Re: [Intel-gfx] [PATCH v2 07/12] drm/i915/icl: Use revid->stepping tables

2021-07-13 Thread Lucas De Marchi

On Tue, Jul 13, 2021 at 12:59:53PM -0700, Matt Roper wrote:

On Tue, Jul 13, 2021 at 12:57:07PM -0700, Lucas De Marchi wrote:

On Fri, Jul 09, 2021 at 08:37:19PM -0700, Matt Roper wrote:
> Switch ICL to use a revid->stepping table as we're trying to do on all
> platforms going forward.  While we're at it, let's include some
> additional steppings that have popped up, even if we don't yet have any
> workarounds tied to those steppings (we probably need to audit our
> workaround list soon to see if any of the bounds have moved or if new
> workarounds have appeared).
>
> Note that the current bspec table is missing information about how to
> map PCI revision ID to GT/display steppings; it only provides an SoC
> stepping.  The mapping to GT/display steppings (which aren't always the
> same as the SoC stepping) used to be in the bspec, but was apparently
> dropped during an update in Nov 2019; I've made my changes here based on
> an older bspec snapshot that still had the necessary information.  We've
> requested that the missing information be restored.
>
> I'm only including the production revids in the table here since we're
> past the point at which we usually stop trying to support pre-production
> hardware.  An appropriate check is added to
> intel_detect_preproduction_hw() to print an error and taint the kernel
> just in case someone still tries to load the driver on old
> pre-production hardware.
>
> v2:
> - Drop pre-production steppings and add error/taint at startup when
>   loading on pre-production hardware.

oh... I forgot to send my review. Here is the commend I had:

It seems we are not actually dropping the WAs. We have several applying
only to A0 or A0/B0. From your first paragraph, is the intention to do
an audit of the WA ranges later?  Because we are currently running
without applying those WAs, so those are effectively dead code.


The actual dropping of workarounds for pre-production steppings happens
in patch #12.  But a more in-depth audit will be done in the future.


ahh, ok. Makes sense then.

Thanks
Lucas De Marchi




Matt



Lucas De Marchi


--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH v2 07/12] drm/i915/icl: Use revid->stepping tables

2021-07-13 Thread Matt Roper
On Tue, Jul 13, 2021 at 12:57:07PM -0700, Lucas De Marchi wrote:
> On Fri, Jul 09, 2021 at 08:37:19PM -0700, Matt Roper wrote:
> > Switch ICL to use a revid->stepping table as we're trying to do on all
> > platforms going forward.  While we're at it, let's include some
> > additional steppings that have popped up, even if we don't yet have any
> > workarounds tied to those steppings (we probably need to audit our
> > workaround list soon to see if any of the bounds have moved or if new
> > workarounds have appeared).
> > 
> > Note that the current bspec table is missing information about how to
> > map PCI revision ID to GT/display steppings; it only provides an SoC
> > stepping.  The mapping to GT/display steppings (which aren't always the
> > same as the SoC stepping) used to be in the bspec, but was apparently
> > dropped during an update in Nov 2019; I've made my changes here based on
> > an older bspec snapshot that still had the necessary information.  We've
> > requested that the missing information be restored.
> > 
> > I'm only including the production revids in the table here since we're
> > past the point at which we usually stop trying to support pre-production
> > hardware.  An appropriate check is added to
> > intel_detect_preproduction_hw() to print an error and taint the kernel
> > just in case someone still tries to load the driver on old
> > pre-production hardware.
> > 
> > v2:
> > - Drop pre-production steppings and add error/taint at startup when
> >   loading on pre-production hardware.
> 
> oh... I forgot to send my review. Here is the commend I had:
> 
> It seems we are not actually dropping the WAs. We have several applying
> only to A0 or A0/B0. From your first paragraph, is the intention to do
> an audit of the WA ranges later?  Because we are currently running
> without applying those WAs, so those are effectively dead code.

The actual dropping of workarounds for pre-production steppings happens
in patch #12.  But a more in-depth audit will be done in the future.


Matt

> 
> Lucas De Marchi

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 07/12] drm/i915/icl: Use revid->stepping tables

2021-07-13 Thread Lucas De Marchi

On Fri, Jul 09, 2021 at 08:37:19PM -0700, Matt Roper wrote:

Switch ICL to use a revid->stepping table as we're trying to do on all
platforms going forward.  While we're at it, let's include some
additional steppings that have popped up, even if we don't yet have any
workarounds tied to those steppings (we probably need to audit our
workaround list soon to see if any of the bounds have moved or if new
workarounds have appeared).

Note that the current bspec table is missing information about how to
map PCI revision ID to GT/display steppings; it only provides an SoC
stepping.  The mapping to GT/display steppings (which aren't always the
same as the SoC stepping) used to be in the bspec, but was apparently
dropped during an update in Nov 2019; I've made my changes here based on
an older bspec snapshot that still had the necessary information.  We've
requested that the missing information be restored.

I'm only including the production revids in the table here since we're
past the point at which we usually stop trying to support pre-production
hardware.  An appropriate check is added to
intel_detect_preproduction_hw() to print an error and taint the kernel
just in case someone still tries to load the driver on old
pre-production hardware.

