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

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

[Ben Widawsky]

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)
 #define _DSPAADDR		(dev_priv->info->display_mmio_offset + 0x70184)
 #define _DSPASTRIDE		(dev_priv->info->display_mmio_offset + 0x70188)
 #define _DSPAPOS		(dev_priv->info->display_mmio_offset + 0x7018C) /* reserved */
@@ -3380,6 +3380,7 @@
 #define DSPLINOFF(plane) DSPADDR(plane)
 #define DSPOFFSET(plane) _PIPE(plane, _DSPAOFFSET, _DSPBOFFSET)
 #define DSPSURFLIVE(plane) _PIPE(plane, _DSPASURFLIVE, _DSPBSURFLIVE)
+#define DSPFLIPCNT(plane) _PIPE(plane, _DSPAFLIPCNT, _DSPBFLIPCNT)
 
 /* Display/Sprite base address macros */
 #define DISP_BASEADDR_MASK	(0xfffff000)
@@ -3410,10 +3411,11 @@
 #define _PIPEBFRAMEHIGH		(dev_priv->info->display_mmio_offset + 0x71040)
 #define _PIPEBFRAMEPIXEL	(dev_priv->info->display_mmio_offset + 0x71044)
 #define _PIPEB_FRMCOUNT_GM45	0x71040
-#define _PIPEB_FLIPCOUNT_GM45	0x71044
+#define _PIPEB_FLIPCOUNT	(dev_priv->info->display_mmio_offset + 0x71044
 
 
 /* Display B control */
+#define _DSPBFLIPCNT		(dev_priv->info->display_mmio_offset + 0x71044)
 #define _DSPBCNTR		(dev_priv->info->display_mmio_offset + 0x71180)
 #define   DISPPLANE_ALPHA_TRANS_ENABLE		(1<<15)
 #define   DISPPLANE_ALPHA_TRANS_DISABLE		0
-- 
1.8.3.4

[PATCH 2/2] drm/i915: Add debugfs interface for planes

[Ben Widawsky]

This interface can enhanced over time to get more per plane information.
I've just flip counts for now. What I'd like to do with flip counts is
integrate them with existing flip tests. It serves a similar, but more
course purpose to some of the CRC work being done.

Unfortunately, I've been unable to get something going on the test side
thus far because I am inexperienced with the APIs, and don't see a way
to map a crtc to a hardware plane (which is what we have flip counts
for).

Chris, maybe you can do something useful with this?

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 236d97e..1cf0461 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1791,6 +1791,26 @@ static int i915_energy_uJ(struct seq_file *m, void *data)
 	return 0;
 }
 
+static int i915_plane_info(struct seq_file *m, void *data)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uintptr_t plane = (uintptr_t) node->info_ent->data;
+
+	if (INTEL_INFO(dev)->gen < 5 && !INTEL_INFO(dev)->is_g4x)
+		return -ENODEV;
+
+	if (plane >= INTEL_INFO(dev)->num_pipes)
+		return -ENODEV;
+
+	DRM_ERROR("Plane was %p\n", data);
+
+	seq_printf(m, "flip count: %u\n", I915_READ(DSPFLIPCNT(plane)));
+
+	return 0;
+}
+
 static int
 i915_wedged_get(void *data, u64 *val)
 {
@@ -2231,6 +2251,9 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_llc", i915_llc, 0},
 	{"i915_edp_psr_status", i915_edp_psr_status, 0},
 	{"i915_energy_uJ", i915_energy_uJ, 0},
+	{"i915_plane_a", i915_plane_info, 0, (void *)0},
+	{"i915_plane_b", i915_plane_info, 0, (void *)1},
+	{"i915_plane_c", i915_plane_info, 0, (void *)2},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
-- 
1.8.3.4

Re: [PATCH 2/2] drm/i915: Add debugfs interface for planes

[Ben Widawsky]

Sent the unfinished commit by accident
On Wed, Aug 21, 2013 at 08:15:53PM -0700, Ben Widawsky wrote:
> This interface can enhanced over time to get more per plane information.
> I've just flip counts for now. What I'd like to do with flip counts is
> integrate them with existing flip tests. It serves a similar, but more
> course purpose to some of the CRC work being done.
  ^^
 coarse.

"It can be used to determine if something is being flipped when we
expect it to be flipped. The immediate issue I want to look into is I've
seen some funny behavior where we're getting two flips per flip queued."


> 
> Unfortunately, I've been unable to get something going on the test side
> thus far because I am inexperienced with the APIs, and don't see a way
> to map a crtc to a hardware plane (which is what we have flip counts
> for).
> 
> Chris, maybe you can do something useful with this?
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 236d97e..1cf0461 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1791,6 +1791,26 @@ static int i915_energy_uJ(struct seq_file *m, void *data)
>  	return 0;
>  }
>  
> +static int i915_plane_info(struct seq_file *m, void *data)
> +{
> +	struct drm_info_node *node = m->private;
> +	struct drm_device *dev = node->minor->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uintptr_t plane = (uintptr_t) node->info_ent->data;
> +
> +	if (INTEL_INFO(dev)->gen < 5 && !INTEL_INFO(dev)->is_g4x)
> +		return -ENODEV;
> +
> +	if (plane >= INTEL_INFO(dev)->num_pipes)
> +		return -ENODEV;
> +
> +	DRM_ERROR("Plane was %p\n", data);
> +
> +	seq_printf(m, "flip count: %u\n", I915_READ(DSPFLIPCNT(plane)));
> +
> +	return 0;
> +}
> +
>  static int
>  i915_wedged_get(void *data, u64 *val)
>  {
> @@ -2231,6 +2251,9 @@ static struct drm_info_list i915_debugfs_list[] = {
>  	{"i915_llc", i915_llc, 0},
>  	{"i915_edp_psr_status", i915_edp_psr_status, 0},
>  	{"i915_energy_uJ", i915_energy_uJ, 0},
> +	{"i915_plane_a", i915_plane_info, 0, (void *)0},
> +	{"i915_plane_b", i915_plane_info, 0, (void *)1},
> +	{"i915_plane_c", i915_plane_info, 0, (void *)2},
>  };
>  #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
>  
> -- 
> 1.8.3.4
> 

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 2/2] drm/i915: Add debugfs interface for planes

