Re: [PATCH 07/13] drm/i915: Add register whitelist for DRM master

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jan 29, 2014 at 03:18:21PM -0800, Volkin, Bradley D wrote:
> On Wed, Jan 29, 2014 at 02:37:25PM -0800, Chris Wilson wrote:
> > On Wed, Jan 29, 2014 at 01:55:08PM -0800, bradley.d.volkin@intel.com wrote:
> > > From: Brad Volkin <bradley.d.volkin@intel.com>
> > > 
> > > These are used to implement scanline waits in the X server.
> > > 
> > > Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_cmd_parser.c | 30 ++++++++++++++++++++++++++++++
> > >  1 file changed, 30 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > > index 18d5b05..296e322 100644
> > > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> > > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > > @@ -234,6 +234,20 @@ static const u32 gen7_blt_regs[] = {
> > >  	BCS_SWCTRL,
> > >  };
> > >  
> > > +/* Whitelists for the DRM master. Magic numbers are taken from sna, to match. */
> > 
> > It would be wiser to use the kernel defines, makes it look like we are
> > actually in charge. ;-)
> 
> Will fix, though based on the sna commit history, it looks like you're in charge
> either way :)

Yeah, for the register tables I think we really should use symbolic values
consistently, adding new ones if i915_reg.h has them lacking. At least as
long as the lists are this short.

Aside: The bkm for getting big feature work which adds lots of register
#defines like this is to split out patches with just the #defines. That
way those can be reviewed independently from any discussions about the
code itself and so merged early. Helps with rebasing pains ;-)

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