2023-08-07 09:09:47

by Maarten Lankhorst

[permalink] [raw]
Subject: [PATCH v3 3/9] ALSA: hda/i915: Add an allow_modprobe argument to snd_hdac_i915_init

Xe is a new GPU driver that re-uses the display (and sound) code from
i915. It's no longer possible to load i915, as the GPU can be driven
by the xe driver instead.

The new behavior will return -EPROBE_DEFER, and wait for a compatible
driver to be loaded instead of modprobing i915.

Converting all drivers at the same time is a lot of work, instead we
will convert each user one by one.

Changes since v1:
- Use dev_err_probe to set a probe reason for debugfs' deferred_devices.

Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Kai Vehmanen <[email protected]>
---
include/sound/hda_i915.h | 4 ++--
sound/hda/hdac_i915.c | 8 ++++----
sound/pci/hda/hda_intel.c | 2 +-
sound/soc/intel/avs/core.c | 2 +-
sound/soc/intel/skylake/skl.c | 2 +-
sound/soc/sof/intel/hda-codec.c | 2 +-
6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/sound/hda_i915.h b/include/sound/hda_i915.h
index 6b79614a893b..f91bd6636086 100644
--- a/include/sound/hda_i915.h
+++ b/include/sound/hda_i915.h
@@ -9,12 +9,12 @@

