Re: [PATCH] drm/i915: Add some more bits to CURSOR_POS_MASK

From: Daniel Vetter <daniel@ffwll.ch>
To: Robert Fekete <robert.fekete@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Add some more bits to CURSOR_POS_MASK
Date: Wed, 18 Nov 2015 10:17:15 +0100	[thread overview]
Message-ID: <20151118091715.GE20799@phenom.ffwll.local> (raw)
In-Reply-To: <20151104095928.GA32357@patrik-desktop.isw.intel.com>

On Wed, Nov 04, 2015 at 10:59:28AM +0100, Patrik Jakobsson wrote:
> On Wed, Nov 04, 2015 at 10:35:19AM +0100, Robert Fekete wrote:
> > The old value of 0x7FF will wrap the position at 2048 giving wrong
> > coordinate values on panels larger than 2048 pixels in any direction.
> > Used in i915_debugfs atm. Looking at all hw specs available at 01.org
> > shows that X position is bit 0:11, and even 0:12 on some hw where
> > remaining bits up to bit 14 is MBZ. For Y position it is bits 16-27
> > where bits 28:30 is MBZ. It should be safe to increase CURSOR_POS_MASK
> > to 13 bits (0x1FFF) making 8192 as a new wrap around value still getting
> > valid cursor positions on platforms with only 12bits available thanks to
> > MBZ on adjacent bits above.
> 
> I cannot find documentation for older hardware and this only touches
> debugfs, so in worst case we get wrong values for really old hardware but good
> ones for newer. I think that's a fair tradeoff.
> 
> Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

If it's only used in debugfs then imo just drop it. Having a _MASK which
isn't valid on all platforms, but where we don't have differnt #defines
for the different platforms is really confusing.
-Daniel

> 
> > 
> > Signed-off-by: Robert Fekete <robert.fekete@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 894253228947..f351f46f8cb9 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4883,7 +4883,7 @@ enum skl_disp_power_wells {
> >  #define   CURSOR_TRICKLE_FEED_DISABLE	(1 << 14)
> >  #define _CURABASE		0x70084
> >  #define _CURAPOS		0x70088
> > -#define   CURSOR_POS_MASK       0x007FF
> > +#define   CURSOR_POS_MASK       0x01FFF
> >  #define   CURSOR_POS_SIGN       0x8000
> >  #define   CURSOR_X_SHIFT        0
> >  #define   CURSOR_Y_SHIFT        16
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx