Re: [PATCH] ASoC: SOF: Intel: Remove deferred probe for SOF

[Takashi Iwai]

On Wed, 19 Jul 2023 14:13:59 +0200,
Maarten Lankhorst wrote:
> 
> 
> On 2023-07-19 13:06, Takashi Iwai wrote:
> > On Wed, 19 Jul 2023 11:48:06 +0200,
> > Maarten Lankhorst wrote:
> >> 
> >>      The 60 seconds timeout is a thing "better than complete disablement",
> >>      so it's not ideal, either.  Maybe we can add something like the
> >>      following:
> >>           - Check when the deferred probe takes too long, and warn
> >> it
> >>      - Provide some runtime option to disable the component binding, so
> >>        that user can work around it if needed
> >>      A module option to snd_hdac_i915_init would probably be the
> >> least of all evils
> >> here.
> > 
> > Yes, probably it's the easiest option and sufficient.
> > 
> > 
> > thanks,
> > 
> > Takashi
> Hey,
> 
> Patch below, can be applied immediately iresspective of the other patches.
> 
> ---->8----------
> 
> Selecting CONFIG_DRM selects CONFIG_VIDEO_NOMODESET, which exports
> video_firmware_drivers_only(). This can be used as a first
> approximation
> on whether i915 will be available. It's safe to use as this is only
> built when CONFIG_SND_HDA_I915 is selected by CONFIG_I915.
> 
> It's not completely fool proof, as you can boot with "nomodeset
> i915.modeset=1" to make i915 load regardless, or use
> "i915.force_probe=!*" to never load i915, but the common case of booting
> with nomodeset to disable all GPU drivers this will work as intended.

The check of video_firmware_drivers_only() may help a bit, but I
believe we still need an option to override the behavior, from the
same reason as why i915.modeset option behaves so.  In general,
nomodeset is for a debugging purpose, and without an option, you'll
have no way to re-enable the HD-audio even if you could reload the
graphics driver.


thanks,

Takashi

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
> index 1637dc6e630a6..90bcf84f7b2ce 100644
> --- a/sound/hda/hdac_i915.c
> +++ b/sound/hda/hdac_i915.c
> @@ -11,6 +11,8 @@
>  #include <sound/hda_i915.h>
>  #include <sound/hda_register.h>
> 
> +#include <video/nomodeset.h>
> +
>  #define IS_HSW_CONTROLLER(pci) (((pci)->device == 0x0a0c) || \
>  				((pci)->device == 0x0c0c) || \
>  				((pci)->device == 0x0d0c) || \
> @@ -122,6 +124,9 @@ static int i915_gfx_present(struct pci_dev *hdac_pci)
>  {
>  	struct pci_dev *display_dev = NULL;
> 
> +	if (video_firmware_drivers_only())
> +		return false;
> +
>  	for_each_pci_dev(display_dev) {
>  		if (display_dev->vendor == PCI_VENDOR_ID_INTEL &&
>  		    (display_dev->class >> 16) == PCI_BASE_CLASS_DISPLAY &&
>