diff mbox series

[2/2] drm/i915/cml: Introduce Comet Lake PCH

Message ID 20190305214657.22399-2-anusha.srivatsa@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915/cml: Add CML PCI IDS | expand

Commit Message

Srivatsa, Anusha March 5, 2019, 9:46 p.m. UTC 
From: Anusha Srivatsa <anusha.srivatsa@intel.com>

Comet Lake PCH is based off of Cannon Point(CNP).
Add PCI ID for Comet Lake PCH.

v2: Code cleanup (DK)

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 4 ++++
 drivers/gpu/drm/i915/i915_drv.h | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Rodrigo Vivi March 6, 2019, 6:07 p.m. UTC | #1 
On Tue, Mar 05, 2019 at 01:46:56PM -0800, Anusha wrote:
> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> 
> Comet Lake PCH is based off of Cannon Point(CNP).
> Add PCI ID for Comet Lake PCH.
> 
> v2: Code cleanup (DK)
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 4 ++++
>  drivers/gpu/drm/i915/i915_drv.h | 3 ++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1b2f5a6f8c25..e787c999b2c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -188,6 +188,10 @@ intel_pch_type(const struct drm_i915_private *dev_priv, unsigned short id)
>  		DRM_DEBUG_KMS("Found Cannon Lake LP PCH (CNP-LP)\n");
>  		WARN_ON(!IS_CANNONLAKE(dev_priv) && !IS_COFFEELAKE(dev_priv));
>  		return PCH_CNP;
> +	case INTEL_PCH_CMP_DEVICE_ID_TYPE:
> +		DRM_DEBUG_KMS("Found Comet Lake LP PCH (CMP)\n");
> +		WARN_ON(!IS_CANNONLAKE(dev_priv) && !IS_COFFEELAKE(dev_priv));

Please remove CNL from this list...

also please add a comment here before the return

     	    /* CM has same south display engine from CNP */

> +		return PCH_CNP;
>  	case INTEL_PCH_ICP_DEVICE_ID_TYPE:
>  		DRM_DEBUG_KMS("Found Ice Lake PCH\n");
>  		WARN_ON(!IS_ICELAKE(dev_priv));
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 453af7438e67..55298e19e740 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -530,7 +530,7 @@ enum intel_pch {
>  	PCH_LPT,	/* Lynxpoint/Wildcatpoint PCH */
>  	PCH_SPT,        /* Sunrisepoint PCH */
>  	PCH_KBP,        /* Kaby Lake PCH */
> -	PCH_CNP,        /* Cannon Lake PCH */
> +	PCH_CNP,        /* Cannon/Comet Lake PCH */
>  	PCH_ICP,	/* Ice Lake PCH */
>  	PCH_NOP,	/* PCH without south display */
>  };
> @@ -2552,6 +2552,7 @@ static inline unsigned int i915_sg_segment_size(void)
>  #define INTEL_PCH_KBP_DEVICE_ID_TYPE		0xA280
>  #define INTEL_PCH_CNP_DEVICE_ID_TYPE		0xA300
>  #define INTEL_PCH_CNP_LP_DEVICE_ID_TYPE		0x9D80
> +#define INTEL_PCH_CMP_DEVICE_ID_TYPE		0x0280
>  #define INTEL_PCH_ICP_DEVICE_ID_TYPE		0x3480
>  #define INTEL_PCH_P2X_DEVICE_ID_TYPE		0x7100
>  #define INTEL_PCH_P3X_DEVICE_ID_TYPE		0x7000
> -- 
> 2.21.0
>
Dhinakaran Pandiyan March 7, 2019, 2:20 a.m. UTC | #2 
On Wed, 2019-03-06 at 10:07 -0800, Rodrigo Vivi wrote:
> On Tue, Mar 05, 2019 at 01:46:56PM -0800, Anusha wrote:
> > From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > 
> > Comet Lake PCH is based off of Cannon Point(CNP).
> > Add PCI ID for Comet Lake PCH.
> > 
> > v2: Code cleanup (DK)
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> >  drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 1b2f5a6f8c25..e787c999b2c6 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -188,6 +188,10 @@ intel_pch_type(const struct drm_i915_private
> > *dev_priv, unsigned short id)
> >  		DRM_DEBUG_KMS("Found Cannon Lake LP PCH (CNP-LP)\n");
> >  		WARN_ON(!IS_CANNONLAKE(dev_priv) &&
> > !IS_COFFEELAKE(dev_priv));
> >  		return PCH_CNP;
> > +	case INTEL_PCH_CMP_DEVICE_ID_TYPE:
> > +		DRM_DEBUG_KMS("Found Comet Lake LP PCH (CMP)\n");
> > +		WARN_ON(!IS_CANNONLAKE(dev_priv) &&
> > !IS_COFFEELAKE(dev_priv));
> 
> Please remove CNL from this list...
> 
> also please add a comment here before the return
> 
>      	    /* CM has same south display engine from CNP */
> 
> > +		return PCH_CNP;
> >  	case INTEL_PCH_ICP_DEVICE_ID_TYPE:
> >  		DRM_DEBUG_KMS("Found Ice Lake PCH\n");
> >  		WARN_ON(!IS_ICELAKE(dev_priv));
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 453af7438e67..55298e19e740 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -530,7 +530,7 @@ enum intel_pch {
> >  	PCH_LPT,	/* Lynxpoint/Wildcatpoint PCH */
> >  	PCH_SPT,        /* Sunrisepoint PCH */
> >  	PCH_KBP,        /* Kaby Lake PCH */
> > -	PCH_CNP,        /* Cannon Lake PCH */
> > +	PCH_CNP,        /* Cannon/Comet Lake PCH */
> >  	PCH_ICP,	/* Ice Lake PCH */
> >  	PCH_NOP,	/* PCH without south display */
> >  };
> > @@ -2552,6 +2552,7 @@ static inline unsigned int
> > i915_sg_segment_size(void)
> >  #define INTEL_PCH_KBP_DEVICE_ID_TYPE		0xA280
> >  #define INTEL_PCH_CNP_DEVICE_ID_TYPE		0xA300
> >  #define INTEL_PCH_CNP_LP_DEVICE_ID_TYPE		0x9D80
> > +#define INTEL_PCH_CMP_DEVICE_ID_TYPE		0x0280
The debug message calls the device "LP" but the device id macro
doesn't. This is not the case with CNP as both are consistent. So, the
question is this an LP PCH?

