All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 0/2] design a way to change audio Jack state by software
@ 2020-12-09 12:43 Hui Wang
  2020-12-09 12:43 ` [RFC][PATCH 1/2] alsa: jack: expand snd_jack_report parameter for jack sw_inject Hui Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Hui Wang @ 2020-12-09 12:43 UTC (permalink / raw)
  To: alsa-devel, tiwai

After we change sth in the userspace audio stack like alsa-ucm or
pulseaudio, we want to perform remote audio auto test to verify if the
change introduce the regression or not, some of the tests are about
the defaut_sink/default_source or active_port switching, this needs
the audio jack state to be changed to trigger the userspace's audio
device switching.

So far, there is no software ways to change the audio jack state, this
block the auto test.

My design is adding a sysfs interface for each sound card if the card
has audio jack, then users could echo different values to sysfs to
change the jack state (Phantom jack is not controlled by injection).
And once the users enable the jack injection via sysfs, this jack's
state will not be controlled by hw events anymore until users disable
the jack injection.

Of course, this could not 100% simulate the plugin or plugout triggered
by hw events, with the hw triggered plugin or plugout, the audio driver
will set codec or does sth else, so the software injection is just
changing the jack state and notify the userspace, it is just for
testing userspace part.

Here is an example to change jack state via sysfs:

After booting up:
/* cd to the jack injection folder for sound card0 in the sysfs */
$cd /sys/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/jack

/* check file nodes in this folder */
$ls
jackin_inject  sw_inject_enable

/* check all jack's software injection enable status, all disabled now */
$ cat sw_inject_enable 
Jack: Mic  0
Jack: Headphone  0
Jack: HDMI/DP,pcm=3  0
Jack: HDMI/DP,pcm=4  0
Jack: HDMI/DP,pcm=5  0

/* enable software injection for Jack Headphone */
$ sudo sh -c "echo Headphone 1 > sw_inject_enable"

/* check all jack's software injection enable status again, now Headphone is enabled */
$ cat sw_inject_enable 
Jack: Mic  0
Jack: Headphone  1
Jack: HDMI/DP,pcm=3  0
Jack: HDMI/DP,pcm=4  0
Jack: HDMI/DP,pcm=5  0

/* trigger plugin to Jack Headphone */
$sudo sh -c "echo Headphone 1 > jackin_inject"

/* check if Jack Headphone is plugged in */
$ sudo amixer contents | grep "Headphone Jack" -3
numid=30,iface=CARD,name='HDMI/DP,pcm=5 Jack'
  ; type=BOOLEAN,access=r-------,values=1
  : values=off
numid=17,iface=CARD,name='Headphone Jack'
  ; type=BOOLEAN,access=r-------,values=1
  : values=on
numid=14,iface=CARD,name='Mic Jack'

/* trigger plugout to Jack Headphone */
$ sudo sh -c "echo Headphone 0 > jackin_inject"

/* check if Jack Headphone is plugged out */
$ sudo amixer contents | grep "Headphone Jack" -3
numid=30,iface=CARD,name='HDMI/DP,pcm=5 Jack'
  ; type=BOOLEAN,access=r-------,values=1
  : values=off
numid=17,iface=CARD,name='Headphone Jack'
  ; type=BOOLEAN,access=r-------,values=1
  : values=off
numid=14,iface=CARD,name='Mic Jack'

/* disable Jack Headphone software injection, this will return the control to non-injection ways */
$ sudo sh -c "echo Headphone 0 > sw_inject_enable"

/* check if the Jack Headphone software injection is disabled, it is disabled now */
$ cat sw_inject_enable 
Jack: Mic  0
Jack: Headphone  0
Jack: HDMI/DP,pcm=3  0
Jack: HDMI/DP,pcm=4  0
Jack: HDMI/DP,pcm=5  0

Hui Wang (2):
  alsa: jack: expand snd_jack_report parameter for jack sw_inject
  alsa: jack: adding support for software jack in or out injection

 include/sound/core.h            |   1 +
 include/sound/jack.h            |   5 +-
 sound/core/jack.c               | 129 +++++++++++++++++++++++++++++++-
 sound/pci/hda/hda_jack.c        |   6 +-
 sound/pci/hda/patch_hdmi.c      |   2 +-
 sound/pci/oxygen/xonar_wm87x6.c |   2 +-
 sound/soc/soc-jack.c            |   2 +-
 sound/x86/intel_hdmi_audio.c    |   4 +-
 8 files changed, 140 insertions(+), 11 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC][PATCH 1/2] alsa: jack: expand snd_jack_report parameter for jack sw_inject
  2020-12-09 12:43 [RFC][PATCH 0/2] design a way to change audio Jack state by software Hui Wang
@ 2020-12-09 12:43 ` Hui Wang
  2020-12-09 12:43 ` [RFC][PATCH 2/2] alsa: jack: adding support for software jack in or out injection Hui Wang
  2020-12-09 14:25 ` [RFC][PATCH 0/2] design a way to change audio Jack state by software Jaroslav Kysela
  2 siblings, 0 replies; 9+ messages in thread
