2014-10-24 11:51 GMT-02:00 Gustavo Padovan gustavo@padovan.org:

From: Gustavo Padovan gustavo.padovan@collabora.co.uk

take out pin_fb code so the commit phase can't fail anymore.

According to my bisection results, this is the first bad commit of https://bugs.freedesktop.org/show_bug.cgi?id=85634.

Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/gpu/drm/i915/intel_sprite.c | 63 +++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 2c060ad..3631b0e 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1192,34 +1192,18 @@ intel_check_sprite_plane(struct drm_plane *plane, }

static int -intel_commit_sprite_plane(struct drm_plane *plane,

•                     struct intel_plane_state *state)
  

+intel_prepare_sprite_plane(struct drm_plane *plane,

•                      struct intel_plane_state *state)
  

{ struct drm_device *dev = plane->dev; struct drm_crtc *crtc = state->crtc; struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

•   struct intel_plane *intel_plane = to_intel_plane(plane);
    enum pipe pipe = intel_crtc->pipe;
    struct drm_framebuffer *fb = state->fb;
  

•   struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
  

•   struct drm_i915_gem_object *obj = intel_fb->obj;
  

•   struct drm_i915_gem_object *old_obj = intel_plane->obj;
  

•   int crtc_x, crtc_y;
  

•   unsigned int crtc_w, crtc_h;
  

•   uint32_t src_x, src_y, src_w, src_h;
  

•   struct drm_rect *dst = &state->dst;
  

•   const struct drm_rect *clip = &state->clip;
  

•   bool primary_enabled;
  

•   struct drm_i915_gem_object *obj = intel_fb_obj(fb);
  

•   struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
    int ret;
  
  

•   /*
  

•    * If the sprite is completely covering the primary plane,
  

•    * we can disable the primary and save power.
  

•    */
  

•   primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);
  

•   WARN_ON(!primary_enabled && !state->visible && intel_crtc->active);
  

•   if (old_obj != obj) {
            mutex_lock(&dev->struct_mutex);
  
  

@@ -1238,6 +1222,36 @@ intel_commit_sprite_plane(struct drm_plane *plane, return ret; }

•   return 0;
  

+}

+static void +intel_commit_sprite_plane(struct drm_plane *plane,

•                     struct intel_plane_state *state)
  

+{

•   struct drm_device *dev = plane->dev;
  

•   struct drm_crtc *crtc = state->crtc;
  

•   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
  

•   struct intel_plane *intel_plane = to_intel_plane(plane);
  

•   enum pipe pipe = intel_crtc->pipe;
  

•   struct drm_framebuffer *fb = state->fb;
  

•   struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
  

•   struct drm_i915_gem_object *obj = intel_fb->obj;
  

•   struct drm_i915_gem_object *old_obj = intel_plane->obj;
  

•   int crtc_x, crtc_y;
  

•   unsigned int crtc_w, crtc_h;
  

•   uint32_t src_x, src_y, src_w, src_h;
  

•   struct drm_rect *dst = &state->dst;
  

•   const struct drm_rect *clip = &state->clip;
  

•   bool primary_enabled;
  

•   /*
  

•    * If the sprite is completely covering the primary plane,
  

•    * we can disable the primary and save power.
  

•    */
  

•   primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);
  

•   WARN_ON(!primary_enabled && !state->visible && intel_crtc->active);
  

•   intel_plane->crtc_x = state->orig_dst.x1;
    intel_plane->crtc_y = state->orig_dst.y1;
    intel_plane->crtc_w = drm_rect_width(&state->orig_dst);
  

@@ -1298,8 +1312,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, intel_unpin_fb_obj(old_obj); mutex_unlock(&dev->struct_mutex); }

•   return 0;
  

}

static int @@ -1339,7 +1351,12 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (ret) return ret;

•   return intel_commit_sprite_plane(plane, &state);
  

•   ret = intel_prepare_sprite_plane(plane, &state);
  

•   if (ret)
  

•           return ret;
  

•   intel_commit_sprite_plane(plane, &state);
  

•   return 0;
  

}

static int

1.9.3

---

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

On Thu, Oct 30, 2014 at 11:32:42AM -0200, Paulo Zanoni wrote:

2014-10-24 11:51 GMT-02:00 Gustavo Padovan gustavo@padovan.org:

...

From: Gustavo Padovan gustavo.padovan@collabora.co.uk