[Chris Wilson]

On Wed, Aug 21, 2013 at 07:16:53PM -0700, Ben Widawsky wrote:
> Sent the unfinished commit by accident
> On Wed, Aug 21, 2013 at 08:15:53PM -0700, Ben Widawsky wrote:
> > This interface can enhanced over time to get more per plane information.
> > I've just flip counts for now. What I'd like to do with flip counts is
> > integrate them with existing flip tests. It serves a similar, but more
> > course purpose to some of the CRC work being done.
>   ^^
>  coarse.
> 
> "It can be used to determine if something is being flipped when we
> expect it to be flipped. The immediate issue I want to look into is I've
> seen some funny behavior where we're getting two flips per flip queued."

Cursed email - I only have your reply. On the machines you are interested
in, the CRTCs are assigned to fixed planes.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/2] drm/i915: Add debugfs interface for planes

[Ben Widawsky]

On Thu, Aug 22, 2013 at 03:21:09AM +0100, Chris Wilson wrote:
> On Wed, Aug 21, 2013 at 07:16:53PM -0700, Ben Widawsky wrote:
> > Sent the unfinished commit by accident
> > On Wed, Aug 21, 2013 at 08:15:53PM -0700, Ben Widawsky wrote:
> > > This interface can enhanced over time to get more per plane information.
> > > I've just flip counts for now. What I'd like to do with flip counts is
> > > integrate them with existing flip tests. It serves a similar, but more
> > > course purpose to some of the CRC work being done.
> >   ^^
> >  coarse.
> > 
> > "It can be used to determine if something is being flipped when we
> > expect it to be flipped. The immediate issue I want to look into is I've
> > seen some funny behavior where we're getting two flips per flip queued."
> 
> Cursed email - I only have your reply. On the machines you are interested
> in, the CRTCs are assigned to fixed planes.
> -Chris
> 

It's not just you, mailman is acting weird for me also.

In any case, can you give me another hint on how to go from crtc_id to
plane?

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 2/2] drm/i915: Add debugfs interface for planes

[Chris Wilson]

On Wed, Aug 21, 2013 at 07:27:11PM -0700, Ben Widawsky wrote:
> On Thu, Aug 22, 2013 at 03:21:09AM +0100, Chris Wilson wrote:
> > On Wed, Aug 21, 2013 at 07:16:53PM -0700, Ben Widawsky wrote:
> > > Sent the unfinished commit by accident
> > > On Wed, Aug 21, 2013 at 08:15:53PM -0700, Ben Widawsky wrote:
> > > > This interface can enhanced over time to get more per plane information.
> > > > I've just flip counts for now. What I'd like to do with flip counts is
> > > > integrate them with existing flip tests. It serves a similar, but more
> > > > course purpose to some of the CRC work being done.
> > >   ^^
> > >  coarse.
> > > 
> > > "It can be used to determine if something is being flipped when we
> > > expect it to be flipped. The immediate issue I want to look into is I've
> > > seen some funny behavior where we're getting two flips per flip queued."
> > 
> > Cursed email - I only have your reply. On the machines you are interested
> > in, the CRTCs are assigned to fixed planes.
> > -Chris
> > 
> 
> It's not just you, mailman is acting weird for me also.
> 
> In any case, can you give me another hint on how to go from crtc_id to
> plane?

crtc-id to plane is easy, use the get_pipe_ioctl and the plane is
identical to the pipe (for these machines). If you want plane to crtc,
then you need to build a little mapping table.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 2/2] [v2] drm/i915: Add debugfs interface for planes

[Ben Widawsky]

