ALSA: usb-audio: Fix missing autopm for MIDI input

The commit [88a8516a: ALSA: usbaudio: implement USB autosuspend] added the support of autopm for USB MIDI output, but it didn't take the MIDI input into account. This patch adds the following for fixing the autopm: - Manage the URB start at the first MIDI input stream open, instead of the time of instance creation - Move autopm code to the common substream_open() - Make snd_usbmidi_input_start/_stop() more robust and add the running state check Reviewd-by: Clemens Ladisch <clemens@ladisch.de> Tested-by: Clemens Ladisch <clemens@ladisch.de> Cc: <stable@vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai@suse.de>

ALSA: usb-audio: Fix missing autopm for MIDI input
The commit [88a8516a: ALSA: usbaudio: implement USB autosuspend] added the support of autopm for USB MIDI output, but it didn't take the MIDI input into account. This patch adds the following for fixing the autopm: - Manage the URB start at the first MIDI input stream open, instead of the time of instance creation - Move autopm code to the common substream_open() - Make snd_usbmidi_input_start/_stop() more robust and add the running state check Reviewd-by: Clemens Ladisch <clemens@ladisch.de> Tested-by: Clemens Ladisch <clemens@ladisch.de> Cc: <stable@vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Takashi Iwai
1 parent 59866da9e4
Showing 1 changed file with 46 additions and 42 deletions Side-by-side Diff
sound/usb/midi.c
@@ -126,8 +126,10 @@
 		struct snd_usb_midi_in_endpoint *in;
 	} endpoints[MIDI_MAX_ENDPOINTS];
 	unsigned long input_triggered;
-	unsigned int opened;
+	bool autopm_reference;
+	unsigned int opened[2];
 	unsigned char disconnected;
+	unsigned char input_running;
  
 	struct snd_kcontrol *roland_load_ctl;
 };
@@ -149,7 +151,6 @@
 		struct snd_usb_midi_out_endpoint* ep;
 		struct snd_rawmidi_substream *substream;
 		int active;
-		bool autopm_reference;
 		uint8_t cable;		/* cable number << 4 */
 		uint8_t state;
 #define STATE_UNKNOWN	0
  
  
  
  
  
  
  
  
@@ -1034,36 +1035,58 @@
 	snd_usbmidi_input_start(&umidi->list);
 }
  