From: Hui Wang @ 2020-12-09 12:43 UTC (permalink / raw)
  To: alsa-devel, tiwai

This is the preparation for supporting jack software plug in/out
injection, when users enable the software injection, the jack state
shouldn't be changed by hw events or other non-injection events
anymore, so adding a parameter in the snd_jack_report() to distinguish
if the function is called from software injection or not.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 include/sound/jack.h            | 4 ++--
 sound/core/jack.c               | 3 ++-
 sound/pci/hda/hda_jack.c        | 6 +++---
 sound/pci/hda/patch_hdmi.c      | 2 +-
 sound/pci/oxygen/xonar_wm87x6.c | 2 +-
 sound/soc/soc-jack.c            | 2 +-
 sound/x86/intel_hdmi_audio.c    | 4 ++--
 7 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/sound/jack.h b/include/sound/jack.h
index 9eb2b5ec1ec4..17f71fe38ed5 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -81,7 +81,7 @@ void snd_jack_set_parent(struct snd_jack *jack, struct device *parent);
 int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type,
 		     int keytype);
 #endif
-void snd_jack_report(struct snd_jack *jack, int status);
+void snd_jack_report(struct snd_jack *jack, int status, bool sw_inject);
 
 #else
 static inline int snd_jack_new(struct snd_card *card, const char *id, int type,
@@ -95,7 +95,7 @@ static inline int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name
 	return 0;
 }
 
-static inline void snd_jack_report(struct snd_jack *jack, int status)
+static inline void snd_jack_report(struct snd_jack *jack, int status, bool sw_inject)
 {
 }
 
diff --git a/sound/core/jack.c b/sound/core/jack.c
index 503c8af79d55..49b9461aef51 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -336,8 +336,9 @@ EXPORT_SYMBOL(snd_jack_set_key);
  *
  * @jack:   The jack to report status for
  * @status: The current status of the jack
+ * @sw_inject: Indicate if this is called from jack software inject
  */
-void snd_jack_report(struct snd_jack *jack, int status)
+void snd_jack_report(struct snd_jack *jack, int status, bool sw_inject)
 {
 	struct snd_jack_kctl *jack_kctl;
 #ifdef CONFIG_SND_JACK_INPUT_DEV
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index 588059428d8f..152d651df74e 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -414,10 +414,10 @@ void snd_hda_jack_report_sync(struct hda_codec *codec)
 			state = jack->button_state;
 			if (get_jack_plug_state(jack->pin_sense))
 				state |= jack->type;
-			snd_jack_report(jack->jack, state);
+			snd_jack_report(jack->jack, state, false);
 			if (jack->button_state) {
 				snd_jack_report(jack->jack,
-						state & ~jack->button_state);
+						state & ~jack->button_state, false);
 				jack->button_state = 0; /* button released */
 			}
 		}
@@ -503,7 +503,7 @@ int snd_hda_jack_add_kctl_mst(struct hda_codec *codec, hda_nid_t nid,
 	}
 
 	state = snd_hda_jack_detect_mst(codec, nid, dev_id);
-	snd_jack_report(jack->jack, state ? jack->type : 0);
+	snd_jack_report(jack->jack, state ? jack->type : 0, false);
 
 	return 0;
 }
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index b0068f8ca46d..f19762a6e9e7 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1590,7 +1590,7 @@ static void update_eld(struct hda_codec *codec,
 	if (eld_changed && pcm_jack)
 		snd_jack_report(pcm_jack,
 				(eld->monitor_present && eld->eld_valid) ?
-				SND_JACK_AVOUT : 0);
+				SND_JACK_AVOUT : 0, false);
 }
 
 /* update ELD and jack state via HD-audio verbs */