take out pin_fb code so the commit phase can't fail anymore.

According to my bisection results, this is the first bad commit of https://bugs.freedesktop.org/show_bug.cgi?id=85634.

...

Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/gpu/drm/i915/intel_sprite.c | 63 +++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 2c060ad..3631b0e 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1192,34 +1192,18 @@ intel_check_sprite_plane(struct drm_plane *plane, }

static int -intel_commit_sprite_plane(struct drm_plane *plane,

•                     struct intel_plane_state *state)
  

+intel_prepare_sprite_plane(struct drm_plane *plane,

•                      struct intel_plane_state *state)
  

<snip>

...

•   struct drm_i915_gem_object *old_obj = intel_plane->obj;
  

•   struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
  

I think this change is the culprit here. The problem is that during a modeset we call intel_plane_disable()->intel_disable_plane() (yeah, yeah the names suck big time :) which will unpin the current buffer and clear intel_plane->obj, but plane->fb will still be set, so the next time .update_plane() is called it will think the buffer was already pinned, and not attempt to pin it again.

So just using intel_plane->obj to figure out the old obj should fix it. But my plan sort of was that even if userspace "enables" a plane with the crtc off, we'd still pin the buffer. Then we can be more sure that when the crtc does get enabled the plane can actually be enabled as well (ie. the pinning can't fail at that point anymore). So if we follow that design I think we need to split intel_disable_plane() into two parts, one which just does the plane disable, and the second part just does unpinning. And then we need to just call the non-unpin variant from intel_plane_disable() so that we don't unpin the buffer during crtc disable. And then we should also kill intel_plane->obj.

...

    int ret;

•   /*
  

•    * If the sprite is completely covering the primary plane,
  

•    * we can disable the primary and save power.
  

•    */
  

•   primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);
  

•   WARN_ON(!primary_enabled && !state->visible && intel_crtc->active);
  

•   if (old_obj != obj) {
            mutex_lock(&dev->struct_mutex);
  
  

@@ -1238,6 +1222,36 @@ intel_commit_sprite_plane(struct drm_plane *plane, return ret; }

•   return 0;
  

+}

+static void +intel_commit_sprite_plane(struct drm_plane *plane,

•                     struct intel_plane_state *state)
  

+{

•   struct drm_device *dev = plane->dev;
  

•   struct drm_crtc *crtc = state->crtc;
  

•   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
  

•   struct intel_plane *intel_plane = to_intel_plane(plane);
  

•   enum pipe pipe = intel_crtc->pipe;
  

•   struct drm_framebuffer *fb = state->fb;
  

•   struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
  

•   struct drm_i915_gem_object *obj = intel_fb->obj;
  

•   struct drm_i915_gem_object *old_obj = intel_plane->obj;
  

•   int crtc_x, crtc_y;
  

•   unsigned int crtc_w, crtc_h;
  

•   uint32_t src_x, src_y, src_w, src_h;
  

•   struct drm_rect *dst = &state->dst;
  

•   const struct drm_rect *clip = &state->clip;
  

•   bool primary_enabled;
  

•   /*
  

•    * If the sprite is completely covering the primary plane,
  

•    * we can disable the primary and save power.
  

•    */
  

•   primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);
  

•   WARN_ON(!primary_enabled && !state->visible && intel_crtc->active);
  

•   intel_plane->crtc_x = state->orig_dst.x1;
    intel_plane->crtc_y = state->orig_dst.y1;
    intel_plane->crtc_w = drm_rect_width(&state->orig_dst);
  

@@ -1298,8 +1312,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, intel_unpin_fb_obj(old_obj); mutex_unlock(&dev->struct_mutex); }

•   return 0;
  

}

static int @@ -1339,7 +1351,12 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (ret) return ret;

•   return intel_commit_sprite_plane(plane, &state);
  

•   ret = intel_prepare_sprite_plane(plane, &state);
  

•   if (ret)
  

•           return ret;
  

•   intel_commit_sprite_plane(plane, &state);
  

•   return 0;
  

}

static int

1.9.3

---

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

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

On Fri, Oct 24, 2014 at 02:51:35PM +0100, Gustavo Padovan wrote:

From: Gustavo Padovan gustavo.padovan@collabora.co.uk

Merge it into the plane update_plane() callback and make other users use the update_plane() functions instead.