-static void substream_open(struct snd_rawmidi_substream *substream, int open)
+static int substream_open(struct snd_rawmidi_substream *substream, int dir,
+			  int open)
 {
 	struct snd_usb_midi* umidi = substream->rmidi->private_data;
 	struct snd_kcontrol *ctl;
+	int err;
  
 	down_read(&umidi->disc_rwsem);
 	if (umidi->disconnected) {
 		up_read(&umidi->disc_rwsem);
-		return;
+		return open ? -ENODEV : 0;
 	}
  
 	mutex_lock(&umidi->mutex);
 	if (open) {
-		if (umidi->opened++ == 0 && umidi->roland_load_ctl) {
-			ctl = umidi->roland_load_ctl;
-			ctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_INACTIVE;
-			snd_ctl_notify(umidi->card,
+		if (!umidi->opened[0] && !umidi->opened[1]) {
+			err = usb_autopm_get_interface(umidi->iface);
+			umidi->autopm_reference = err >= 0;
+			if (err < 0 && err != -EACCES) {
+				mutex_unlock(&umidi->mutex);
+				up_read(&umidi->disc_rwsem);
+				return -EIO;
+			}
+			if (umidi->roland_load_ctl) {
+				ctl = umidi->roland_load_ctl;
+				ctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_INACTIVE;
+				snd_ctl_notify(umidi->card,
 				       SNDRV_CTL_EVENT_MASK_INFO, &ctl->id);
-			update_roland_altsetting(umidi);
+				update_roland_altsetting(umidi);
+			}
 		}
+		umidi->opened[dir]++;
+		if (umidi->opened[1])
+			snd_usbmidi_input_start(&umidi->list);
 	} else {
-		if (--umidi->opened == 0 && umidi->roland_load_ctl) {
-			ctl = umidi->roland_load_ctl;
-			ctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_INACTIVE;
-			snd_ctl_notify(umidi->card,
+		umidi->opened[dir]--;
+		if (!umidi->opened[1])
+			snd_usbmidi_input_stop(&umidi->list);
+		if (!umidi->opened[0] && !umidi->opened[1]) {
+			if (umidi->roland_load_ctl) {
+				ctl = umidi->roland_load_ctl;
+				ctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_INACTIVE;
+				snd_ctl_notify(umidi->card,
 				       SNDRV_CTL_EVENT_MASK_INFO, &ctl->id);
+			}
+			if (umidi->autopm_reference)
+				usb_autopm_put_interface(umidi->iface);
 		}
 	}
 	mutex_unlock(&umidi->mutex);
 	up_read(&umidi->disc_rwsem);
+	return 0;
 }
  
 static int snd_usbmidi_output_open(struct snd_rawmidi_substream *substream)
@@ -1071,7 +1094,6 @@
 	struct snd_usb_midi* umidi = substream->rmidi->private_data;
 	struct usbmidi_out_port* port = NULL;
 	int i, j;
-	int err;
  
 	for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i)
 		if (umidi->endpoints[i].out)
  
  
@@ -1085,33 +1107,14 @@
 		return -ENXIO;
 	}
  
-	down_read(&umidi->disc_rwsem);
-	if (umidi->disconnected) {
-		up_read(&umidi->disc_rwsem);
-		return -ENODEV;
-	}
-	err = usb_autopm_get_interface(umidi->iface);
-	port->autopm_reference = err >= 0;
-	up_read(&umidi->disc_rwsem);
-	if (err < 0 && err != -EACCES)
-		return -EIO;
 	substream->runtime->private_data = port;
 	port->state = STATE_UNKNOWN;
-	substream_open(substream, 1);
-	return 0;
+	return substream_open(substream, 0, 1);
 }
  
 static int snd_usbmidi_output_close(struct snd_rawmidi_substream *substream)
 {
-	struct snd_usb_midi* umidi = substream->rmidi->private_data;
-	struct usbmidi_out_port *port = substream->runtime->private_data;
-
-	substream_open(substream, 0);
-	down_read(&umidi->disc_rwsem);
-	if (!umidi->disconnected && port->autopm_reference)
-		usb_autopm_put_interface(umidi->iface);
-	up_read(&umidi->disc_rwsem);
-	return 0;
+	return substream_open(substream, 0, 0);
 }
  
 static void snd_usbmidi_output_trigger(struct snd_rawmidi_substream *substream, int up)
  
@@ -1164,14 +1167,12 @@
  
 static int snd_usbmidi_input_open(struct snd_rawmidi_substream *substream)
 {
-	substream_open(substream, 1);
-	return 0;
+	return substream_open(substream, 1, 1);
 }
  
 static int snd_usbmidi_input_close(struct snd_rawmidi_substream *substream)
 {
-	substream_open(substream, 0);
-	return 0;
+	return substream_open(substream, 1, 0);
 }
  
 static void snd_usbmidi_input_trigger(struct snd_rawmidi_substream *substream, int up)
  
@@ -2080,12 +2081,15 @@
 	unsigned int i, j;
  
 	umidi = list_entry(p, struct snd_usb_midi, list);
+	if (!umidi->input_running)
+		return;
 	for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i) {
 		struct snd_usb_midi_endpoint* ep = &umidi->endpoints[i];
 		if (ep->in)
 			for (j = 0; j < INPUT_URBS; ++j)
 				usb_kill_urb(ep->in->urbs[j]);
 	}
+	umidi->input_running = 0;
 }
  
 static void snd_usbmidi_input_start_ep(struct snd_usb_midi_in_endpoint* ep)
  
@@ -2110,8 +2114,11 @@
 	int i;
  
 	umidi = list_entry(p, struct snd_usb_midi, list);
+	if (umidi->input_running || !umidi->opened[1])
+		return;
 	for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i)
 		snd_usbmidi_input_start_ep(umidi->endpoints[i].in);
