Re: [PATCH v3 1/2] drm/i915/jsl: Split EHL/JSL platform info and PCI ids

On Fri, Oct 02, 2020 at 02:54:34AM +0530, Tejas Upadhyay wrote:

Split the basic platform definition, macros, and PCI IDs to differentiate between EHL and JSL platforms.

Changes since V2 :

• Added IS_EHL_JSL to replace IS_ELKHARTLAKE
• EHL/JSL PCI ids split added

Changes since V1 :

• IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
• Reverted EHL/JSL PCI ids split change

Signed-off-by: Tejas Upadhyay tejaskumarx.surendrakumar.upadhyay@intel.com

drivers/gpu/drm/i915/display/icl_dsi.c | 4 ++-- drivers/gpu/drm/i915/display/intel_cdclk.c | 4 ++-- drivers/gpu/drm/i915/display/intel_combo_phy.c | 6 +++--- drivers/gpu/drm/i915/display/intel_display.c | 8 ++++---- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 16 ++++++++-------- drivers/gpu/drm/i915/gt/intel_sseu.c | 2 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 5 +++-- drivers/gpu/drm/i915/i915_pci.c | 9 +++++++++ drivers/gpu/drm/i915/intel_device_info.c | 1 + drivers/gpu/drm/i915/intel_device_info.h | 1 + drivers/gpu/drm/i915/intel_pch.c | 6 +++--- include/drm/i915_pciids.h | 9 ++++++--- 14 files changed, 45 insertions(+), 30 deletions(-)

<snip>

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 6c580d0d9ea8..5d81a55993e5 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1839,7 +1839,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) GEN12_FF_TESSELATION_DOP_GATE_DISABLE);

/* Wa_22010271021:ehl */

• if (IS_ELKHARTLAKE(i915))
  

• if (IS_EHL_JSL(i915))
  wa_masked_en(wal,
  	     GEN9_CS_DEBUG_MODE1,
  	     FF_DOP_CLOCK_GATE_DISABLE);
  

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index eef9a821c49c..2c75bcb76932 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1417,7 +1417,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define IS_COMETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_COMETLAKE) #define IS_CANNONLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_CANNONLAKE) #define IS_ICELAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ICELAKE) -#define IS_ELKHARTLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)

Why are we removing this? I would just keep it + add the IS_JASPERLAKE() too.

+#define IS_EHL_JSL(dev_priv) (IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE) || \

• 		IS_PLATFORM(dev_priv, INTEL_JASPERLAKE))
  

A bit non-standard thing, but I guess it's fine. We already have the oddball IS_GEN9_{LP,BC}() anyway.

But since JSL is the canonical name used by Bspec I would make that our "primary" name for this. Ie. I would call the macro IS_JSL_EHL(). And since I'm a bit ocd about ordering things I would generally make it so that JSL comes first, followed by EHL, everywhere.

#define IS_TIGERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_TIGERLAKE) #define IS_ROCKETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE) #define IS_DG1(dev_priv) IS_PLATFORM(dev_priv, INTEL_DG1) @@ -1559,7 +1560,7 @@ extern const struct i915_rev_steppings kbl_revids[]; #define EHL_REVID_A0 0x0

#define IS_EHL_REVID(p, since, until) \

• (IS_ELKHARTLAKE(p) && IS_REVID(p, since, until))

• (IS_EHL_JSL(p) && IS_REVID(p, since, until))

That is confusing. Either we want just IS_ELKHARTLAKE() here, or this macro should also be renamed to contain JSL in the name.

enum { TGL_REVID_A0, diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 366ddfc8df6b..8690b69fcf33 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -846,6 +846,14 @@ static const struct intel_device_info ehl_info = { .ppgtt_size = 36, };

+static const struct intel_device_info jsl_info = {

• GEN11_FEATURES,
• PLATFORM(INTEL_JASPERLAKE),
• .require_force_probe = 1,
• .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VCS0) | BIT(VECS0),
• .ppgtt_size = 36,

+};

#define GEN12_FEATURES \ GEN11_FEATURES, \ GEN(12), \ @@ -985,6 +993,7 @@ static const struct pci_device_id pciidlist[] = { INTEL_CNL_IDS(&cnl_info), INTEL_ICL_11_IDS(&icl_info), INTEL_EHL_IDS(&ehl_info),