The fb != crtc->cursor->fb was already inside intel_crtc_cursor_set_obj() so we fold intel_crtc_cursor_set_obj() inside intel_commit_cursor_plane() and merge both paths into one.

Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk

drivers/gpu/drm/i915/intel_display.c | 215 ++++++++++++++++------------------- 1 file changed, 100 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9a913f5..60ec165 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8453,109 +8453,6 @@ static bool cursor_size_ok(struct drm_device *dev, return true; }

-static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,

• 		     struct drm_i915_gem_object *obj,
  

• 		     uint32_t width, uint32_t height)
  

-{

• struct drm_device *dev = crtc->dev;
• struct drm_i915_private *dev_priv = dev->dev_private;
• struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
• enum pipe pipe = intel_crtc->pipe;
• unsigned old_width;
• uint32_t addr;
• int ret;
• /* if we want to turn off the cursor ignore width and height */
• if (!obj) {

• DRM_DEBUG_KMS("cursor off\n");
  

• addr = 0;
  

• mutex_lock(&dev->struct_mutex);
  

• goto finish;
  

• }
• /* we only need to pin inside GTT if cursor is non-phy */
• mutex_lock(&dev->struct_mutex);
• if (!INTEL_INFO(dev)->cursor_needs_physical) {

• unsigned alignment;
  

• /*
  

•  * Global gtt pte registers are special registers which actually
  

•  * forward writes to a chunk of system memory. Which means that
  

•  * there is no risk that the register values disappear as soon
  

•  * as we call intel_runtime_pm_put(), so it is correct to wrap
  

•  * only the pin/unpin/fence and not more.
  

•  */
  

• intel_runtime_pm_get(dev_priv);
  

• /* Note that the w/a also requires 2 PTE of padding following
  

•  * the bo. We currently fill all unused PTE with the shadow
  

•  * page and so we should always have valid PTE following the
  

•  * cursor preventing the VT-d warning.
  

•  */
  

• alignment = 0;
  

• if (need_vtd_wa(dev))
  

• 	alignment = 64*1024;
  

• ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n");
  

• 	intel_runtime_pm_put(dev_priv);
  

• 	goto fail_locked;
  

• }
  

• ret = i915_gem_object_put_fence(obj);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to release fence for cursor");
  

• 	intel_runtime_pm_put(dev_priv);
  

• 	goto fail_unpin;
  

• }
  

• addr = i915_gem_obj_ggtt_offset(obj);
  

• intel_runtime_pm_put(dev_priv);
  

• } else {

• int align = IS_I830(dev) ? 16 * 1024 : 256;
  

• ret = i915_gem_object_attach_phys(obj, align);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to attach phys object\n");
  

• 	goto fail_locked;
  

• }
  

• addr = obj->phys_handle->busaddr;
  

• }
• finish:
• if (intel_crtc->cursor_bo) {

• if (!INTEL_INFO(dev)->cursor_needs_physical)
  

• 	i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo);
  

• }
• i915_gem_track_fb(intel_crtc->cursor_bo, obj,

• 	  INTEL_FRONTBUFFER_CURSOR(pipe));
  

• mutex_unlock(&dev->struct_mutex);
• old_width = intel_crtc->cursor_width;
• intel_crtc->cursor_addr = addr;
• intel_crtc->cursor_bo = obj;
• intel_crtc->cursor_width = width;
• intel_crtc->cursor_height = height;
• if (intel_crtc->active) {

• if (old_width != width)
  

• 	intel_update_watermarks(crtc);
  

• intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
  

• intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe));
  

• }
• return 0;

-fail_unpin:

• i915_gem_object_unpin_from_display_plane(obj);

-fail_locked:

• mutex_unlock(&dev->struct_mutex);
• return ret;

-}

static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, uint32_t start, uint32_t size) { @@ -11906,7 +11803,8 @@ intel_cursor_plane_disable(struct drm_plane *plane)

BUG_ON(!plane->crtc);

• return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0);

• return plane->funcs->update_plane(plane, plane->crtc, NULL,

• 			  0, 0, 0, 0, 0, 0, 0, 0);
  

}

