[Intel-gfx] [PATCH 07/71] drm/i915/chv: Add DPFLIPSTAT register bits for Cherryview

[Daniel Vetter]

On Fri, May 02, 2014 at 11:29:16AM +0300, Ville Syrjälä wrote:
> On Thu, May 01, 2014 at 01:55:23PM +0000, Barbalho, Rafael wrote:
> > > -----Original Message-----
> > > From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf
> > > Of ville.syrjala at linux.intel.com
> > > Sent: Wednesday, April 09, 2014 11:28 AM
> > > To: intel-gfx at lists.freedesktop.org
> > > Subject: [Intel-gfx] [PATCH 07/71] drm/i915/chv: Add DPFLIPSTAT register
> > > bits for Cherryview
> > > 
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > CHV has pipe C and PSR which cause changes to DPFLIPSTAT.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > If we add the _CHV to some of the bitfield defines would you also add them to here? These are also CHV specific.
> 
> I figured these are pretty clear even w/o the suffix. Everyone ought to
> know VLV has two pipes and CHV has three.

I agree with Ville here - if the bitfields are just evenly laid out
extensions of previously reserved fields then adding a suffix just
confuses. Otoh if the bits are all over the place and reuse bits from
older generations the suffix is good as a signal to the reader to pay
closer attention.

In the end it's about what reads easier and conveys more useful
information to the reader.
-Daniel

> 
> > 
> > Reviewed-by: Rafael Barbalho <rafael.barbalho at intel.com>
> > 
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index 0fb6b6f..81d4b83 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -3451,12 +3451,19 @@ enum punit_power_well {
> > >  #define   SPRITED_FLIP_DONE_INT_EN		(1<<26)
> > >  #define   SPRITEC_FLIP_DONE_INT_EN		(1<<25)
> > >  #define   PLANEB_FLIP_DONE_INT_EN		(1<<24)
> > > +#define   PIPE_PSR_INT_EN			(1<<22)
> > >  #define   PIPEA_LINE_COMPARE_INT_EN		(1<<21)
> > >  #define   PIPEA_HLINE_INT_EN			(1<<20)
> > >  #define   PIPEA_VBLANK_INT_EN			(1<<19)
> > >  #define   SPRITEB_FLIP_DONE_INT_EN		(1<<18)
> > >  #define   SPRITEA_FLIP_DONE_INT_EN		(1<<17)
> > >  #define   PLANEA_FLIPDONE_INT_EN		(1<<16)
> > > +#define   PIPEC_LINE_COMPARE_INT_EN		(1<<13)
> > > +#define   PIPEC_HLINE_INT_EN			(1<<12)
> > > +#define   PIPEC_VBLANK_INT_EN			(1<<11)
> > > +#define   SPRITEF_FLIPDONE_INT_EN		(1<<10)
> > > +#define   SPRITEE_FLIPDONE_INT_EN		(1<<9)
> > > +#define   PLANEC_FLIPDONE_INT_EN		(1<<8)
> > > 
> > >  #define DPINVGTT				(VLV_DISPLAY_BASE +
> > > 0x7002c) /* VLV only */
> > >  #define   CURSORB_INVALID_GTT_INT_EN		(1<<23)
> > > --
> > > 1.8.3.2
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch