Re: [PATCH 1/2] drm/i915: Update plane flip count registers

From: Ben Widawsky <ben@bwidawsk.net>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>,
	Ben Widawsky <benjamin.widawsky@intel.com>
Subject: Re: [PATCH 1/2] drm/i915: Update plane flip count registers
Date: Thu, 22 Aug 2013 11:12:13 -0700	[thread overview]
Message-ID: <20130822181212.GA7181@bwidawsk.net> (raw)
In-Reply-To: <20130822171844.GB29682@intel.com>

On Thu, Aug 22, 2013 at 08:18:44PM +0300, Ville Syrjälä wrote:
> On Wed, Aug 21, 2013 at 08:15:52PM -0700, Ben Widawsky wrote:
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 53d0e70..d1079db 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3277,7 +3277,6 @@
> >  #define   PIPE_PIXEL_SHIFT        0
> >  /* GM45+ just has to be different */
> >  #define _PIPEA_FRMCOUNT_GM45	0x70040
> > -#define _PIPEA_FLIPCOUNT_GM45	0x70044
> >  #define PIPE_FRMCOUNT_GM45(pipe) _PIPE(pipe, _PIPEA_FRMCOUNT_GM45, _PIPEB_FRMCOUNT_GM45)
> >  
> >  /* Cursor A & B regs */
> > @@ -3361,6 +3360,7 @@
> >  #define   DISPPLANE_STEREO_POLARITY_SECOND	(1<<18)
> >  #define   DISPPLANE_TRICKLE_FEED_DISABLE	(1<<14) /* Ironlake */
> >  #define   DISPPLANE_TILED			(1<<10)
> > +#define _DSPAFLIPCNT		(dev_priv->info->display_mmio_offset + 0x70044)
> 
> Hmm. I don't quite get it. Why rename and move it? Sure it should really
> be called DSPFLIPCNT since it applies to the primary plane, but BSpec
> doesn't actually call it that, never has AFAICS.

The rename and move was just to keep everything in a nice place.
_PIPEA_FLIPCOUNT_GM45 was never actually used AFAICT. Since it seemed I
needed to define a PIPEB anyway, I figured I'd try to adhere to the
other register convention.

I don't know what naming scheme we typically use to define these
registers, as I infrequently touch them. What is your recommendation,
I'll gladly update. I just used what the code around me used (and I
personally dislike that we call the stuff DSP[AB] anyway).

> 
> Also I was first sceptical about the mmio_offset, since I remembered
> seeing the pre-CTG two part frame counter registers in VLV specs. But after
> re-checking, the TOC only has the old regs, while the actual text has only
> the CTG style regs. So I guess VLV does indeed have the CTG style registers.
> I don't have a VLV board on me to verify though.
> 

As a broader question, is that how we denote whether or not the register
exists on VLV, by not use dev_priv->info->display_mmio_offset + ? I was
sort of curious about this since I too was unsure about VLV.

[snip]

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx