From: "Souza, Jose" <jose.souza@intel.com>
To: "Swarup, Aditya" <aditya.swarup@intel.com>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>
Cc: "Nikula, Jani" <jani.nikula@intel.com>,
"De Marchi, Lucas" <lucas.demarchi@intel.com>
Subject: Re: [Intel-gfx] [PATCH 02/21] drm/i915/tgl: Fix macros for TGL SOC based WA
Date: Tue, 17 Nov 2020 19:03:05 +0000 [thread overview]
Message-ID: <02ea7d2b7eb151787abe8cf42771db40a7f0ef7d.camel@intel.com> (raw)
In-Reply-To: <20201117185029.22078-3-aditya.swarup@intel.com>
On Tue, 2020-11-17 at 10:50 -0800, Aditya Swarup wrote:
> Fix macros for applying TGL SOC WAs by using INTEL_REVID()
> as index to fetch correct revision offset in TGL GT/DISP stepping
> table.
Please explain what exactly is the issue you are fixing, the change you did in tgl_revids_get() + IS_TGL_GT_REVID looks a improvement but not a fix.
>
> Also, remove redundant macros and simplify it to use GT and DISP
> macros for getting applicable stepping for TGL.
>
> 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>
> ---
> .../drm/i915/display/intel_display_power.c | 2 +-
> drivers/gpu/drm/i915/display/intel_psr.c | 4 ++--
> drivers/gpu/drm/i915/display/intel_sprite.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 20 ++++++++--------
> drivers/gpu/drm/i915/i915_drv.h | 24 +++++++------------
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> 6 files changed, 24 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index fe2d90bba536..06c036e2092c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -5283,7 +5283,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> int config, i;
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> if (IS_DG1_REVID(dev_priv, DG1_REVID_A0, DG1_REVID_A0) ||
> - IS_TGL_DISP_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B0))
> + IS_TGL_DISP_REVID(dev_priv, REVID_A0, REVID_B0))
> /* Wa_1409767108:tgl,dg1 */
> table = wa_1409767108_buddy_page_masks;
> else
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index b3631b722de3..c057a03b2ed4 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -550,7 +550,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> if (dev_priv->psr.psr2_sel_fetch_enabled) {
> /* WA 1408330847 */
> - if (IS_TGL_DISP_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0) ||
> + if (IS_TGL_DISP_REVID(dev_priv, REVID_A0, REVID_A0) ||
> IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_A0))
> intel_de_rmw(dev_priv, CHICKEN_PAR1_1,
> DIS_RAM_BYPASS_PSR2_MAN_TRACK,
> @@ -1102,7 +1102,7 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp)
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> /* WA 1408330847 */
> if (dev_priv->psr.psr2_sel_fetch_enabled &&
> - (IS_TGL_DISP_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0) ||
> + (IS_TGL_DISP_REVID(dev_priv, REVID_A0, REVID_A0) ||
> IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_A0)))
> intel_de_rmw(dev_priv, CHICKEN_PAR1_1,
> DIS_RAM_BYPASS_PSR2_MAN_TRACK, 0);
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index a3ab44694118..f7da4a56054e 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -3022,7 +3022,7 @@ static bool gen12_plane_supports_mc_ccs(struct drm_i915_private *dev_priv,
> {
> /* Wa_14010477008:tgl[a0..c0],rkl[all],dg1[all] */
> if (IS_DG1(dev_priv) || IS_ROCKETLAKE(dev_priv) ||
> - IS_TGL_DISP_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_C0))
> + IS_TGL_DISP_REVID(dev_priv, REVID_A0, REVID_C0))
> return false;
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> return plane_id < PLANE_SPRITE4;
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index a82554baa6ac..d756155d82ea 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -71,16 +71,16 @@ const struct i915_rev_steppings kbl_revids[] = {
> };
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> const struct i915_rev_steppings tgl_uy_revids[] = {
> - [0] = { .gt_stepping = TGL_REVID_A0, .disp_stepping = TGL_REVID_A0 },
> - [1] = { .gt_stepping = TGL_REVID_B0, .disp_stepping = TGL_REVID_C0 },
> - [2] = { .gt_stepping = TGL_REVID_B1, .disp_stepping = TGL_REVID_C0 },
> - [3] = { .gt_stepping = TGL_REVID_C0, .disp_stepping = TGL_REVID_D0 },
> + [0] = { .gt_stepping = REVID_A0, .disp_stepping = REVID_A0 },
> + [1] = { .gt_stepping = REVID_B0, .disp_stepping = REVID_C0 },
> + [2] = { .gt_stepping = REVID_B1, .disp_stepping = REVID_C0 },
> + [3] = { .gt_stepping = REVID_C0, .disp_stepping = REVID_D0 },
> };
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> /* Same GT stepping between tgl_uy_revids and tgl_revids don't mean the same HW */
> const struct i915_rev_steppings tgl_revids[] = {
> - [0] = { .gt_stepping = TGL_REVID_A0, .disp_stepping = TGL_REVID_B0 },
> - [1] = { .gt_stepping = TGL_REVID_B0, .disp_stepping = TGL_REVID_D0 },
> + [0] = { .gt_stepping = REVID_A0, .disp_stepping = REVID_B0 },
> + [1] = { .gt_stepping = REVID_B0, .disp_stepping = REVID_D0 },
> };
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> static void wa_init_start(struct i915_wa_list *wal, const char *name, const char *engine_name)
> @@ -1250,13 +1250,13 @@ tgl_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
> gen12_gt_workarounds_init(i915, wal);
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> /* Wa_1409420604:tgl */
> - if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> + if (IS_TGL_GT_REVID(i915, REVID_A0, REVID_A0))
> wa_write_or(wal,
> SUBSLICE_UNIT_LEVEL_CLKGATE2,
> CPSSUNIT_CLKGATE_DIS);
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> /* Wa_1607087056:tgl also know as BUG:1409180338 */
> - if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> + if (IS_TGL_GT_REVID(i915, REVID_A0, REVID_A0))
> wa_write_or(wal,
> SLICE_UNIT_LEVEL_CLKGATE,
> L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
> @@ -1734,7 +1734,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> struct drm_i915_private *i915 = engine->i915;
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> if (IS_DG1_REVID(i915, DG1_REVID_A0, DG1_REVID_A0) ||
> - IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> + IS_TGL_GT_REVID(i915, REVID_A0, REVID_A0)) {
> /*
> * Wa_1607138336:tgl[a0],dg1[a0]
> * Wa_1607063988:tgl[a0],dg1[a0]
> @@ -1744,7 +1744,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> GEN12_DISABLE_POSH_BUSY_FF_DOP_CG);
> }
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> - if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> + if (IS_TGL_GT_REVID(i915, REVID_A0, REVID_A0)) {
> /*
> * Wa_1606679103:tgl
> * (see also Wa_1606682166:icl)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 15be8debae54..437916aacaa6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1565,11 +1565,11 @@ extern const struct i915_rev_steppings kbl_revids[];
> (IS_JSL_EHL(p) && IS_REVID(p, since, until))
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> enum {
> - TGL_REVID_A0,
> - TGL_REVID_B0,
> - TGL_REVID_B1,
> - TGL_REVID_C0,
> - TGL_REVID_D0,
> + REVID_A0,
> + REVID_B0,
> + REVID_B1,
> + REVID_C0,
> + REVID_D0,
Better keep "TGL_" otherwise this could be used in other platforms that have different values for each revision.
> };
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> extern const struct i915_rev_steppings tgl_uy_revids[];
> @@ -1579,9 +1579,9 @@ static inline const struct i915_rev_steppings *
> tgl_revids_get(struct drm_i915_private *dev_priv)
> {
> if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv))
> - return tgl_uy_revids;
> + return tgl_uy_revids + INTEL_REVID(dev_priv);
> else
> - return tgl_revids;
> + return tgl_revids + INTEL_REVID(dev_priv);
better do tgl_revids[INTEL_REVID(dev_priv)] with a array size check first.
> }
>
>
>
>
>
>
>
>
> #define IS_TGL_DISP_REVID(p, since, until) \
> @@ -1589,16 +1589,10 @@ tgl_revids_get(struct drm_i915_private *dev_priv)
> tgl_revids_get(p)->disp_stepping >= (since) && \
> tgl_revids_get(p)->disp_stepping <= (until))
>
>
>
>
>
>
>
>
> -#define IS_TGL_UY_GT_REVID(p, since, until) \
> - ((IS_TGL_U(p) || IS_TGL_Y(p)) && \
> - tgl_uy_revids->gt_stepping >= (since) && \
> - tgl_uy_revids->gt_stepping <= (until))
> -
> #define IS_TGL_GT_REVID(p, since, until) \
> (IS_TIGERLAKE(p) && \
> - !(IS_TGL_U(p) || IS_TGL_Y(p)) && \
> - tgl_revids->gt_stepping >= (since) && \
> - tgl_revids->gt_stepping <= (until))
> + tgl_revids_get(p)->gt_stepping >= (since) && \
> + tgl_revids_get(p)->gt_stepping <= (until))
>
>
>
>
>
>
>
>
> #define RKL_REVID_A0 0x0
> #define RKL_REVID_B0 0x1
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a20b5051f18c..69840aa0d4db 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7110,7 +7110,7 @@ static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
> ILK_DPFC_CHICKEN_COMP_DUMMY_PIXEL);
>
>
>
>
>
>
>
>
> /* Wa_1409825376:tgl (pre-prod)*/
> - if (IS_TGL_DISP_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B1))
> + if (IS_TGL_DISP_REVID(dev_priv, REVID_A0, REVID_B1))
> I915_WRITE(GEN9_CLKGATE_DIS_3, I915_READ(GEN9_CLKGATE_DIS_3) |
> TGL_VRH_GATING_DIS);
>
>
>
>
>
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-11-17 19:03 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-17 18:50 [Intel-gfx] [PATCH 00/21] Introduce Alderlake-S Aditya Swarup
2020-11-17 18:50 ` [Intel-gfx] [PATCH 01/21] drm/i915/dg1: Enable ports Aditya Swarup
2020-11-17 18:50 ` [Intel-gfx] [PATCH 02/21] drm/i915/tgl: Fix macros for TGL SOC based WA Aditya Swarup
2020-11-17 19:03 ` Souza, Jose [this message]
2020-11-17 19:28 ` Lucas De Marchi
2020-11-17 19:33 ` Souza, Jose
2020-11-18 7:56 ` Lucas De Marchi
2020-11-17 19:31 ` Lucas De Marchi
2020-11-18 9:18 ` Jani Nikula
2020-11-24 1:32 ` Aditya Swarup
2020-11-24 13:14 ` Lucas De Marchi
2020-11-24 14:20 ` Jani Nikula
2020-11-24 20:11 ` Lucas De Marchi
2020-11-25 0:48 ` Aditya Swarup
2020-11-25 8:36 ` Jani Nikula
2020-11-17 18:50 ` [Intel-gfx] [PATCH 03/21] drm/i915/adl_s: Add ADL-S platform info and PCI ids Aditya Swarup
2020-11-17 19:17 ` Jani Nikula
2020-11-24 1:50 ` Aditya Swarup
2020-11-24 9:28 ` Jani Nikula
2020-11-17 18:50 ` [Intel-gfx] [PATCH 04/21] x86/gpu: add ADL_S stolen memory support Aditya Swarup
2020-11-17 18:50 ` [Intel-gfx] [PATCH 05/21] drm/i915/adl_s: Add PCH support Aditya Swarup
2020-11-20 0:09 ` Matt Roper
2020-11-17 18:50 ` [Intel-gfx] [PATCH 06/21] drm/i915/adl_s: Add Interrupt Support Aditya Swarup
2020-11-20 0:12 ` Matt Roper
2020-11-17 18:50 ` [Intel-gfx] [PATCH 07/21] drm/i915/adl_s: Add PHYs for Alderlake S Aditya Swarup
2020-11-20 0:20 ` Matt Roper
2020-11-17 18:50 ` [Intel-gfx] [PATCH 08/21] drm/i915/adl_s: Configure DPLL for ADL-S Aditya Swarup
2020-11-17 18:50 ` [Intel-gfx] [PATCH 09/21] drm/i915/adl_s: Configure Port clock registers " Aditya Swarup
2020-11-17 18:50 ` [Intel-gfx] [PATCH 10/21] drm/i915/adl_s: Add HTI support and initialize display " Aditya Swarup
2020-11-20 0:27 ` Matt Roper
2020-11-17 18:50 ` [Intel-gfx] [PATCH 11/21] drm/i915/adl_s: Add adl-s ddc pin mapping Aditya Swarup
2020-11-20 0:33 ` Matt Roper
2020-11-17 18:50 ` [Intel-gfx] [PATCH 12/21] drm/i915/adl_s: Add vbt port and aux channel settings for adls Aditya Swarup
2020-11-17 18:50 ` [Intel-gfx] [PATCH 13/21] drm/i915/adl_s: Update combo PHY master/slave relationships Aditya Swarup
2020-11-25 23:38 ` Srivatsa, Anusha
2020-11-17 18:50 ` [Intel-gfx] [PATCH 14/21] drm/i915/adl_s: Update PHY_MISC programming Aditya Swarup
2020-11-17 18:50 ` [Intel-gfx] [PATCH 15/21] drm/i915/adl_s: Add display, gt, ctx and ADL-S Aditya Swarup
2020-12-01 18:46 ` Srivatsa, Anusha
2020-12-01 20:51 ` Lucas De Marchi
2020-11-17 18:50 ` [Intel-gfx] [PATCH 16/21] drm/i915/adl_s: MCHBAR memory info registers are moved Aditya Swarup
2020-11-20 20:18 ` Lucas De Marchi
2020-11-20 20:39 ` Caz Yokoyama
2020-11-25 0:11 ` Lucas De Marchi
2020-11-17 18:50 ` [Intel-gfx] [PATCH 17/21] drm/i915/adl_s: Add power wells Aditya Swarup
2020-11-17 18:50 ` [Intel-gfx] [PATCH 18/21] drm/i915/adl_s: Re-use TGL GuC/HuC firmware Aditya Swarup
2020-11-25 22:52 ` Srivatsa, Anusha
2020-11-17 18:50 ` [Intel-gfx] [PATCH 19/21] drm/i915/display: Add HAS_D12_PLANE_MINIMIZATION Aditya Swarup
2020-12-01 18:35 ` Srivatsa, Anusha
2020-11-17 18:50 ` [Intel-gfx] [PATCH 20/21] drm/i915/adl_s: Load DMC Aditya Swarup
2020-11-17 18:50 ` [Intel-gfx] [PATCH 21/21] drm/i915/adl_s: Update memory bandwidth parameters Aditya Swarup
2020-11-25 22:46 ` Srivatsa, Anusha
2020-11-18 1:28 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Introduce Alderlake-S (rev2) Patchwork
2020-11-18 1:29 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-11-18 1:57 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-11-18 7:53 ` [Intel-gfx] [PATCH 00/21] Introduce Alderlake-S Lucas De Marchi
2020-11-18 15:14 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for Introduce Alderlake-S (rev2) Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=02ea7d2b7eb151787abe8cf42771db40a7f0ef7d.camel@intel.com \
--to=jose.souza@intel.com \
--cc=aditya.swarup@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
--cc=lucas.demarchi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox