Re: [PATCH 2/2] drm/i915: Make plane fb tracking work correctly.

Op 02-09-15 om 10:42 schreef Maarten Lankhorst:

atomic->disabled_planes is a hack that had to exist because prepare_fb was only called when a new fb was set. This messed up fb tracking in some circumstances like aborts from interruptible waits. As a result interruptible waiting in prepare_plane_fb was forbidden, but other errors could still cause frontbuffer tracking to be messed up.

Now that prepare_fb is always called, this hack is no longer required and prepare_fb may fail without consequences.

Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++++++---------------------- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index edcf8746440e..6ebc7661ec8c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4718,17 +4718,6 @@ static void intel_pre_plane_update(struct intel_crtc *crtc) struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc_atomic_commit *atomic = &crtc->atomic;

• struct drm_plane *p;

• /* Track fb's for any planes being disabled */

• drm_for_each_plane_mask(p, dev, atomic->disabled_planes) {

• struct intel_plane *plane = to_intel_plane(p);
  

• mutex_lock(&dev->struct_mutex);
  

• i915_gem_track_fb(intel_fb_obj(plane->base.fb), NULL,
  

• 		  plane->frontbuffer_bit);
  

• mutex_unlock(&dev->struct_mutex);
  

• }

  if (atomic->wait_for_flips) intel_crtc_wait_for_pending_flips(&crtc->base);

@@ -11511,14 +11500,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, return ret; }

• /*

• * Disabling a plane is always okay; we just need to update
  

• * fb tracking in a special way since cleanup_fb() won't
  

• * get called by the plane helpers.
  

• */
  

• if (old_plane_state->base.fb && !fb)

• intel_crtc->atomic.disabled_planes |= 1 << i;
  

• was_visible = old_plane_state->visible; visible = to_intel_plane_state(plane_state)->visible;

@@ -13272,15 +13253,17 @@ intel_prepare_plane_fb(struct drm_plane *plane, struct drm_framebuffer *fb = new_state->fb; struct intel_plane *intel_plane = to_intel_plane(plane); 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_fb_obj(plane->state->fb); int ret = 0;

• if (!obj)

• if (!obj && !old_obj) return 0;

  mutex_lock(&dev->struct_mutex);

• if (plane->type == DRM_PLANE_TYPE_CURSOR &&

• if (!obj) {

• ret = 0;
  

• } else if (plane->type == DRM_PLANE_TYPE_CURSOR && INTEL_INFO(dev)->cursor_needs_physical) { int align = IS_I830(dev) ? 16 * 1024 : 256; ret = i915_gem_object_attach_phys(obj, align);

@@ -13310,17 +13293,23 @@ intel_cleanup_plane_fb(struct drm_plane *plane, const struct drm_plane_state *old_state) { struct drm_device *dev = plane->dev;

• struct drm_i915_gem_object *obj = intel_fb_obj(old_state->fb);

• struct intel_plane *intel_plane = to_intel_plane(plane);
• struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
• struct drm_i915_gem_object *obj = intel_fb_obj(plane->fb);

^Oops, this one needs to be plane->state->fb, or a frontbuffer tracking mismatch would still occur when applying the interruptible pinning series.

• if (!obj)

• if (!obj && !old_obj) return;

• if (plane->type != DRM_PLANE_TYPE_CURSOR ||

•    !INTEL_INFO(dev)->cursor_needs_physical) {
  

• mutex_lock(&dev->struct_mutex);
  

• mutex_lock(&dev->struct_mutex);
• if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR ||

•    !INTEL_INFO(dev)->cursor_needs_physical))
  

  intel_unpin_fb_obj(old_state->fb, old_state);

• mutex_unlock(&dev->struct_mutex);
  

• }

• /* prepare_fb aborted? */
• if ((old_obj && (old_obj->frontbuffer_bits & intel_plane->frontbuffer_bit)) ||

•    (obj && !(obj->frontbuffer_bits & intel_plane->frontbuffer_bit)))
  

• i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
  

• mutex_unlock(&dev->struct_mutex);

}

int diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 104a7f8b650d..176f0a15e41b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -505,7 +505,6 @@ struct intel_crtc_atomic_commit { bool disable_cxsr; bool pre_disable_primary; bool update_wm_pre, update_wm_post;

• unsigned disabled_planes;

  /* Sleepable operations to perform after commit */ unsigned fb_bits;