diff --git a/sound/pci/oxygen/xonar_wm87x6.c b/sound/pci/oxygen/xonar_wm87x6.c
index 8aa92f3e5ee8..595e10275bd9 100644
--- a/sound/pci/oxygen/xonar_wm87x6.c
+++ b/sound/pci/oxygen/xonar_wm87x6.c
@@ -251,7 +251,7 @@ static void xonar_ds_handle_hp_jack(struct oxygen *chip)
 		reg |= WM8766_MUTEALL;
 	wm8766_write_cached(chip, WM8766_DAC_CTRL, reg);
 
-	snd_jack_report(data->hp_jack, hp_plugged ? SND_JACK_HEADPHONE : 0);
+	snd_jack_report(data->hp_jack, hp_plugged ? SND_JACK_HEADPHONE : 0, false);
 
 	mutex_unlock(&chip->mutex);
 }
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index 0f1820f36b4d..5b98f5fa4537 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -78,7 +78,7 @@ void snd_soc_jack_report(struct snd_soc_jack *jack, int status, int mask)
 	if (sync)
 		snd_soc_dapm_sync(dapm);
 
-	snd_jack_report(jack->jack, jack->status);
+	snd_jack_report(jack->jack, jack->status, false);
 
 	mutex_unlock(&jack->mutex);
 }
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 9f9fcd2749f2..e12dce8daed0 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1363,7 +1363,7 @@ static void had_process_hot_plug(struct snd_intelhad *intelhaddata)
 		had_substream_put(intelhaddata);
 	}
 
-	snd_jack_report(intelhaddata->jack, SND_JACK_AVOUT);
+	snd_jack_report(intelhaddata->jack, SND_JACK_AVOUT, false);
 }
 
 /* process hot unplug, called from wq with mutex locked */
@@ -1398,7 +1398,7 @@ static void had_process_hot_unplug(struct snd_intelhad *intelhaddata)
 		had_substream_put(intelhaddata);
 	}
 
-	snd_jack_report(intelhaddata->jack, 0);
+	snd_jack_report(intelhaddata->jack, 0, false);
 }
 
 /*
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC][PATCH 2/2] alsa: jack: adding support for software jack in or out injection
  2020-12-09 12:43 [RFC][PATCH 0/2] design a way to change audio Jack state by software Hui Wang
  2020-12-09 12:43 ` [RFC][PATCH 1/2] alsa: jack: expand snd_jack_report parameter for jack sw_inject Hui Wang
@ 2020-12-09 12:43 ` Hui Wang
  2020-12-10 10:51   ` kernel test robot
  2020-12-09 14:25 ` [RFC][PATCH 0/2] design a way to change audio Jack state by software Jaroslav Kysela
  2 siblings, 1 reply; 9+ messages in thread
From: Hui Wang @ 2020-12-09 12:43 UTC (permalink / raw)
  To: alsa-devel, tiwai

We want to perform remote audio auto test, need the audio jack to
change from plugout to plugin or vice versa by software ways.

Here the design is if the sound card has at least one Jack, the kernel
will build a sysfs interface of jack injection for this sound card, it
will create 2 files: jackin_inject and sw_inject_enable

users need to cat sw_inject_enable first to check all jacks and their
injection enable status, like below (0 means disabled):
Jack: Mic  0
Jack: Headphone  0
Jack: HDMI/DP,pcm=3  0
Jack: HDMI/DP,pcm=4  0
Jack: HDMI/DP,pcm=5  0

Suppose users want to enable jack injection for Headphone, they need
to run $sudo sh -c 'echo Headphone 1 > sw_inject_enable', then users
could change the Headphone Jack state through jackin_inject and this
Jack's state will not changed by non-injection ways anymore.

Users could run $sudo sh -c 'echo Headphone 1 > jackin_inject' to
trigger the Headphone jack to plugin or echo Headphone 0 to trigger
it to plugout.

If users finish their test, they could run
$sudo sh -c 'echo Headphone 0 > sw_inject_enable' to disable injection
and let non-injection ways control this Jack.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 include/sound/core.h |   1 +
 include/sound/jack.h |   1 +
 sound/core/jack.c    | 126 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 128 insertions(+)

diff --git a/include/sound/core.h b/include/sound/core.h
index 0462c577d7a3..6860bfe5bb46 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -122,6 +122,7 @@ struct snd_card {
 
 	size_t total_pcm_alloc_bytes;	/* total amount of allocated buffers */
 	struct mutex memory_mutex;	/* protection for the above */