v2:
- Drop pre-production steppings and add error/taint at startup when
  loading on pre-production hardware.


oh... I forgot to send my review. Here is the commend I had:

It seems we are not actually dropping the WAs. We have several applying
only to A0 or A0/B0. From your first paragraph, is the intention to do
an audit of the WA ranges later?  Because we are currently running
without applying those WAs, so those are effectively dead code.

Lucas De Marchi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 07/12] drm/i915/icl: Use revid->stepping tables

2021-07-13 Thread Souza, Jose
On Fri, 2021-07-09 at 20:37 -0700, Matt Roper wrote:
> Switch ICL to use a revid->stepping table as we're trying to do on all
> platforms going forward.  While we're at it, let's include some
> additional steppings that have popped up, even if we don't yet have any
> workarounds tied to those steppings (we probably need to audit our
> workaround list soon to see if any of the bounds have moved or if new
> workarounds have appeared).
> 
> Note that the current bspec table is missing information about how to
> map PCI revision ID to GT/display steppings; it only provides an SoC
> stepping.  The mapping to GT/display steppings (which aren't always the
> same as the SoC stepping) used to be in the bspec, but was apparently
> dropped during an update in Nov 2019; I've made my changes here based on
> an older bspec snapshot that still had the necessary information.  We've
> requested that the missing information be restored.
> 
> I'm only including the production revids in the table here since we're
> past the point at which we usually stop trying to support pre-production
> hardware.  An appropriate check is added to
> intel_detect_preproduction_hw() to print an error and taint the kernel
> just in case someone still tries to load the driver on old
> pre-production hardware.
> 
> v2:
>  - Drop pre-production steppings and add error/taint at startup when
>loading on pre-production hardware.
> 

Reviewed-by: José Roberto de Souza 

> Bspec: 21141  # pre-Nov 2019 snapshot
> Signed-off-by: Matt Roper 
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 12 ++--
>  drivers/gpu/drm/i915/i915_drv.c |  1 +
>  drivers/gpu/drm/i915/i915_drv.h | 10 ++
>  drivers/gpu/drm/i915/intel_step.c   |  7 +++
>  4 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 6dfd564e078f..e2d8acb8c1c9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -557,7 +557,7 @@ static void icl_ctx_workarounds_init(struct 
> intel_engine_cs *engine,
>   /* Wa_1604370585:icl (pre-prod)
>* Formerly known as WaPushConstantDereferenceHoldDisable
>*/
> - if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_B0))
> + if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_B0))
>   wa_masked_en(wal, GEN7_ROW_CHICKEN2,
>PUSH_CONSTANT_DEREF_DISABLE);
>  
> @@ -573,12 +573,12 @@ static void icl_ctx_workarounds_init(struct 
> intel_engine_cs *engine,
>   /* Wa_2006611047:icl (pre-prod)
>* Formerly known as WaDisableImprovedTdlClkGating
>*/
> - if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0))
> + if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_A0))
>   wa_masked_en(wal, GEN7_ROW_CHICKEN2,
>GEN11_TDL_CLOCK_GATING_FIX_DISABLE);
>  
>   /* Wa_2006665173:icl (pre-prod) */
> - if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0))
> + if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_A0))
>   wa_masked_en(wal, GEN11_COMMON_SLICE_CHICKEN3,
>GEN11_BLEND_EMB_FIX_DISABLE_IN_RCC);
>  
> @@ -1023,13 +1023,13 @@ icl_gt_workarounds_init(struct drm_i915_private 
> *i915, struct i915_wa_list *wal)
>   GAMW_ECO_DEV_CTX_RELOAD_DISABLE);
>  
>   /* Wa_1405779004:icl (pre-prod) */
> - if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0))
> + if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_A0))
>   wa_write_or(wal,
>   SLICE_UNIT_LEVEL_CLKGATE,
>   MSCUNIT_CLKGATE_DIS);
>  
>   /* Wa_1406838659:icl (pre-prod) */
> - if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_B0))
> + if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_B0))
>   wa_write_or(wal,
>   INF_UNIT_LEVEL_CLKGATE,
>   CGPSF_CLKGATE_DIS);
> @@ -1725,7 +1725,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, 
> struct i915_wa_list *wal)
>   PMFLUSHDONE_LNEBLK);
>  
>   /* Wa_1406609255:icl (pre-prod) */
> - if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_B0))
> + if (IS_ICL_GT_STEP(i915, STEP_A0, STEP_B0))
>   wa_write_or(wal,
>   GEN7_SARCHKMD,
>   GEN7_DISABLE_DEMAND_PREFETCH);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 90136995f5eb..c43b698bf0b9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -275,6 +275,7 @@ static void intel_detect_preproduction_hw(struct 
> drm_i915_private *dev_priv)
>   pre |= IS_BROXTON(dev_priv) && INTEL_REVID(dev_priv) < 0xA;
>   pre |= IS_KABYLAKE(dev_priv) && INTEL_REVID(dev_priv) < 0x1;
>   pre |= IS_GEMINILAKE(dev_priv) &&