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

[Aditya Swarup]

On 11/25/20 11:29 AM, Lucas De Marchi wrote:
> 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.

Thanks for the suggestion. I will implement it this way.

Aditya

> 
> Lucas De Marchi
> 
>>
>>> -Chris
>>>
>>