• INTEL_JSL_IDS(&jsl_info), INTEL_TGL_12_IDS(&tgl_info), INTEL_RKL_IDS(&rkl_info), {0, 0, 0}

diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index adc836f15fde..e67cec8fa2aa 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -62,6 +62,7 @@ static const char * const platform_names[] = { PLATFORM_NAME(CANNONLAKE), PLATFORM_NAME(ICELAKE), PLATFORM_NAME(ELKHARTLAKE),

• PLATFORM_NAME(JASPERLAKE), PLATFORM_NAME(TIGERLAKE), PLATFORM_NAME(ROCKETLAKE), PLATFORM_NAME(DG1),

diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 6a3d607218aa..d92fa041c700 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -79,6 +79,7 @@ enum intel_platform { /* gen11 */ INTEL_ICELAKE, INTEL_ELKHARTLAKE,

• INTEL_JASPERLAKE, /* gen12 */ INTEL_TIGERLAKE, INTEL_ROCKETLAKE,

diff --git a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c index 6c97192e9ca8..72ffb6d4d5fc 100644 --- a/drivers/gpu/drm/i915/intel_pch.c +++ b/drivers/gpu/drm/i915/intel_pch.c @@ -115,7 +115,7 @@ intel_pch_type(const struct drm_i915_private *dev_priv, unsigned short id) return PCH_ICP; case INTEL_PCH_MCC_DEVICE_ID_TYPE: drm_dbg_kms(&dev_priv->drm, "Found Mule Creek Canyon PCH\n");

• drm_WARN_ON(&dev_priv->drm, !IS_ELKHARTLAKE(dev_priv));
  

• drm_WARN_ON(&dev_priv->drm, !IS_EHL_JSL(dev_priv));
  

  return PCH_MCC; case INTEL_PCH_TGP_DEVICE_ID_TYPE: case INTEL_PCH_TGP2_DEVICE_ID_TYPE:

@@ -126,7 +126,7 @@ intel_pch_type(const struct drm_i915_private *dev_priv, unsigned short id) case INTEL_PCH_JSP_DEVICE_ID_TYPE: case INTEL_PCH_JSP2_DEVICE_ID_TYPE: drm_dbg_kms(&dev_priv->drm, "Found Jasper Lake PCH\n");

• drm_WARN_ON(&dev_priv->drm, !IS_ELKHARTLAKE(dev_priv));
  

• drm_WARN_ON(&dev_priv->drm, !IS_EHL_JSL(dev_priv));
  

  return PCH_JSP; default: return PCH_NONE;

@@ -157,7 +157,7 @@ intel_virt_detect_pch(const struct drm_i915_private *dev_priv)

if (IS_TIGERLAKE(dev_priv) || IS_ROCKETLAKE(dev_priv)) id = INTEL_PCH_TGP_DEVICE_ID_TYPE;

• else if (IS_ELKHARTLAKE(dev_priv))

• else if (IS_EHL_JSL(dev_priv)) id = INTEL_PCH_MCC_DEVICE_ID_TYPE; else if (IS_ICELAKE(dev_priv)) id = INTEL_PCH_ICP_DEVICE_ID_TYPE;

diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 7eeecb07c9a1..1b5e09cfa11e 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -579,15 +579,18 @@ INTEL_VGA_DEVICE(0x8A51, info), \ INTEL_VGA_DEVICE(0x8A5D, info)

-/* EHL/JSL */ +/* EHL */ #define INTEL_EHL_IDS(info) \ INTEL_VGA_DEVICE(0x4500, info), \ INTEL_VGA_DEVICE(0x4571, info), \ INTEL_VGA_DEVICE(0x4551, info), \ INTEL_VGA_DEVICE(0x4541, info), \

• INTEL_VGA_DEVICE(0x4E71, info), \ INTEL_VGA_DEVICE(0x4557, info), \
• INTEL_VGA_DEVICE(0x4555, info), \

• INTEL_VGA_DEVICE(0x4555, info)

+/* JSL */ +#define INTEL_JSL_IDS(info) \

• INTEL_VGA_DEVICE(0x4E71, info), \ INTEL_VGA_DEVICE(0x4E61, info), \ INTEL_VGA_DEVICE(0x4E57, info), \ INTEL_VGA_DEVICE(0x4E55, info), \

-- 2.28.0