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

[Takashi Iwai <tiwai@suse.de>]

On Thu, 22 Feb 2024 21:54:42 +0100,
Rodrigo Vivi wrote:
> 
> On Thu, Feb 22, 2024 at 06:53:12PM +0100, Cezary Rojewski wrote:
> > On 2024-02-22 6:24 PM, Ville Syrjälä wrote:
> > > On Thu, Feb 22, 2024 at 06:06:11PM +0100, Cezary Rojewski wrote:
> > > > Commit 78f613ba1efb ("drm/i915: finish removal of CNL") and its friends
> > > > removed support for i915 for all CNL-based platforms. HDAudio library,
> > > > however, still treats such platforms as valid candidates for i915
> > > > binding. Update query mechanism to reflect changes made in drm tree.
> > > > 
> > > > At the same time, i915 support for LKF-based platforms has not been
> > > > provided so remove them from valid binding candidates.
> > 
> > ...
> > 
> > > > @@ -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.


thanks,

Takashi