+	umidi->input_running = 1;
 }
  
 /*
@@ -2250,9 +2257,6 @@
 	}
  
 	list_add_tail(&umidi->list, midi_list);
-
-	for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i)
-		snd_usbmidi_input_start_ep(umidi->endpoints[i].in);
 	return 0;
 }
...	...	@@ -126,8 +126,10 @@
126	126	struct snd_usb_midi_in_endpoint *in;
127	127	} endpoints[MIDI_MAX_ENDPOINTS];
128	128	unsigned long input_triggered;
129		- unsigned int opened;
	129	+ bool autopm_reference;
	130	+ unsigned int opened[2];
130	131	unsigned char disconnected;
	132	+ unsigned char input_running;
131	133
132	134	struct snd_kcontrol *roland_load_ctl;
133	135	};
...	...	@@ -149,7 +151,6 @@
149	151	struct snd_usb_midi_out_endpoint* ep;
150	152	struct snd_rawmidi_substream *substream;
151	153	int active;
152		- bool autopm_reference;
153	154	uint8_t cable; /* cable number << 4 */
154	155	uint8_t state;
155	156	#define STATE_UNKNOWN 0
156	157
157	158
158	159
159	160
160	161
161	162
162	163
163	164
...	...	@@ -1034,36 +1035,58 @@
1034	1035	snd_usbmidi_input_start(&umidi->list);
1035	1036	}
1036	1037
1037		-static void substream_open(struct snd_rawmidi_substream *substream, int open)
	1038	+static int substream_open(struct snd_rawmidi_substream *substream, int dir,
	1039	+ int open)
