On Sat, Apr 12, 2014 at 04:20:49PM +0100, Chris Wilson wrote:
> It is possible for userspace to create a big object large enough for a
> 256x256, and then switch over to using it as a 64x64 cursor. This
> requires the cursor update routines to check for a change in width on
> every update, rather than just when the cursor is originally enabled.
>
> This also fixes an issue with 845g/865g which cannot change the base
> address of the cursor whilst it is active.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Hm, can we have a testcase for this please? I'm thinking of a black cursor
on black fb at full size, then using the same cursor at smaller size and
filling out the oversize with white. If the CRCs don't match we know that
we scan out a bit of white.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 106 +++++++++++++++++------------------
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> 3 files changed, 53 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 4c5de60e5757..7799b1394741 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2450,7 +2450,7 @@ static int i915_display_info(struct seq_file *m, void *unused)
>
> active = cursor_position(dev, crtc->pipe, &x, &y);
> seq_printf(m, "\tcursor visible? %s, position (%d, %d), addr 0x%08x, active? %s\n",
> - yesno(crtc->cursor_visible),
> + yesno(crtc->cursor_base),
> x, y, crtc->cursor_addr,
> yesno(active));
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5e9cb9383f05..92d152694464 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7564,29 +7564,33 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - bool visible = base != 0;
> - u32 cntl;
> + uint32_t cntl;
>
> - if (intel_crtc->cursor_visible == visible)
> - return;
> -
> - cntl = I915_READ(_CURACNTR);
> - if (visible) {
> + if (base != intel_crtc->cursor_base) {
> /* On these chipsets we can only modify the base whilst
> * the cursor is disabled.
> */
> + if (intel_crtc->cursor_cntl) {
> + I915_WRITE(_CURACNTR, 0);
> + POSTING_READ(_CURACNTR);
> + intel_crtc->cursor_cntl = 0;
> + }
> +
> I915_WRITE(_CURABASE, base);
> + POSTING_READ(_CURABASE);
> + }
>
> - cntl &= ~(CURSOR_FORMAT_MASK);
> - /* XXX width must be 64, stride 256 => 0x00 << 28 */
> - cntl |= CURSOR_ENABLE |
> + /* XXX width must be 64, stride 256 => 0x00 << 28 */
> + cntl = 0;
> + if (base)
> + cntl = (CURSOR_ENABLE |
> CURSOR_GAMMA_ENABLE |
> - CURSOR_FORMAT_ARGB;
> - } else
> - cntl &= ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
> - I915_WRITE(_CURACNTR, cntl);
> -
> - intel_crtc->cursor_visible = visible;
> + CURSOR_FORMAT_ARGB);
> + if (intel_crtc->cursor_cntl != cntl) {
> + I915_WRITE(_CURACNTR, cntl);
> + POSTING_READ(_CURACNTR);
> + intel_crtc->cursor_cntl = cntl;
> + }
> }
>
> static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> @@ -7595,16 +7599,12 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int pipe = intel_crtc->pipe;
> - bool visible = base != 0;
> -
> - if (intel_crtc->cursor_visible != visible) {
> - int16_t width = intel_crtc->cursor_width;
> - uint32_t cntl = I915_READ(CURCNTR(pipe));
> - if (base) {
> - cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> - cntl |= MCURSOR_GAMMA_ENABLE;
> + uint32_t cntl;
>
> - switch (width) {
> + cntl = 0;
> + if (base) {
> + cntl = MCURSOR_GAMMA_ENABLE;
> + switch (intel_crtc->cursor_width) {
> case 64:
> cntl |= CURSOR_MODE_64_ARGB_AX;
> break;
> @@ -7617,18 +7617,16 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> default:
> WARN_ON(1);
> return;
> - }
> - cntl |= pipe << 28; /* Connect to correct pipe */
> - } else {
> - cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> - cntl |= CURSOR_MODE_DISABLE;
> }
> + cntl |= pipe << 28; /* Connect to correct pipe */
> + }
> + if (intel_crtc->cursor_cntl != cntl) {
> I915_WRITE(CURCNTR(pipe), cntl);
> -
> - intel_crtc->cursor_visible = visible;
> + POSTING_READ(CURCNTR(pipe));
> + intel_crtc->cursor_cntl = cntl;
> }
> +
> /* and commit changes on next vblank */
> - POSTING_READ(CURCNTR(pipe));
> I915_WRITE(CURBASE(pipe), base);
> POSTING_READ(CURBASE(pipe));
> }
> @@ -7639,15 +7637,12 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int pipe = intel_crtc->pipe;
> - bool visible = base != 0;
> -
> - if (intel_crtc->cursor_visible != visible) {
> - int16_t width = intel_crtc->cursor_width;
> - uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
> - if (base) {
> - cntl &= ~CURSOR_MODE;
> - cntl |= MCURSOR_GAMMA_ENABLE;
> - switch (width) {
> + uint32_t cntl;
> +
> + cntl = 0;
> + if (base) {
> + cntl = MCURSOR_GAMMA_ENABLE;
> + switch (intel_crtc->cursor_width) {
> case 64:
> cntl |= CURSOR_MODE_64_ARGB_AX;
> break;
> @@ -7660,21 +7655,18 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> default:
> WARN_ON(1);
> return;
> - }
> - } else {
> - cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> - cntl |= CURSOR_MODE_DISABLE;
> - }
> - if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> - cntl |= CURSOR_PIPE_CSC_ENABLE;
> - cntl &= ~CURSOR_TRICKLE_FEED_DISABLE;
> }
> - I915_WRITE(CURCNTR_IVB(pipe), cntl);
> + }
> + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + cntl |= CURSOR_PIPE_CSC_ENABLE;
>
> - intel_crtc->cursor_visible = visible;
> + if (intel_crtc->cursor_cntl != cntl) {
> + I915_WRITE(CURCNTR_IVB(pipe), cntl);
> + POSTING_READ(CURCNTR_IVB(pipe));
> + intel_crtc->cursor_cntl = cntl;
> }
> +
> /* and commit changes on next vblank */
> - POSTING_READ(CURCNTR_IVB(pipe));
> I915_WRITE(CURBASE_IVB(pipe), base);
> POSTING_READ(CURBASE_IVB(pipe));
> }
> @@ -7690,7 +7682,6 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> int x = intel_crtc->cursor_x;
> int y = intel_crtc->cursor_y;
> u32 base = 0, pos = 0;
> - bool visible;
>
> if (on)
> base = intel_crtc->cursor_addr;
> @@ -7719,8 +7710,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> }
> pos |= y << CURSOR_Y_SHIFT;
>
> - visible = base != 0;
> - if (!visible && !intel_crtc->cursor_visible)
> + if (base == 0 && intel_crtc->cursor_base == 0)
> return;
>
> if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> @@ -7733,6 +7723,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> else
> i9xx_update_cursor(crtc, base);
> }
> + intel_crtc->cursor_base = base;
> }
>
> static int intel_crtc_cursor_set(struct drm_crtc *crtc,
> @@ -10681,6 +10672,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> intel_crtc->plane = !pipe;
> }
>
> + intel_crtc->cursor_base = ~0;
> + intel_crtc->cursor_cntl = ~0;
> +
> BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
> dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
> dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2da2efcfbd47..c6ab4ac4063a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -379,7 +379,8 @@ struct intel_crtc {
> uint32_t cursor_addr;
> int16_t cursor_x, cursor_y;
> int16_t cursor_width, cursor_height;
> - bool cursor_visible;
> + uint32_t cursor_cntl;
> + uint32_t cursor_base;
>
> struct intel_plane_config plane_config;
> struct intel_crtc_config config;
> --
> 1.9.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
