[PATCH] drm/i915: Fix cursor visibility check with negative coordinates

[PATCH] drm/i915: Fix cursor visibility check with negative coordinates

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When the cursor x coordinate is exactly -cursor_width, the cursor is
invisible. And obviously the same holds for the y coordinate and
cursor_height.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 17fe7ed..42e65ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6857,7 +6857,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 		base = 0;
 
 	if (x < 0) {
-		if (x + intel_crtc->cursor_width < 0)
+		if (x + intel_crtc->cursor_width <= 0)
 			base = 0;
 
 		pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
@@ -6866,7 +6866,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	pos |= x << CURSOR_X_SHIFT;
 
 	if (y < 0) {
-		if (y + intel_crtc->cursor_height < 0)
+		if (y + intel_crtc->cursor_height <= 0)
 			base = 0;
 
 		pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix cursor visibility check with negative coordinates

[Chris Wilson]

On Mon, Sep 02, 2013 at 09:16:50PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When the cursor x coordinate is exactly -cursor_width, the cursor is
> invisible. And obviously the same holds for the y coordinate and
> cursor_height.

And similarly for the earlier x == fb->width checks.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

First of all we should not be looking at fb->{width,height} as those do
not tell us what the actual pipe size is. Second of all we need to use
>= for the comparison.

So fix the comparison, and make use of the new pipe_src_{w,h} to
determine the real pipe source dimensions.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bbaa689..5055d2f8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6847,19 +6847,16 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	int pipe = intel_crtc->pipe;
 	int x = intel_crtc->cursor_x;
 	int y = intel_crtc->cursor_y;
-	u32 base, pos;
+	u32 base = 0, pos = 0;
 	bool visible;
 
-	pos = 0;
-
-	if (on && crtc->enabled && crtc->fb) {
+	if (on)
 		base = intel_crtc->cursor_addr;
-		if (x > (int) crtc->fb->width)
-			base = 0;
 
-		if (y > (int) crtc->fb->height)
-			base = 0;
-	} else
+	if (x >= intel_crtc->config.pipe_src_w)
+		base = 0;
+
+	if (y >= intel_crtc->config.pipe_src_h)
 		base = 0;
 
 	if (x < 0) {
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges

[Chris Wilson]

On Tue, Sep 03, 2013 at 11:04:30AM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> First of all we should not be looking at fb->{width,height} as those do
> not tell us what the actual pipe size is. Second of all we need to use
> >= for the comparison.
> 
> So fix the comparison, and make use of the new pipe_src_{w,h} to
> determine the real pipe source dimensions.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Whoops.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

And r-b for its sibling as well. I am pretty sure there are real-world
bugs out there, but they are going to pretty be rare and the same user
is never likely to see it twice... And just maybe more recent hw isn't
quite so hang-happy!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges

[Daniel Vetter]

On Tue, Sep 3, 2013 at 10:13 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Sep 03, 2013 at 11:04:30AM +0300, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> First of all we should not be looking at fb->{width,height} as those do
>> not tell us what the actual pipe size is. Second of all we need to use
>> >= for the comparison.
>>
>> So fix the comparison, and make use of the new pipe_src_{w,h} to
>> determine the real pipe source dimensions.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Whoops.
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> And r-b for its sibling as well. I am pretty sure there are real-world
> bugs out there, but they are going to pretty be rare and the same user
> is never likely to see it twice... And just maybe more recent hw isn't
> quite so hang-happy!

Hm, can we have an igt for this? We kinda don't any sprite/cursor test
at all right now, but we need to start somewhere ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch