[Intel-gfx] [PATCH] drm/i915: Add some more bits to CURSOR_POS_MASK

[Patrik Jakobsson]

On Wed, Nov 25, 2015 at 1:54 PM, Robert Fekete
<robert.fekete at linux.intel.com> wrote:
> On ons, 2015-11-18 at 10:17 +0100, Daniel Vetter wrote:
>> 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 at 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

Yes, seems fair.

>
> Well, not the most important patch around, but still the value of 0x7ff
> is still too conservative and gives wrong cursor pos values on large
> panels. I have hard times digging up really old register specs so I
> still can't see which Intel platform this new value of mine isn't valid
> on.

I think Daniel meant that we can remove the define altogether and
hardcode a sane mask in debugfs instead. Keeping the define around
might give people the wrong idea what the bspec actually says. Also
adding a comment along with the hardcoded mask in debugfs would be
nice.

-Patrik

>
> It is this patch by Chris in debugfs that is broken on large panels
> wrapping coords at (x_pos/y_pos > 2048)
> http://marc.info/?l=git-commits-head&m=139697989108096&w=1
>
>> >
>> > >
>> > > Signed-off-by: Robert Fekete <robert.fekete at 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 at lists.freedesktop.org
>> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>
> --
> BR
> /Robert Fekete
> Intel Open Source Technology Center
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx