[Intel-gfx] [PATCH] drm/i915/tgl: Fix REVID macros for TGL to fetch correct stepping

[Lucas De Marchi]

On Wed, Nov 25, 2020 at 09:51:04AM -0800, Aditya Swarup wrote:
>On 11/25/20 7:33 AM, Chris Wilson wrote:
>> Quoting Jani Nikula (2020-11-25 11:45:56)
>>> On Tue, 24 Nov 2020, Aditya Swarup <aditya.swarup at intel.com> wrote:
>>>> +     if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
>>>> +             if (TGL_UY_REVID_RANGE(revid)) {
>>>> +                     return tgl_uy_revids + revid;
>>>> +             } else {
>>>> +                     drm_dbg_kms(&dev_priv->drm,
>>>> +                                 "Unsupported SOC stepping found %u, using %lu instead\n",
>>>> +                                 revid, ARRAY_SIZE(tgl_uy_revids) - 1);
>>
>> Also please don't have a dbg for every single IS_TGL_*_REVID
>> invocation. And this is not _kms, but driver; better yet, don't bother
>> with a drm_dbg_kms here at all.
>>
>> If you want to actually check, add something like
>> intel_detect_preproduction_hw() and warn about unknown future revids.
>> Or include the info when we print the revid in the caps.
>
>So, what you are suggesting is add an info print in that function intel_detect_preproduction_hw() right?
>Or something else?

since this is all going away soon, just removing the dbg would be ok

And in that case, just doing something like below would be shorter and
clearer IMO (untested):

	if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
		arr = tgl_uy_revids;
		size = ARRAY_SIZE(tgl_uy_revids);
	} else {
		arr = tgl_revids;
		size = ARRAY_SIZE(tgl_revids);
	}
		
	revid = min(revid, size - 1);

	return &arr[revid];

That may also be 2 patches:  one adding the revid so we actually apply
the correct workarounds (this needs the "Fixes" tag) and the other to
add the bounds check.

Lucas De Marchi

>
>> -Chris
>>
>