#ifdef CONFIG_SND_HDA_I915
void snd_hdac_i915_set_bclk(struct hdac_bus *bus);
-int snd_hdac_i915_init(struct hdac_bus *bus);
+int snd_hdac_i915_init(struct hdac_bus *bus, bool allow_modprobe);
#else
static inline void snd_hdac_i915_set_bclk(struct hdac_bus *bus)
{
}
-static inline int snd_hdac_i915_init(struct hdac_bus *bus)
+static inline int snd_hdac_i915_init(struct hdac_bus *bus, bool allow_modprobe)
{
return -ENODEV;
}
diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index c32709fa4115..961fcd3397f4 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -155,7 +155,7 @@ static int i915_gfx_present(struct pci_dev *hdac_pci)
*
* Returns zero for success or a negative error code.
*/
-int snd_hdac_i915_init(struct hdac_bus *bus)
+int snd_hdac_i915_init(struct hdac_bus *bus, bool allow_modprobe)
{
struct drm_audio_component *acomp;
int err;
@@ -171,7 +171,7 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
acomp = bus->audio_component;
if (!acomp)
return -ENODEV;
- if (!acomp->ops) {
+ if (allow_modprobe && !acomp->ops) {
if (!IS_ENABLED(CONFIG_MODULES) ||
!request_module("i915")) {
/* 60s timeout */
@@ -180,9 +180,9 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
}
}
if (!acomp->ops) {
- dev_info(bus->dev, "couldn't bind with audio component\n");
+ int err = allow_modprobe ? -ENODEV : -EPROBE_DEFER;
snd_hdac_acomp_exit(bus);
- return -ENODEV;
+ return dev_err_probe(bus->dev, err, "couldn't bind with audio component\n");
}
return 0;
}
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 0d2d6bc6c75e..11cf9907f039 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2277,7 +2277,7 @@ static int azx_probe_continue(struct azx *chip)

/* bind with i915 if needed */
if (chip->driver_caps & AZX_DCAPS_I915_COMPONENT) {
- err = snd_hdac_i915_init(bus);
+ err = snd_hdac_i915_init(bus, true);
if (err < 0) {
/* if the controller is bound only with HDMI/DP
* (for HSW and BDW), we need to abort the probe;
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index 637501850728..3311a6f14200 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -191,7 +191,7 @@ static void avs_hda_probe_work(struct work_struct *work)

pm_runtime_set_active(bus->dev); /* clear runtime_error flag */

- ret = snd_hdac_i915_init(bus);
+ ret = snd_hdac_i915_init(bus, true);
if (ret < 0)
dev_info(bus->dev, "i915 init unsuccessful: %d\n", ret);

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 998bd0232cf1..4d93b8690467 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -791,7 +791,7 @@ static int skl_i915_init(struct hdac_bus *bus)
* The HDMI codec is in GPU so we need to ensure that it is powered
* up and ready for probe
*/
- err = snd_hdac_i915_init(bus);
+ err = snd_hdac_i915_init(bus, true);
if (err < 0)
return err;

diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
index 8a5e99a898ec..f1fd5b44aaac 100644
--- a/sound/soc/sof/intel/hda-codec.c
+++ b/sound/soc/sof/intel/hda-codec.c
@@ -415,7 +415,7 @@ int hda_codec_i915_init(struct snd_sof_dev *sdev)
return 0;

/* i915 exposes a HDA codec for HDMI audio */
- ret = snd_hdac_i915_init(bus);
+ ret = snd_hdac_i915_init(bus, true);
if (ret < 0)
return ret;

--
2.39.2



2023-08-07 16:30:56

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [PATCH v3 3/9] ALSA: hda/i915: Add an allow_modprobe argument to snd_hdac_i915_init



On 8/7/23 04:00, Maarten Lankhorst wrote:
> Xe is a new GPU driver that re-uses the display (and sound) code from
> i915. It's no longer possible to load i915, as the GPU can be driven
> by the xe driver instead.
>
> The new behavior will return -EPROBE_DEFER, and wait for a compatible
> driver to be loaded instead of modprobing i915.
>
> Converting all drivers at the same time is a lot of work, instead we
> will convert each user one by one.
>
> Changes since v1:
> - Use dev_err_probe to set a probe reason for debugfs' deferred_devices.

You want the changes below the --- line ...
>
> Signed-off-by: Maarten Lankhorst <[email protected]>
> Reviewed-by: Kai Vehmanen <[email protected]>

Reviewed-by: Pierre-Louis Bossart <[email protected]>

> ---

... here


> include/sound/hda_i915.h | 4 ++--
> sound/hda/hdac_i915.c | 8 ++++----
> sound/pci/hda/hda_intel.c | 2 +-
> sound/soc/intel/avs/core.c | 2 +-
> sound/soc/intel/skylake/skl.c | 2 +-
> sound/soc/sof/intel/hda-codec.c | 2 +-
> 6 files changed, 10 insertions(+), 10 deletions(-)


2023-08-12 08:59:09

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH v3 3/9] ALSA: hda/i915: Add an allow_modprobe argument to snd_hdac_i915_init

On Mon, 07 Aug 2023 16:08:32 +0200,
Pierre-Louis Bossart wrote:
>
>
>
> On 8/7/23 04:00, Maarten Lankhorst wrote:
> > Xe is a new GPU driver that re-uses the display (and sound) code from
> > i915. It's no longer possible to load i915, as the GPU can be driven
> > by the xe driver instead.
> >
> > The new behavior will return -EPROBE_DEFER, and wait for a compatible
> > driver to be loaded instead of modprobing i915.
> >
> > Converting all drivers at the same time is a lot of work, instead we
> > will convert each user one by one.
> >
> > Changes since v1:
> > - Use dev_err_probe to set a probe reason for debugfs' deferred_devices.
>
> You want the changes below the --- line ...

Note that there are subsystems preferring keeping the version change
logs in the commit log (typically found in drm trees), although
majority of subsystems (including sound) want rather cleaner logs,
AFAIK.


Takashi

2023-08-12 15:13:13

by Maarten Lankhorst

[permalink] [raw]
Subject: Re: [PATCH v3 3/9] ALSA: hda/i915: Add an allow_modprobe argument to snd_hdac_i915_init

Hey,

On 2023-08-12 10:21, Takashi Iwai wrote:
> On Mon, 07 Aug 2023 16:08:32 +0200,
> Pierre-Louis Bossart wrote:
>>
>>
>>
>> On 8/7/23 04:00, Maarten Lankhorst wrote:
>>> Xe is a new GPU driver that re-uses the display (and sound) code from
>>> i915. It's no longer possible to load i915, as the GPU can be driven
>>> by the xe driver instead.
>>>
>>> The new behavior will return -EPROBE_DEFER, and wait for a compatible
>>> driver to be loaded instead of modprobing i915.
>>>
>>> Converting all drivers at the same time is a lot of work, instead we
>>> will convert each user one by one.
>>>
>>> Changes since v1:
>>> - Use dev_err_probe to set a probe reason for debugfs' deferred_devices.
>>
>> You want the changes below the --- line ...
>
> Note that there are subsystems preferring keeping the version change
> logs in the commit log (typically found in drm trees), although
> majority of subsystems (including sound) want rather cleaner logs,
> AFAIK.
Yeah, I usually maintain stuff in drm. :)

Cheers,
~Maarten