[v2] sound: Use -EPROBE_DEFER instead of i915 module loading.

[PATCH v2 4/9] ALSA: hda/i915: Allow xe as match for i915_component_master_match

Posted by Maarten Lankhorst 2 years, 6 months ago

xe is a new driver for intel GPU's that shares the sound related code
with i915.

Don't allow it to be modprobed though; the module is not upstream yet
and we should exclusively use the EPROBE_DEFER mechanism.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 sound/hda/hdac_i915.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index 961fcd3397f40..12c1f8d93499f 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -115,7 +115,8 @@ static int i915_component_master_match(struct device *dev, int subcomponent,
 	hdac_pci = to_pci_dev(bus->dev);
 	i915_pci = to_pci_dev(dev);
 
-	if (!strcmp(dev->driver->name, "i915") &&
+	if ((!strcmp(dev->driver->name, "i915") ||
+		 !strcmp(dev->driver->name, "xe")) &&
 	    subcomponent == I915_COMPONENT_AUDIO &&
 	    connectivity_check(i915_pci, hdac_pci))
 		return 1;
-- 
2.39.2

Re: [PATCH v2 4/9] ALSA: hda/i915: Allow xe as match for i915_component_master_match

Posted by Pierre-Louis Bossart 2 years, 6 months ago


On 7/19/23 18:41, Maarten Lankhorst wrote:
> xe is a new driver for intel GPU's that shares the sound related code
> with i915.
> 
> Don't allow it to be modprobed though; the module is not upstream yet
> and we should exclusively use the EPROBE_DEFER mechanism.

I can't figure out what this comment means.

how would the -EPROBE_DEFER mechanism help if the driver that will
trigger a new probe is not upstream?

Not following at all what you intended to explain.

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  sound/hda/hdac_i915.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
> index 961fcd3397f40..12c1f8d93499f 100644
> --- a/sound/hda/hdac_i915.c
> +++ b/sound/hda/hdac_i915.c
> @@ -115,7 +115,8 @@ static int i915_component_master_match(struct device *dev, int subcomponent,
>  	hdac_pci = to_pci_dev(bus->dev);
>  	i915_pci = to_pci_dev(dev);
>  
> -	if (!strcmp(dev->driver->name, "i915") &&
> +	if ((!strcmp(dev->driver->name, "i915") ||
> +		 !strcmp(dev->driver->name, "xe")) &&
>  	    subcomponent == I915_COMPONENT_AUDIO &&
>  	    connectivity_check(i915_pci, hdac_pci))
>  		return 1;

Re: [PATCH v2 4/9] ALSA: hda/i915: Allow xe as match for i915_component_master_match

Posted by Maarten Lankhorst 2 years, 6 months ago

Hey,

On 2023-07-24 12:28, Pierre-Louis Bossart wrote:
> 
> 
> On 7/19/23 18:41, Maarten Lankhorst wrote:
>> xe is a new driver for intel GPU's that shares the sound related code
>> with i915.
>>
>> Don't allow it to be modprobed though; the module is not upstream yet
>> and we should exclusively use the EPROBE_DEFER mechanism.
> 
> I can't figure out what this comment means.
> 
> how would the -EPROBE_DEFER mechanism help if the driver that will
> trigger a new probe is not upstream?
> 
> Not following at all what you intended to explain.

What I mean is that there is code inside the current code that does 
request_module("i915"), the comment meant I didn't try to add any logic 
for request_module("xe"), as the driver is not merged yet.

Additionally I am removing the request_module logic, but this comment 
was written when I first tried the simple solution of request_module("xe").

Turns out telepathy is hard, and using -EPROBE_DEFER is much simpler. :-)

Cheers,
~Maarten

Re: [PATCH v2 4/9] ALSA: hda/i915: Allow xe as match for i915_component_master_match

Posted by Péter Ujfalusi 2 years, 6 months ago


On 19/07/2023 19:41, Maarten Lankhorst wrote:
> xe is a new driver for intel GPU's that shares the sound related code
> with i915.
> 
> Don't allow it to be modprobed though; the module is not upstream yet
> and we should exclusively use the EPROBE_DEFER mechanism.

Reviewed-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  sound/hda/hdac_i915.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
> index 961fcd3397f40..12c1f8d93499f 100644
> --- a/sound/hda/hdac_i915.c
> +++ b/sound/hda/hdac_i915.c
> @@ -115,7 +115,8 @@ static int i915_component_master_match(struct device *dev, int subcomponent,
>  	hdac_pci = to_pci_dev(bus->dev);
>  	i915_pci = to_pci_dev(dev);
>  
> -	if (!strcmp(dev->driver->name, "i915") &&
> +	if ((!strcmp(dev->driver->name, "i915") ||
> +		 !strcmp(dev->driver->name, "xe")) &&
>  	    subcomponent == I915_COMPONENT_AUDIO &&
>  	    connectivity_check(i915_pci, hdac_pci))
>  		return 1;

-- 
Péter