Re: [Intel-gfx] [PATCH v2] drm/i915: Always apply cursor width changes
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 ch...@chris-wilson.co.uk
[Antti:rebased, adjusted macro names and moved some lines, no functional
changes]
Reviewed-by: Antti Koskipaa antti.koski...@linux.intel.com
Tested-by: Antti Koskipaa antti.koski...@linux.intel.com
Still missing the igt testcase. Is Antti still working on that one? And I
guess now we also need an Cc: sta...@vger.kernel.org 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
Re: [Intel-gfx] [PATCH v2] drm/i915: Always apply cursor width changes
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 ch...@chris-wilson.co.uk
[Antti:rebased, adjusted macro names and moved some lines, no functional
changes]
Reviewed-by: Antti Koskipaa antti.koski...@linux.intel.com
Tested-by: Antti Koskipaa antti.koski...@linux.intel.com
Still missing the igt testcase. Is Antti still working on that one?
The testcase was just posted. It just needed some cleanup.
And I
guess now we also need an Cc: sta...@vger.kernel.org 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;
-
Re: [Intel-gfx] [PATCH v2] drm/i915: Always apply cursor width changes
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 ch...@chris-wilson.co.uk
[Antti:rebased, adjusted macro names and moved some lines, no functional
changes]
Reviewed-by: Antti Koskipaa antti.koski...@linux.intel.com
Tested-by: Antti Koskipaa antti.koski...@linux.intel.com
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: sta...@vger.kernel.org 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)
