On Mon, Jun 02, 2014 at 01:46:11PM +0300, Antti Koskipää wrote: > On 06/02/2014 11:49 AM, Daniel Vetter wrote: > > On Fri, May 30, 2014 at 04:35:26PM +0300, 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 <[email protected]> > >> [Antti:rebased, adjusted macro names and moved some lines, no functional > >> changes] > >> Reviewed-by: Antti Koskipaa <[email protected]> > >> Tested-by: Antti Koskipaa <[email protected]> > > > > Still missing the igt testcase. Is Antti still working on that one? > > The testcase was just posted. It just needed some cleanup.
Cool. Patch is queued for -next, thanks for the patch. -Daniel > > > And I > > guess now we also need an Cc: [email protected] on this one here. > > -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 2e5f76a..04d0b7c 100644 > >> --- a/drivers/gpu/drm/i915/i915_debugfs.c > >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c > >> @@ -2353,7 +2353,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 731cd01..955f92d 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -7872,29 +7872,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) > >> @@ -7903,16 +7907,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; > >> @@ -7925,18 +7925,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)); > >> } > >> @@ -7947,15 +7945,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(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; > >> @@ -7968,21 +7963,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(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(pipe), cntl); > >> + 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)); > >> } > >> @@ -7998,7 +7990,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; > >> @@ -8027,8 +8018,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; > >> > >> I915_WRITE(CURPOS(pipe), pos); > >> @@ -8039,6 +8029,7 @@ static void intel_crtc_update_cursor(struct drm_crtc > >> *crtc, > >> i845_update_cursor(crtc, base); > >> else > >> i9xx_update_cursor(crtc, base); > >> + intel_crtc->cursor_base = base; > >> } > >> > >> static int intel_crtc_cursor_set(struct drm_crtc *crtc, > >> @@ -10970,6 +10961,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; > >> + > >> init_waitqueue_head(&intel_crtc->vbl_wait); > >> > >> BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) || > >> diff --git a/drivers/gpu/drm/i915/intel_drv.h > >> b/drivers/gpu/drm/i915/intel_drv.h > >> index 9bb70dc..9d1ff7b 100644 > >> --- a/drivers/gpu/drm/i915/intel_drv.h > >> +++ b/drivers/gpu/drm/i915/intel_drv.h > >> @@ -386,7 +386,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.8.3.2 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> [email protected] > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > - Antti > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
