On Thu, 2014-10-30 at 11:32 -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.
I saw the same issue, when checking #82939. AFAICS when disabling the crtc intel_plane->obj will be set to NULL and the object will be unpinned, but plane->fb remains set. Next if drm_set_plane with the same fb is called intel_prepare_sprite_plane() won't pin the object since it sees that same fb is still set, but sets intel_plane->obj. Next if drm_set_plane called with a NULL fb, it will try unpin the object, and the unbalanced refcount throws a WARN. The following got rid of the problem for me:
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 8b80d68..36762b5 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1198,9 +1198,10 @@ intel_prepare_sprite_plane(struct drm_plane *plane, struct drm_crtc *crtc = state->crtc; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); enum pipe pipe = intel_crtc->pipe; + struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_framebuffer *fb = state->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); + struct drm_i915_gem_object *old_obj = intel_plane->obj; int ret;
if (old_obj != obj) {
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