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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@intel.com> wrote:
> >>> Fix TGL REVID macros to fetch correct display/gt stepping based
> >>> on SOC rev id from INTEL_REVID() macro. Previously, we were just
> >>> returning the first element of the revid array instead of using
> >>> the correct index based on SOC rev id.
> >>>
> >>> Also, add array bound checks for TGL REV ID array. Since, there
> >>> might be a possibility of using older kernels on latest platform
> >>> revision, resulting in out of bounds access for rev ID array.
> >>> In this scenario, print message for unsupported rev ID and apply
> >>> settings for latest rev ID available.
> >>>
> >>> Fixes: ("drm/i915/tgl: Fix stepping WA matching")
> >>> Cc: José Roberto de Souza <jose.souza@intel.com>
> >>> Cc: Matt Roper <matthew.d.roper@intel.com>
> >>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >>> Cc: Jani Nikula <jani.nikula@intel.com>
> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/i915_drv.h | 35 +++++++++++++++++++++++++++------
> >>>  1 file changed, 29 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>> index 15be8debae54..29d55b7017be 100644
> >>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>> @@ -1572,16 +1572,37 @@ enum {
> >>>       TGL_REVID_D0,
> >>>  };
> >>>  
> >>> -extern const struct i915_rev_steppings tgl_uy_revids[];
> >>> -extern const struct i915_rev_steppings tgl_revids[];
> >>> +extern const struct i915_rev_steppings tgl_uy_revids[4];
> >>> +extern const struct i915_rev_steppings tgl_revids[2];
> >>
> >> Just a quick note, the compiler does not check that the size in the
> >> extern declaration matches the size in the array definition. So you
> >> might end up with a mismatch without noticing.
> 
> Yes.. We will have to take care of it if we are adding rev id to array table(which mostly
> should remain a const once we decide to go upstream). Without this declaration, I cannot
> use ARRAY_SIZE() macro with revid arrays as the sizeof() operator complains about not
> knowing the size of the array in question as it is an extern declaration. 

Can't you replace the ARRAY_SIZE() with a sentinel? I guess
Making it a struct with a size member would be another option.

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