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

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

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

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