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@xxxxxxxxx> 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
>>>
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