static int @@ -11970,26 +11868,113 @@ intel_commit_cursor_plane(struct drm_plane *plane, struct intel_plane_state *state) { struct drm_crtc *crtc = state->crtc;

• struct drm_device *dev = crtc->dev;
• struct drm_i915_private *dev_priv = dev->dev_private; struct drm_framebuffer *fb = state->fb; struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

• struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
• struct drm_i915_gem_object *obj = intel_fb->obj;
• int crtc_w, crtc_h;

• struct drm_i915_gem_object *obj = intel_fb_obj(fb);

• enum pipe pipe = intel_crtc->pipe;

• unsigned old_width;

• uint32_t addr;

• int ret;

  crtc->cursor_x = state->orig_dst.x1; crtc->cursor_y = state->orig_dst.y1;

• if (fb != crtc->cursor->fb) {

• crtc_w = drm_rect_width(&state->orig_dst);
  

• crtc_h = drm_rect_height(&state->orig_dst);
  

• return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h);
  

• if (intel_crtc->cursor_bo == obj)

• goto update;
  

• /* if we want to turn off the cursor ignore width and height */
• if (!obj) {

• DRM_DEBUG_KMS("cursor off\n");
  

• addr = 0;
  

• mutex_lock(&dev->struct_mutex);
  

• goto finish;
  

• }
• /* we only need to pin inside GTT if cursor is non-phy */
• mutex_lock(&dev->struct_mutex);
• if (!INTEL_INFO(dev)->cursor_needs_physical) {

• unsigned alignment;
  

• /*
  

•  * Global gtt pte registers are special registers which actually
  

•  * forward writes to a chunk of system memory. Which means that
  

•  * there is no risk that the register values disappear as soon
  

•  * as we call intel_runtime_pm_put(), so it is correct to wrap
  

•  * only the pin/unpin/fence and not more.
  

•  */
  

• intel_runtime_pm_get(dev_priv);
  

• /* Note that the w/a also requires 2 PTE of padding following
  

•  * the bo. We currently fill all unused PTE with the shadow
  

•  * page and so we should always have valid PTE following the
  

•  * cursor preventing the VT-d warning.
  

•  */
  

• alignment = 0;
  

• if (need_vtd_wa(dev))
  

• 	alignment = 64*1024;
  

• ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n");
  

• 	intel_runtime_pm_put(dev_priv);
  

• 	goto fail_locked;
  

• }
  

• ret = i915_gem_object_put_fence(obj);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to release fence for cursor");
  

• 	intel_runtime_pm_put(dev_priv);
  

• 	goto fail_unpin;
  

• }
  

• addr = i915_gem_obj_ggtt_offset(obj);
  

• intel_runtime_pm_put(dev_priv);
  

  } else {

• intel_crtc_update_cursor(crtc, state->visible);
  

• int align = IS_I830(dev) ? 16 * 1024 : 256;
  

• ret = i915_gem_object_attach_phys(obj, align);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to attach phys object\n");
  

• 	goto fail_locked;
  

• }
  

• addr = obj->phys_handle->busaddr;
  

• }

• intel_frontbuffer_flip(crtc->dev,
  

• 		       INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe));
  

+finish:

• if (intel_crtc->cursor_bo) {

• if (!INTEL_INFO(dev)->cursor_needs_physical)
  

• 	i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo);
  

• }

• return 0;
  

• i915_gem_track_fb(intel_crtc->cursor_bo, obj,

• 	  INTEL_FRONTBUFFER_CURSOR(pipe));
  

• mutex_unlock(&dev->struct_mutex);
• intel_crtc->cursor_addr = addr;
• intel_crtc->cursor_bo = obj;

+update:

• old_width = intel_crtc->cursor_width;
• intel_crtc->cursor_width = drm_rect_width(&state->orig_dst);
• intel_crtc->cursor_height = drm_rect_height(&state->orig_dst);
• if (intel_crtc->active) {

• if (old_width != intel_crtc->cursor_width)
  

• 	intel_update_watermarks(crtc);
  

• intel_crtc_update_cursor(crtc, state->visible);
  

So we need to make sure state->visible==false when there's no fb. I suppose we should just do that in drm_plane_helper_check_update().

• intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe));
  

  }
• return 0;

+fail_unpin:

• i915_gem_object_unpin_from_display_plane(obj);

+fail_locked:

• mutex_unlock(&dev->struct_mutex);
• drm_gem_object_unreference_unlocked(&obj->base);
• return ret;

}

static int

1.9.3

---

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

On Fri, Oct 24, 2014 at 02:51:35PM +0100, Gustavo Padovan wrote:

From: Gustavo Padovan gustavo.padovan@collabora.co.uk

