[Intel-gfx] [PATCH 2/2] drm/i915/cml: Introduce Comet Lake PCH

[Dhinakaran Pandiyan]

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 at 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 at intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa at 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
> >