Re: [PATCH 1/4] ALSA: hda: Skip i915 initialization on CNL/LKF-based platforms

[Cezary Rojewski <cezary.rojewski@xxxxxxxxx>]

On 2024-02-23 9:47 AM, Takashi Iwai wrote:
> On Thu, 22 Feb 2024 21:54:42 +0100,
> Rodrigo Vivi wrote:

...

> > > > > @@ -127,15 +128,26 @@ static int i915_component_master_match(struct device *dev, int subcomponent,
> > > > >    /* check whether Intel graphics is present and reachable */
> > > > >    static int i915_gfx_present(struct pci_dev *hdac_pci)
> > > > >    {
> > > > > +	/* List of known platforms with no i915 support. */
> > > > > +	static struct pci_device_id denylist[] = {
> > > > > +		INTEL_CNL_IDS(NULL),
> > > > > +		INTEL_LKF_IDS(NULL),
> > > > > +		{ 0 }
> > > > > +	};
> > > >
> > > > I thought these don't actually exist in the wild?
> > >
> > > To my knowledge the opposite is true - while LKFs were shipped in limited
> > > number, they still were. I did ask few weeks ago my friends from Windows
> > > side about the support and they're still running full-scopes on HDMI
> > > endpoints on LKF platforms in their CIs. It seems the drm support is there
> > > though. Once you re-boot to linux we get -19 during probe().
> > >
> > > In regard to CNL, the commit removing CNL-support left the IDs intact what's
> >
> > I would prefer to go the other way around and remove the unused/unsupported
> > IDs entirely and for good.
> >
> > > very handy to us - we have a lot of spare CNL boards for our validation
> > > purposes - CNL-based AudioDSP spans multiple platforms, e.g.:
> > > CNL/CFL/WHL/CML. The number of newer boards is lower, unfortunately.
> >
> > Well, I do see your point here and you are not asking for us to add gfx
> > support back, but only help to have this protection here.
> >
> > However I'm afraid that these entries in the list would only cause
> > further confusion. Couldn't they get defined inside your .c directly as
> > a const deny_list? so when we go there and remove the missing bits
> > of CNL we don't conflict or cause undersired issues to you.
>
> That makes sense.  Maybe drm people would get rid of the dead CNL*()
> definitions from the header as a cleanup in near future, and we'll hit
> a trouble.

Another, more robust solution could be to expose list of recognized 
devices by drivers/gpu/drm/i915/i915_pci.c in a header. Rather than 
guess, as we do in i915_gfx_present(), we would just loop over IDs in 
such list.

As I'm unsure if such solution is viable, what I'll do for now is: send 
v2 with relevant IDs moved over to sound/ tree, leaving the i915 header 
alone. Incremental update can be provided later if a neater solution 
appears on the horizon.


Kind regards,
Czarek