Re: [Intel-gfx] [PATCH v7 11/15] drm/i915: Support variable cursor height on ivb+

2017-05-05 Thread Imre Deak 
On Mon, Mar 27, 2017 at 09:55:42PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> 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.
> 
> v2: Reverse the gen check to make it sane
> v3: Only enable CUR_FBC_CTL when cursor is enabled, adapt to
> earlier code changes which means we now actually turn off
> the cursor when we're supposed to unlike v2
> v4: Add a comment about rotation vs. CUR_FBC_CTL,
> rebase due to 'dirty' (Chris)
> v5: Rebase to the atomic world
> Handle 180 degree rotation
> Add HAS_CUR_FBC()
> v6: Rebase
> v7: Rebase due to I915_WRITE_FW/uncore.lock
> s/size/fbc_ctl/
> 
> Signed-off-by: Ville Syrjälä 

Nitpick: cursor.size could've been changed to be a union of size,fb_ctl
too. Either way looks ok:
Reviewed-by: Imre Deak 

> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/i915_reg.h  |  5 -
>  drivers/gpu/drm/i915/intel_display.c | 38 
> +---
>  3 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 86f097db8ef6..9524ce0442ad 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2927,6 +2927,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 4d97d9fb2ad0..9027debb1cf9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5447,7 +5447,9 @@ enum {
>  #define   CURSOR_POS_SIGN   0x8000
>  #define   CURSOR_X_SHIFT0
>  #define   CURSOR_Y_SHIFT16
> -#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 _CURBCNTR0x700c0
>  #define _CURBBASE0x700c4
>  #define _CURBPOS 0x700c8
> @@ -5463,6 +5465,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 c6e026e617e5..53ec9d30437e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9364,17 +9364,16 @@ static u32 i9xx_cursor_ctl(const struct 
> intel_crtc_state *crtc_state,
>  
>  static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
>  {
> + struct drm_i915_private *dev_priv =
> + to_i915(plane_state->base.plane->dev);
>   int width = plane_state->base.crtc_w;
>   int height = plane_state->base.crtc_h;
>  
>   if (!intel_cursor_size_ok(plane_state))
>   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:
>   case 64:
> @@ -9383,6 +9382,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;
>  }
>  
> @@ -9444,12 +9458,15 @@ static void i9xx_update_cursor(struct intel_plane 
>

[Intel-gfx] [PATCH v7 11/15] drm/i915: Support variable cursor height on ivb+

2017-03-27 Thread ville . syrjala 
From: Ville Syrjälä 

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.

v2: Reverse the gen check to make it sane
v3: Only enable CUR_FBC_CTL when cursor is enabled, adapt to
earlier code changes which means we now actually turn off
the cursor when we're supposed to unlike v2
v4: Add a comment about rotation vs. CUR_FBC_CTL,
rebase due to 'dirty' (Chris)
v5: Rebase to the atomic world
Handle 180 degree rotation
Add HAS_CUR_FBC()
v6: Rebase
v7: Rebase due to I915_WRITE_FW/uncore.lock
s/size/fbc_ctl/

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/i915_reg.h  |  5 -
 drivers/gpu/drm/i915/intel_display.c | 38 +---
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 86f097db8ef6..9524ce0442ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2927,6 +2927,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 4d97d9fb2ad0..9027debb1cf9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5447,7 +5447,9 @@ enum {
 #define   CURSOR_POS_SIGN   0x8000
 #define   CURSOR_X_SHIFT0
 #define   CURSOR_Y_SHIFT16
-#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
@@ -5463,6 +5465,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 c6e026e617e5..53ec9d30437e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9364,17 +9364,16 @@ static u32 i9xx_cursor_ctl(const struct 
intel_crtc_state *crtc_state,
 
 static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
 {
+   struct drm_i915_private *dev_priv =
+   to_i915(plane_state->base.plane->dev);
int width = plane_state->base.crtc_w;
int height = plane_state->base.crtc_h;
 
if (!intel_cursor_size_ok(plane_state))
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:
case 64:
@@ -9383,6 +9382,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;
 }
 
@@ -9444,12 +9458,15 @@ 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, fbc_ctl = 0;
unsigned long irqflags;
 
if (plane_state && plane_state->base.visible) {
cntl = plane_state->ctl;
 
+   if (plane_state->base.crtc_h !=