+	bool jack_inject_attr_registered; /* jack software inject sysfs interface registered? */
 
 #ifdef CONFIG_PM
 	unsigned int power_state;	/* power state */
diff --git a/include/sound/jack.h b/include/sound/jack.h
index 17f71fe38ed5..082664f2aff2 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -67,6 +67,7 @@ struct snd_jack {
 	char name[100];
 	unsigned int key[6];   /* Keep in sync with definitions above */
 #endif /* CONFIG_SND_JACK_INPUT_DEV */
+	bool sw_inject_enable;
 	void *private_data;
 	void (*private_free)(struct snd_jack *);
 };
diff --git a/sound/core/jack.c b/sound/core/jack.c
index 49b9461aef51..2d8eef9dcab8 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -8,6 +8,7 @@
 #include <linux/input.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/ctype.h>
 #include <sound/jack.h>
 #include <sound/core.h>
 #include <sound/control.h>
@@ -180,6 +181,123 @@ int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name, int mask)
 }
 EXPORT_SYMBOL(snd_jack_add_new_kctl);
 
+static struct snd_jack *parsing_jack_and_enable(struct snd_card *card, const char *buf,
+						size_t count, unsigned long *enable)
+{
+	struct snd_device *sdev;
+	struct snd_jack *jack = NULL;
+	bool jack_found = 0;
+	char *jackid, *ena;
+	int i, err;
+
+	/* skip the '\n\r' at the end of buf */
+	for (i = count - 2; i > 0; i--)
+		if (isspace(buf[i]))
+			break;
+	if (i == 0)
+		return NULL;
+
+	jackid = kstrndup(buf, i, GFP_KERNEL);
+	if (!jackid)
+		return NULL;
+
+	if (strstr(jackid, "Phantom"))
+		goto exit1;
+
+	ena = kstrndup(&buf[i+1], count - i - 2, GFP_KERNEL);
+	if (!ena)
+		goto exit1;
+
+	err = kstrtoul(ena, 0, enable);
+	if (err)
+		goto exit;
+
+	list_for_each_entry(sdev, &card->devices, list)
+		if (sdev->type == SNDRV_DEV_JACK) {
+			jack = (struct snd_jack *) sdev->device_data;
+			if (!strcmp(jack->id, jackid)) {
+				jack_found = 1;
+				break;
+			}
+		}
+
+	if (!jack_found)
+		jack = NULL;
+ exit:
+	kfree(ena);
+ exit1:
+	kfree(jackid);
+	return jack;
+}
+
+static ssize_t
+jackin_inject_store(struct device *dev, struct device_attribute *attr,
+		    const char *buf, size_t count)
+{
+	struct snd_card *card = container_of(dev, struct snd_card, card_dev);
+	struct snd_jack *jack;
+	unsigned long enable;
+
+	jack = parsing_jack_and_enable(card, buf, count, &enable);
+	if (!jack)
+		return -EINVAL;
+
+	if (jack->sw_inject_enable)
+		snd_jack_report(jack, enable ? jack->type : 0, true);
+
+	return count;
+}
+static DEVICE_ATTR_WO(jackin_inject);
+
+static ssize_t
+sw_inject_enable_show(struct device *dev,
+		      struct device_attribute *attr, char *buf)
+{
+	struct snd_card *card = container_of(dev, struct snd_card, card_dev);
+	struct snd_device *sdev;
+	struct snd_jack *jack = NULL;
+	ssize_t ret_count = 0;
+
+	list_for_each_entry(sdev, &card->devices, list)
+		if (sdev->type == SNDRV_DEV_JACK) {
+			jack = (struct snd_jack *) sdev->device_data;
+			if (strstr(jack->id, "Phantom"))
+				continue;
+			ret_count += scnprintf(buf + ret_count, PAGE_SIZE, "%s: %s  %i\n",
+					       "Jack", jack->id, jack->sw_inject_enable);
+		}
+	return ret_count;
+}
+
+static ssize_t
+sw_inject_enable_store(struct device *dev, struct device_attribute *attr,
+		       const char *buf, size_t count)
+{
+	struct snd_card *card = container_of(dev, struct snd_card, card_dev);
+	struct snd_jack *jack;
+	unsigned long enable;
+
+	jack = parsing_jack_and_enable(card, buf, count, &enable);
+	if (!jack)
+		return -EINVAL;
+
+	jack->sw_inject_enable = !!enable;
+
+	return count;
+}
+static DEVICE_ATTR_RW(sw_inject_enable);
+
+static struct attribute *snd_jack_dev_attrs[] = {
+	&dev_attr_jackin_inject.attr,
+	&dev_attr_sw_inject_enable.attr,
+	NULL
+};
+
+static const struct attribute_group snd_jack_dev_attr_group = {
+	.name = "jack",
+	.attrs = snd_jack_dev_attrs,
+};
+
 /**
  * snd_jack_new - Create a new jack
  * @card:  the card instance
@@ -256,6 +374,11 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 
 	*jjack = jack;
 
+	if (!jack->card->jack_inject_attr_registered) {
+		jack->card->jack_inject_attr_registered = true;
+		snd_card_add_dev_attr(jack->card, &snd_jack_dev_attr_group);
+	}
+
 	return 0;
 
 fail_input:
@@ -348,6 +471,9 @@ void snd_jack_report(struct snd_jack *jack, int status, bool sw_inject)
 	if (!jack)
 		return;
 
+	if (jack->sw_inject_enable && !sw_inject)
+		return;
+
 	list_for_each_entry(jack_kctl, &jack->kctl_list, list)
 		snd_kctl_jack_report(jack->card, jack_kctl->kctl,
 					    status & jack_kctl->mask_bits);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC][PATCH 0/2] design a way to change audio Jack state by software
  2020-12-09 12:43 [RFC][PATCH 0/2] design a way to change audio Jack state by software Hui Wang
  2020-12-09 12:43 ` [RFC][PATCH 1/2] alsa: jack: expand snd_jack_report parameter for jack sw_inject Hui Wang
  2020-12-09 12:43 ` [RFC][PATCH 2/2] alsa: jack: adding support for software jack in or out injection Hui Wang
@ 2020-12-09 14:25 ` Jaroslav Kysela
  2020-12-09 14:44   ` Takashi Iwai
  2020-12-11 13:36   ` Kai Vehmanen
  2 siblings, 2 replies; 9+ messages in thread
From: Jaroslav Kysela @ 2020-12-09 14:25 UTC (permalink / raw)
  To: Hui Wang, alsa-devel, tiwai

Dne 09. 12. 20 v 13:43 Hui Wang napsal(a):
> After we change sth in the userspace audio stack like alsa-ucm or
> pulseaudio, we want to perform remote audio auto test to verify if the
> change introduce the regression or not, some of the tests are about
> the defaut_sink/default_source or active_port switching, this needs
> the audio jack state to be changed to trigger the userspace's audio
> device switching.
> 
> So far, there is no software ways to change the audio jack state, this
> block the auto test.

I'm not convinced to pollute the kernel space with this code. You can use
LD_PRELOAD and simulate this via a shared library or the alsa-lib may be extended.

Also, the first patch can be omitted if you just create another
snd_jack_report() function for the user API and put the common code to the
static function in jack.c.

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][PATCH 0/2] design a way to change audio Jack state by software
  2020-12-09 14:25 ` [RFC][PATCH 0/2] design a way to change audio Jack state by software Jaroslav Kysela
@ 2020-12-09 14:44   ` Takashi Iwai
  2020-12-10  1:54     ` Hui Wang
  2020-12-11 13:36   ` Kai Vehmanen
  1 sibling, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2020-12-09 14:44 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: Hui Wang, alsa-devel

On Wed, 09 Dec 2020 15:25:42 +0100,
Jaroslav Kysela wrote:
> 
> Dne 09. 12. 20 v 13:43 Hui Wang napsal(a):
> > After we change sth in the userspace audio stack like alsa-ucm or
> > pulseaudio, we want to perform remote audio auto test to verify if the
> > change introduce the regression or not, some of the tests are about
> > the defaut_sink/default_source or active_port switching, this needs
> > the audio jack state to be changed to trigger the userspace's audio
> > device switching.
> > 
> > So far, there is no software ways to change the audio jack state, this
> > block the auto test.
> 
> I'm not convinced to pollute the kernel space with this code. You can use
> LD_PRELOAD and simulate this via a shared library or the alsa-lib may be extended.

While I understand this argument, I see the merit of having the
kernel-side injection, too.  Wrapping with LD_PRELOAD (or use alsa-lib
plugin) doesn't verify whether the whole jack system works.  OTOH, the
jack injection in kernel simulates the closer path to the real use
case, which covers also the call paths inside the kernel.

Though, I'm not sure whether the current design is the best choice.
Basically sysfs is expressed in one value per file (although there are
many exceptions, of course).  So creating a node per jack object might
fit better?  Or can it better be in debugfs?

> Also, the first patch can be omitted if you just create another
> snd_jack_report() function for the user API and put the common code to the
> static function in jack.c.

Fully agreed on this point.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][PATCH 0/2] design a way to change audio Jack state by software
  2020-12-09 14:44   ` Takashi Iwai
@ 2020-12-10  1:54     ` Hui Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Hui Wang @ 2020-12-10  1:54 UTC (permalink / raw)
  To: Takashi Iwai, Jaroslav Kysela; +Cc: alsa-devel


On 12/9/20 10:44 PM, Takashi Iwai wrote:
> On Wed, 09 Dec 2020 15:25:42 +0100,
> Jaroslav Kysela wrote:
>> Dne 09. 12. 20 v 13:43 Hui Wang napsal(a):
>>> After we change sth in the userspace audio stack like alsa-ucm or
>>> pulseaudio, we want to perform remote audio auto test to verify if the
>>> change introduce the regression or not, some of the tests are about
>>> the defaut_sink/default_source or active_port switching, this needs
>>> the audio jack state to be changed to trigger the userspace's audio
>>> device switching.
>>>
>>> So far, there is no software ways to change the audio jack state, this
>>> block the auto test.
>> I'm not convinced to pollute the kernel space with this code. You can use
>> LD_PRELOAD and simulate this via a shared library or the alsa-lib may be extended.
> While I understand this argument, I see the merit of having the
> kernel-side injection, too.  Wrapping with LD_PRELOAD (or use alsa-lib
> plugin) doesn't verify whether the whole jack system works.  OTOH, the
> jack injection in kernel simulates the closer path to the real use
> case, which covers also the call paths inside the kernel.
>
> Though, I'm not sure whether the current design is the best choice.
> Basically sysfs is expressed in one value per file (although there are
> many exceptions, of course).  So creating a node per jack object might
> fit better?  Or can it better be in debugfs?
OK, got it, will investigate it.
>
>> Also, the first patch can be omitted if you just create another
>> snd_jack_report() function for the user API and put the common code to the
>> static function in jack.c.
> Fully agreed on this point.

Indeed, it is a better and cleaner way, will think about it and 
implement it.

Thanks.

>
> thanks,
>
> Takashi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][PATCH 2/2] alsa: jack: adding support for software jack in or out injection
  2020-12-09 12:43 ` [RFC][PATCH 2/2] alsa: jack: adding support for software jack in or out injection Hui Wang
@ 2020-12-10 10:51   ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-12-10 10:51 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2719 bytes --]

Hi Hui,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on sound/for-next]
[also build test ERROR on asoc/for-next v5.10-rc7 next-20201209]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Hui-Wang/design-a-way-to-change-audio-Jack-state-by-software/20201209-204735
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: powerpc64-randconfig-r035-20201209 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1968804ac726e7674d5de22bc2204b45857da344)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/db33ddf3088e8952179c2908fa69c9e0795a445d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hui-Wang/design-a-way-to-change-audio-Jack-state-by-software/20201209-204735
        git checkout db33ddf3088e8952179c2908fa69c9e0795a445d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> sound/core/jack.c:246:40: error: no member named 'type' in 'struct snd_jack'
                   snd_jack_report(jack, enable ? jack->type : 0, true);
                                                  ~~~~  ^
   1 error generated.

vim +246 sound/core/jack.c

   232	
   233	static ssize_t
   234	jackin_inject_store(struct device *dev, struct device_attribute *attr,
   235			    const char *buf, size_t count)
   236	{
   237		struct snd_card *card = container_of(dev, struct snd_card, card_dev);
   238		struct snd_jack *jack;
   239		unsigned long enable;
   240	
   241		jack = parsing_jack_and_enable(card, buf, count, &enable);
   242		if (!jack)
   243			return -EINVAL;
   244	
   245		if (jack->sw_inject_enable)
 > 246			snd_jack_report(jack, enable ? jack->type : 0, true);
   247	
   248		return count;
   249	}
   250	static DEVICE_ATTR_WO(jackin_inject);
   251	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 27056 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][PATCH 0/2] design a way to change audio Jack state by software
  2020-12-09 14:25 ` [RFC][PATCH 0/2] design a way to change audio Jack state by software Jaroslav Kysela
  2020-12-09 14:44   ` Takashi Iwai
@ 2020-12-11 13:36   ` Kai Vehmanen
  2020-12-14 10:48     ` Hui Wang
  1 sibling, 1 reply; 9+ messages in thread
From: Kai Vehmanen @ 2020-12-11 13:36 UTC (permalink / raw)
  To: Jaroslav Kysela, Hui Wang; +Cc: Takashi Iwai, alsa-devel

Hi,

On Wed, 9 Dec 2020, Jaroslav Kysela wrote:

> Dne 09. 12. 20 v 13:43 Hui Wang napsal(a):
> > After we change sth in the userspace audio stack like alsa-ucm or
> > pulseaudio, we want to perform remote audio auto test to verify if the
> > change introduce the regression or not, some of the tests are about
> > the defaut_sink/default_source or active_port switching, this needs

thanks Hui for the RFC.

I do think this is a very tempting capability to add. I understand 
Jaroslav's concerns as well, but there is clearly a category of issues 
where user-space functionality is broken due to a mismatch of element 
names between UCM file and the driver. I.e. the actual jack detection 
(codec hw and its driver) is working, but due to wrong UCM file chosen, 
wrong driven is chosen, or errors in either UCM or driver, event does not 
get parsed right and user ends with no audio.

A pure user-space test harness would not catch these, and building full 
automation of external codec events, is a complex task and coverage of 
devices is likely limited. 

The 'edid_override' debugfs interface in i915 is a bit similar. It allows
inject EDID content irrespectively of the actual monitor connected.

> Also, the first patch can be omitted if you just create another
> snd_jack_report() function for the user API and put the common code to the
> static function in jack.c.

++votes on this

Br, Kai

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][PATCH 0/2] design a way to change audio Jack state by software
  2020-12-11 13:36   ` Kai Vehmanen
@ 2020-12-14 10:48     ` Hui Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Hui Wang @ 2020-12-14 10:48 UTC (permalink / raw)
  To: Kai Vehmanen, Jaroslav Kysela; +Cc: Takashi Iwai, alsa-devel


On 12/11/20 9:36 PM, Kai Vehmanen wrote:
> Hi,
>
> On Wed, 9 Dec 2020, Jaroslav Kysela wrote:
>
>> Dne 09. 12. 20 v 13:43 Hui Wang napsal(a):
>>> After we change sth in the userspace audio stack like alsa-ucm or
>>> pulseaudio, we want to perform remote audio auto test to verify if the
>>> change introduce the regression or not, some of the tests are about
>>> the defaut_sink/default_source or active_port switching, this needs
> thanks Hui for the RFC.
>
> I do think this is a very tempting capability to add. I understand
> Jaroslav's concerns as well, but there is clearly a category of issues
> where user-space functionality is broken due to a mismatch of element
> names between UCM file and the driver. I.e. the actual jack detection
> (codec hw and its driver) is working, but due to wrong UCM file chosen,
> wrong driven is chosen, or errors in either UCM or driver, event does not
> get parsed right and user ends with no audio.
>
> A pure user-space test harness would not catch these, and building full
> automation of external codec events, is a complex task and coverage of
> devices is likely limited.
>
> The 'edid_override' debugfs interface in i915 is a bit similar. It allows
> inject EDID content irrespectively of the actual monitor connected.
>
>> Also, the first patch can be omitted if you just create another
>> snd_jack_report() function for the user API and put the common code to the
>> static function in jack.c.
> ++votes on this
OK, got it,  am preparing the v2 RFC, will send it out soon. Thanks for 
your comment.
>
> Br, Kai

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-12-14 10:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-09 12:43 [RFC][PATCH 0/2] design a way to change audio Jack state by software Hui Wang
2020-12-09 12:43 ` [RFC][PATCH 1/2] alsa: jack: expand snd_jack_report parameter for jack sw_inject Hui Wang
2020-12-09 12:43 ` [RFC][PATCH 2/2] alsa: jack: adding support for software jack in or out injection Hui Wang
2020-12-10 10:51   ` kernel test robot
2020-12-09 14:25 ` [RFC][PATCH 0/2] design a way to change audio Jack state by software Jaroslav Kysela
2020-12-09 14:44   ` Takashi Iwai
2020-12-10  1:54     ` Hui Wang
2020-12-11 13:36   ` Kai Vehmanen
2020-12-14 10:48     ` Hui Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.