Re: [Intel-gfx] [PATCH v3 4/4] drm/i915: Support variable cursor height on ivb+

2014-09-15 Thread Ville Syrjälä
On Sat, Sep 13, 2014 at 05:23:23PM +0100, Chris Wilson wrote:
 On Fri, Sep 12, 2014 at 08:53:35PM +0300, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@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.
  
  Commandeer the otherwise unused intel_crtc-cursor_size to track the
  current value of CUR_FBC_CTL so that we can avoid duplicating the
  complicated device type checks in i9xx_update_cursor().
  
  One caveat to note is that CUR_FBC_CTL can't be used with 180 degree
  rotation, so when cursor rotation gets introduced some extra checks are
  needed.
 
 Where would be a good place to put that note into a comment?

I guess I could stick it into cursor_size_ok().

 
 So other than the mystery of rotated cursors, the code looks clear
 enough. One side question, is the CHV/VLV conflation correct here?

Yes. It's still the same old g4x derived display controller.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 4/4] drm/i915: Support variable cursor height on ivb+

2014-09-13 Thread Chris Wilson
On Fri, Sep 12, 2014 at 08:53:35PM +0300, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@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.
 
 Commandeer the otherwise unused intel_crtc-cursor_size to track the
 current value of CUR_FBC_CTL so that we can avoid duplicating the
 complicated device type checks in i9xx_update_cursor().
 
 One caveat to note is that CUR_FBC_CTL can't be used with 180 degree
 rotation, so when cursor rotation gets introduced some extra checks are
 needed.

Where would be a good place to put that note into a comment?

So other than the mystery of rotated cursors, the code looks clear
enough. One side question, is the CHV/VLV conflation correct here?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v3 4/4] drm/i915: Support variable cursor height on ivb+

2014-09-12 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@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.

Commandeer the otherwise unused intel_crtc-cursor_size to track the
current value of CUR_FBC_CTL so that we can avoid duplicating the
complicated device type checks in i9xx_update_cursor().

One caveat to note is that CUR_FBC_CTL can't be used with 180 degree
rotation, so when cursor rotation gets introduced some extra checks are
needed.

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

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_reg.h  |  5 -
 drivers/gpu/drm/i915/intel_display.c | 25 +
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ad8179b..4bb2c31 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4181,7 +4181,9 @@ enum punit_power_well {
 #define   CURSOR_POS_SIGN   0x8000
 #define   CURSOR_X_SHIFT0
 #define   CURSOR_Y_SHIFT16
-#define CURSIZE0x700a0
+#define CURSIZE0x700a0 /* 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
@@ -4197,6 +4199,7 @@ enum punit_power_well {
 #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 60c1aa4..7c9c514 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8297,9 +8297,8 @@ 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;
-   uint32_t cntl;
+   uint32_t cntl = 0, fbc_ctl = 0;
 
-   cntl = 0;
if (base) {
cntl = MCURSOR_GAMMA_ENABLE;
switch (intel_crtc-cursor_width) {
@@ -8320,12 +8319,19 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, 
u32 base)
 
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
cntl |= CURSOR_PIPE_CSC_ENABLE;
+
+   if (intel_crtc-cursor_height != intel_crtc-cursor_width)
+   fbc_ctl = CUR_FBC_CTL_EN | (intel_crtc-cursor_height - 
1);
}
 
+   if (intel_crtc-cursor_size != fbc_ctl)
+   I915_WRITE(CUR_FBC_CTL(pipe), fbc_ctl);
+
if (intel_crtc-cursor_cntl != cntl)
I915_WRITE(CURCNTR(pipe), cntl);
 
-   if (intel_crtc-cursor_cntl == cntl 
+   if (intel_crtc-cursor_size == fbc_ctl 
+   intel_crtc-cursor_cntl == cntl 
intel_crtc-cursor_base == base)
return;
 
@@ -8333,6 +8339,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 
base)
I915_WRITE(CURBASE(pipe), base);
POSTING_READ(CURBASE(pipe));
 
+   intel_crtc-cursor_size = fbc_ctl;
intel_crtc-cursor_cntl = cntl;
intel_crtc-cursor_base = base;
 }
@@ -8398,6 +8405,8 @@ static bool cursor_size_ok(struct drm_device *dev,
 * the width of their cursors, the height is arbitrary up to
 * the precision of the register. Everything else requires
 * square cursors, limited to a few power-of-two sizes.
+* ivb+ have CUR_FBC_CTL which allows reducing the cursor
+* height down to a minimum of 8 lines.
 */
if (IS_845G(dev) || IS_I865G(dev)) {
if ((width  63) != 0)
@@ -8409,7 +8418,7 @@ static bool cursor_size_ok(struct drm_device *dev,
if (height  1023)
return false;
} else {
-   switch (width | height) {
+   switch (width) {
case 256:
case 128:
if (IS_GEN2(dev))
@@ -8419,6 +8428,14 @@ static bool cursor_size_ok(struct drm_device *dev,
default:
return false;
}
+
+   if (INTEL_INFO(dev)-gen  7 || IS_VALLEYVIEW(dev)) {
+   if (height != width)
+   return false;
+   } else {
+   if (height  8 || height  width)
+   return false;
+   }
}