[PATCH 1/2] Revert "drm/i915: Workaround CHV pipe C cursor fail"

[PATCH 1/2] Revert "drm/i915: Workaround CHV pipe C cursor fail"

[Akshu Agrawal]

This reverts commit ef8dd37af85a8f37ca3a29074647511e52c56181.

Will use cropped cursor image for -ve co-ordinates instead of using
swcursor. Advantages of cropped cursor being, it will work for non X11
based platform like Chrome and also will use hwcursor.

Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 12ff795..bca9245 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14287,7 +14287,6 @@ intel_check_cursor_plane(struct drm_plane *plane,
 	struct drm_crtc *crtc = crtc_state->base.crtc;
 	struct drm_framebuffer *fb = state->base.fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	enum pipe pipe = to_intel_plane(plane)->pipe;
 	unsigned stride;
 	int ret;
 
@@ -14321,22 +14320,6 @@ intel_check_cursor_plane(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
-	/*
-	 * There's something wrong with the cursor on CHV pipe C.
-	 * If it straddles the left edge of the screen then
-	 * moving it away from the edge or disabling it often
-	 * results in a pipe underrun, and often that can lead to
-	 * dead pipe (constant underrun reported, and it scans
-	 * out just a solid color). To recover from that, the
-	 * display power well must be turned off and on again.
-	 * Refuse the put the cursor into that compromised position.
-	 */
-	if (IS_CHERRYVIEW(plane->dev) && pipe == PIPE_C &&
-	    state->visible && state->base.crtc_x < 0) {
-		DRM_DEBUG_KMS("CHV cursor C not allowed to straddle the left screen edge\n");
-		return -EINVAL;
-	}
-
 	return 0;
 }
 
-- 
1.9.1

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

[PATCH 2/2] drm/i915: Crop cursor image for CHV pipe C cursor issue

[Akshu Agrawal]

CHV pipe C hits underrun when we get -ve X values of cursor. To avoid
this we crop the cursor image for by -ve X value and thus use '0' as
least X value.

Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 113 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |   3 +
 2 files changed, 116 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bca9245..e6e6568 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14279,6 +14279,81 @@ void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *
 				plane->base.state->rotation);
 }
 
