From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 1/2] drm/i915: Update plane flip count registers Date: Thu, 22 Aug 2013 21:25:57 +0300 Message-ID: <20130822182557.GD29682@intel.com> References: <1377141353-11532-1-git-send-email-benjamin.widawsky@intel.com> <20130822171844.GB29682@intel.com> <20130822181212.GA7181@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 3DBA9E6CBC for ; Thu, 22 Aug 2013 11:26:01 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130822181212.GA7181@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky Cc: Intel GFX , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org 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=E4l=E4 wrote: > > On Wed, Aug 21, 2013 at 08:15:52PM -0700, Ben Widawsky wrote: > > > Signed-off-by: Ben Widawsky > > > --- > > > 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/i= 915_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 a= fter > > re-checking, the TOC only has the old regs, while the actual text has o= nly > > the CTG style regs. So I guess VLV does indeed have the CTG style regis= ters. > > 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=E4l=E4 Intel OTC