> >  #define INTEL_PCH_ICP_DEVICE_ID_TYPE		0x3480
> >  #define INTEL_PCH_P2X_DEVICE_ID_TYPE		0x7100
> >  #define INTEL_PCH_P3X_DEVICE_ID_TYPE		0x7000
> > -- 
> > 2.21.0
> >
Rodrigo Vivi March 7, 2019, 5:27 p.m. UTC | #3 
On Wed, Mar 06, 2019 at 06:20:08PM -0800, Dhinakaran Pandiyan wrote:
> On Wed, 2019-03-06 at 10:07 -0800, Rodrigo Vivi wrote:
> > On Tue, Mar 05, 2019 at 01:46:56PM -0800, Anusha wrote:
> > > From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > 
> > > Comet Lake PCH is based off of Cannon Point(CNP).
> > > Add PCI ID for Comet Lake PCH.
> > > 
> > > v2: Code cleanup (DK)
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> > >  drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> > >  2 files changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 1b2f5a6f8c25..e787c999b2c6 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -188,6 +188,10 @@ intel_pch_type(const struct drm_i915_private
> > > *dev_priv, unsigned short id)
> > >  		DRM_DEBUG_KMS("Found Cannon Lake LP PCH (CNP-LP)\n");
> > >  		WARN_ON(!IS_CANNONLAKE(dev_priv) &&
> > > !IS_COFFEELAKE(dev_priv));
> > >  		return PCH_CNP;
> > > +	case INTEL_PCH_CMP_DEVICE_ID_TYPE:
> > > +		DRM_DEBUG_KMS("Found Comet Lake LP PCH (CMP)\n");
> > > +		WARN_ON(!IS_CANNONLAKE(dev_priv) &&
> > > !IS_COFFEELAKE(dev_priv));
> > 
> > Please remove CNL from this list...
> > 
> > also please add a comment here before the return
> > 
> >      	    /* CM has same south display engine from CNP */
> > 
> > > +		return PCH_CNP;
> > >  	case INTEL_PCH_ICP_DEVICE_ID_TYPE:
> > >  		DRM_DEBUG_KMS("Found Ice Lake PCH\n");
> > >  		WARN_ON(!IS_ICELAKE(dev_priv));
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 453af7438e67..55298e19e740 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -530,7 +530,7 @@ enum intel_pch {
> > >  	PCH_LPT,	/* Lynxpoint/Wildcatpoint PCH */
> > >  	PCH_SPT,        /* Sunrisepoint PCH */
> > >  	PCH_KBP,        /* Kaby Lake PCH */
> > > -	PCH_CNP,        /* Cannon Lake PCH */
> > > +	PCH_CNP,        /* Cannon/Comet Lake PCH */
> > >  	PCH_ICP,	/* Ice Lake PCH */
> > >  	PCH_NOP,	/* PCH without south display */
> > >  };
> > > @@ -2552,6 +2552,7 @@ static inline unsigned int
> > > i915_sg_segment_size(void)
> > >  #define INTEL_PCH_KBP_DEVICE_ID_TYPE		0xA280
> > >  #define INTEL_PCH_CNP_DEVICE_ID_TYPE		0xA300
> > >  #define INTEL_PCH_CNP_LP_DEVICE_ID_TYPE		0x9D80
> > > +#define INTEL_PCH_CMP_DEVICE_ID_TYPE		0x0280
> The debug message calls the device "LP" but the device id macro
> doesn't. This is not the case with CNP as both are consistent. So, the
> question is this an LP PCH?