1038	1040	{
1039	1041	struct snd_usb_midi* umidi = substream->rmidi->private_data;
1040	1042	struct snd_kcontrol *ctl;
	1043	+ int err;
1041	1044
1042	1045	down_read(&umidi->disc_rwsem);
1043	1046	if (umidi->disconnected) {
1044	1047	up_read(&umidi->disc_rwsem);
1045		- return;
	1048	+ return open ? -ENODEV : 0;
1046	1049	}
1047	1050
1048	1051	mutex_lock(&umidi->mutex);
1049	1052	if (open) {
1050		- if (umidi->opened++ == 0 && umidi->roland_load_ctl) {
1051		- ctl = umidi->roland_load_ctl;
1052		- ctl->vd[0].access \|= SNDRV_CTL_ELEM_ACCESS_INACTIVE;
1053		- snd_ctl_notify(umidi->card,
	1053	+ if (!umidi->opened[0] && !umidi->opened[1]) {
	1054	+ err = usb_autopm_get_interface(umidi->iface);
	1055	+ umidi->autopm_reference = err >= 0;
	1056	+ if (err < 0 && err != -EACCES) {
	1057	+ mutex_unlock(&umidi->mutex);
	1058	+ up_read(&umidi->disc_rwsem);
	1059	+ return -EIO;
	1060	+ }
	1061	+ if (umidi->roland_load_ctl) {
	1062	+ ctl = umidi->roland_load_ctl;
	1063	+ ctl->vd[0].access \|= SNDRV_CTL_ELEM_ACCESS_INACTIVE;
	1064	+ snd_ctl_notify(umidi->card,
1054	1065	SNDRV_CTL_EVENT_MASK_INFO, &ctl->id);
1055		- update_roland_altsetting(umidi);
	1066	+ update_roland_altsetting(umidi);
	1067	+ }
1056	1068	}
	1069	+ umidi->opened[dir]++;
	1070	+ if (umidi->opened[1])
	1071	+ snd_usbmidi_input_start(&umidi->list);
1057	1072	} else {
1058		- if (--umidi->opened == 0 && umidi->roland_load_ctl) {
1059		- ctl = umidi->roland_load_ctl;
1060		- ctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_INACTIVE;
1061		- snd_ctl_notify(umidi->card,
	1073	+ umidi->opened[dir]--;
	1074	+ if (!umidi->opened[1])
	1075	+ snd_usbmidi_input_stop(&umidi->list);
	1076	+ if (!umidi->opened[0] && !umidi->opened[1]) {
	1077	+ if (umidi->roland_load_ctl) {
	1078	+ ctl = umidi->roland_load_ctl;
	1079	+ ctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_INACTIVE;
	1080	+ snd_ctl_notify(umidi->card,
1062	1081	SNDRV_CTL_EVENT_MASK_INFO, &ctl->id);
	1082	+ }
	1083	+ if (umidi->autopm_reference)
	1084	+ usb_autopm_put_interface(umidi->iface);
1063	1085	}
1064	1086	}
1065	1087	mutex_unlock(&umidi->mutex);
1066	1088	up_read(&umidi->disc_rwsem);
	1089	+ return 0;
1067	1090	}
1068	1091
1069	1092	static int snd_usbmidi_output_open(struct snd_rawmidi_substream *substream)
...	...	@@ -1071,7 +1094,6 @@
1071	1094	struct snd_usb_midi* umidi = substream->rmidi->private_data;
1072	1095	struct usbmidi_out_port* port = NULL;
1073	1096	int i, j;
1074		- int err;
1075	1097
1076	1098	for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i)
1077	1099	if (umidi->endpoints[i].out)
1078	1100
1079	1101
...	...	@@ -1085,33 +1107,14 @@
1085	1107	return -ENXIO;
1086	1108	}
1087	1109
1088		- down_read(&umidi->disc_rwsem);
1089		- if (umidi->disconnected) {
1090		- up_read(&umidi->disc_rwsem);
1091		- return -ENODEV;
1092		- }
1093		- err = usb_autopm_get_interface(umidi->iface);
1094		- port->autopm_reference = err >= 0;
1095		- up_read(&umidi->disc_rwsem);
1096		- if (err < 0 && err != -EACCES)
1097		- return -EIO;
1098	1110	substream->runtime->private_data = port;
1099	1111	port->state = STATE_UNKNOWN;
1100		- substream_open(substream, 1);
1101		- return 0;
	1112	+ return substream_open(substream, 0, 1);
1102	1113	}
1103	1114
1104	1115	static int snd_usbmidi_output_close(struct snd_rawmidi_substream *substream)
1105	1116	{
1106		- struct snd_usb_midi* umidi = substream->rmidi->private_data;
1107		- struct usbmidi_out_port *port = substream->runtime->private_data;
1108		-
1109		- substream_open(substream, 0);
1110		- down_read(&umidi->disc_rwsem);
1111		- if (!umidi->disconnected && port->autopm_reference)
1112		- usb_autopm_put_interface(umidi->iface);
1113		- up_read(&umidi->disc_rwsem);
1114		- return 0;
	1117	+ return substream_open(substream, 0, 0);
1115	1118	}
1116	1119
1117	1120	static void snd_usbmidi_output_trigger(struct snd_rawmidi_substream *substream, int up)
1118	1121
...	...	@@ -1164,14 +1167,12 @@
1164	1167
1165	1168	static int snd_usbmidi_input_open(struct snd_rawmidi_substream *substream)
1166	1169	{
1167		- substream_open(substream, 1);
1168		- return 0;
	1170	+ return substream_open(substream, 1, 1);
1169	1171	}
1170	1172
1171	1173	static int snd_usbmidi_input_close(struct snd_rawmidi_substream *substream)
1172	1174	{
1173		- substream_open(substream, 0);
1174		- return 0;
	1175	+ return substream_open(substream, 1, 0);
1175	1176	}
1176	1177
1177	1178	static void snd_usbmidi_input_trigger(struct snd_rawmidi_substream *substream, int up)
1178	1179
...	...	@@ -2080,12 +2081,15 @@
2080	2081	unsigned int i, j;
2081	2082
2082	2083	umidi = list_entry(p, struct snd_usb_midi, list);
	2084	+ if (!umidi->input_running)
	2085	+ return;
2083	2086	for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i) {
2084	2087	struct snd_usb_midi_endpoint* ep = &umidi->endpoints[i];
2085	2088	if (ep->in)
2086	2089	for (j = 0; j < INPUT_URBS; ++j)
2087	2090	usb_kill_urb(ep->in->urbs[j]);
2088	2091	}
	2092	+ umidi->input_running = 0;
2089	2093	}
2090	2094
2091	2095	static void snd_usbmidi_input_start_ep(struct snd_usb_midi_in_endpoint* ep)
2092	2096
...	...	@@ -2110,8 +2114,11 @@
2110	2114	int i;
2111	2115
2112	2116	umidi = list_entry(p, struct snd_usb_midi, list);
	2117	+ if (umidi->input_running \|\| !umidi->opened[1])
	2118	+ return;
2113	2119	for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i)
2114	2120	snd_usbmidi_input_start_ep(umidi->endpoints[i].in);
	2121	+ umidi->input_running = 1;
2115	2122	}
2116	2123
2117	2124	/*
...	...	@@ -2250,9 +2257,6 @@
2250	2257	}
2251	2258
2252	2259	list_add_tail(&umidi->list, midi_list);
2253		-
2254		- for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i)
2255		- snd_usbmidi_input_start_ep(umidi->endpoints[i].in);
2256	2260	return 0;
2257	2261	}
2258	2262