Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 19 Jul 2023 08:01:00 +0200
Message-ID: <874jm0modf.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Kai Vehmanen <kai.vehmanen@linux.intel.com>
Cc:     Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Alsa-devel <alsa-devel@alsa-project.org>,
        sound-open-firmware@alsa-project.org, linux-kernel@vger.kernel.org,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>,
        Cezary Rojewski <cezary.rojewski@intel.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
        Liam Girdwood <liam.r.girdwood@linux.intel.com>,
        Peter Ujfalusi <peter.ujfalusi@linux.intel.com>,
        Bard Liao <yung-chuan.liao@linux.intel.com>,
        Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
        Mark Brown <broonie@kernel.org>,
        Daniel Baluta <daniel.baluta@nxp.com>,
        Matthew Auld <matthew.auld@intel.com>
Subject: Re: [PATCH 6/7] ASoC: SOF: Intel: Remove deferred probe for SOF
In-Reply-To: <alpine.DEB.2.22.394.2307181922160.3532114@eliteleevi.tm.intel.com>
References: <20230718084522.116952-1-maarten.lankhorst@linux.intel.com>
        <20230718084522.116952-7-maarten.lankhorst@linux.intel.com>
        <alpine.DEB.2.22.394.2307181922160.3532114@eliteleevi.tm.intel.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

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