| Message ID | 20170307152709.31957-11-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 07, 2017 at 05:27:09PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > IVB introduced the CUR_FBC_CTL register which allows reducing the cursor > height down to 8 lines from the otherwise square cursor dimensions. > Implement support for it. CUR_FBC_CTL can't be used when the cursor > is rotated. > > Commandeer the otherwise unused cursor->cursor.size to track the > current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL > writes, and to notice when we need to arm the update via CURBASE if > just CUR_FBC_CTL changes. For userspace to discover this, they should just use trial and error during startup (using the legacy SetCursor)? Though the easiest way would cause the cursor to flicker - just hope the cursor is off/invisible on startup. Code makes a lot of sense after all the refactoring, but I'll leave the register checking to somebody else unless they are lazier than I am. -Chris
On Tue, Mar 07, 2017 at 10:32:14PM +0000, Chris Wilson wrote: > On Tue, Mar 07, 2017 at 05:27:09PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > IVB introduced the CUR_FBC_CTL register which allows reducing the cursor > > height down to 8 lines from the otherwise square cursor dimensions. > > Implement support for it. CUR_FBC_CTL can't be used when the cursor > > is rotated. > > > > Commandeer the otherwise unused cursor->cursor.size to track the > > current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL > > writes, and to notice when we need to arm the update via CURBASE if > > just CUR_FBC_CTL changes. > > For userspace to discover this, they should just use trial and error > during startup (using the legacy SetCursor)? Though the easiest way > would cause the cursor to flicker - just hope the cursor is > off/invisible on startup. I don't have any decent answer for how to discover this feature. Atomic would allow the trial and error approach without flickers. Otherwise I guess one could defer the check until the cursor size actually changes. Although maybe that won't really work for Xorg. ISTR it wants to know things about the cursor size upfront? > Code makes a lot of sense after all the refactoring, but I'll leave the > register checking to somebody else unless they are lazier than I am. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
On Tue, Mar 07, 2017 at 10:32:14PM +0000, Chris Wilson wrote: > On Tue, Mar 07, 2017 at 05:27:09PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > IVB introduced the CUR_FBC_CTL register which allows reducing the cursor > > height down to 8 lines from the otherwise square cursor dimensions. > > Implement support for it. CUR_FBC_CTL can't be used when the cursor > > is rotated. > > > > Commandeer the otherwise unused cursor->cursor.size to track the > > current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL > > writes, and to notice when we need to arm the update via CURBASE if > > just CUR_FBC_CTL changes. > > For userspace to discover this, they should just use trial and error > during startup (using the legacy SetCursor)? Though the easiest way > would cause the cursor to flicker - just hope the cursor is > off/invisible on startup. > > Code makes a lot of sense after all the refactoring, but I'll leave the > register checking to somebody else unless they are lazier than I am. TEST_ONLY atomic commits on the cursor plane. That's pretty much how it's meant to be used (although no one yet figured out how to tell generic userspace about fancy constraints like this one here). -Daniel
On Wed, Mar 08, 2017 at 12:40:53PM +0200, Ville Syrjälä wrote: > On Tue, Mar 07, 2017 at 10:32:14PM +0000, Chris Wilson wrote: > > On Tue, Mar 07, 2017 at 05:27:09PM +0200, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > IVB introduced the CUR_FBC_CTL register which allows reducing the cursor > > > height down to 8 lines from the otherwise square cursor dimensions. > > > Implement support for it. CUR_FBC_CTL can't be used when the cursor > > > is rotated. > > > > > > Commandeer the otherwise unused cursor->cursor.size to track the > > > current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL > > > writes, and to notice when we need to arm the update via CURBASE if > > > just CUR_FBC_CTL changes. > > > > For userspace to discover this, they should just use trial and error > > during startup (using the legacy SetCursor)? Though the easiest way > > would cause the cursor to flicker - just hope the cursor is > > off/invisible on startup. > > I don't have any decent answer for how to discover this feature. Atomic > would allow the trial and error approach without flickers. Otherwise I > guess one could defer the check until the cursor size actually changes. > Although maybe that won't really work for Xorg. ISTR it wants to know > things about the cursor size upfront? We resize the cursor on the fly, so we could do a trial-and-error after a resize. But yes, the old cursor code used a static one-size fits all. (gem bo alloc is signalsafe, but malloc isn't -- so we just need to make sure we have a spare slot to create a new cursor in.) -Chris
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b7ec6636e2a0..c96b667d6e6e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2900,6 +2900,7 @@ intel_info(const struct drm_i915_private *dev_priv) #define HAS_FW_BLC(dev_priv) (INTEL_GEN(dev_priv) > 2) #define HAS_PIPE_CXSR(dev_priv) ((dev_priv)->info.has_pipe_cxsr) #define HAS_FBC(dev_priv) ((dev_priv)->info.has_fbc) +#define HAS_CUR_FBC(dev_priv) (!HAS_GMCH_DISPLAY(dev_priv) && INTEL_INFO(dev_priv)->gen >= 7) #define HAS_IPS(dev_priv) (IS_HSW_ULT(dev_priv) || IS_BROADWELL(dev_priv)) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 07cae03e8727..cfcb139e5952 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5449,7 +5449,9 @@ enum { #define CURSOR_POS_SIGN 0x8000 #define CURSOR_X_SHIFT 0 #define CURSOR_Y_SHIFT 16 -#define CURSIZE _MMIO(0x700a0) +#define CURSIZE _MMIO(0x700a0) /* 845/865 */ +#define _CUR_FBC_CTL_A 0x700a0 /* ivb+ */ +#define CUR_FBC_CTL_EN (1 << 31) #define _CURBCNTR 0x700c0 #define _CURBBASE 0x700c4 #define _CURBPOS 0x700c8 @@ -5465,6 +5467,7 @@ enum { #define CURCNTR(pipe) _CURSOR2(pipe, _CURACNTR) #define CURBASE(pipe) _CURSOR2(pipe, _CURABASE) #define CURPOS(pipe) _CURSOR2(pipe, _CURAPOS) +#define CUR_FBC_CTL(pipe) _CURSOR2(pipe, _CUR_FBC_CTL_A) #define CURSOR_A_OFFSET 0x70080 #define CURSOR_B_OFFSET 0x700c0 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 17362dc9f438..13b31e929631 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9347,7 +9347,7 @@ static void i9xx_update_cursor(struct intel_plane *plane, { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe; - u32 cntl = 0, base = 0, pos = 0; + u32 cntl = 0, base = 0, pos = 0, size = 0; if (plane_state && plane_state->base.visible) { cntl = MCURSOR_GAMMA_ENABLE; @@ -9376,15 +9376,22 @@ static void i9xx_update_cursor(struct intel_plane *plane, base = intel_cursor_base(plane, plane_state); pos = intel_cursor_position(plane, plane_state); + + if (plane_state->base.crtc_h != plane_state->base.crtc_w) + size = CUR_FBC_CTL_EN | (plane_state->base.crtc_h - 1); } if (plane->cursor.cntl != cntl) I915_WRITE(CURCNTR(pipe), cntl); + if (plane->cursor.size != size) + I915_WRITE(CUR_FBC_CTL(pipe), size); + if (cntl) I915_WRITE(CURPOS(pipe), pos); if (plane->cursor.cntl != cntl || + plane->cursor.size != size || plane->cursor.base != base) I915_WRITE(CURBASE(pipe), base); @@ -9392,6 +9399,7 @@ static void i9xx_update_cursor(struct intel_plane *plane, plane->cursor.cntl = cntl; plane->cursor.base = base; + plane->cursor.size = size; } static void i9xx_disable_cursor(struct intel_plane *plane, @@ -9410,11 +9418,8 @@ static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state) if (width == 0 || height == 0) return false; - /* - * Cursors are limited to a few power-of-two - * sizes, and they must be square. - */ - switch (width | height) { + /* Cursor width is limited to a few power-of-two sizes */ + switch (width) { case 256: case 128: if (IS_GEN2(dev_priv)) @@ -9425,6 +9430,21 @@ static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state) return false; } + /* + * IVB+ have CUR_FBC_CTL which allows an arbitrary cursor + * height from 8 lines up to the cursor width, when the + * cursor is not rotated. Everything else requires square + * cursors. + */ + if (HAS_CUR_FBC(dev_priv) && + plane_state->base.rotation & DRM_ROTATE_0) { + if (height < 8 || height > width) + return false; + } else { + if (height != width) + return false; + } + return true; } @@ -13833,7 +13853,9 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, cursor->cursor.base = ~0; cursor->cursor.cntl = ~0; - cursor->cursor.size = ~0; + + if (IS_I845G(dev_priv) || IS_I865G(dev_priv) || HAS_CUR_FBC(dev_priv)) + cursor->cursor.size = ~0; ret = drm_universal_plane_init(&dev_priv->drm, &cursor->base, 0, &intel_cursor_plane_funcs,