Better to remove the LP from the comment.

> 
> > >  #define INTEL_PCH_ICP_DEVICE_ID_TYPE		0x3480
> > >  #define INTEL_PCH_P2X_DEVICE_ID_TYPE		0x7100
> > >  #define INTEL_PCH_P3X_DEVICE_ID_TYPE		0x7000
> > > -- 
> > > 2.21.0
> > > 
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1b2f5a6f8c25..e787c999b2c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -188,6 +188,10 @@  intel_pch_type(const struct drm_i915_private *dev_priv, unsigned short id)
 		DRM_DEBUG_KMS("Found Cannon Lake LP PCH (CNP-LP)\n");
 		WARN_ON(!IS_CANNONLAKE(dev_priv) && !IS_COFFEELAKE(dev_priv));
 		return PCH_CNP;
+	case INTEL_PCH_CMP_DEVICE_ID_TYPE:
+		DRM_DEBUG_KMS("Found Comet Lake LP PCH (CMP)\n");
+		WARN_ON(!IS_CANNONLAKE(dev_priv) && !IS_COFFEELAKE(dev_priv));
+		return PCH_CNP;
 	case INTEL_PCH_ICP_DEVICE_ID_TYPE:
 		DRM_DEBUG_KMS("Found Ice Lake PCH\n");
 		WARN_ON(!IS_ICELAKE(dev_priv));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 453af7438e67..55298e19e740 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -530,7 +530,7 @@  enum intel_pch {
 	PCH_LPT,	/* Lynxpoint/Wildcatpoint PCH */
 	PCH_SPT,        /* Sunrisepoint PCH */
 	PCH_KBP,        /* Kaby Lake PCH */
-	PCH_CNP,        /* Cannon Lake PCH */
+	PCH_CNP,        /* Cannon/Comet Lake PCH */
 	PCH_ICP,	/* Ice Lake PCH */
 	PCH_NOP,	/* PCH without south display */
 };
@@ -2552,6 +2552,7 @@  static inline unsigned int i915_sg_segment_size(void)
 #define INTEL_PCH_KBP_DEVICE_ID_TYPE		0xA280
 #define INTEL_PCH_CNP_DEVICE_ID_TYPE		0xA300
 #define INTEL_PCH_CNP_LP_DEVICE_ID_TYPE		0x9D80
+#define INTEL_PCH_CMP_DEVICE_ID_TYPE		0x0280
 #define INTEL_PCH_ICP_DEVICE_ID_TYPE		0x3480
 #define INTEL_PCH_P2X_DEVICE_ID_TYPE		0x7100
 #define INTEL_PCH_P3X_DEVICE_ID_TYPE		0x7000