* [PATCH] drm/i915: Add some more bits to CURSOR_POS_MASK @ 2015-11-04 9:35 Robert Fekete 2015-11-04 9:59 ` Patrik Jakobsson 0 siblings, 1 reply; 5+ messages in thread From: Robert Fekete @ 2015-11-04 9:35 UTC (permalink / raw) To: intel-gfx 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. 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Add some more bits to CURSOR_POS_MASK 2015-11-04 9:35 [PATCH] drm/i915: Add some more bits to CURSOR_POS_MASK Robert Fekete @ 2015-11-04 9:59 ` Patrik Jakobsson 2015-11-18 9:17 ` Daniel Vetter 0 siblings, 1 reply; 5+ messages in thread From: Patrik Jakobsson @ 2015-11-04 9:59 UTC (permalink / raw) To: Robert Fekete; +Cc: intel-gfx 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> > > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Add some more bits to CURSOR_POS_MASK 2015-11-04 9:59 ` Patrik Jakobsson @ 2015-11-18 9:17 ` Daniel Vetter 2015-11-25 12:54 ` Robert Fekete 0 siblings, 1 reply; 5+ messages in thread From: Daniel Vetter @ 2015-11-18 9:17 UTC (permalink / raw) To: Robert Fekete, intel-gfx 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Add some more bits to CURSOR_POS_MASK 2015-11-18 9:17 ` Daniel Vetter @ 2015-11-25 12:54 ` Robert Fekete 2015-11-25 16:54 ` Patrik Jakobsson 0 siblings, 1 reply; 5+ messages in thread From: Robert Fekete @ 2015-11-25 12:54 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx 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@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 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. 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@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 > -- BR /Robert Fekete Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Add some more bits to CURSOR_POS_MASK 2015-11-25 12:54 ` Robert Fekete @ 2015-11-25 16:54 ` Patrik Jakobsson 0 siblings, 0 replies; 5+ messages in thread From: Patrik Jakobsson @ 2015-11-25 16:54 UTC (permalink / raw) To: Robert Fekete; +Cc: Intel Graphics Development On Wed, Nov 25, 2015 at 1:54 PM, Robert Fekete <robert.fekete@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@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@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 >> > > -- > BR > /Robert Fekete > Intel Open Source Technology Center > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-11-25 16:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-11-04 9:35 [PATCH] drm/i915: Add some more bits to CURSOR_POS_MASK Robert Fekete 2015-11-04 9:59 ` Patrik Jakobsson 2015-11-18 9:17 ` Daniel Vetter 2015-11-25 12:54 ` Robert Fekete 2015-11-25 16:54 ` Patrik Jakobsson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.