diff mbox series

[3/3] drm/i915: Nuke PCH_JSP

Message ID 20220630150600.24611-4-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: PCH type cleanup | expand

Commit Message

Ville Syrjälä June 30, 2022, 3:06 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

JSP is based on ICP and we don't really need to differentiate
between the two. So let's just delcare JSP to be ICP.

The only slight change here is for Wa_14011294188 which we
used to apply for JSP but now we'll only apply to MCC. This
should be fine since the issue being dealt with was introduced
in TGP and inherited into MCC. JSP being derived from ICP
should not need this workaround.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
 drivers/gpu/drm/i915/intel_pch.c                   | 3 ++-
 drivers/gpu/drm/i915/intel_pch.h                   | 4 +---
 3 files changed, 4 insertions(+), 5 deletions(-)

Comments

Murthy, Arun R June 30, 2022, 3:55 p.m. UTC | #1
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Thursday, June 30, 2022 8:36 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 3/3] drm/i915: Nuke PCH_JSP
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> JSP is based on ICP and we don't really need to differentiate between the
> two. So let's just delcare JSP to be ICP.
> 
> The only slight change here is for Wa_14011294188 which we used to apply
> for JSP but now we'll only apply to MCC. This should be fine since the issue
> being dealt with was introduced in TGP and inherited into MCC. JSP being
> derived from ICP should not need this workaround.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
>  drivers/gpu/drm/i915/intel_pch.c                   | 3 ++-
>  drivers/gpu/drm/i915/intel_pch.h                   | 4 +---
>  3 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> b/drivers/gpu/drm/i915/display/intel_display_power.c
> index a9cb27f1c964..589af257edeb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1608,7 +1608,7 @@ static void icl_display_core_init(struct
> drm_i915_private *dev_priv,
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> 
>  	/* Wa_14011294188:ehl,jsl,tgl,rkl,adl-s */
> -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_JSP &&
> +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP &&
>  	    INTEL_PCH_TYPE(dev_priv) < PCH_DG1)
>  		intel_de_rmw(dev_priv, SOUTH_DSPCLK_GATE_D, 0,
>  			     PCH_DPMGUNIT_CLOCK_GATE_DISABLE); diff --git
> a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
> index b45c504c6f03..0fec25be146a 100644
> --- a/drivers/gpu/drm/i915/intel_pch.c
> +++ b/drivers/gpu/drm/i915/intel_pch.c
> @@ -128,7 +128,8 @@ intel_pch_type(const struct drm_i915_private
> *dev_priv, unsigned short id)
>  	case INTEL_PCH_JSP_DEVICE_ID_TYPE:
>  		drm_dbg_kms(&dev_priv->drm, "Found Jasper Lake PCH\n");
>  		drm_WARN_ON(&dev_priv->drm, !IS_JSL_EHL(dev_priv));
> -		return PCH_JSP;
> +		/* JSP is ICP compatible */
> +		return PCH_ICP;
>  	case INTEL_PCH_ADP_DEVICE_ID_TYPE:
>  	case INTEL_PCH_ADP2_DEVICE_ID_TYPE:
>  	case INTEL_PCH_ADP3_DEVICE_ID_TYPE:
> diff --git a/drivers/gpu/drm/i915/intel_pch.h
> b/drivers/gpu/drm/i915/intel_pch.h
> index 07f6f5517968..7c8ce9781d1a 100644
> --- a/drivers/gpu/drm/i915/intel_pch.h
> +++ b/drivers/gpu/drm/i915/intel_pch.h
> @@ -22,8 +22,7 @@ enum intel_pch {
>  	PCH_LPT,	/* Lynxpoint/Wildcatpoint PCH */
>  	PCH_SPT,        /* Sunrisepoint/Kaby Lake PCH */
>  	PCH_CNP,        /* Cannon/Comet Lake PCH */
> -	PCH_ICP,	/* Ice Lake PCH */
> -	PCH_JSP,	/* Jasper Lake PCH */
> +	PCH_ICP,	/* Ice Lake/Jasper Lake PCH */

Only in i915_irq icp is used and in the rest of the i915 code icl is used leading to confusion.
I would rather suggest to use ICL since most of the driver references it as icl and also change icp in i915_irq to icl.
This is just my opinion, other can comment over here.

Thanks and Regards,
Arun R Murthy
--------------------
Ville Syrjälä June 30, 2022, 4:59 p.m. UTC | #2
On Thu, Jun 30, 2022 at 03:55:57PM +0000, Murthy, Arun R wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> > Syrjala
> > Sent: Thursday, June 30, 2022 8:36 PM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: [Intel-gfx] [PATCH 3/3] drm/i915: Nuke PCH_JSP
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > JSP is based on ICP and we don't really need to differentiate between the
> > two. So let's just delcare JSP to be ICP.
> > 
> > The only slight change here is for Wa_14011294188 which we used to apply
> > for JSP but now we'll only apply to MCC. This should be fine since the issue
> > being dealt with was introduced in TGP and inherited into MCC. JSP being
> > derived from ICP should not need this workaround.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
> >  drivers/gpu/drm/i915/intel_pch.c                   | 3 ++-
> >  drivers/gpu/drm/i915/intel_pch.h                   | 4 +---
> >  3 files changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index a9cb27f1c964..589af257edeb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -1608,7 +1608,7 @@ static void icl_display_core_init(struct
> > drm_i915_private *dev_priv,
> >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > 
> >  	/* Wa_14011294188:ehl,jsl,tgl,rkl,adl-s */
> > -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_JSP &&
> > +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP &&
> >  	    INTEL_PCH_TYPE(dev_priv) < PCH_DG1)
> >  		intel_de_rmw(dev_priv, SOUTH_DSPCLK_GATE_D, 0,
> >  			     PCH_DPMGUNIT_CLOCK_GATE_DISABLE); diff --git
> > a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
> > index b45c504c6f03..0fec25be146a 100644
> > --- a/drivers/gpu/drm/i915/intel_pch.c
> > +++ b/drivers/gpu/drm/i915/intel_pch.c
> > @@ -128,7 +128,8 @@ intel_pch_type(const struct drm_i915_private
> > *dev_priv, unsigned short id)
> >  	case INTEL_PCH_JSP_DEVICE_ID_TYPE:
> >  		drm_dbg_kms(&dev_priv->drm, "Found Jasper Lake PCH\n");
> >  		drm_WARN_ON(&dev_priv->drm, !IS_JSL_EHL(dev_priv));
> > -		return PCH_JSP;
> > +		/* JSP is ICP compatible */
> > +		return PCH_ICP;
> >  	case INTEL_PCH_ADP_DEVICE_ID_TYPE:
> >  	case INTEL_PCH_ADP2_DEVICE_ID_TYPE:
> >  	case INTEL_PCH_ADP3_DEVICE_ID_TYPE:
> > diff --git a/drivers/gpu/drm/i915/intel_pch.h
> > b/drivers/gpu/drm/i915/intel_pch.h
> > index 07f6f5517968..7c8ce9781d1a 100644
> > --- a/drivers/gpu/drm/i915/intel_pch.h
> > +++ b/drivers/gpu/drm/i915/intel_pch.h
> > @@ -22,8 +22,7 @@ enum intel_pch {
> >  	PCH_LPT,	/* Lynxpoint/Wildcatpoint PCH */
> >  	PCH_SPT,        /* Sunrisepoint/Kaby Lake PCH */
> >  	PCH_CNP,        /* Cannon/Comet Lake PCH */
> > -	PCH_ICP,	/* Ice Lake PCH */
> > -	PCH_JSP,	/* Jasper Lake PCH */
> > +	PCH_ICP,	/* Ice Lake/Jasper Lake PCH */
> 
> Only in i915_irq icp is used and in the rest of the i915 code icl is used leading to confusion.
> I would rather suggest to use ICL since most of the driver references it as icl and also change icp in i915_irq to icl.
> This is just my opinion, other can comment over here.

ICL is a CPU, ICP is a PCH.
Jani Nikula July 1, 2022, 9:57 a.m. UTC | #3
On Thu, 30 Jun 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> JSP is based on ICP and we don't really need to differentiate
> between the two. So let's just delcare JSP to be ICP.
>
> The only slight change here is for Wa_14011294188 which we
> used to apply for JSP but now we'll only apply to MCC. This
> should be fine since the issue being dealt with was introduced
> in TGP and inherited into MCC. JSP being derived from ICP
> should not need this workaround.

I'll take your word for it.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
>  drivers/gpu/drm/i915/intel_pch.c                   | 3 ++-
>  drivers/gpu/drm/i915/intel_pch.h                   | 4 +---
>  3 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index a9cb27f1c964..589af257edeb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1608,7 +1608,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
>  	/* Wa_14011294188:ehl,jsl,tgl,rkl,adl-s */
> -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_JSP &&
> +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP &&
>  	    INTEL_PCH_TYPE(dev_priv) < PCH_DG1)
>  		intel_de_rmw(dev_priv, SOUTH_DSPCLK_GATE_D, 0,
>  			     PCH_DPMGUNIT_CLOCK_GATE_DISABLE);
> diff --git a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
> index b45c504c6f03..0fec25be146a 100644
> --- a/drivers/gpu/drm/i915/intel_pch.c
> +++ b/drivers/gpu/drm/i915/intel_pch.c
> @@ -128,7 +128,8 @@ intel_pch_type(const struct drm_i915_private *dev_priv, unsigned short id)
>  	case INTEL_PCH_JSP_DEVICE_ID_TYPE:
>  		drm_dbg_kms(&dev_priv->drm, "Found Jasper Lake PCH\n");
>  		drm_WARN_ON(&dev_priv->drm, !IS_JSL_EHL(dev_priv));
> -		return PCH_JSP;
> +		/* JSP is ICP compatible */
> +		return PCH_ICP;
>  	case INTEL_PCH_ADP_DEVICE_ID_TYPE:
>  	case INTEL_PCH_ADP2_DEVICE_ID_TYPE:
>  	case INTEL_PCH_ADP3_DEVICE_ID_TYPE:
> diff --git a/drivers/gpu/drm/i915/intel_pch.h b/drivers/gpu/drm/i915/intel_pch.h
> index 07f6f5517968..7c8ce9781d1a 100644
> --- a/drivers/gpu/drm/i915/intel_pch.h
> +++ b/drivers/gpu/drm/i915/intel_pch.h
> @@ -22,8 +22,7 @@ enum intel_pch {
>  	PCH_LPT,	/* Lynxpoint/Wildcatpoint PCH */
>  	PCH_SPT,        /* Sunrisepoint/Kaby Lake PCH */
>  	PCH_CNP,        /* Cannon/Comet Lake PCH */
> -	PCH_ICP,	/* Ice Lake PCH */
> -	PCH_JSP,	/* Jasper Lake PCH */
> +	PCH_ICP,	/* Ice Lake/Jasper Lake PCH */
>  	PCH_TGP,	/* Tiger Lake/Mule Creek Canyon PCH */
>  	PCH_ADP,	/* Alder Lake PCH */
>  
> @@ -67,7 +66,6 @@ enum intel_pch {
>  #define HAS_PCH_DG2(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_DG2)
>  #define HAS_PCH_ADP(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_ADP)
>  #define HAS_PCH_DG1(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_DG1)
> -#define HAS_PCH_JSP(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_JSP)
>  #define HAS_PCH_TGP(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_TGP)
>  #define HAS_PCH_ICP(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_ICP)
>  #define HAS_PCH_CNP(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_CNP)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index a9cb27f1c964..589af257edeb 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -1608,7 +1608,7 @@  static void icl_display_core_init(struct drm_i915_private *dev_priv,
 	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 
 	/* Wa_14011294188:ehl,jsl,tgl,rkl,adl-s */
-	if (INTEL_PCH_TYPE(dev_priv) >= PCH_JSP &&
+	if (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP &&
 	    INTEL_PCH_TYPE(dev_priv) < PCH_DG1)
 		intel_de_rmw(dev_priv, SOUTH_DSPCLK_GATE_D, 0,
 			     PCH_DPMGUNIT_CLOCK_GATE_DISABLE);
diff --git a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
index b45c504c6f03..0fec25be146a 100644
--- a/drivers/gpu/drm/i915/intel_pch.c
+++ b/drivers/gpu/drm/i915/intel_pch.c
@@ -128,7 +128,8 @@  intel_pch_type(const struct drm_i915_private *dev_priv, unsigned short id)
 	case INTEL_PCH_JSP_DEVICE_ID_TYPE:
 		drm_dbg_kms(&dev_priv->drm, "Found Jasper Lake PCH\n");
 		drm_WARN_ON(&dev_priv->drm, !IS_JSL_EHL(dev_priv));
-		return PCH_JSP;
+		/* JSP is ICP compatible */
+		return PCH_ICP;
 	case INTEL_PCH_ADP_DEVICE_ID_TYPE:
 	case INTEL_PCH_ADP2_DEVICE_ID_TYPE:
 	case INTEL_PCH_ADP3_DEVICE_ID_TYPE:
diff --git a/drivers/gpu/drm/i915/intel_pch.h b/drivers/gpu/drm/i915/intel_pch.h
index 07f6f5517968..7c8ce9781d1a 100644
--- a/drivers/gpu/drm/i915/intel_pch.h
+++ b/drivers/gpu/drm/i915/intel_pch.h
@@ -22,8 +22,7 @@  enum intel_pch {
 	PCH_LPT,	/* Lynxpoint/Wildcatpoint PCH */
 	PCH_SPT,        /* Sunrisepoint/Kaby Lake PCH */
 	PCH_CNP,        /* Cannon/Comet Lake PCH */
-	PCH_ICP,	/* Ice Lake PCH */
-	PCH_JSP,	/* Jasper Lake PCH */
+	PCH_ICP,	/* Ice Lake/Jasper Lake PCH */
 	PCH_TGP,	/* Tiger Lake/Mule Creek Canyon PCH */
 	PCH_ADP,	/* Alder Lake PCH */
 
@@ -67,7 +66,6 @@  enum intel_pch {
 #define HAS_PCH_DG2(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_DG2)
 #define HAS_PCH_ADP(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_ADP)
 #define HAS_PCH_DG1(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_DG1)
-#define HAS_PCH_JSP(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_JSP)
 #define HAS_PCH_TGP(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_TGP)
 #define HAS_PCH_ICP(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_ICP)
 #define HAS_PCH_CNP(dev_priv)			(INTEL_PCH_TYPE(dev_priv) == PCH_CNP)