+static void vlv_unpin_buffer_obj(struct drm_i915_gem_object *obj,
+			  char __iomem *buffer_start)
+{
+	iounmap(buffer_start);
+	i915_gem_object_ggtt_unpin(obj);
+}
+
+static char __iomem *vlv_pin_and_map_buffer_obj
+				(struct drm_i915_gem_object *obj)
+{
+	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
+	char __iomem *buffer_start;
+	int ret;
+
+	ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
+	if (ret)
+		return NULL;
+
+	ret = i915_gem_object_set_to_gtt_domain(obj, true);
+	if (ret) {
+		i915_gem_object_ggtt_unpin(obj);
+		return NULL;
+	}
+
+	buffer_start = ioremap_wc(dev_priv->gtt.mappable_base +
+			i915_gem_obj_ggtt_offset(obj), obj->base.size);
+	if (buffer_start == NULL) {
+		i915_gem_object_ggtt_unpin(obj);
+		return NULL;
+	}
+
+	return buffer_start;
+}
+
+static int vlv_cursor_crop(struct intel_plane_state *state,
+				      int prev_x)
+{
+	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
+	struct drm_framebuffer *fb = state->base.fb;
+	char __iomem *buffer = state->vlv_cursor_image;
+	char __iomem *base, *cursor_base;
+	int size = obj->base.size;
+	int i, x = state->base.crtc_x;
+	int bytes_per_pixel = fb->bits_per_pixel / 8;
+
+	base = vlv_pin_and_map_buffer_obj(obj);
+	if (base == NULL)
+		return -ENOMEM;
+
+	if (prev_x >= 0) {
+		if (buffer != NULL)
+			kfree(buffer);
+		buffer = kzalloc(size, GFP_KERNEL);
+		state->vlv_cursor_image = buffer;
+		if (buffer == NULL)
+			return -ENOMEM;
+		memcpy(buffer, base, size);
+	}
+	cursor_base = buffer;
+	x = -x;
+	for (i = 0; i < state->base.crtc_h; i++) {
+		cursor_base += x * bytes_per_pixel;
+		memcpy(base, cursor_base,
+			(state->base.crtc_w - x) * bytes_per_pixel);
+		base += (state->base.crtc_w - x) * bytes_per_pixel;
+		memset(base, 0, x * bytes_per_pixel);
+		base += x * bytes_per_pixel;
+		cursor_base += (state->base.crtc_w - x) * bytes_per_pixel;
+	}
+
+	vlv_unpin_buffer_obj(obj, base);
+
+	return 0;
+}
+
 static int
 intel_check_cursor_plane(struct drm_plane *plane,
 			 struct intel_crtc_state *crtc_state,
@@ -14287,8 +14362,10 @@ intel_check_cursor_plane(struct drm_plane *plane,
 	struct drm_crtc *crtc = crtc_state->base.crtc;
 	struct drm_framebuffer *fb = state->base.fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+	enum pipe pipe = to_intel_plane(plane)->pipe;
 	unsigned stride;
 	int ret;
+	int crtc_prev_x = state->vlv_cursor_prev_x;
 
 	ret = drm_plane_helper_check_update(plane, crtc, fb, &state->src,
 					    &state->dst, &state->clip,
@@ -14309,6 +14386,32 @@ intel_check_cursor_plane(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
+	/*
+	 * There is an issue in CHV PIPE C where we hit underrun on
+	 * -ve value of cursor. To avoid this we are cropping the
+	 *  image for all PIPE C -ve values.
+	 */
+	if (IS_CHERRYVIEW(plane->dev)) {
+		if (pipe == PIPE_C && state->visible &&
+		    state->base.crtc_x < 0) {
+			ret = vlv_cursor_crop(state, crtc_prev_x);
+			if (ret)
+				return -ENOMEM;
+		} else if (crtc_prev_x < 0) { /* Restore the image back */
+			char __iomem *base;
+			char __iomem *org_image = state->vlv_cursor_image;
+			int size = obj->base.size;
+
+			if (org_image == NULL)
+				return -ENOMEM;
+			base = vlv_pin_and_map_buffer_obj(obj);
+			if (base == NULL)
+				return -ENOMEM;
+			memcpy(base, org_image, size);
+			vlv_unpin_buffer_obj(obj, base);
+		}
+	}
+
 	stride = roundup_pow_of_two(state->base.crtc_w) * 4;
 	if (obj->base.size < stride * state->base.crtc_h) {
 		DRM_DEBUG_KMS("buffer is too small\n");
@@ -14320,6 +14423,16 @@ intel_check_cursor_plane(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
+	if (IS_CHERRYVIEW(plane->dev)) {
+		if (pipe == PIPE_C &&
+		    state->visible && state->base.crtc_x < 0) {
+			state->vlv_cursor_prev_x = state->base.crtc_x;
+			state->base.crtc_x = 0;
+		} else {
+			state->vlv_cursor_prev_x = state->base.crtc_x;
+		}
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ebe7b34..0c406d1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -350,6 +350,9 @@ struct intel_plane_state {
 
 	/* async flip related structures */
 	struct drm_i915_gem_request *wait_req;
+
+	char __iomem *vlv_cursor_image;
+	int vlv_cursor_prev_x;
 };
 
 struct intel_initial_plane_config {
-- 
1.9.1

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

✗ Ro.CI.BAT: failure for series starting with [1/2] Revert "drm/i915: Workaround CHV pipe C cursor fail"

[Patchwork]

== Series Details ==

Series: series starting with [1/2] Revert "drm/i915: Workaround CHV pipe C cursor fail"
URL   : https://patchwork.freedesktop.org/series/8431/
State : failure

== Summary ==

  CC [M]  drivers/gpu/drm/i915/intel_dp_mst.o
  CC [M]  drivers/gpu/drm/i915/intel_dp.o
  CC [M]  drivers/gpu/drm/i915/intel_dsi.o
  CC [M]  drivers/gpu/drm/i915/intel_dsi_dcs_backlight.o
  CC [M]  drivers/gpu/drm/i915/intel_dsi_panel_vbt.o
  CC [M]  drivers/gpu/drm/i915/intel_dvo.o
  CC [M]  drivers/gpu/drm/i915/intel_dsi_pll.o
  CC [M]  drivers/gpu/drm/i915/intel_hdmi.o
  CC [M]  drivers/gpu/drm/i915/intel_lvds.o
  CC [M]  drivers/gpu/drm/i915/intel_i2c.o
  CC [M]  drivers/gpu/drm/i915/intel_panel.o
  CC [M]  drivers/gpu/drm/i915/intel_sdvo.o
  CC [M]  drivers/gpu/drm/i915/intel_tv.o
  CC [M]  drivers/gpu/drm/i915/i915_vgpu.o
  CC [M]  drivers/gpu/drm/i915/i915_dma.o
drivers/gpu/drm/i915/intel_display.c: In function ‘vlv_pin_and_map_buffer_obj’:
drivers/gpu/drm/i915/intel_display.c:14306:36: error: ‘struct drm_i915_private’ has no member named ‘gtt’
  buffer_start = ioremap_wc(dev_priv->gtt.mappable_base +
                                    ^
scripts/Makefile.build:289: recipe for target 'drivers/gpu/drm/i915/intel_display.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_display.o] Error 1
make[4]: *** Waiting for unfinished jobs....
scripts/Makefile.build:440: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:440: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:440: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:985: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

Re: [PATCH 2/2] drm/i915: Crop cursor image for CHV pipe C cursor issue

[Daniel Vetter]

On Wed, Jun 08, 2016 at 01:57:44PM +0530, Akshu Agrawal wrote:
> CHV pipe C hits underrun when we get -ve X values of cursor. To avoid
> this we crop the cursor image for by -ve X value and thus use '0' as
> least X value.

You're talking about "-ve" here and there's absolutely no "-ve" anywhere
in your patch. That makes your commit message non-understandable.

But I think I get what you're doing here, and nope, you can't override the
cursor image like that. If we go with this w/a then you need to allocate a
new cursor gem bo instead. This is userspace-visible.

For similar reasons you're also not allowed to change crtc->state.
-Daniel

> Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 113 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |   3 +
>  2 files changed, 116 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bca9245..e6e6568 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14279,6 +14279,81 @@ void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *
>  				plane->base.state->rotation);
>  }
>  
> +static void vlv_unpin_buffer_obj(struct drm_i915_gem_object *obj,
> +			  char __iomem *buffer_start)
> +{
> +	iounmap(buffer_start);
> +	i915_gem_object_ggtt_unpin(obj);
> +}
> +
> +static char __iomem *vlv_pin_and_map_buffer_obj
> +				(struct drm_i915_gem_object *obj)
> +{
> +	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> +	char __iomem *buffer_start;
> +	int ret;
> +
> +	ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
> +	if (ret)
> +		return NULL;
> +
> +	ret = i915_gem_object_set_to_gtt_domain(obj, true);
> +	if (ret) {
> +		i915_gem_object_ggtt_unpin(obj);
> +		return NULL;
> +	}
> +
> +	buffer_start = ioremap_wc(dev_priv->gtt.mappable_base +
> +			i915_gem_obj_ggtt_offset(obj), obj->base.size);
> +	if (buffer_start == NULL) {
> +		i915_gem_object_ggtt_unpin(obj);
> +		return NULL;
> +	}
> +
> +	return buffer_start;
> +}
> +
> +static int vlv_cursor_crop(struct intel_plane_state *state,
> +				      int prev_x)
> +{
> +	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
> +	struct drm_framebuffer *fb = state->base.fb;
> +	char __iomem *buffer = state->vlv_cursor_image;
> +	char __iomem *base, *cursor_base;
> +	int size = obj->base.size;
> +	int i, x = state->base.crtc_x;
> +	int bytes_per_pixel = fb->bits_per_pixel / 8;
> +
> +	base = vlv_pin_and_map_buffer_obj(obj);
> +	if (base == NULL)
> +		return -ENOMEM;
> +
> +	if (prev_x >= 0) {
> +		if (buffer != NULL)
> +			kfree(buffer);
> +		buffer = kzalloc(size, GFP_KERNEL);
> +		state->vlv_cursor_image = buffer;
> +		if (buffer == NULL)
> +			return -ENOMEM;
> +		memcpy(buffer, base, size);
> +	}
> +	cursor_base = buffer;
> +	x = -x;
> +	for (i = 0; i < state->base.crtc_h; i++) {
> +		cursor_base += x * bytes_per_pixel;
> +		memcpy(base, cursor_base,
> +			(state->base.crtc_w - x) * bytes_per_pixel);
> +		base += (state->base.crtc_w - x) * bytes_per_pixel;
> +		memset(base, 0, x * bytes_per_pixel);
> +		base += x * bytes_per_pixel;
> +		cursor_base += (state->base.crtc_w - x) * bytes_per_pixel;
> +	}
> +
> +	vlv_unpin_buffer_obj(obj, base);
> +
> +	return 0;
> +}
> +
>  static int
>  intel_check_cursor_plane(struct drm_plane *plane,
>  			 struct intel_crtc_state *crtc_state,
> @@ -14287,8 +14362,10 @@ intel_check_cursor_plane(struct drm_plane *plane,
>  	struct drm_crtc *crtc = crtc_state->base.crtc;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> +	enum pipe pipe = to_intel_plane(plane)->pipe;
>  	unsigned stride;
>  	int ret;
> +	int crtc_prev_x = state->vlv_cursor_prev_x;
>  
>  	ret = drm_plane_helper_check_update(plane, crtc, fb, &state->src,
>  					    &state->dst, &state->clip,
> @@ -14309,6 +14386,32 @@ intel_check_cursor_plane(struct drm_plane *plane,
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * There is an issue in CHV PIPE C where we hit underrun on
> +	 * -ve value of cursor. To avoid this we are cropping the
> +	 *  image for all PIPE C -ve values.
> +	 */
> +	if (IS_CHERRYVIEW(plane->dev)) {
> +		if (pipe == PIPE_C && state->visible &&
> +		    state->base.crtc_x < 0) {
> +			ret = vlv_cursor_crop(state, crtc_prev_x);
> +			if (ret)
> +				return -ENOMEM;
> +		} else if (crtc_prev_x < 0) { /* Restore the image back */
> +			char __iomem *base;
> +			char __iomem *org_image = state->vlv_cursor_image;
> +			int size = obj->base.size;
> +
> +			if (org_image == NULL)
> +				return -ENOMEM;
> +			base = vlv_pin_and_map_buffer_obj(obj);
> +			if (base == NULL)
> +				return -ENOMEM;
> +			memcpy(base, org_image, size);
> +			vlv_unpin_buffer_obj(obj, base);
> +		}
> +	}
> +
>  	stride = roundup_pow_of_two(state->base.crtc_w) * 4;
>  	if (obj->base.size < stride * state->base.crtc_h) {
>  		DRM_DEBUG_KMS("buffer is too small\n");
> @@ -14320,6 +14423,16 @@ intel_check_cursor_plane(struct drm_plane *plane,
>  		return -EINVAL;
>  	}
>  
> +	if (IS_CHERRYVIEW(plane->dev)) {
> +		if (pipe == PIPE_C &&
> +		    state->visible && state->base.crtc_x < 0) {
> +			state->vlv_cursor_prev_x = state->base.crtc_x;
> +			state->base.crtc_x = 0;
> +		} else {
> +			state->vlv_cursor_prev_x = state->base.crtc_x;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ebe7b34..0c406d1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -350,6 +350,9 @@ struct intel_plane_state {
>  
>  	/* async flip related structures */
>  	struct drm_i915_gem_request *wait_req;
> +
> +	char __iomem *vlv_cursor_image;
> +	int vlv_cursor_prev_x;
>  };
>  
>  struct intel_initial_plane_config {
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Crop cursor image for CHV pipe C cursor issue

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1512 bytes --]

Hi,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.7-rc2 next-20160608]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Akshu-Agrawal/Revert-drm-i915-Workaround-CHV-pipe-C-cursor-fail/20160608-163007
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-defconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_display.c: In function 'vlv_pin_and_map_buffer_obj':
>> drivers/gpu/drm/i915/intel_display.c:14306:36: error: 'struct drm_i915_private' has no member named 'gtt'; did you mean 'ggtt'?
     buffer_start = ioremap_wc(dev_priv->gtt.mappable_base +
                                       ^~

vim +14306 drivers/gpu/drm/i915/intel_display.c

 14300		ret = i915_gem_object_set_to_gtt_domain(obj, true);
 14301		if (ret) {
 14302			i915_gem_object_ggtt_unpin(obj);
 14303			return NULL;
 14304		}
 14305	
 14306		buffer_start = ioremap_wc(dev_priv->gtt.mappable_base +
 14307				i915_gem_obj_ggtt_offset(obj), obj->base.size);
 14308		if (buffer_start == NULL) {
 14309			i915_gem_object_ggtt_unpin(obj);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24879 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 2/2] drm/i915: Crop cursor image for CHV pipe C cursor issue

[Dave Gordon]

On 08/06/16 09:40, Daniel Vetter wrote:
> On Wed, Jun 08, 2016 at 01:57:44PM +0530, Akshu Agrawal wrote:
>> CHV pipe C hits underrun when we get -ve X values of cursor. To avoid
>> this we crop the cursor image for by -ve X value and thus use '0' as
>> least X value.
>
> You're talking about "-ve" here and there's absolutely no "-ve" anywhere
> in your patch. That makes your commit message non-understandable.

That's shorthand for "negative", and some of the code below is indeed 
testing for a negative X coordinate, e.g:

[snip]

>> +	/*
>> +	 * There is an issue in CHV PIPE C where we hit underrun on
>> +	 * -ve value of cursor. To avoid this we are cropping the
>> +	 *  image for all PIPE C -ve values.
>> +	 */
>> +	if (IS_CHERRYVIEW(plane->dev)) {
>> +		if (pipe == PIPE_C && state->visible &&
>> +		    state->base.crtc_x < 0) {
>> +			ret = vlv_cursor_crop(state, crtc_prev_x);
>> +			if (ret)
>> +				return -ENOMEM;
>> +		} else if (crtc_prev_x < 0) { /* Restore the image back */
>> +			char __iomem *base;
>> +			char __iomem *org_image = state->vlv_cursor_image;
>> +			int size = obj->base.size;
>> +
>> +			if (org_image == NULL)
>> +				return -ENOMEM;
>> +			base = vlv_pin_and_map_buffer_obj(obj);
>> +			if (base == NULL)
>> +				return -ENOMEM;
>> +			memcpy(base, org_image, size);
>> +			vlv_unpin_buffer_obj(obj, base);
>> +		}
>> +	}

HTH,
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Crop cursor image for CHV pipe C cursor issue

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1482 bytes --]

Hi,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.7-rc2 next-20160608]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Akshu-Agrawal/Revert-drm-i915-Workaround-CHV-pipe-C-cursor-fail/20160608-163007
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-rhel (attached as .config)
compiler: gcc-4.9 (Debian 4.9.3-14) 4.9.3
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_display.c: In function 'vlv_pin_and_map_buffer_obj':
>> drivers/gpu/drm/i915/intel_display.c:14306:36: error: 'struct drm_i915_private' has no member named 'gtt'
     buffer_start = ioremap_wc(dev_priv->gtt.mappable_base +
                                       ^

vim +14306 drivers/gpu/drm/i915/intel_display.c

 14300		ret = i915_gem_object_set_to_gtt_domain(obj, true);
 14301		if (ret) {
 14302			i915_gem_object_ggtt_unpin(obj);
 14303			return NULL;
 14304		}
 14305	
 14306		buffer_start = ioremap_wc(dev_priv->gtt.mappable_base +
 14307				i915_gem_obj_ggtt_offset(obj), obj->base.size);
 14308		if (buffer_start == NULL) {
 14309			i915_gem_object_ggtt_unpin(obj);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 37214 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 2/2] drm/i915: Crop cursor image for CHV pipe C cursor issue

[Daniel Vetter]

On Wed, Jun 8, 2016 at 12:18 PM, Dave Gordon <david.s.gordon@intel.com> wrote:
> On 08/06/16 09:40, Daniel Vetter wrote:
>>
>> On Wed, Jun 08, 2016 at 01:57:44PM +0530, Akshu Agrawal wrote:
>>>
>>> CHV pipe C hits underrun when we get -ve X values of cursor. To avoid
>>> this we crop the cursor image for by -ve X value and thus use '0' as
>>> least X value.
>>
>>
>> You're talking about "-ve" here and there's absolutely no "-ve" anywhere
>> in your patch. That makes your commit message non-understandable.
>
>
> That's shorthand for "negative", and some of the code below is indeed
> testing for a negative X coordinate, e.g:

Just type it out, thanks. Not everyone is a native speaker and knows
all this stuff.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Crop cursor image for CHV pipe C cursor issue

[Agrawal, Akshu]

On 6/8/2016 2:10 PM, Daniel Vetter wrote:
> On Wed, Jun 08, 2016 at 01:57:44PM +0530, Akshu Agrawal wrote:
>> CHV pipe C hits underrun when we get -ve X values of cursor. To avoid
>> this we crop the cursor image for by -ve X value and thus use '0' as
>> least X value.
>
> You're talking about "-ve" here and there's absolutely no "-ve" anywhere
> in your patch. That makes your commit message non-understandable.

Will change -ve to "negative" for better readability.

>
> But I think I get what you're doing here, and nope, you can't override the
> cursor image like that. If we go with this w/a then you need to allocate a
> new cursor gem bo instead. This is userspace-visible.

Can't we use the same gem bo? As we are calling 
"i915_gem_object_set_to_gtt_domain" to get write access for CPU after 
unbinding from GTT.

Regards,
Akshu
>
> For similar reasons you're also not allowed to change crtc->state.
> -Daniel
>
>> Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 113 +++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_drv.h     |   3 +
>>  2 files changed, 116 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index bca9245..e6e6568 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -14279,6 +14279,81 @@ void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *
>>  				plane->base.state->rotation);
>>  }
>>
>> +static void vlv_unpin_buffer_obj(struct drm_i915_gem_object *obj,
>> +			  char __iomem *buffer_start)
>> +{
>> +	iounmap(buffer_start);
>> +	i915_gem_object_ggtt_unpin(obj);
>> +}
>> +
>> +static char __iomem *vlv_pin_and_map_buffer_obj
>> +				(struct drm_i915_gem_object *obj)
>> +{
>> +	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
>> +	char __iomem *buffer_start;
>> +	int ret;
>> +
>> +	ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
>> +	if (ret)
>> +		return NULL;
>> +
>> +	ret = i915_gem_object_set_to_gtt_domain(obj, true);
>> +	if (ret) {
>> +		i915_gem_object_ggtt_unpin(obj);
>> +		return NULL;
>> +	}
>> +
>> +	buffer_start = ioremap_wc(dev_priv->gtt.mappable_base +
>> +			i915_gem_obj_ggtt_offset(obj), obj->base.size);
>> +	if (buffer_start == NULL) {
>> +		i915_gem_object_ggtt_unpin(obj);
>> +		return NULL;
>> +	}
>> +
>> +	return buffer_start;
>> +}
>> +
>> +static int vlv_cursor_crop(struct intel_plane_state *state,
>> +				      int prev_x)
>> +{
>> +	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
>> +	struct drm_framebuffer *fb = state->base.fb;
>> +	char __iomem *buffer = state->vlv_cursor_image;
>> +	char __iomem *base, *cursor_base;
>> +	int size = obj->base.size;
>> +	int i, x = state->base.crtc_x;
>> +	int bytes_per_pixel = fb->bits_per_pixel / 8;
>> +
>> +	base = vlv_pin_and_map_buffer_obj(obj);
>> +	if (base == NULL)
>> +		return -ENOMEM;
>> +
>> +	if (prev_x >= 0) {
>> +		if (buffer != NULL)
>> +			kfree(buffer);
>> +		buffer = kzalloc(size, GFP_KERNEL);
>> +		state->vlv_cursor_image = buffer;
>> +		if (buffer == NULL)
>> +			return -ENOMEM;
>> +		memcpy(buffer, base, size);
>> +	}
>> +	cursor_base = buffer;
>> +	x = -x;
>> +	for (i = 0; i < state->base.crtc_h; i++) {
>> +		cursor_base += x * bytes_per_pixel;
>> +		memcpy(base, cursor_base,
>> +			(state->base.crtc_w - x) * bytes_per_pixel);
>> +		base += (state->base.crtc_w - x) * bytes_per_pixel;
>> +		memset(base, 0, x * bytes_per_pixel);
>> +		base += x * bytes_per_pixel;
>> +		cursor_base += (state->base.crtc_w - x) * bytes_per_pixel;
>> +	}
>> +
>> +	vlv_unpin_buffer_obj(obj, base);
>> +
>> +	return 0;
>> +}
>> +
>>  static int
>>  intel_check_cursor_plane(struct drm_plane *plane,
>>  			 struct intel_crtc_state *crtc_state,
>> @@ -14287,8 +14362,10 @@ intel_check_cursor_plane(struct drm_plane *plane,
>>  	struct drm_crtc *crtc = crtc_state->base.crtc;
>>  	struct drm_framebuffer *fb = state->base.fb;
>>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>> +	enum pipe pipe = to_intel_plane(plane)->pipe;
>>  	unsigned stride;
>>  	int ret;
>> +	int crtc_prev_x = state->vlv_cursor_prev_x;
>>
>>  	ret = drm_plane_helper_check_update(plane, crtc, fb, &state->src,
>>  					    &state->dst, &state->clip,
>> @@ -14309,6 +14386,32 @@ intel_check_cursor_plane(struct drm_plane *plane,
>>  		return -EINVAL;
>>  	}
>>
>> +	/*
>> +	 * There is an issue in CHV PIPE C where we hit underrun on
>> +	 * -ve value of cursor. To avoid this we are cropping the
>> +	 *  image for all PIPE C -ve values.
>> +	 */
>> +	if (IS_CHERRYVIEW(plane->dev)) {
>> +		if (pipe == PIPE_C && state->visible &&
>> +		    state->base.crtc_x < 0) {
>> +			ret = vlv_cursor_crop(state, crtc_prev_x);
>> +			if (ret)
>> +				return -ENOMEM;
>> +		} else if (crtc_prev_x < 0) { /* Restore the image back */
>> +			char __iomem *base;
>> +			char __iomem *org_image = state->vlv_cursor_image;
>> +			int size = obj->base.size;
>> +
>> +			if (org_image == NULL)
>> +				return -ENOMEM;
>> +			base = vlv_pin_and_map_buffer_obj(obj);
>> +			if (base == NULL)
>> +				return -ENOMEM;
>> +			memcpy(base, org_image, size);
>> +			vlv_unpin_buffer_obj(obj, base);
>> +		}
>> +	}
>> +
>>  	stride = roundup_pow_of_two(state->base.crtc_w) * 4;
>>  	if (obj->base.size < stride * state->base.crtc_h) {
>>  		DRM_DEBUG_KMS("buffer is too small\n");
>> @@ -14320,6 +14423,16 @@ intel_check_cursor_plane(struct drm_plane *plane,
>>  		return -EINVAL;
>>  	}
>>
>> +	if (IS_CHERRYVIEW(plane->dev)) {
>> +		if (pipe == PIPE_C &&
>> +		    state->visible && state->base.crtc_x < 0) {
>> +			state->vlv_cursor_prev_x = state->base.crtc_x;
>> +			state->base.crtc_x = 0;
>> +		} else {
>> +			state->vlv_cursor_prev_x = state->base.crtc_x;
>> +		}
>> +	}
>> +
>>  	return 0;
>>  }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index ebe7b34..0c406d1 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -350,6 +350,9 @@ struct intel_plane_state {
>>
>>  	/* async flip related structures */
>>  	struct drm_i915_gem_request *wait_req;
>> +
>> +	char __iomem *vlv_cursor_image;
>> +	int vlv_cursor_prev_x;
>>  };
>>
>>  struct intel_initial_plane_config {
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Crop cursor image for CHV pipe C cursor issue

[Daniel Vetter]

On Fri, Jun 10, 2016 at 03:14:36PM +0530, Agrawal, Akshu wrote:
> On 6/8/2016 2:10 PM, Daniel Vetter wrote:
> > On Wed, Jun 08, 2016 at 01:57:44PM +0530, Akshu Agrawal wrote:
> > > CHV pipe C hits underrun when we get -ve X values of cursor. To avoid
> > > this we crop the cursor image for by -ve X value and thus use '0' as
> > > least X value.
> > 
> > You're talking about "-ve" here and there's absolutely no "-ve" anywhere
> > in your patch. That makes your commit message non-understandable.
> 
> Will change -ve to "negative" for better readability.
> 
> > 
> > But I think I get what you're doing here, and nope, you can't override the
> > cursor image like that. If we go with this w/a then you need to allocate a
> > new cursor gem bo instead. This is userspace-visible.
> 
> Can't we use the same gem bo? As we are calling
> "i915_gem_object_set_to_gtt_domain" to get write access for CPU after
> unbinding from GTT.

Userspace can write to the underlying bo without telling us. That'll cause
tearing. And it could also read from it, which would result in even more
serious bugs.

We can't just change userspace-visible state for a w/a. Same reasoning
applies to the crtc_state stuff. Again we need copies, to avoid
accidentally confusion userspace.

With all those copies it's probably best to write a special
atomic_update_plane function for cursor pipe C, to avoid spreading all of
them all over the driver.
-Daniel

> 
> Regards,
> Akshu
> > 
> > For similar reasons you're also not allowed to change crtc->state.
> > -Daniel
> > 
> > > Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 113 +++++++++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_drv.h     |   3 +
> > >  2 files changed, 116 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index bca9245..e6e6568 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -14279,6 +14279,81 @@ void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *
> > >  				plane->base.state->rotation);
> > >  }
> > > 
> > > +static void vlv_unpin_buffer_obj(struct drm_i915_gem_object *obj,
> > > +			  char __iomem *buffer_start)
> > > +{
> > > +	iounmap(buffer_start);
> > > +	i915_gem_object_ggtt_unpin(obj);
> > > +}
> > > +
> > > +static char __iomem *vlv_pin_and_map_buffer_obj
> > > +				(struct drm_i915_gem_object *obj)
> > > +{
> > > +	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> > > +	char __iomem *buffer_start;
> > > +	int ret;
> > > +
> > > +	ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
> > > +	if (ret)
> > > +		return NULL;
> > > +
> > > +	ret = i915_gem_object_set_to_gtt_domain(obj, true);
> > > +	if (ret) {
> > > +		i915_gem_object_ggtt_unpin(obj);
> > > +		return NULL;
> > > +	}
> > > +
> > > +	buffer_start = ioremap_wc(dev_priv->gtt.mappable_base +
> > > +			i915_gem_obj_ggtt_offset(obj), obj->base.size);
> > > +	if (buffer_start == NULL) {
> > > +		i915_gem_object_ggtt_unpin(obj);
> > > +		return NULL;
> > > +	}
> > > +
> > > +	return buffer_start;
> > > +}
> > > +
> > > +static int vlv_cursor_crop(struct intel_plane_state *state,
> > > +				      int prev_x)
> > > +{
> > > +	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
> > > +	struct drm_framebuffer *fb = state->base.fb;
> > > +	char __iomem *buffer = state->vlv_cursor_image;
> > > +	char __iomem *base, *cursor_base;
> > > +	int size = obj->base.size;
> > > +	int i, x = state->base.crtc_x;
> > > +	int bytes_per_pixel = fb->bits_per_pixel / 8;
> > > +
> > > +	base = vlv_pin_and_map_buffer_obj(obj);
> > > +	if (base == NULL)
> > > +		return -ENOMEM;
> > > +
> > > +	if (prev_x >= 0) {
> > > +		if (buffer != NULL)
> > > +			kfree(buffer);
> > > +		buffer = kzalloc(size, GFP_KERNEL);
> > > +		state->vlv_cursor_image = buffer;
> > > +		if (buffer == NULL)
> > > +			return -ENOMEM;
> > > +		memcpy(buffer, base, size);
> > > +	}
> > > +	cursor_base = buffer;
> > > +	x = -x;
> > > +	for (i = 0; i < state->base.crtc_h; i++) {
> > > +		cursor_base += x * bytes_per_pixel;
> > > +		memcpy(base, cursor_base,
> > > +			(state->base.crtc_w - x) * bytes_per_pixel);
> > > +		base += (state->base.crtc_w - x) * bytes_per_pixel;
> > > +		memset(base, 0, x * bytes_per_pixel);
> > > +		base += x * bytes_per_pixel;
> > > +		cursor_base += (state->base.crtc_w - x) * bytes_per_pixel;
> > > +	}
> > > +
> > > +	vlv_unpin_buffer_obj(obj, base);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  static int
> > >  intel_check_cursor_plane(struct drm_plane *plane,
> > >  			 struct intel_crtc_state *crtc_state,
> > > @@ -14287,8 +14362,10 @@ intel_check_cursor_plane(struct drm_plane *plane,
> > >  	struct drm_crtc *crtc = crtc_state->base.crtc;
> > >  	struct drm_framebuffer *fb = state->base.fb;
> > >  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> > > +	enum pipe pipe = to_intel_plane(plane)->pipe;
> > >  	unsigned stride;
> > >  	int ret;
> > > +	int crtc_prev_x = state->vlv_cursor_prev_x;
> > > 
> > >  	ret = drm_plane_helper_check_update(plane, crtc, fb, &state->src,
> > >  					    &state->dst, &state->clip,
> > > @@ -14309,6 +14386,32 @@ intel_check_cursor_plane(struct drm_plane *plane,
> > >  		return -EINVAL;
> > >  	}
> > > 
> > > +	/*
> > > +	 * There is an issue in CHV PIPE C where we hit underrun on
> > > +	 * -ve value of cursor. To avoid this we are cropping the
> > > +	 *  image for all PIPE C -ve values.
> > > +	 */
> > > +	if (IS_CHERRYVIEW(plane->dev)) {
> > > +		if (pipe == PIPE_C && state->visible &&
> > > +		    state->base.crtc_x < 0) {
> > > +			ret = vlv_cursor_crop(state, crtc_prev_x);
> > > +			if (ret)
> > > +				return -ENOMEM;
> > > +		} else if (crtc_prev_x < 0) { /* Restore the image back */
> > > +			char __iomem *base;
> > > +			char __iomem *org_image = state->vlv_cursor_image;
> > > +			int size = obj->base.size;
> > > +
> > > +			if (org_image == NULL)
> > > +				return -ENOMEM;
> > > +			base = vlv_pin_and_map_buffer_obj(obj);
> > > +			if (base == NULL)
> > > +				return -ENOMEM;
> > > +			memcpy(base, org_image, size);
> > > +			vlv_unpin_buffer_obj(obj, base);
> > > +		}
> > > +	}
> > > +
> > >  	stride = roundup_pow_of_two(state->base.crtc_w) * 4;
> > >  	if (obj->base.size < stride * state->base.crtc_h) {
> > >  		DRM_DEBUG_KMS("buffer is too small\n");
> > > @@ -14320,6 +14423,16 @@ intel_check_cursor_plane(struct drm_plane *plane,
> > >  		return -EINVAL;
> > >  	}
> > > 
> > > +	if (IS_CHERRYVIEW(plane->dev)) {
> > > +		if (pipe == PIPE_C &&
> > > +		    state->visible && state->base.crtc_x < 0) {
> > > +			state->vlv_cursor_prev_x = state->base.crtc_x;
> > > +			state->base.crtc_x = 0;
> > > +		} else {
> > > +			state->vlv_cursor_prev_x = state->base.crtc_x;
> > > +		}
> > > +	}
> > > +
> > >  	return 0;
> > >  }
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index ebe7b34..0c406d1 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -350,6 +350,9 @@ struct intel_plane_state {
> > > 
> > >  	/* async flip related structures */
> > >  	struct drm_i915_gem_request *wait_req;
> > > +
> > > +	char __iomem *vlv_cursor_image;
> > > +	int vlv_cursor_prev_x;
> > >  };
> > > 
> > >  struct intel_initial_plane_config {
> > > --
> > > 1.9.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Crop cursor image for CHV pipe C cursor issue

[Shobhit Kumar]

On Mon, Jun 13, 2016 at 7:52 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Jun 10, 2016 at 03:14:36PM +0530, Agrawal, Akshu wrote:
>> On 6/8/2016 2:10 PM, Daniel Vetter wrote:
>> > On Wed, Jun 08, 2016 at 01:57:44PM +0530, Akshu Agrawal wrote:
>> > > CHV pipe C hits underrun when we get -ve X values of cursor. To avoid
>> > > this we crop the cursor image for by -ve X value and thus use '0' as
>> > > least X value.
>> >
>> > You're talking about "-ve" here and there's absolutely no "-ve" anywhere
>> > in your patch. That makes your commit message non-understandable.
>>
>> Will change -ve to "negative" for better readability.
>>
>> >
>> > But I think I get what you're doing here, and nope, you can't override the
>> > cursor image like that. If we go with this w/a then you need to allocate a
>> > new cursor gem bo instead. This is userspace-visible.
>>
>> Can't we use the same gem bo? As we are calling
>> "i915_gem_object_set_to_gtt_domain" to get write access for CPU after
>> unbinding from GTT.
>
> Userspace can write to the underlying bo without telling us. That'll cause
> tearing. And it could also read from it, which would result in even more
> serious bugs.
>
> We can't just change userspace-visible state for a w/a. Same reasoning
> applies to the crtc_state stuff. Again we need copies, to avoid
> accidentally confusion userspace.
>

Agreed Daniel. Though chances user space doing something like this are
less, but nevertheless it can happen.

> With all those copies it's probably best to write a special
> atomic_update_plane function for cursor pipe C, to avoid spreading all of
> them all over the driver.

Is implementing a intel_chv_update_pipe_c_cursor_plane and
initializing that as the update_plane callback in case of PIPE C on
CHV during cursor plane creation is what you have in mind ? We can
then implement the copies inside this function. No too sure on the
atomic path so just want to confirm if this will work well.

Regards
Shobhit

> -Daniel
>
>>
>> Regards,
>> Akshu
>> >
>> > For similar reasons you're also not allowed to change crtc->state.
>> > -Daniel
>> >
>> > > Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
>> > > ---
>> > >  drivers/gpu/drm/i915/intel_display.c | 113 +++++++++++++++++++++++++++++++++++
>> > >  drivers/gpu/drm/i915/intel_drv.h     |   3 +
>> > >  2 files changed, 116 insertions(+)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > > index bca9245..e6e6568 100644
>> > > --- a/drivers/gpu/drm/i915/intel_display.c
>> > > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > > @@ -14279,6 +14279,81 @@ void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *
>> > >                           plane->base.state->rotation);
>> > >  }
>> > >
>> > > +static void vlv_unpin_buffer_obj(struct drm_i915_gem_object *obj,
>> > > +                   char __iomem *buffer_start)
>> > > +{
>> > > + iounmap(buffer_start);
>> > > + i915_gem_object_ggtt_unpin(obj);
>> > > +}
>> > > +
>> > > +static char __iomem *vlv_pin_and_map_buffer_obj
>> > > +                         (struct drm_i915_gem_object *obj)
>> > > +{
>> > > + struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
>> > > + char __iomem *buffer_start;
>> > > + int ret;
>> > > +
>> > > + ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
>> > > + if (ret)
>> > > +         return NULL;
>> > > +
>> > > + ret = i915_gem_object_set_to_gtt_domain(obj, true);
>> > > + if (ret) {
>> > > +         i915_gem_object_ggtt_unpin(obj);
>> > > +         return NULL;
>> > > + }
>> > > +
>> > > + buffer_start = ioremap_wc(dev_priv->gtt.mappable_base +
>> > > +                 i915_gem_obj_ggtt_offset(obj), obj->base.size);
>> > > + if (buffer_start == NULL) {
>> > > +         i915_gem_object_ggtt_unpin(obj);
>> > > +         return NULL;
>> > > + }
>> > > +
>> > > + return buffer_start;
>> > > +}
>> > > +
>> > > +static int vlv_cursor_crop(struct intel_plane_state *state,
>> > > +                               int prev_x)
>> > > +{
>> > > + struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
>> > > + struct drm_framebuffer *fb = state->base.fb;
>> > > + char __iomem *buffer = state->vlv_cursor_image;
>> > > + char __iomem *base, *cursor_base;
>> > > + int size = obj->base.size;
>> > > + int i, x = state->base.crtc_x;
>> > > + int bytes_per_pixel = fb->bits_per_pixel / 8;
>> > > +
>> > > + base = vlv_pin_and_map_buffer_obj(obj);
>> > > + if (base == NULL)
>> > > +         return -ENOMEM;
>> > > +
>> > > + if (prev_x >= 0) {
>> > > +         if (buffer != NULL)
>> > > +                 kfree(buffer);
>> > > +         buffer = kzalloc(size, GFP_KERNEL);
>> > > +         state->vlv_cursor_image = buffer;
>> > > +         if (buffer == NULL)
>> > > +                 return -ENOMEM;
>> > > +         memcpy(buffer, base, size);
>> > > + }
>> > > + cursor_base = buffer;
>> > > + x = -x;
>> > > + for (i = 0; i < state->base.crtc_h; i++) {
>> > > +         cursor_base += x * bytes_per_pixel;
>> > > +         memcpy(base, cursor_base,
>> > > +                 (state->base.crtc_w - x) * bytes_per_pixel);
>> > > +         base += (state->base.crtc_w - x) * bytes_per_pixel;
>> > > +         memset(base, 0, x * bytes_per_pixel);
>> > > +         base += x * bytes_per_pixel;
>> > > +         cursor_base += (state->base.crtc_w - x) * bytes_per_pixel;
>> > > + }
>> > > +
>> > > + vlv_unpin_buffer_obj(obj, base);
>> > > +
>> > > + return 0;
>> > > +}
>> > > +
>> > >  static int
>> > >  intel_check_cursor_plane(struct drm_plane *plane,
>> > >                    struct intel_crtc_state *crtc_state,
>> > > @@ -14287,8 +14362,10 @@ intel_check_cursor_plane(struct drm_plane *plane,
>> > >   struct drm_crtc *crtc = crtc_state->base.crtc;
>> > >   struct drm_framebuffer *fb = state->base.fb;
>> > >   struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>> > > + enum pipe pipe = to_intel_plane(plane)->pipe;
>> > >   unsigned stride;
>> > >   int ret;
>> > > + int crtc_prev_x = state->vlv_cursor_prev_x;
>> > >
>> > >   ret = drm_plane_helper_check_update(plane, crtc, fb, &state->src,
>> > >                                       &state->dst, &state->clip,
>> > > @@ -14309,6 +14386,32 @@ intel_check_cursor_plane(struct drm_plane *plane,
>> > >           return -EINVAL;
>> > >   }
>> > >
>> > > + /*
>> > > +  * There is an issue in CHV PIPE C where we hit underrun on
>> > > +  * -ve value of cursor. To avoid this we are cropping the
>> > > +  *  image for all PIPE C -ve values.
>> > > +  */
>> > > + if (IS_CHERRYVIEW(plane->dev)) {
>> > > +         if (pipe == PIPE_C && state->visible &&
>> > > +             state->base.crtc_x < 0) {
>> > > +                 ret = vlv_cursor_crop(state, crtc_prev_x);
>> > > +                 if (ret)
>> > > +                         return -ENOMEM;
>> > > +         } else if (crtc_prev_x < 0) { /* Restore the image back */
>> > > +                 char __iomem *base;
>> > > +                 char __iomem *org_image = state->vlv_cursor_image;
>> > > +                 int size = obj->base.size;
>> > > +
>> > > +                 if (org_image == NULL)
>> > > +                         return -ENOMEM;
>> > > +                 base = vlv_pin_and_map_buffer_obj(obj);
>> > > +                 if (base == NULL)
>> > > +                         return -ENOMEM;
>> > > +                 memcpy(base, org_image, size);
>> > > +                 vlv_unpin_buffer_obj(obj, base);
>> > > +         }
>> > > + }
>> > > +
>> > >   stride = roundup_pow_of_two(state->base.crtc_w) * 4;
>> > >   if (obj->base.size < stride * state->base.crtc_h) {
>> > >           DRM_DEBUG_KMS("buffer is too small\n");
>> > > @@ -14320,6 +14423,16 @@ intel_check_cursor_plane(struct drm_plane *plane,
>> > >           return -EINVAL;
>> > >   }
>> > >
>> > > + if (IS_CHERRYVIEW(plane->dev)) {
>> > > +         if (pipe == PIPE_C &&
>> > > +             state->visible && state->base.crtc_x < 0) {
>> > > +                 state->vlv_cursor_prev_x = state->base.crtc_x;
>> > > +                 state->base.crtc_x = 0;
>> > > +         } else {
>> > > +                 state->vlv_cursor_prev_x = state->base.crtc_x;
>> > > +         }
>> > > + }
>> > > +
>> > >   return 0;
>> > >  }
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > > index ebe7b34..0c406d1 100644
>> > > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > > @@ -350,6 +350,9 @@ struct intel_plane_state {
>> > >
>> > >   /* async flip related structures */
>> > >   struct drm_i915_gem_request *wait_req;
>> > > +
>> > > + char __iomem *vlv_cursor_image;
>> > > + int vlv_cursor_prev_x;
>> > >  };
>> > >
>> > >  struct intel_initial_plane_config {
>> > > --
>> > > 1.9.1
>> > >
>> > > _______________________________________________
>> > > Intel-gfx mailing list
>> > > Intel-gfx@lists.freedesktop.org
>> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC] drm/i915/chv: Clip cursor for CHV pipe C HW Cursor pos < 0

[Shobhit Kumar]

From: Shobhit Kumar <shobhit.kumar@intel.com>

CHV pipe C hits underrun when we get negative crtc_x values of cursor.
To avoid this we clip and shift the cursor image by negative crtc_x
value.

v2: Make a copy of cursor plane state and allocate new gem object and fb
    for clipped cursor and use that in case of negative cursor position

Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   7 ++
 drivers/gpu/drm/i915/intel_display.c | 122 ++++++++++++++++++++++++++++++++++-
 2 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 724d34b..1e59c02 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2041,6 +2041,13 @@ struct drm_i915_private {
 	struct intel_encoder *dig_port_map[I915_MAX_PORTS];
 
 	/*
+	* Temporary copy of cursor plane state for CHV PIPE_C
+	* Will be initialized only when crtc_x < 0 as there is a
+	* HW bug causing pipe underrun
+	*/
+	struct intel_plane_state *cursor_state;
+
+	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
 	 */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c3b5dc8..c4988d5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14456,6 +14456,123 @@ intel_update_cursor_plane(struct drm_plane *plane,
 	intel_crtc_update_cursor(crtc, state);
 }
 
+static void
+intel_update_chv_pipe_c_cursor_plane(struct drm_plane *plane,
+		const struct intel_crtc_state *crtc_state,
+		const struct intel_plane_state *state)
+{
+	struct drm_crtc *crtc = crtc_state->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
+	struct drm_i915_gem_object *cur_obj = NULL, *use_obj = NULL;
+	uint32_t addr;
+	struct intel_plane_state *cursor_state = dev_priv->cursor_state;
+	const struct intel_plane_state *use_state;
+	char __iomem *src, *dst;
+
+	if (state->visible && state->base.crtc_x < 0) {
+		int bytes_per_pixel = state->base.fb->bits_per_pixel / 8;
+		int x = state->base.crtc_x;
+		int width = state->base.crtc_w;
+		int height = state->base.crtc_h;
+		struct drm_mode_fb_cmd2 mode_cmd = { 0 };
+		int i;
+
+		if (!cursor_state) {
+			cursor_state = kzalloc(sizeof(*cursor_state), GFP_KERNEL);
+			if (!cursor_state) {
+				use_state = state;
+				use_obj = obj;
+				goto update;
+			}
+
+			memcpy(cursor_state, state, sizeof(*state));
+			cursor_state->base.crtc_x = 0;
+
+			/* Allocate new gem object */
+			cur_obj = i915_gem_object_create(dev, obj->base.size);
+			if (IS_ERR(cur_obj)) {
+				kfree(cursor_state);
+				use_state = state;
+				use_obj = obj;
+				goto update;
+			}
+
+			mode_cmd.width = cursor_state->base.fb->width;
+			mode_cmd.height = cursor_state->base.fb->height;
+			mode_cmd.pitches[0] = cursor_state->base.fb->pitches[0];
+			mode_cmd.pixel_format = cursor_state->base.fb->pixel_format;
+
+			cursor_state->base.fb = intel_framebuffer_create(dev, &mode_cmd, cur_obj);
+			if (IS_ERR(cursor_state->base.fb)) {
+				drm_gem_object_unreference_unlocked(&cur_obj->base);
+				kfree(cursor_state);
+				use_state = state;
+				use_obj = obj;
+				goto update;
+			}
+
+			dev_priv->cursor_state = cursor_state;
+		} else
+			cur_obj = intel_fb_obj(cursor_state->base.fb);
+
+		src = ioremap_wc(dev_priv->ggtt.mappable_base +
+				i915_gem_obj_ggtt_offset(obj),
+				obj->base.size);
+
+		dst = ioremap_wc(dev_priv->ggtt.mappable_base +
+				i915_gem_obj_ggtt_offset(cur_obj),
+				cur_obj->base.size);
+
+		/* shift the original cusrsor in to copy buffer offsetting -ive pos */
+		x = -x;
+		for (i = 0; i < height; i++) {
+			src += x * bytes_per_pixel;
+			memcpy(dst, src, (width - x) * bytes_per_pixel);
+			dst += (width - x) * bytes_per_pixel;
+			memset(dst, 0, x * bytes_per_pixel);
+			dst += x * bytes_per_pixel;
+			src += (width -x) * bytes_per_pixel;
+		}
+
+		iounmap(src);
+		iounmap(dst);
+
+		use_obj = cur_obj;
+		use_state = cursor_state;
+	} else {
+		/* free the cursor_state if cached */
+		if (cursor_state) {
+			struct intel_framebuffer *intel_fb = to_intel_framebuffer(cursor_state->base.fb);
+			drm_framebuffer_cleanup(cursor_state->base.fb);
+			mutex_lock(&dev->struct_mutex);
+			drm_gem_object_unreference(&intel_fb->obj->base);
+			mutex_unlock(&dev->struct_mutex);
+			kfree(intel_fb);
+
+			kfree(cursor_state);
+			cursor_state = NULL;
+		}
+
+		use_obj = obj;
+		use_state = state;
+	}
+
+update:
+	if (!use_obj)
+		addr = 0;
+	else if (!INTEL_INFO(dev)->cursor_needs_physical)
+		addr = i915_gem_obj_ggtt_offset(use_obj);
+	else
+		addr = use_obj->phys_handle->busaddr;
+
+	intel_crtc->cursor_addr = addr;
+
+	intel_crtc_update_cursor(crtc, use_state);
+}
+
 static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
 						   int pipe)
 {
@@ -14478,7 +14595,10 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
 	cursor->plane = pipe;
 	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
 	cursor->check_plane = intel_check_cursor_plane;
-	cursor->update_plane = intel_update_cursor_plane;
+	if (IS_CHERRYVIEW(dev) && pipe == PIPE_C)
+		cursor->update_plane = intel_update_chv_pipe_c_cursor_plane;
+	else
+		cursor->update_plane = intel_update_cursor_plane;
 	cursor->disable_plane = intel_disable_cursor_plane;
 
 	ret = drm_universal_plane_init(dev, &cursor->base, 0,
-- 
1.9.1

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

Re: [RFC] drm/i915/chv: Clip cursor for CHV pipe C HW Cursor pos < 0

[Shobhit Kumar]

Daniel,

On 06/28/2016 05:57 PM, Shobhit Kumar wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> CHV pipe C hits underrun when we get negative crtc_x values of cursor.
> To avoid this we clip and shift the cursor image by negative crtc_x
> value.
>
> v2: Make a copy of cursor plane state and allocate new gem object and fb
>     for clipped cursor and use that in case of negative cursor position
>

This patch is not yet tested and most likely not even complete and is 
sent for early review of the approach. Will test it and update the 
status here.

Regards
Shobhit

> Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
>  drivers/gpu/drm/i915/intel_display.c | 122 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 128 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 724d34b..1e59c02 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2041,6 +2041,13 @@ struct drm_i915_private {
>  	struct intel_encoder *dig_port_map[I915_MAX_PORTS];
>
>  	/*
> +	* Temporary copy of cursor plane state for CHV PIPE_C
> +	* Will be initialized only when crtc_x < 0 as there is a
> +	* HW bug causing pipe underrun
> +	*/
> +	struct intel_plane_state *cursor_state;
> +
> +	/*
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
>  	 */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c3b5dc8..c4988d5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14456,6 +14456,123 @@ intel_update_cursor_plane(struct drm_plane *plane,
>  	intel_crtc_update_cursor(crtc, state);
>  }
>
> +static void
> +intel_update_chv_pipe_c_cursor_plane(struct drm_plane *plane,
> +		const struct intel_crtc_state *crtc_state,
> +		const struct intel_plane_state *state)
> +{
> +	struct drm_crtc *crtc = crtc_state->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
> +	struct drm_i915_gem_object *cur_obj = NULL, *use_obj = NULL;
> +	uint32_t addr;
> +	struct intel_plane_state *cursor_state = dev_priv->cursor_state;
> +	const struct intel_plane_state *use_state;
> +	char __iomem *src, *dst;
> +
> +	if (state->visible && state->base.crtc_x < 0) {
> +		int bytes_per_pixel = state->base.fb->bits_per_pixel / 8;
> +		int x = state->base.crtc_x;
> +		int width = state->base.crtc_w;
> +		int height = state->base.crtc_h;
> +		struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> +		int i;
> +
> +		if (!cursor_state) {
> +			cursor_state = kzalloc(sizeof(*cursor_state), GFP_KERNEL);
> +			if (!cursor_state) {
> +				use_state = state;
> +				use_obj = obj;
> +				goto update;
> +			}
> +
> +			memcpy(cursor_state, state, sizeof(*state));
> +			cursor_state->base.crtc_x = 0;
> +
> +			/* Allocate new gem object */
> +			cur_obj = i915_gem_object_create(dev, obj->base.size);
> +			if (IS_ERR(cur_obj)) {
> +				kfree(cursor_state);
> +				use_state = state;
> +				use_obj = obj;
> +				goto update;
> +			}
> +
> +			mode_cmd.width = cursor_state->base.fb->width;
> +			mode_cmd.height = cursor_state->base.fb->height;
> +			mode_cmd.pitches[0] = cursor_state->base.fb->pitches[0];
> +			mode_cmd.pixel_format = cursor_state->base.fb->pixel_format;
> +
> +			cursor_state->base.fb = intel_framebuffer_create(dev, &mode_cmd, cur_obj);
> +			if (IS_ERR(cursor_state->base.fb)) {
> +				drm_gem_object_unreference_unlocked(&cur_obj->base);
> +				kfree(cursor_state);
> +				use_state = state;
> +				use_obj = obj;
> +				goto update;
> +			}
> +
> +			dev_priv->cursor_state = cursor_state;
> +		} else
> +			cur_obj = intel_fb_obj(cursor_state->base.fb);
> +
> +		src = ioremap_wc(dev_priv->ggtt.mappable_base +
> +				i915_gem_obj_ggtt_offset(obj),
> +				obj->base.size);
> +
> +		dst = ioremap_wc(dev_priv->ggtt.mappable_base +
> +				i915_gem_obj_ggtt_offset(cur_obj),
> +				cur_obj->base.size);
> +
> +		/* shift the original cusrsor in to copy buffer offsetting -ive pos */
> +		x = -x;
> +		for (i = 0; i < height; i++) {
> +			src += x * bytes_per_pixel;
> +			memcpy(dst, src, (width - x) * bytes_per_pixel);
> +			dst += (width - x) * bytes_per_pixel;
> +			memset(dst, 0, x * bytes_per_pixel);
> +			dst += x * bytes_per_pixel;
> +			src += (width -x) * bytes_per_pixel;
> +		}
> +
> +		iounmap(src);
> +		iounmap(dst);
> +
> +		use_obj = cur_obj;
> +		use_state = cursor_state;
> +	} else {
> +		/* free the cursor_state if cached */
> +		if (cursor_state) {
> +			struct intel_framebuffer *intel_fb = to_intel_framebuffer(cursor_state->base.fb);
> +			drm_framebuffer_cleanup(cursor_state->base.fb);
> +			mutex_lock(&dev->struct_mutex);
> +			drm_gem_object_unreference(&intel_fb->obj->base);
> +			mutex_unlock(&dev->struct_mutex);
> +			kfree(intel_fb);
> +
> +			kfree(cursor_state);
> +			cursor_state = NULL;
> +		}
> +
> +		use_obj = obj;
> +		use_state = state;
> +	}
> +
> +update:
> +	if (!use_obj)
> +		addr = 0;
> +	else if (!INTEL_INFO(dev)->cursor_needs_physical)
> +		addr = i915_gem_obj_ggtt_offset(use_obj);
> +	else
> +		addr = use_obj->phys_handle->busaddr;
> +
> +	intel_crtc->cursor_addr = addr;
> +
> +	intel_crtc_update_cursor(crtc, use_state);
> +}
> +
>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  						   int pipe)
>  {
> @@ -14478,7 +14595,10 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  	cursor->plane = pipe;
>  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
>  	cursor->check_plane = intel_check_cursor_plane;
> -	cursor->update_plane = intel_update_cursor_plane;
> +	if (IS_CHERRYVIEW(dev) && pipe == PIPE_C)
> +		cursor->update_plane = intel_update_chv_pipe_c_cursor_plane;
> +	else
> +		cursor->update_plane = intel_update_cursor_plane;
>  	cursor->disable_plane = intel_disable_cursor_plane;
>
>  	ret = drm_universal_plane_init(dev, &cursor->base, 0,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for series starting with [1/2] Revert "drm/i915: Workaround CHV pipe C cursor fail" (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] Revert "drm/i915: Workaround CHV pipe C cursor fail" (rev2)
URL   : https://patchwork.freedesktop.org/series/8431/
State : failure

== Summary ==

Series 8431v2 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/8431/revisions/2/mbox

Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (ro-bdw-i7-5557U)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-b:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)

fi-hsw-i7-4770k  total:229  pass:194  dwarn:0   dfail:0   fail:2   skip:33 
fi-skl-i5-6260u  total:229  pass:202  dwarn:0   dfail:0   fail:2   skip:25 
fi-skl-i7-6700k  total:229  pass:188  dwarn:0   dfail:0   fail:2   skip:39 
fi-snb-i7-2600   total:229  pass:174  dwarn:0   dfail:0   fail:2   skip:53 
ro-bdw-i5-5250u  total:229  pass:202  dwarn:2   dfail:1   fail:2   skip:22 
ro-bdw-i7-5557U  total:229  pass:201  dwarn:1   dfail:1   fail:3   skip:23 
ro-bdw-i7-5600u  total:229  pass:190  dwarn:0   dfail:1   fail:0   skip:38 
ro-bsw-n3050     total:229  pass:177  dwarn:0   dfail:1   fail:2   skip:49 
ro-byt-n2820     total:229  pass:178  dwarn:0   dfail:1   fail:5   skip:45 
ro-hsw-i3-4010u  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
ro-hsw-i7-4770r  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
ro-ilk-i7-620lm  total:229  pass:155  dwarn:0   dfail:1   fail:3   skip:70 
ro-ilk1-i5-650   total:224  pass:155  dwarn:0   dfail:1   fail:3   skip:65 
ro-ivb-i7-3770   total:229  pass:186  dwarn:0   dfail:1   fail:2   skip:40 
ro-ivb2-i7-3770  total:229  pass:190  dwarn:0   dfail:1   fail:2   skip:36 
ro-skl3-i5-6260u total:229  pass:206  dwarn:1   dfail:1   fail:2   skip:19 
ro-snb-i7-2620M  total:229  pass:179  dwarn:0   dfail:1   fail:1   skip:48 

Results at /archive/results/CI_IGT_test/RO_Patchwork_1319/

3a50b42 drm-intel-nightly: 2016y-06m-28d-11h-06m-59s UTC integration manifest
88c4ebf drm/i915/chv: Clip cursor for CHV pipe C HW Cursor pos < 0
452d762 Revert "drm/i915: Workaround CHV pipe C cursor fail"

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

[RFC v2] drm/i915/chv: Clip cursor for CHV pipe C HW Cursor pos < 0

[Shobhit Kumar]

From: Shobhit Kumar <shobhit.kumar@intel.com>

CHV pipe C hits underrun when we get negative crtc_x values of cursor.
To avoid this we clip and shift the cursor image by negative crtc_x
value.

v2: Make a copy of cursor plane state and allocate new gem object and fb
    for clipped cursor and use that in case of negative cursor position

v3: Updated error handling
    Pin the gem object before use.

Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   7 ++
 drivers/gpu/drm/i915/intel_display.c | 131 ++++++++++++++++++++++++++++++++++-
 2 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 724d34b..1e59c02 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2041,6 +2041,13 @@ struct drm_i915_private {
 	struct intel_encoder *dig_port_map[I915_MAX_PORTS];
 
 	/*
+	* Temporary copy of cursor plane state for CHV PIPE_C
+	* Will be initialized only when crtc_x < 0 as there is a
+	* HW bug causing pipe underrun
+	*/
+	struct intel_plane_state *cursor_state;
+
+	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
 	 */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c3b5dc8..e6c103a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14456,6 +14456,132 @@ intel_update_cursor_plane(struct drm_plane *plane,
 	intel_crtc_update_cursor(crtc, state);
 }
 
+static void
+intel_update_chv_pipe_c_cursor_plane(struct drm_plane *plane,
+		const struct intel_crtc_state *crtc_state,
+		const struct intel_plane_state *state)
+{
+	struct drm_crtc *crtc = crtc_state->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
+	struct drm_i915_gem_object *cur_obj = NULL, *use_obj = NULL;
+	uint32_t addr;
+	struct intel_plane_state *cursor_state = dev_priv->cursor_state;
+	const struct intel_plane_state *use_state;
+	char __iomem *src, *dst;
+	bool pinned = true;
+
+	if (state->visible && state->base.crtc_x < 0) {
+		int bytes_per_pixel = state->base.fb->bits_per_pixel / 8;
+		int x = state->base.crtc_x;
+		int width = state->base.crtc_w;
+		int height = state->base.crtc_h;
+		struct drm_mode_fb_cmd2 mode_cmd = { 0 };
+		int i;
+
+		if (!cursor_state) {
+			cursor_state = kzalloc(sizeof(*cursor_state), GFP_KERNEL);
+			if (!cursor_state) {
+				use_state = state;
+				use_obj = obj;
+				goto update;
+			}
+
+			memcpy(cursor_state, state, sizeof(*state));
+
+			/* Allocate new gem object */
+			cur_obj = i915_gem_object_create(dev, obj->base.size);
+			if (IS_ERR(cur_obj))
+				goto gem_err;
+
+			mode_cmd.width = cursor_state->base.fb->width;
+			mode_cmd.height = cursor_state->base.fb->height;
+			mode_cmd.pitches[0] = cursor_state->base.fb->pitches[0];
+			mode_cmd.pixel_format = cursor_state->base.fb->pixel_format;
+
+			cursor_state->base.fb = intel_framebuffer_create(dev, &mode_cmd, cur_obj);
+			if (IS_ERR(cursor_state->base.fb)) {
+				drm_gem_object_unreference_unlocked(&cur_obj->base);
+				goto gem_err;
+			}
+
+			if (i915_gem_obj_ggtt_pin(cur_obj, 0, 0) < 0) {
+				drm_gem_object_unreference_unlocked(&cur_obj->base);
+				pinned = false;
+				goto cleanup;
+			}
+
+			dev_priv->cursor_state = cursor_state;
+		} else
+			cur_obj = intel_fb_obj(cursor_state->base.fb);
+
+		src = ioremap_wc(dev_priv->ggtt.mappable_base +
+				i915_gem_obj_ggtt_offset(obj),
+				obj->base.size);
+
+		dst = ioremap_wc(dev_priv->ggtt.mappable_base +
+				i915_gem_obj_ggtt_offset(cur_obj),
+				cur_obj->base.size);
+
+		/* shift the original cusrsor in to copy buffer offsetting -ive pos */
+		x = -x;
+		for (i = 0; i < height; i++) {
+			src += x * bytes_per_pixel;
+			memcpy(dst, src, (width - x) * bytes_per_pixel);
+			dst += (width - x) * bytes_per_pixel;
+			memset(dst, 0, x * bytes_per_pixel);
+			dst += x * bytes_per_pixel;
+			src += (width -x) * bytes_per_pixel;
+		}
+
+		iounmap(src);
+		iounmap(dst);
+
+		cursor_state->base.crtc_x = 0;
+		use_obj = cur_obj;
+		use_state = cursor_state;
+
+		goto update;
+	}
+
+cleanup:
+	if (cursor_state) {
+		struct intel_framebuffer *intel_fb = to_intel_framebuffer(cursor_state->base.fb);
+
+		if (pinned)
+			i915_gem_object_ggtt_unpin(cur_obj);
+
+		drm_framebuffer_cleanup(cursor_state->base.fb);
+		mutex_lock(&dev->struct_mutex);
+		drm_gem_object_unreference(&intel_fb->obj->base);
+		mutex_unlock(&dev->struct_mutex);
+		kfree(intel_fb);
+	}
+
+gem_err:
+	if (dev_priv->cursor_state) {
+		kfree(dev_priv->cursor_state);
+		dev_priv->cursor_state = NULL;
+	}
+
+	use_state = state;
+	use_obj = obj;
+
+update:
+	if (!use_obj)
+		addr = 0;
+	else if (!INTEL_INFO(dev)->cursor_needs_physical)
+		addr = i915_gem_obj_ggtt_offset(use_obj);
+	else
+		addr = use_obj->phys_handle->busaddr;
+
+	intel_crtc->cursor_addr = addr;
+
+	intel_crtc_update_cursor(crtc, use_state);
+}
+
 static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
 						   int pipe)
 {
@@ -14478,7 +14604,10 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
 	cursor->plane = pipe;
 	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
 	cursor->check_plane = intel_check_cursor_plane;
-	cursor->update_plane = intel_update_cursor_plane;
+	if (IS_CHERRYVIEW(dev) && pipe == PIPE_C)
+		cursor->update_plane = intel_update_chv_pipe_c_cursor_plane;
+	else
+		cursor->update_plane = intel_update_cursor_plane;
 	cursor->disable_plane = intel_disable_cursor_plane;
 
 	ret = drm_universal_plane_init(dev, &cursor->base, 0,
-- 
1.9.1

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

Re: [RFC v2] drm/i915/chv: Clip cursor for CHV pipe C HW Cursor pos < 0

[Shobhit Kumar]

On 06/29/2016 06:24 PM, Shobhit Kumar wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> CHV pipe C hits underrun when we get negative crtc_x values of cursor.
> To avoid this we clip and shift the cursor image by negative crtc_x
> value.
>
> v2: Make a copy of cursor plane state and allocate new gem object and fb
>     for clipped cursor and use that in case of negative cursor position
>
> v3: Updated error handling
>     Pin the gem object before use.

I tested a modified version of this on 3.18 kernel on ChromeOS. Does 
work fine, but few WARN dumps from might_sleep() in atomic context while 
allocating cursor gem bo. I can allocate it one time during plane 
creation but how do I know the size of incoming bo ? If I allocate for 
say large cursor 256x256, ioremap_wc also has same WARN dumps. Need to 
remap in update function. Any hints how to go about it ? Maybe I should 
do this hack in check_plane rather than update plane ?

Regards
Shobhit

>
> Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
>  drivers/gpu/drm/i915/intel_display.c | 131 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 137 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 724d34b..1e59c02 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2041,6 +2041,13 @@ struct drm_i915_private {
>  	struct intel_encoder *dig_port_map[I915_MAX_PORTS];
>
>  	/*
> +	* Temporary copy of cursor plane state for CHV PIPE_C
> +	* Will be initialized only when crtc_x < 0 as there is a
> +	* HW bug causing pipe underrun
> +	*/
> +	struct intel_plane_state *cursor_state;
> +
> +	/*
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
>  	 */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c3b5dc8..e6c103a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14456,6 +14456,132 @@ intel_update_cursor_plane(struct drm_plane *plane,
>  	intel_crtc_update_cursor(crtc, state);
>  }
>
> +static void
> +intel_update_chv_pipe_c_cursor_plane(struct drm_plane *plane,
> +		const struct intel_crtc_state *crtc_state,
> +		const struct intel_plane_state *state)
> +{
> +	struct drm_crtc *crtc = crtc_state->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
> +	struct drm_i915_gem_object *cur_obj = NULL, *use_obj = NULL;
> +	uint32_t addr;
> +	struct intel_plane_state *cursor_state = dev_priv->cursor_state;
> +	const struct intel_plane_state *use_state;
> +	char __iomem *src, *dst;
> +	bool pinned = true;
> +
> +	if (state->visible && state->base.crtc_x < 0) {
> +		int bytes_per_pixel = state->base.fb->bits_per_pixel / 8;
> +		int x = state->base.crtc_x;
> +		int width = state->base.crtc_w;
> +		int height = state->base.crtc_h;
> +		struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> +		int i;
> +
> +		if (!cursor_state) {
> +			cursor_state = kzalloc(sizeof(*cursor_state), GFP_KERNEL);
> +			if (!cursor_state) {
> +				use_state = state;
> +				use_obj = obj;
> +				goto update;
> +			}
> +
> +			memcpy(cursor_state, state, sizeof(*state));
> +
> +			/* Allocate new gem object */
> +			cur_obj = i915_gem_object_create(dev, obj->base.size);
> +			if (IS_ERR(cur_obj))
> +				goto gem_err;
> +
> +			mode_cmd.width = cursor_state->base.fb->width;
> +			mode_cmd.height = cursor_state->base.fb->height;
> +			mode_cmd.pitches[0] = cursor_state->base.fb->pitches[0];
> +			mode_cmd.pixel_format = cursor_state->base.fb->pixel_format;
> +
> +			cursor_state->base.fb = intel_framebuffer_create(dev, &mode_cmd, cur_obj);
> +			if (IS_ERR(cursor_state->base.fb)) {
> +				drm_gem_object_unreference_unlocked(&cur_obj->base);
> +				goto gem_err;
> +			}
> +
> +			if (i915_gem_obj_ggtt_pin(cur_obj, 0, 0) < 0) {
> +				drm_gem_object_unreference_unlocked(&cur_obj->base);
> +				pinned = false;
> +				goto cleanup;
> +			}
> +
> +			dev_priv->cursor_state = cursor_state;
> +		} else
> +			cur_obj = intel_fb_obj(cursor_state->base.fb);
> +
> +		src = ioremap_wc(dev_priv->ggtt.mappable_base +
> +				i915_gem_obj_ggtt_offset(obj),
> +				obj->base.size);
> +
> +		dst = ioremap_wc(dev_priv->ggtt.mappable_base +
> +				i915_gem_obj_ggtt_offset(cur_obj),
> +				cur_obj->base.size);
> +
> +		/* shift the original cusrsor in to copy buffer offsetting -ive pos */
> +		x = -x;
> +		for (i = 0; i < height; i++) {
> +			src += x * bytes_per_pixel;
> +			memcpy(dst, src, (width - x) * bytes_per_pixel);
> +			dst += (width - x) * bytes_per_pixel;
> +			memset(dst, 0, x * bytes_per_pixel);
> +			dst += x * bytes_per_pixel;
> +			src += (width -x) * bytes_per_pixel;
> +		}
> +
> +		iounmap(src);
> +		iounmap(dst);
> +
> +		cursor_state->base.crtc_x = 0;
> +		use_obj = cur_obj;
> +		use_state = cursor_state;
> +
> +		goto update;
> +	}
> +
> +cleanup:
> +	if (cursor_state) {
> +		struct intel_framebuffer *intel_fb = to_intel_framebuffer(cursor_state->base.fb);
> +
> +		if (pinned)
> +			i915_gem_object_ggtt_unpin(cur_obj);
> +
> +		drm_framebuffer_cleanup(cursor_state->base.fb);
> +		mutex_lock(&dev->struct_mutex);
> +		drm_gem_object_unreference(&intel_fb->obj->base);
> +		mutex_unlock(&dev->struct_mutex);
> +		kfree(intel_fb);
> +	}
> +
> +gem_err:
> +	if (dev_priv->cursor_state) {
> +		kfree(dev_priv->cursor_state);
> +		dev_priv->cursor_state = NULL;
> +	}
> +
> +	use_state = state;
> +	use_obj = obj;
> +
> +update:
> +	if (!use_obj)
> +		addr = 0;
> +	else if (!INTEL_INFO(dev)->cursor_needs_physical)
> +		addr = i915_gem_obj_ggtt_offset(use_obj);
> +	else
> +		addr = use_obj->phys_handle->busaddr;
> +
> +	intel_crtc->cursor_addr = addr;
> +
> +	intel_crtc_update_cursor(crtc, use_state);
> +}
> +
>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  						   int pipe)
>  {
> @@ -14478,7 +14604,10 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  	cursor->plane = pipe;
>  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
>  	cursor->check_plane = intel_check_cursor_plane;
> -	cursor->update_plane = intel_update_cursor_plane;
> +	if (IS_CHERRYVIEW(dev) && pipe == PIPE_C)
> +		cursor->update_plane = intel_update_chv_pipe_c_cursor_plane;
> +	else
> +		cursor->update_plane = intel_update_cursor_plane;
>  	cursor->disable_plane = intel_disable_cursor_plane;
>
>  	ret = drm_universal_plane_init(dev, &cursor->base, 0,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Ro.CI.BAT: success for series starting with [1/2] Revert "drm/i915: Workaround CHV pipe C cursor fail" (rev3)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] Revert "drm/i915: Workaround CHV pipe C cursor fail" (rev3)
URL   : https://patchwork.freedesktop.org/series/8431/
State : success

== Summary ==

Series 8431v3 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/8431/revisions/3/mbox

Test drv_module_reload_basic:
                dmesg-warn -> PASS       (ro-byt-n2820)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)

fi-hsw-i7-4770k  total:229  pass:194  dwarn:0   dfail:0   fail:2   skip:33 
fi-kbl-qkkr      total:229  pass:160  dwarn:29  dfail:0   fail:0   skip:40 
fi-skl-i5-6260u  total:229  pass:202  dwarn:0   dfail:0   fail:2   skip:25 
fi-skl-i7-6700k  total:229  pass:188  dwarn:0   dfail:0   fail:2   skip:39 
fi-snb-i7-2600   total:229  pass:174  dwarn:0   dfail:0   fail:2   skip:53 
ro-bdw-i5-5250u  total:229  pass:202  dwarn:1   dfail:1   fail:2   skip:23 
ro-bdw-i7-5600u  total:229  pass:190  dwarn:0   dfail:1   fail:0   skip:38 
ro-byt-n2820     total:229  pass:178  dwarn:0   dfail:1   fail:5   skip:45 
ro-hsw-i3-4010u  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
ro-hsw-i7-4770r  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
ro-ilk-i7-620lm  total:229  pass:155  dwarn:0   dfail:1   fail:3   skip:70 
ro-ilk1-i5-650   total:224  pass:155  dwarn:0   dfail:1   fail:3   skip:65 
ro-ivb2-i7-3770  total:229  pass:190  dwarn:0   dfail:1   fail:2   skip:36 
ro-skl3-i5-6260u total:229  pass:206  dwarn:1   dfail:1   fail:2   skip:19 
ro-snb-i7-2620M  total:229  pass:179  dwarn:0   dfail:1   fail:1   skip:48 
ro-bdw-i7-5557U failed to connect after reboot
ro-bsw-n3050 failed to connect after reboot
ro-ivb-i7-3770 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1328/

a90c989 drm-intel-nightly: 2016y-06m-29d-09h-24m-21s UTC integration manifest
9cc39c6 drm/i915/chv: Clip cursor for CHV pipe C HW Cursor pos < 0
3f44ddb Revert "drm/i915: Workaround CHV pipe C cursor fail"

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

Re: [RFC v2] drm/i915/chv: Clip cursor for CHV pipe C HW Cursor pos < 0

[Shobhit Kumar]

On 06/29/2016 06:24 PM, Shobhit Kumar wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> CHV pipe C hits underrun when we get negative crtc_x values of cursor.
> To avoid this we clip and shift the cursor image by negative crtc_x
> value.
>
> v2: Make a copy of cursor plane state and allocate new gem object and fb
>     for clipped cursor and use that in case of negative cursor position
>
> v3: Updated error handling
>     Pin the gem object before use.
>

Need someone to look at this patch. Daniel does this align with your 
suggestions ?

Regards
Shobhit

> Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
>  drivers/gpu/drm/i915/intel_display.c | 131 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 137 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 724d34b..1e59c02 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2041,6 +2041,13 @@ struct drm_i915_private {
>  	struct intel_encoder *dig_port_map[I915_MAX_PORTS];
>
>  	/*
> +	* Temporary copy of cursor plane state for CHV PIPE_C
> +	* Will be initialized only when crtc_x < 0 as there is a
> +	* HW bug causing pipe underrun
> +	*/
> +	struct intel_plane_state *cursor_state;
> +
> +	/*
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
>  	 */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c3b5dc8..e6c103a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14456,6 +14456,132 @@ intel_update_cursor_plane(struct drm_plane *plane,
>  	intel_crtc_update_cursor(crtc, state);
>  }
>
> +static void
> +intel_update_chv_pipe_c_cursor_plane(struct drm_plane *plane,
> +		const struct intel_crtc_state *crtc_state,
> +		const struct intel_plane_state *state)
> +{
> +	struct drm_crtc *crtc = crtc_state->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
> +	struct drm_i915_gem_object *cur_obj = NULL, *use_obj = NULL;
> +	uint32_t addr;
> +	struct intel_plane_state *cursor_state = dev_priv->cursor_state;
> +	const struct intel_plane_state *use_state;
> +	char __iomem *src, *dst;
> +	bool pinned = true;
> +
> +	if (state->visible && state->base.crtc_x < 0) {
> +		int bytes_per_pixel = state->base.fb->bits_per_pixel / 8;
> +		int x = state->base.crtc_x;
> +		int width = state->base.crtc_w;
> +		int height = state->base.crtc_h;
> +		struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> +		int i;
> +
> +		if (!cursor_state) {
> +			cursor_state = kzalloc(sizeof(*cursor_state), GFP_KERNEL);
> +			if (!cursor_state) {
> +				use_state = state;
> +				use_obj = obj;
> +				goto update;
> +			}
> +
> +			memcpy(cursor_state, state, sizeof(*state));
> +
> +			/* Allocate new gem object */
> +			cur_obj = i915_gem_object_create(dev, obj->base.size);
> +			if (IS_ERR(cur_obj))
> +				goto gem_err;
> +
> +			mode_cmd.width = cursor_state->base.fb->width;
> +			mode_cmd.height = cursor_state->base.fb->height;
> +			mode_cmd.pitches[0] = cursor_state->base.fb->pitches[0];
> +			mode_cmd.pixel_format = cursor_state->base.fb->pixel_format;
> +
> +			cursor_state->base.fb = intel_framebuffer_create(dev, &mode_cmd, cur_obj);
> +			if (IS_ERR(cursor_state->base.fb)) {
> +				drm_gem_object_unreference_unlocked(&cur_obj->base);
> +				goto gem_err;
> +			}
> +
> +			if (i915_gem_obj_ggtt_pin(cur_obj, 0, 0) < 0) {
> +				drm_gem_object_unreference_unlocked(&cur_obj->base);
> +				pinned = false;
> +				goto cleanup;
> +			}
> +
> +			dev_priv->cursor_state = cursor_state;
> +		} else
> +			cur_obj = intel_fb_obj(cursor_state->base.fb);
> +
> +		src = ioremap_wc(dev_priv->ggtt.mappable_base +
> +				i915_gem_obj_ggtt_offset(obj),
> +				obj->base.size);
> +
> +		dst = ioremap_wc(dev_priv->ggtt.mappable_base +
> +				i915_gem_obj_ggtt_offset(cur_obj),
> +				cur_obj->base.size);
> +
> +		/* shift the original cusrsor in to copy buffer offsetting -ive pos */
> +		x = -x;
> +		for (i = 0; i < height; i++) {
> +			src += x * bytes_per_pixel;
> +			memcpy(dst, src, (width - x) * bytes_per_pixel);
> +			dst += (width - x) * bytes_per_pixel;
> +			memset(dst, 0, x * bytes_per_pixel);
> +			dst += x * bytes_per_pixel;
> +			src += (width -x) * bytes_per_pixel;
> +		}
> +
> +		iounmap(src);
> +		iounmap(dst);
> +
> +		cursor_state->base.crtc_x = 0;
> +		use_obj = cur_obj;
> +		use_state = cursor_state;
> +
> +		goto update;
> +	}
> +
> +cleanup:
> +	if (cursor_state) {
> +		struct intel_framebuffer *intel_fb = to_intel_framebuffer(cursor_state->base.fb);
> +
> +		if (pinned)
> +			i915_gem_object_ggtt_unpin(cur_obj);
> +
> +		drm_framebuffer_cleanup(cursor_state->base.fb);
> +		mutex_lock(&dev->struct_mutex);
> +		drm_gem_object_unreference(&intel_fb->obj->base);
> +		mutex_unlock(&dev->struct_mutex);
> +		kfree(intel_fb);
> +	}
> +
> +gem_err:
> +	if (dev_priv->cursor_state) {
> +		kfree(dev_priv->cursor_state);
> +		dev_priv->cursor_state = NULL;
> +	}
> +
> +	use_state = state;
> +	use_obj = obj;
> +
> +update:
> +	if (!use_obj)
> +		addr = 0;
> +	else if (!INTEL_INFO(dev)->cursor_needs_physical)
> +		addr = i915_gem_obj_ggtt_offset(use_obj);
> +	else
> +		addr = use_obj->phys_handle->busaddr;
> +
> +	intel_crtc->cursor_addr = addr;
> +
> +	intel_crtc_update_cursor(crtc, use_state);
> +}
> +
>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  						   int pipe)
>  {
> @@ -14478,7 +14604,10 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  	cursor->plane = pipe;
>  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
>  	cursor->check_plane = intel_check_cursor_plane;
> -	cursor->update_plane = intel_update_cursor_plane;
> +	if (IS_CHERRYVIEW(dev) && pipe == PIPE_C)
> +		cursor->update_plane = intel_update_chv_pipe_c_cursor_plane;
> +	else
> +		cursor->update_plane = intel_update_cursor_plane;
>  	cursor->disable_plane = intel_disable_cursor_plane;
>
>  	ret = drm_universal_plane_init(dev, &cursor->base, 0,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC v2] drm/i915/chv: Clip cursor for CHV pipe C HW Cursor pos < 0

[Shobhit Kumar]

On Wed, Jun 29, 2016 at 6:24 PM, Shobhit Kumar
<shobhit.kumar@linux.intel.com> wrote:
>
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> CHV pipe C hits underrun when we get negative crtc_x values of cursor.
> To avoid this we clip and shift the cursor image by negative crtc_x
> value.
>
> v2: Make a copy of cursor plane state and allocate new gem object and fb
>     for clipped cursor and use that in case of negative cursor position
>
> v3: Updated error handling
>     Pin the gem object before use.

Summarizing the discussion with Ville and Chris on IRC few days back -
1. This indeed does break the uabi which expects coherent writing into
cursor plane (Chris)
2. Doing this with single buffer in kernel might result in tearing. We
will need triple buffering (Ville)
3. Some continuous rendering tools might break (Chris)
4. Animated cursor  might break (Chris). But then update plane call
should copy back the original buffer every time and animated cursor
should work as long as this is not filtered.

So still recommendation remains to use SW cursor, though in theory
clipping is possible but with additional complexity.

Regards
Shobhit

>
> Signed-off-by: Akshu Agrawal <akshu.agrawal@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
>  drivers/gpu/drm/i915/intel_display.c | 131 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 137 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 724d34b..1e59c02 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2041,6 +2041,13 @@ struct drm_i915_private {
>         struct intel_encoder *dig_port_map[I915_MAX_PORTS];
>
>         /*
> +       * Temporary copy of cursor plane state for CHV PIPE_C
> +       * Will be initialized only when crtc_x < 0 as there is a
> +       * HW bug causing pipe underrun
> +       */
> +       struct intel_plane_state *cursor_state;
> +
> +       /*
>          * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>          * will be rejected. Instead look for a better place.
>          */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c3b5dc8..e6c103a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14456,6 +14456,132 @@ intel_update_cursor_plane(struct drm_plane *plane,
>         intel_crtc_update_cursor(crtc, state);
>  }
>
> +static void
> +intel_update_chv_pipe_c_cursor_plane(struct drm_plane *plane,
> +               const struct intel_crtc_state *crtc_state,
> +               const struct intel_plane_state *state)
> +{
> +       struct drm_crtc *crtc = crtc_state->base.crtc;
> +       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +       struct drm_device *dev = plane->dev;
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
> +       struct drm_i915_gem_object *cur_obj = NULL, *use_obj = NULL;
> +       uint32_t addr;
> +       struct intel_plane_state *cursor_state = dev_priv->cursor_state;
> +       const struct intel_plane_state *use_state;
> +       char __iomem *src, *dst;
> +       bool pinned = true;
> +
> +       if (state->visible && state->base.crtc_x < 0) {
> +               int bytes_per_pixel = state->base.fb->bits_per_pixel / 8;
> +               int x = state->base.crtc_x;
> +               int width = state->base.crtc_w;
> +               int height = state->base.crtc_h;
> +               struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> +               int i;
> +
> +               if (!cursor_state) {
> +                       cursor_state = kzalloc(sizeof(*cursor_state), GFP_KERNEL);
> +                       if (!cursor_state) {
> +                               use_state = state;
> +                               use_obj = obj;
> +                               goto update;
> +                       }
> +
> +                       memcpy(cursor_state, state, sizeof(*state));
> +
> +                       /* Allocate new gem object */
> +                       cur_obj = i915_gem_object_create(dev, obj->base.size);
> +                       if (IS_ERR(cur_obj))
> +                               goto gem_err;
> +
> +                       mode_cmd.width = cursor_state->base.fb->width;
> +                       mode_cmd.height = cursor_state->base.fb->height;
> +                       mode_cmd.pitches[0] = cursor_state->base.fb->pitches[0];
> +                       mode_cmd.pixel_format = cursor_state->base.fb->pixel_format;
> +
> +                       cursor_state->base.fb = intel_framebuffer_create(dev, &mode_cmd, cur_obj);
> +                       if (IS_ERR(cursor_state->base.fb)) {
> +                               drm_gem_object_unreference_unlocked(&cur_obj->base);
> +                               goto gem_err;
> +                       }
> +
> +                       if (i915_gem_obj_ggtt_pin(cur_obj, 0, 0) < 0) {
> +                               drm_gem_object_unreference_unlocked(&cur_obj->base);
> +                               pinned = false;
> +                               goto cleanup;
> +                       }
> +
> +                       dev_priv->cursor_state = cursor_state;
> +               } else
> +                       cur_obj = intel_fb_obj(cursor_state->base.fb);
> +
> +               src = ioremap_wc(dev_priv->ggtt.mappable_base +
> +                               i915_gem_obj_ggtt_offset(obj),
> +                               obj->base.size);
> +
> +               dst = ioremap_wc(dev_priv->ggtt.mappable_base +
> +                               i915_gem_obj_ggtt_offset(cur_obj),
> +                               cur_obj->base.size);
> +
> +               /* shift the original cusrsor in to copy buffer offsetting -ive pos */
> +               x = -x;
> +               for (i = 0; i < height; i++) {
> +                       src += x * bytes_per_pixel;
> +                       memcpy(dst, src, (width - x) * bytes_per_pixel);
> +                       dst += (width - x) * bytes_per_pixel;
> +                       memset(dst, 0, x * bytes_per_pixel);
> +                       dst += x * bytes_per_pixel;
> +                       src += (width -x) * bytes_per_pixel;
> +               }
> +
> +               iounmap(src);
> +               iounmap(dst);
> +
> +               cursor_state->base.crtc_x = 0;
> +               use_obj = cur_obj;
> +               use_state = cursor_state;
> +
> +               goto update;
> +       }
> +
> +cleanup:
> +       if (cursor_state) {
> +               struct intel_framebuffer *intel_fb = to_intel_framebuffer(cursor_state->base.fb);
> +
> +               if (pinned)
> +                       i915_gem_object_ggtt_unpin(cur_obj);
> +
> +               drm_framebuffer_cleanup(cursor_state->base.fb);
> +               mutex_lock(&dev->struct_mutex);
> +               drm_gem_object_unreference(&intel_fb->obj->base);
> +               mutex_unlock(&dev->struct_mutex);
> +               kfree(intel_fb);
> +       }
> +
> +gem_err:
> +       if (dev_priv->cursor_state) {
> +               kfree(dev_priv->cursor_state);
> +               dev_priv->cursor_state = NULL;
> +       }
> +
> +       use_state = state;
> +       use_obj = obj;
> +
> +update:
> +       if (!use_obj)
> +               addr = 0;
> +       else if (!INTEL_INFO(dev)->cursor_needs_physical)
> +               addr = i915_gem_obj_ggtt_offset(use_obj);
> +       else
> +               addr = use_obj->phys_handle->busaddr;
> +
> +       intel_crtc->cursor_addr = addr;
> +
> +       intel_crtc_update_cursor(crtc, use_state);
> +}
> +
>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>                                                    int pipe)
>  {
> @@ -14478,7 +14604,10 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>         cursor->plane = pipe;
>         cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
>         cursor->check_plane = intel_check_cursor_plane;
> -       cursor->update_plane = intel_update_cursor_plane;
> +       if (IS_CHERRYVIEW(dev) && pipe == PIPE_C)
> +               cursor->update_plane = intel_update_chv_pipe_c_cursor_plane;
> +       else
> +               cursor->update_plane = intel_update_cursor_plane;
>         cursor->disable_plane = intel_disable_cursor_plane;
>
>         ret = drm_universal_plane_init(dev, &cursor->base, 0,
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx