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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
