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.swa...@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


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to