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@xxxxxxxxxxxx> > > > --- > > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
