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

Takashi Iwai
2023-07-19T12:47:50Z

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@linux.intel.com>
> ---
> 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 &&
>