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

[Cezary Rojewski <cezary.rojewski@intel.com>]

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