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

Takashi Iwai
2023-07-19T06:02:06Z

On Tue, 18 Jul 2023 19:04:41 +0200,
Kai Vehmanen wrote:
>
> > diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
> > index f1fd5b44aaac9..344b61576c0e3 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, true);
> > + ret = snd_hdac_i915_init(bus, false);
> > if (ret < 0)
> > return ret;
>
> My only bigger concern is corner cases where the display PCI device is on
> the bus and visible to kernel, but for some reason there is no working
> driver in the system or it is disabled.
>
> With this patch, not having a workign display driver means that there is
> also no audio in the system as the SOF driver will never get probed.
>
> In current mainline, one will get the 60sec timeout warning and then
> audio driver will proceed to probe and you'll have audio support (minus
> HDMI/DP).

Yeah, that was a concern in the past, too. e.g. when you pass
"nomodeset" boot option, the driver will become unusable, even if the
bus is used generically for both analog and HDMI codecs.

> This is mostly an issue with very new hardware (e.g. hw is still
> behind force_probe flag in xe/i915 driver), but we've had some odd
> cases with e.g. systems with both Intel IGFX and other vendors' DGPU.
> Audio drivers see the Intel VGA controller in system and will
> call snd_hdac_i915_init(), but the audio component bind will never
> succeed if the the Intel IGFX is not in actual use.
>
> Will need a bit of time to think about possible scenarios. Possibly this
> is not an issue outside early development systems. In theory if IGFX is
> disabled in BIOS, and not visible to OS, we are good, and if it's visible,
> the i915/xe driver should be loaded, so we are good again.

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

thanks,

Takashi