Merge it into the plane update_plane() callback and make other users use the update_plane() functions instead.

The fb != crtc->cursor->fb was already inside intel_crtc_cursor_set_obj() so we fold intel_crtc_cursor_set_obj() inside intel_commit_cursor_plane() and merge both paths into one.

Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk

drivers/gpu/drm/i915/intel_display.c | 215 ++++++++++++++++------------------- 1 file changed, 100 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9a913f5..60ec165 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8453,109 +8453,6 @@ static bool cursor_size_ok(struct drm_device *dev, return true; }

-static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,

• 		     struct drm_i915_gem_object *obj,
  

• 		     uint32_t width, uint32_t height)
  

-{

• struct drm_device *dev = crtc->dev;
• struct drm_i915_private *dev_priv = dev->dev_private;
• struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
• enum pipe pipe = intel_crtc->pipe;
• unsigned old_width;
• uint32_t addr;
• int ret;
• /* if we want to turn off the cursor ignore width and height */
• if (!obj) {

• DRM_DEBUG_KMS("cursor off\n");
  

• addr = 0;
  

• mutex_lock(&dev->struct_mutex);
  

• goto finish;
  

• }
• /* we only need to pin inside GTT if cursor is non-phy */
• mutex_lock(&dev->struct_mutex);
• if (!INTEL_INFO(dev)->cursor_needs_physical) {

• unsigned alignment;
  

• /*
  

•  * Global gtt pte registers are special registers which actually
  

•  * forward writes to a chunk of system memory. Which means that
  

•  * there is no risk that the register values disappear as soon
  

•  * as we call intel_runtime_pm_put(), so it is correct to wrap
  

•  * only the pin/unpin/fence and not more.
  

•  */
  

• intel_runtime_pm_get(dev_priv);
  

• /* Note that the w/a also requires 2 PTE of padding following
  

•  * the bo. We currently fill all unused PTE with the shadow
  

•  * page and so we should always have valid PTE following the
  

•  * cursor preventing the VT-d warning.
  

•  */
  

• alignment = 0;
  

• if (need_vtd_wa(dev))
  

• 	alignment = 64*1024;
  

• ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n");
  

• 	intel_runtime_pm_put(dev_priv);
  

• 	goto fail_locked;
  

• }
  

• ret = i915_gem_object_put_fence(obj);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to release fence for cursor");
  

• 	intel_runtime_pm_put(dev_priv);
  

• 	goto fail_unpin;
  

• }
  

• addr = i915_gem_obj_ggtt_offset(obj);
  

• intel_runtime_pm_put(dev_priv);
  

• } else {

• int align = IS_I830(dev) ? 16 * 1024 : 256;
  

• ret = i915_gem_object_attach_phys(obj, align);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to attach phys object\n");
  

• 	goto fail_locked;
  

• }
  

• addr = obj->phys_handle->busaddr;
  

• }
• finish:
• if (intel_crtc->cursor_bo) {

• if (!INTEL_INFO(dev)->cursor_needs_physical)
  

• 	i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo);
  

• }
• i915_gem_track_fb(intel_crtc->cursor_bo, obj,

• 	  INTEL_FRONTBUFFER_CURSOR(pipe));
  

• mutex_unlock(&dev->struct_mutex);
• old_width = intel_crtc->cursor_width;
• intel_crtc->cursor_addr = addr;
• intel_crtc->cursor_bo = obj;
• intel_crtc->cursor_width = width;
• intel_crtc->cursor_height = height;
• if (intel_crtc->active) {

• if (old_width != width)
  

• 	intel_update_watermarks(crtc);
  

• intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
  

• intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe));
  

• }
• return 0;

-fail_unpin:

• i915_gem_object_unpin_from_display_plane(obj);

-fail_locked:

• mutex_unlock(&dev->struct_mutex);
• return ret;

-}

static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, uint32_t start, uint32_t size) { @@ -11906,7 +11803,8 @@ intel_cursor_plane_disable(struct drm_plane *plane)

BUG_ON(!plane->crtc);

• return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0);

• return plane->funcs->update_plane(plane, plane->crtc, NULL,

• 			  0, 0, 0, 0, 0, 0, 0, 0);
  

}