This interface can enhanced over time to get more per plane information.
I've just flip counts for now. What I'd like to do with flip counts is
integrate them with existing flip tests. It serves a similar, but more
coarse purpose to some of the CRC work being done. It can be used to
determine if something is being flipped when we expect it to be flipped.
The immediate issue I want to look into is I've seen some funny behavior
where we're getting two flips per flip queued.

Unfortunately, I've been unable to get something going on the test side
thus far because I am inexperienced with the APIs, and don't see a way
to map a crtc to a hardware plane (which is what we have flip counts
for).

Chris, maybe you can do something useful with this?

v2: Use the right patch, with the right commit msg.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 236d97e..27aee95 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1791,6 +1791,24 @@ static int i915_energy_uJ(struct seq_file *m, void *data)
 	return 0;
 }
 
+static int i915_plane_info(struct seq_file *m, void *data)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uintptr_t plane = (uintptr_t) node->info_ent->data;
+
+	if (INTEL_INFO(dev)->gen < 5 && !INTEL_INFO(dev)->is_g4x)
+		return -ENODEV;
+
+	if (plane >= INTEL_INFO(dev)->num_pipes)
+		return -ENODEV;
+
+	seq_printf(m, "flip count: %u\n", I915_READ(DSPFLIPCNT(plane)));
+
+	return 0;
+}
+
 static int
 i915_wedged_get(void *data, u64 *val)
 {
@@ -2231,6 +2249,9 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_llc", i915_llc, 0},
 	{"i915_edp_psr_status", i915_edp_psr_status, 0},
 	{"i915_energy_uJ", i915_energy_uJ, 0},
+	{"i915_plane_a", i915_plane_info, 0, (void *)0},
+	{"i915_plane_b", i915_plane_info, 0, (void *)1},
+	{"i915_plane_c", i915_plane_info, 0, (void *)2},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
-- 
1.8.3.4

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

[Ville Syrjälä]

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.

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.

>  #define _DSPAADDR		(dev_priv->info->display_mmio_offset + 0x70184)
>  #define _DSPASTRIDE		(dev_priv->info->display_mmio_offset + 0x70188)
>  #define _DSPAPOS		(dev_priv->info->display_mmio_offset + 0x7018C) /* reserved */
> @@ -3380,6 +3380,7 @@
>  #define DSPLINOFF(plane) DSPADDR(plane)
>  #define DSPOFFSET(plane) _PIPE(plane, _DSPAOFFSET, _DSPBOFFSET)
>  #define DSPSURFLIVE(plane) _PIPE(plane, _DSPASURFLIVE, _DSPBSURFLIVE)
> +#define DSPFLIPCNT(plane) _PIPE(plane, _DSPAFLIPCNT, _DSPBFLIPCNT)
>  
>  /* Display/Sprite base address macros */
>  #define DISP_BASEADDR_MASK	(0xfffff000)
> @@ -3410,10 +3411,11 @@
>  #define _PIPEBFRAMEHIGH		(dev_priv->info->display_mmio_offset + 0x71040)
>  #define _PIPEBFRAMEPIXEL	(dev_priv->info->display_mmio_offset + 0x71044)
>  #define _PIPEB_FRMCOUNT_GM45	0x71040
> -#define _PIPEB_FLIPCOUNT_GM45	0x71044
> +#define _PIPEB_FLIPCOUNT	(dev_priv->info->display_mmio_offset + 0x71044
>  
>  
>  /* Display B control */
> +#define _DSPBFLIPCNT		(dev_priv->info->display_mmio_offset + 0x71044)
>  #define _DSPBCNTR		(dev_priv->info->display_mmio_offset + 0x71180)
>  #define   DISPPLANE_ALPHA_TRANS_ENABLE		(1<<15)
>  #define   DISPPLANE_ALPHA_TRANS_DISABLE		0
> -- 
> 1.8.3.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

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

[Ben Widawsky]

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

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

[Ville Syrjälä]

On Thu, Aug 22, 2013 at 11:12:13AM -0700, Ben Widawsky wrote:
> 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).

I guess generally the name has more or less come from the earliest BSpec
that had the register. I probably need to break that rule soon tough as
I want to unify all the sprite registers. We have too many copies which
basically just differ by some constant offset.

> 
> > 
> > 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.

Yes, more or less.

The basic rule has been that register which exists only on VLV gets a
hardcoded +VLV_DISPLAY_BASE, register which exists on both VLV and
non-VLV gets a +display_mmio_offset, and registers that don't exist on
VLV get nothing.

Sometimes we have VLV prefix/suffix in the register name as well if the
register is VLV specific.

There's also some things like the ADPA register where we could have
used display_mmio_offset for VLV, but since we already had a pre-PCH
and PCH versions of it, we also added a specific VLV version. Also the
GMBUS registers already had offset handling through gmbus_mmio_offset,
so we didn't add display_mmio_offset to them.

-- 
Ville Syrjälä
Intel OTC