static int @@ -11970,26 +11868,113 @@ intel_commit_cursor_plane(struct drm_plane *plane, struct intel_plane_state *state) { struct drm_crtc *crtc = state->crtc;

• struct drm_device *dev = crtc->dev;
• struct drm_i915_private *dev_priv = dev->dev_private; struct drm_framebuffer *fb = state->fb; struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

• struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
• struct drm_i915_gem_object *obj = intel_fb->obj;
• int crtc_w, crtc_h;

• struct drm_i915_gem_object *obj = intel_fb_obj(fb);

• enum pipe pipe = intel_crtc->pipe;

• unsigned old_width;

• uint32_t addr;

• int ret;

  crtc->cursor_x = state->orig_dst.x1; crtc->cursor_y = state->orig_dst.y1;

• if (fb != crtc->cursor->fb) {

• crtc_w = drm_rect_width(&state->orig_dst);
  

• crtc_h = drm_rect_height(&state->orig_dst);
  

• return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h);
  

• if (intel_crtc->cursor_bo == obj)

• goto update;
  

• /* if we want to turn off the cursor ignore width and height */
• if (!obj) {

• DRM_DEBUG_KMS("cursor off\n");
  

• addr = 0;
  

• mutex_lock(&dev->struct_mutex);
  

• goto finish;
  

• }
• /* we only need to pin inside GTT if cursor is non-phy */
• mutex_lock(&dev->struct_mutex);
• if (!INTEL_INFO(dev)->cursor_needs_physical) {

• unsigned alignment;
  

• /*
  

•  * Global gtt pte registers are special registers which actually
  

•  * forward writes to a chunk of system memory. Which means that
  

•  * there is no risk that the register values disappear as soon
  

•  * as we call intel_runtime_pm_put(), so it is correct to wrap
  

•  * only the pin/unpin/fence and not more.
  

•  */
  

• intel_runtime_pm_get(dev_priv);
  

• /* Note that the w/a also requires 2 PTE of padding following
  

•  * the bo. We currently fill all unused PTE with the shadow
  

•  * page and so we should always have valid PTE following the
  

•  * cursor preventing the VT-d warning.
  

•  */
  

• alignment = 0;
  

• if (need_vtd_wa(dev))
  

• 	alignment = 64*1024;
  

• ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n");
  

• 	intel_runtime_pm_put(dev_priv);
  

• 	goto fail_locked;
  

• }
  

• ret = i915_gem_object_put_fence(obj);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to release fence for cursor");
  

• 	intel_runtime_pm_put(dev_priv);
  

• 	goto fail_unpin;
  

• }
  

• addr = i915_gem_obj_ggtt_offset(obj);
  

• intel_runtime_pm_put(dev_priv);
  

  } else {

• intel_crtc_update_cursor(crtc, state->visible);
  

• int align = IS_I830(dev) ? 16 * 1024 : 256;
  

• ret = i915_gem_object_attach_phys(obj, align);
  

• if (ret) {
  

• 	DRM_DEBUG_KMS("failed to attach phys object\n");
  

• 	goto fail_locked;
  

• }
  

• addr = obj->phys_handle->busaddr;
  

• }

• intel_frontbuffer_flip(crtc->dev,
  

• 		       INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe));
  

+finish:

• if (intel_crtc->cursor_bo) {

• if (!INTEL_INFO(dev)->cursor_needs_physical)
  

• 	i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo);
  

• }

• return 0;
  

• i915_gem_track_fb(intel_crtc->cursor_bo, obj,

• 	  INTEL_FRONTBUFFER_CURSOR(pipe));
  

• mutex_unlock(&dev->struct_mutex);
• intel_crtc->cursor_addr = addr;
• intel_crtc->cursor_bo = obj;

+update:

• old_width = intel_crtc->cursor_width;
• intel_crtc->cursor_width = drm_rect_width(&state->orig_dst);
• intel_crtc->cursor_height = drm_rect_height(&state->orig_dst);
• if (intel_crtc->active) {

• if (old_width != intel_crtc->cursor_width)
  

• 	intel_update_watermarks(crtc);
  

• intel_crtc_update_cursor(crtc, state->visible);
  

• intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe));
  

  }
• return 0;

+fail_unpin:

• i915_gem_object_unpin_from_display_plane(obj);

+fail_locked:

• mutex_unlock(&dev->struct_mutex);
• drm_gem_object_unreference_unlocked(&obj->base);

That unref needs to be dropped. With that change the patch is

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

• return ret;

}

static int

1.9.3

---

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