From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Fold intel_pipe_set_base() in the update primary plane path merging pieces of code that are common to both paths.
Basically the the pin/unpin procedures are the same for both paths and some checks can also be shared (some of the were moved to the check() stage)
v2: take Ville's comments: - remove unnecessary plane check - move mutex lock to inside the conditional - make the pin fail message a debug one - add a fixme for the fastboot hack - call intel_frontbuffer_flip() after FBC update
v3: take more Ville's comments: - fold update code under if (intel_crtc->active), and do the visible/!visible split inside. - check ret inside the same conditional we assign it
v4: don't use intel_enable_primary_hw_plane(), the primary_enabled check inside will break page flips
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/gpu/drm/i915/intel_display.c | 141 +++++++++++++++++++++-------------- 1 file changed, 84 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5b05ddb..1fd9b70 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11792,12 +11792,23 @@ intel_check_primary_plane(struct drm_plane *plane, struct drm_rect *dest = &state->dst; struct drm_rect *src = &state->src; const struct drm_rect *clip = &state->clip; + int ret;
- return drm_plane_helper_check_update(plane, crtc, fb, + ret = drm_plane_helper_check_update(plane, crtc, fb, src, dest, clip, DRM_PLANE_HELPER_NO_SCALING, DRM_PLANE_HELPER_NO_SCALING, false, true, &state->visible); + if (ret) + return ret; + + /* no fb bound */ + if (state->visible && !fb) { + DRM_ERROR("No FB bound\n"); + return -EINVAL; + } + + return 0; }
static int @@ -11809,6 +11820,8 @@ intel_commit_primary_plane(struct drm_plane *plane, 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; + struct drm_framebuffer *old_fb = plane->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); struct intel_plane *intel_plane = to_intel_plane(plane); @@ -11817,67 +11830,28 @@ intel_commit_primary_plane(struct drm_plane *plane,
intel_crtc_wait_for_pending_flips(crtc);
- /* - * If clipping results in a non-visible primary plane, we'll disable - * the primary plane. Note that this is a bit different than what - * happens if userspace explicitly disables the plane by passing fb=0 - * because plane->fb still gets set and pinned. - */ - if (!state->visible) { - mutex_lock(&dev->struct_mutex); - - /* - * Try to pin the new fb first so that we can bail out if we - * fail. - */ - if (plane->fb != fb) { - ret = intel_pin_and_fence_fb_obj(dev, obj, NULL); - if (ret) { - mutex_unlock(&dev->struct_mutex); - return ret; - } - } - - i915_gem_track_fb(old_obj, obj, - INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe)); - - if (intel_crtc->primary_enabled) - intel_disable_primary_hw_plane(plane, crtc); - - - if (plane->fb != fb) - if (plane->fb) - intel_unpin_fb_obj(old_obj); + if (intel_crtc_has_pending_flip(crtc)) { + DRM_ERROR("pipe is still busy with an old pageflip\n"); + return -EBUSY; + }
+ if (plane->fb != fb) { + mutex_lock(&dev->struct_mutex); + ret = intel_pin_and_fence_fb_obj(dev, obj, NULL); + if (ret == 0) + i915_gem_track_fb(old_obj, obj, + INTEL_FRONTBUFFER_PRIMARY(pipe)); mutex_unlock(&dev->struct_mutex); - - } else { - if (intel_crtc && intel_crtc->active && - intel_crtc->primary_enabled) { - /* - * FBC does not work on some platforms for rotated - * planes, so disable it when rotation is not 0 and - * update it when rotation is set back to 0. - * - * FIXME: This is redundant with the fbc update done in - * the primary plane enable function except that that - * one is done too late. We eventually need to unify - * this. - */ - if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && - dev_priv->fbc.plane == intel_crtc->plane && - intel_plane->rotation != BIT(DRM_ROTATE_0)) { - intel_disable_fbc(dev); - } - } - ret = intel_pipe_set_base(crtc, src->x1, src->y1, fb); - if (ret) + if (ret != 0) { + DRM_DEBUG_KMS("pin & fence failed\n"); return ret; - - if (!intel_crtc->primary_enabled) - intel_enable_primary_hw_plane(plane, crtc); + } }
+ crtc->primary->fb = fb; + crtc->x = src->x1; + crtc->y = src->y1; + 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); @@ -11888,6 +11862,59 @@ intel_commit_primary_plane(struct drm_plane *plane, intel_plane->src_h = drm_rect_height(&state->orig_src); intel_plane->obj = obj;
+ if (intel_crtc->active) { + /* + * FBC does not work on some platforms for rotated + * planes, so disable it when rotation is not 0 and + * update it when rotation is set back to 0. + * + * FIXME: This is redundant with the fbc update done in + * the primary plane enable function except that that + * one is done too late. We eventually need to unify + * this. + */ + if (intel_crtc->primary_enabled && + INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && + dev_priv->fbc.plane == intel_crtc->plane && + intel_plane->rotation != BIT(DRM_ROTATE_0)) { + intel_disable_fbc(dev); + } + + if (state->visible) { + /* FIXME: kill this fastboot hack */ + intel_update_pipe_size(intel_crtc); + + dev_priv->display.update_primary_plane(crtc, plane->fb, + crtc->x, crtc->y); + } else { + /* + * If clipping results in a non-visible primary plane, + * we'll disable the primary plane. Note that this is + * a bit different than what happens if userspace + * explicitly disables the plane by passing fb=0 + * because plane->fb still gets set and pinned. + */ + intel_disable_primary_hw_plane(plane, crtc); + } + + intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe)); + + mutex_lock(&dev->struct_mutex); + intel_update_fbc(dev); + mutex_unlock(&dev->struct_mutex); + } + + if (old_fb) { + if (intel_crtc->active && old_fb != fb) + intel_wait_for_vblank(dev, intel_crtc->pipe); + + if (old_fb != fb) { + mutex_lock(&dev->struct_mutex); + intel_unpin_fb_obj(old_obj); + mutex_unlock(&dev->struct_mutex); + } + } + return 0; }
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Move checks inside intel_crtc_cursor_set_obj() to intel_check_cursor_plane(), we only use they there so move them out to make the merge of intel_crtc_cursor_set_obj() into intel_check_cursor_plane() easier.
This is another step toward the atomic modesetting support and unification of plane operations such pin/unpin of fb objects on i915.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/gpu/drm/i915/intel_display.c | 66 ++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1fd9b70..a68befb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8377,7 +8377,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, 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, stride; + unsigned old_width; uint32_t addr; int ret;
@@ -8389,30 +8389,11 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, goto finish; }
- /* Check for which cursor types we support */ - if (!cursor_size_ok(dev, width, height)) { - DRM_DEBUG("Cursor dimension not supported\n"); - return -EINVAL; - } - - stride = roundup_pow_of_two(width) * 4; - if (obj->base.size < stride * height) { - DRM_DEBUG_KMS("buffer is too small\n"); - ret = -ENOMEM; - goto fail; - } - /* 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;
- if (obj->tiling_mode) { - DRM_DEBUG_KMS("cursor cannot be tiled\n"); - ret = -EINVAL; - goto fail_locked; - } - /* * Global gtt pte registers are special registers which actually * forward writes to a chunk of system memory. Which means that @@ -8488,7 +8469,6 @@ fail_unpin: i915_gem_object_unpin_from_display_plane(obj); fail_locked: mutex_unlock(&dev->struct_mutex); -fail: drm_gem_object_unreference_unlocked(&obj->base); return ret; } @@ -12039,16 +12019,58 @@ intel_check_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_framebuffer *fb = state->fb; struct drm_rect *dest = &state->dst; struct drm_rect *src = &state->src; const struct drm_rect *clip = &state->clip; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + int crtc_w, crtc_h; + unsigned stride; + int ret;
- return drm_plane_helper_check_update(plane, crtc, fb, + ret = drm_plane_helper_check_update(plane, crtc, fb, src, dest, clip, DRM_PLANE_HELPER_NO_SCALING, DRM_PLANE_HELPER_NO_SCALING, true, true, &state->visible); + if (ret) + return ret; + + crtc_w = drm_rect_width(&state->orig_dst); + crtc_h = drm_rect_height(&state->orig_dst); + + /* if we want to turn off the cursor ignore width and height */ + if (!obj) + return 0; + + if (fb == crtc->cursor->fb) + return 0; + + /* Check for which cursor types we support */ + if (!cursor_size_ok(dev, crtc_w, crtc_h)) { + DRM_DEBUG("Cursor dimension not supported\n"); + return -EINVAL; + } + + stride = roundup_pow_of_two(crtc_w) * 4; + if (obj->base.size < stride * crtc_h) { + DRM_DEBUG_KMS("buffer is too small\n"); + ret = -ENOMEM; + goto fail; + } + + /* we only need to pin inside GTT if cursor is non-phy */ + mutex_lock(&dev->struct_mutex); + if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) { + DRM_DEBUG_KMS("cursor cannot be tiled\n"); + ret = -EINVAL; + } + mutex_unlock(&dev->struct_mutex); + +fail: + drm_gem_object_unreference_unlocked(&obj->base); + return ret; }
static int
On Thu, Sep 18, 2014 at 04:43:13PM -0300, Gustavo Padovan wrote:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Move checks inside intel_crtc_cursor_set_obj() to intel_check_cursor_plane(), we only use they there so move them out to make the merge of intel_crtc_cursor_set_obj() into intel_check_cursor_plane() easier.
This is another step toward the atomic modesetting support and unification of plane operations such pin/unpin of fb objects on i915.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/gpu/drm/i915/intel_display.c | 66 ++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1fd9b70..a68befb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8377,7 +8377,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, 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, stride;
- unsigned old_width; uint32_t addr; int ret;
@@ -8389,30 +8389,11 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, goto finish; }
/* Check for which cursor types we support */
if (!cursor_size_ok(dev, width, height)) {
DRM_DEBUG("Cursor dimension not supported\n");return -EINVAL;}
stride = roundup_pow_of_two(width) * 4;
if (obj->base.size < stride * height) {
DRM_DEBUG_KMS("buffer is too small\n");ret = -ENOMEM;goto fail;}
/* 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;
if (obj->tiling_mode) {DRM_DEBUG_KMS("cursor cannot be tiled\n");ret = -EINVAL;goto fail_locked;}
Hmm. I was going to say this check needs to remain here due to struct_mutex getting dropped between check() and here. But it looks like these days obj->framebuffer_references should protect us from anyone changing the tiling mode while the object is wrapped in framebuffer. So seems moving it should still work fine.
- /*
- Global gtt pte registers are special registers which actually
- forward writes to a chunk of system memory. Which means that
@@ -8488,7 +8469,6 @@ fail_unpin: i915_gem_object_unpin_from_display_plane(obj); fail_locked: mutex_unlock(&dev->struct_mutex); -fail: drm_gem_object_unreference_unlocked(&obj->base);
That unref looks like a leftover from the days before universal planes. Should be just removed AFAICS and the stale comments about reference consumption should be removed. Please send a separate patch for this stuff.
return ret; } @@ -12039,16 +12019,58 @@ intel_check_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_framebuffer *fb = state->fb; struct drm_rect *dest = &state->dst; struct drm_rect *src = &state->src; const struct drm_rect *clip = &state->clip;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- int crtc_w, crtc_h;
- unsigned stride;
- int ret;
- return drm_plane_helper_check_update(plane, crtc, fb,
- ret = drm_plane_helper_check_update(plane, crtc, fb, src, dest, clip, DRM_PLANE_HELPER_NO_SCALING, DRM_PLANE_HELPER_NO_SCALING, true, true, &state->visible);
- if (ret)
return ret;- crtc_w = drm_rect_width(&state->orig_dst);
- crtc_h = drm_rect_height(&state->orig_dst);
Could move these a bit later since they're not needed immediately.
- /* if we want to turn off the cursor ignore width and height */
- if (!obj)
return 0;- if (fb == crtc->cursor->fb)
return 0;- /* Check for which cursor types we support */
- if (!cursor_size_ok(dev, crtc_w, crtc_h)) {
DRM_DEBUG("Cursor dimension not supported\n");return -EINVAL;- }
- stride = roundup_pow_of_two(crtc_w) * 4;
- if (obj->base.size < stride * crtc_h) {
DRM_DEBUG_KMS("buffer is too small\n");ret = -ENOMEM;goto fail;- }
- /* we only need to pin inside GTT if cursor is non-phy */
- mutex_lock(&dev->struct_mutex);
- if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) {
DRM_DEBUG_KMS("cursor cannot be tiled\n");ret = -EINVAL;- }
- mutex_unlock(&dev->struct_mutex);
+fail:
- drm_gem_object_unreference_unlocked(&obj->base);
and this bug should not get copied here.
- return ret;
}
static int
1.9.3
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
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 | 229 ++++++++++++++++------------------- 1 file changed, 106 insertions(+), 123 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a68befb..ad86fa3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8362,117 +8362,6 @@ static bool cursor_size_ok(struct drm_device *dev, return true; }
-/* - * intel_crtc_cursor_set_obj - Set cursor to specified GEM object - * - * Note that the object's reference will be consumed if the update fails. If - * the update succeeds, the reference of the old object (if any) will be - * consumed. - */ -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); - drm_gem_object_unreference_unlocked(&obj->base); - return ret; -} - static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, uint32_t start, uint32_t size) { @@ -12011,7 +11900,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 @@ -12078,26 +11968,119 @@ 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; + bool on; + 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; + } + +finish: + if (intel_crtc->cursor_bo) { + if (!INTEL_INFO(dev)->cursor_needs_physical) + i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); + }
- intel_frontbuffer_flip(crtc->dev, - INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe)); + i915_gem_track_fb(intel_crtc->cursor_bo, obj, + INTEL_FRONTBUFFER_CURSOR(pipe)); + mutex_unlock(&dev->struct_mutex);
- return 0; + 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->cursor_bo == obj) + on = state->visible; + else + on = !obj; + + if (intel_crtc->active) { + if (old_width != intel_crtc->cursor_width) + intel_update_watermarks(crtc); + intel_crtc_update_cursor(crtc, on); } + + 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
From: Daniel Stone daniels@collabora.com
Start the work of splitting the intel_crtc_page_flip() for later use by the atomic modesetting API.
Signed-off-by: Daniel Stone daniels@collabora.com Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ad86fa3..6c61c8f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9757,23 +9757,11 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) spin_unlock(&dev->event_lock); }
-static int intel_crtc_page_flip(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - struct drm_pending_vblank_event *event, - uint32_t page_flip_flags) +static int intel_crtc_check_page_flip(struct drm_crtc *crtc, + struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = dev->dev_private; struct drm_framebuffer *old_fb = crtc->primary->fb; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; - struct intel_unpin_work *work; - struct intel_engine_cs *ring; - int ret; - - //trigger software GT busyness calculation - gen8_flip_interrupt(dev);
/* * drm_mode_page_flip_ioctl() should already catch this, but double @@ -9796,6 +9784,27 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, fb->pitches[0] != crtc->primary->fb->pitches[0])) return -EINVAL;
+ return 0; +} + +static int intel_crtc_commit_page_flip(struct drm_crtc *crtc, + struct drm_framebuffer *fb, + struct drm_pending_vblank_event *event, + uint32_t page_flip_flags) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_framebuffer *old_fb = crtc->primary->fb; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + enum pipe pipe = intel_crtc->pipe; + struct intel_unpin_work *work; + struct intel_engine_cs *ring; + int ret; + + /* trigger software GT busyness calculation */ + gen8_flip_interrupt(dev); + if (i915_terminally_wedged(&dev_priv->gpu_error)) goto out_hang;
@@ -9939,6 +9948,20 @@ out_hang: return ret; }
+static int intel_crtc_page_flip(struct drm_crtc *crtc, + struct drm_framebuffer *fb, + struct drm_pending_vblank_event *event, + uint32_t page_flip_flags) +{ + int ret; + + ret = intel_crtc_check_page_flip(crtc, fb); + if (ret) + return ret; + + return intel_crtc_commit_page_flip(crtc, fb, event, page_flip_flags); +} + static struct drm_crtc_helper_funcs intel_helper_funcs = { .mode_set_base_atomic = intel_pipe_set_base_atomic, .load_lut = intel_crtc_load_lut,
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
After some refactor intel_primary_plane_setplane() does the same as intel_pipe_set_base() so we can get rid of it and replace the calls with intel_primary_plane_setplane().
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/gpu/drm/i915/intel_display.c | 79 ++++-------------------------------- 1 file changed, 8 insertions(+), 71 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6c61c8f..2477587 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2763,74 +2763,6 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) crtc->config.pipe_src_h = adjusted_mode->crtc_vdisplay; }
-static int -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, - struct drm_framebuffer *fb) -{ - 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; - struct drm_framebuffer *old_fb = crtc->primary->fb; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb); - int ret; - - if (intel_crtc_has_pending_flip(crtc)) { - DRM_ERROR("pipe is still busy with an old pageflip\n"); - return -EBUSY; - } - - /* no fb bound */ - if (!fb) { - DRM_ERROR("No FB bound\n"); - return 0; - } - - if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) { - DRM_ERROR("no plane for crtc: plane %c, num_pipes %d\n", - plane_name(intel_crtc->plane), - INTEL_INFO(dev)->num_pipes); - return -EINVAL; - } - - mutex_lock(&dev->struct_mutex); - ret = intel_pin_and_fence_fb_obj(dev, obj, NULL); - if (ret == 0) - i915_gem_track_fb(old_obj, obj, - INTEL_FRONTBUFFER_PRIMARY(pipe)); - mutex_unlock(&dev->struct_mutex); - if (ret != 0) { - DRM_ERROR("pin & fence failed\n"); - return ret; - } - - intel_update_pipe_size(intel_crtc); - - dev_priv->display.update_primary_plane(crtc, fb, x, y); - - if (intel_crtc->active) - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe)); - - crtc->primary->fb = fb; - crtc->x = x; - crtc->y = y; - - if (old_fb) { - if (intel_crtc->active && old_fb != fb) - intel_wait_for_vblank(dev, intel_crtc->pipe); - mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(old_obj); - mutex_unlock(&dev->struct_mutex); - } - - mutex_lock(&dev->struct_mutex); - intel_update_fbc(dev); - mutex_unlock(&dev->struct_mutex); - - return 0; -} - static void intel_fdi_normal_train(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -9797,6 +9729,7 @@ static int intel_crtc_commit_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *old_fb = crtc->primary->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_plane *primary = crtc->primary; enum pipe pipe = intel_crtc->pipe; struct intel_unpin_work *work; struct intel_engine_cs *ring; @@ -9938,7 +9871,9 @@ free_work: if (ret == -EIO) { out_hang: intel_crtc_wait_for_pending_flips(crtc); - ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb); + ret = primary->funcs->update_plane(primary, crtc, fb, + 0, 0, 0, 0, + crtc->x, crtc->y, 0, 0); if (ret == 0 && event) { spin_lock_irq(&dev->event_lock); drm_send_vblank_event(dev, pipe, event); @@ -11475,11 +11410,13 @@ static int intel_crtc_set_config(struct drm_mode_set *set) set->x, set->y, set->fb); } else if (config->fb_changed) { struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc); + struct drm_plane *primary = set->crtc->primary;
intel_crtc_wait_for_pending_flips(set->crtc);
- ret = intel_pipe_set_base(set->crtc, - set->x, set->y, set->fb); + ret = primary->funcs->update_plane(primary, set->crtc, set->fb, + 0, 0, 0, 0, + set->x, set->y, 0, 0);
/* * We need to make sure the primary plane is re-enabled if it
On Thu, Sep 18, 2014 at 04:43:16PM -0300, Gustavo Padovan wrote:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
After some refactor intel_primary_plane_setplane() does the same as intel_pipe_set_base() so we can get rid of it and replace the calls with intel_primary_plane_setplane().
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/gpu/drm/i915/intel_display.c | 79 ++++-------------------------------- 1 file changed, 8 insertions(+), 71 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6c61c8f..2477587 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2763,74 +2763,6 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) crtc->config.pipe_src_h = adjusted_mode->crtc_vdisplay; }
-static int -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *fb)-{
- 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;
- struct drm_framebuffer *old_fb = crtc->primary->fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
- int ret;
- if (intel_crtc_has_pending_flip(crtc)) {
DRM_ERROR("pipe is still busy with an old pageflip\n");return -EBUSY;- }
- /* no fb bound */
- if (!fb) {
DRM_ERROR("No FB bound\n");return 0;- }
- if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) {
DRM_ERROR("no plane for crtc: plane %c, num_pipes %d\n",plane_name(intel_crtc->plane),INTEL_INFO(dev)->num_pipes);return -EINVAL;- }
- mutex_lock(&dev->struct_mutex);
- ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
- if (ret == 0)
i915_gem_track_fb(old_obj, obj,INTEL_FRONTBUFFER_PRIMARY(pipe));- mutex_unlock(&dev->struct_mutex);
- if (ret != 0) {
DRM_ERROR("pin & fence failed\n");return ret;- }
- intel_update_pipe_size(intel_crtc);
- dev_priv->display.update_primary_plane(crtc, fb, x, y);
- if (intel_crtc->active)
intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));- crtc->primary->fb = fb;
- crtc->x = x;
- crtc->y = y;
- if (old_fb) {
if (intel_crtc->active && old_fb != fb)intel_wait_for_vblank(dev, intel_crtc->pipe);mutex_lock(&dev->struct_mutex);intel_unpin_fb_obj(old_obj);mutex_unlock(&dev->struct_mutex);- }
- mutex_lock(&dev->struct_mutex);
- intel_update_fbc(dev);
- mutex_unlock(&dev->struct_mutex);
- return 0;
-}
static void intel_fdi_normal_train(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -9797,6 +9729,7 @@ static int intel_crtc_commit_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *old_fb = crtc->primary->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct drm_plane *primary = crtc->primary; enum pipe pipe = intel_crtc->pipe; struct intel_unpin_work *work; struct intel_engine_cs *ring;
@@ -9938,7 +9871,9 @@ free_work: if (ret == -EIO) { out_hang: intel_crtc_wait_for_pending_flips(crtc);
ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb);
ret = primary->funcs->update_plane(primary, crtc, fb,0, 0, 0, 0,crtc->x, crtc->y, 0, 0);
I think we want page flips to not change the current plane setup so these should be: intel_plane->crtc_x/y/w/h intel_plane->src_x/y/w/h
if (ret == 0 && event) { spin_lock_irq(&dev->event_lock); drm_send_vblank_event(dev, pipe, event);@@ -11475,11 +11410,13 @@ static int intel_crtc_set_config(struct drm_mode_set *set) set->x, set->y, set->fb); } else if (config->fb_changed) { struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
struct drm_plane *primary = set->crtc->primary;intel_crtc_wait_for_pending_flips(set->crtc);
ret = intel_pipe_set_base(set->crtc,set->x, set->y, set->fb);
ret = primary->funcs->update_plane(primary, set->crtc, set->fb,0, 0, 0, 0,set->x, set->y, 0, 0);
And these should really be: 0, 0, hdisplay, vdisplay, set->x << 16, set->y << 16, hdisplay << 16, vdisplay << 16,
Oh, one extra complication here is the stereo modes, so I think we'll be needing to borrow the stereo doubling trick from intel_modeset_pipe_config() to adjust hdisplay/vdisplay when appropriate.
And we'll be needing something also for the full modeset path so that intel_plane->crtc_* and intel_plane->src_* get initialized properly. Perhaps it's enough to rip out the fb pin/unpin stuff from __intel_set_mode() and just replace it with a call to ->update_plane().
/* * We need to make sure the primary plane is re-enabled if it-- 1.9.3
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
2014-09-19 Ville Syrjälä ville.syrjala@linux.intel.com:
On Thu, Sep 18, 2014 at 04:43:16PM -0300, Gustavo Padovan wrote:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
After some refactor intel_primary_plane_setplane() does the same as intel_pipe_set_base() so we can get rid of it and replace the calls with intel_primary_plane_setplane().
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/gpu/drm/i915/intel_display.c | 79 ++++-------------------------------- 1 file changed, 8 insertions(+), 71 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6c61c8f..2477587 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2763,74 +2763,6 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) crtc->config.pipe_src_h = adjusted_mode->crtc_vdisplay; }
-static int -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *fb)-{
- 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;
- struct drm_framebuffer *old_fb = crtc->primary->fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
- int ret;
- if (intel_crtc_has_pending_flip(crtc)) {
DRM_ERROR("pipe is still busy with an old pageflip\n");return -EBUSY;- }
- /* no fb bound */
- if (!fb) {
DRM_ERROR("No FB bound\n");return 0;- }
- if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) {
DRM_ERROR("no plane for crtc: plane %c, num_pipes %d\n",plane_name(intel_crtc->plane),INTEL_INFO(dev)->num_pipes);return -EINVAL;- }
- mutex_lock(&dev->struct_mutex);
- ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
- if (ret == 0)
i915_gem_track_fb(old_obj, obj,INTEL_FRONTBUFFER_PRIMARY(pipe));- mutex_unlock(&dev->struct_mutex);
- if (ret != 0) {
DRM_ERROR("pin & fence failed\n");return ret;- }
- intel_update_pipe_size(intel_crtc);
- dev_priv->display.update_primary_plane(crtc, fb, x, y);
- if (intel_crtc->active)
intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));- crtc->primary->fb = fb;
- crtc->x = x;
- crtc->y = y;
- if (old_fb) {
if (intel_crtc->active && old_fb != fb)intel_wait_for_vblank(dev, intel_crtc->pipe);mutex_lock(&dev->struct_mutex);intel_unpin_fb_obj(old_obj);mutex_unlock(&dev->struct_mutex);- }
- mutex_lock(&dev->struct_mutex);
- intel_update_fbc(dev);
- mutex_unlock(&dev->struct_mutex);
- return 0;
-}
static void intel_fdi_normal_train(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -9797,6 +9729,7 @@ static int intel_crtc_commit_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *old_fb = crtc->primary->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct drm_plane *primary = crtc->primary; enum pipe pipe = intel_crtc->pipe; struct intel_unpin_work *work; struct intel_engine_cs *ring;
@@ -9938,7 +9871,9 @@ free_work: if (ret == -EIO) { out_hang: intel_crtc_wait_for_pending_flips(crtc);
ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb);
ret = primary->funcs->update_plane(primary, crtc, fb,0, 0, 0, 0,crtc->x, crtc->y, 0, 0);I think we want page flips to not change the current plane setup so these should be: intel_plane->crtc_x/y/w/h intel_plane->src_x/y/w/h
Okay.
if (ret == 0 && event) { spin_lock_irq(&dev->event_lock); drm_send_vblank_event(dev, pipe, event);@@ -11475,11 +11410,13 @@ static int intel_crtc_set_config(struct drm_mode_set *set) set->x, set->y, set->fb); } else if (config->fb_changed) { struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
struct drm_plane *primary = set->crtc->primary;intel_crtc_wait_for_pending_flips(set->crtc);
ret = intel_pipe_set_base(set->crtc,set->x, set->y, set->fb);
ret = primary->funcs->update_plane(primary, set->crtc, set->fb,0, 0, 0, 0,set->x, set->y, 0, 0);And these should really be: 0, 0, hdisplay, vdisplay, set->x << 16, set->y << 16, hdisplay << 16, vdisplay << 16,
Oh, one extra complication here is the stereo modes, so I think we'll be needing to borrow the stereo doubling trick from intel_modeset_pipe_config() to adjust hdisplay/vdisplay when appropriate.
I see, so just calling intel_modeset_pipe_config() and getting pipe_config's pipe_src_w and pipe_src_h will fix the problem?
And we'll be needing something also for the full modeset path so that intel_plane->crtc_* and intel_plane->src_* get initialized properly. Perhaps it's enough to rip out the fb pin/unpin stuff from __intel_set_mode() and just replace it with a call to ->update_plane().
I see. Is it okay to update the primary planes from there?
Gustavo
On Fri, Sep 19, 2014 at 06:31:25PM -0300, Gustavo Padovan wrote:
2014-09-19 Ville Syrjälä ville.syrjala@linux.intel.com:
On Thu, Sep 18, 2014 at 04:43:16PM -0300, Gustavo Padovan wrote:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
After some refactor intel_primary_plane_setplane() does the same as intel_pipe_set_base() so we can get rid of it and replace the calls with intel_primary_plane_setplane().
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/gpu/drm/i915/intel_display.c | 79 ++++-------------------------------- 1 file changed, 8 insertions(+), 71 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6c61c8f..2477587 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2763,74 +2763,6 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) crtc->config.pipe_src_h = adjusted_mode->crtc_vdisplay; }
-static int -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *fb)-{
- 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;
- struct drm_framebuffer *old_fb = crtc->primary->fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
- int ret;
- if (intel_crtc_has_pending_flip(crtc)) {
DRM_ERROR("pipe is still busy with an old pageflip\n");return -EBUSY;- }
- /* no fb bound */
- if (!fb) {
DRM_ERROR("No FB bound\n");return 0;- }
- if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) {
DRM_ERROR("no plane for crtc: plane %c, num_pipes %d\n",plane_name(intel_crtc->plane),INTEL_INFO(dev)->num_pipes);return -EINVAL;- }
- mutex_lock(&dev->struct_mutex);
- ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
- if (ret == 0)
i915_gem_track_fb(old_obj, obj,INTEL_FRONTBUFFER_PRIMARY(pipe));- mutex_unlock(&dev->struct_mutex);
- if (ret != 0) {
DRM_ERROR("pin & fence failed\n");return ret;- }
- intel_update_pipe_size(intel_crtc);
- dev_priv->display.update_primary_plane(crtc, fb, x, y);
- if (intel_crtc->active)
intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));- crtc->primary->fb = fb;
- crtc->x = x;
- crtc->y = y;
- if (old_fb) {
if (intel_crtc->active && old_fb != fb)intel_wait_for_vblank(dev, intel_crtc->pipe);mutex_lock(&dev->struct_mutex);intel_unpin_fb_obj(old_obj);mutex_unlock(&dev->struct_mutex);- }
- mutex_lock(&dev->struct_mutex);
- intel_update_fbc(dev);
- mutex_unlock(&dev->struct_mutex);
- return 0;
-}
static void intel_fdi_normal_train(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -9797,6 +9729,7 @@ static int intel_crtc_commit_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *old_fb = crtc->primary->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct drm_plane *primary = crtc->primary; enum pipe pipe = intel_crtc->pipe; struct intel_unpin_work *work; struct intel_engine_cs *ring;
@@ -9938,7 +9871,9 @@ free_work: if (ret == -EIO) { out_hang: intel_crtc_wait_for_pending_flips(crtc);
ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb);
ret = primary->funcs->update_plane(primary, crtc, fb,0, 0, 0, 0,crtc->x, crtc->y, 0, 0);I think we want page flips to not change the current plane setup so these should be: intel_plane->crtc_x/y/w/h intel_plane->src_x/y/w/h
Okay.
if (ret == 0 && event) { spin_lock_irq(&dev->event_lock); drm_send_vblank_event(dev, pipe, event);@@ -11475,11 +11410,13 @@ static int intel_crtc_set_config(struct drm_mode_set *set) set->x, set->y, set->fb); } else if (config->fb_changed) { struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
struct drm_plane *primary = set->crtc->primary;intel_crtc_wait_for_pending_flips(set->crtc);
ret = intel_pipe_set_base(set->crtc,set->x, set->y, set->fb);
ret = primary->funcs->update_plane(primary, set->crtc, set->fb,0, 0, 0, 0,set->x, set->y, 0, 0);And these should really be: 0, 0, hdisplay, vdisplay, set->x << 16, set->y << 16, hdisplay << 16, vdisplay << 16,
Oh, one extra complication here is the stereo modes, so I think we'll be needing to borrow the stereo doubling trick from intel_modeset_pipe_config() to adjust hdisplay/vdisplay when appropriate.
I see, so just calling intel_modeset_pipe_config() and getting pipe_config's pipe_src_w and pipe_src_h will fix the problem?
No, we shouldn't call intel_modeset_pipe_config() here. You just need to extract the code to compute pipe_src_w/h when dealing with a doubled stereo mode. We also use the same trick in drm_crtc_check_viewport(). That's three instances of the same thing basically so maybe we should put that bit of code into a small helper function and just call that from all three places.
And we'll be needing something also for the full modeset path so that intel_plane->crtc_* and intel_plane->src_* get initialized properly. Perhaps it's enough to rip out the fb pin/unpin stuff from __intel_set_mode() and just replace it with a call to ->update_plane().
I see. Is it okay to update the primary planes from there?
At that point the pipe should be disabled, so in theory the .update_plane() should just end up doing pin+unpin+update the saved plane coords. But I might be missing something subtle here.
On Thu, Sep 18, 2014 at 04:43:12PM -0300, Gustavo Padovan wrote:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Fold intel_pipe_set_base() in the update primary plane path merging pieces of code that are common to both paths.
Basically the the pin/unpin procedures are the same for both paths and some checks can also be shared (some of the were moved to the check() stage)
v2: take Ville's comments:
- remove unnecessary plane check
- move mutex lock to inside the conditional
- make the pin fail message a debug one
- add a fixme for the fastboot hack
- call intel_frontbuffer_flip() after FBC update
v3: take more Ville's comments:
- fold update code under if (intel_crtc->active), and do the
visible/!visible split inside.
- check ret inside the same conditional we assign it
v4: don't use intel_enable_primary_hw_plane(), the primary_enabled check inside will break page flips
Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/gpu/drm/i915/intel_display.c | 141 +++++++++++++++++++++-------------- 1 file changed, 84 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5b05ddb..1fd9b70 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11792,12 +11792,23 @@ intel_check_primary_plane(struct drm_plane *plane, struct drm_rect *dest = &state->dst; struct drm_rect *src = &state->src; const struct drm_rect *clip = &state->clip;
- int ret;
- return drm_plane_helper_check_update(plane, crtc, fb,
- ret = drm_plane_helper_check_update(plane, crtc, fb, src, dest, clip, DRM_PLANE_HELPER_NO_SCALING, DRM_PLANE_HELPER_NO_SCALING, false, true, &state->visible);
- if (ret)
return ret;- /* no fb bound */
- if (state->visible && !fb) {
DRM_ERROR("No FB bound\n");return -EINVAL;- }
- return 0;
}
static int @@ -11809,6 +11820,8 @@ intel_commit_primary_plane(struct drm_plane *plane, 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;
- struct drm_framebuffer *old_fb = plane->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -11817,67 +11830,28 @@ intel_commit_primary_plane(struct drm_plane *plane,
intel_crtc_wait_for_pending_flips(crtc);
- /*
* If clipping results in a non-visible primary plane, we'll disable* the primary plane. Note that this is a bit different than what* happens if userspace explicitly disables the plane by passing fb=0* because plane->fb still gets set and pinned.*/- if (!state->visible) {
mutex_lock(&dev->struct_mutex);/** Try to pin the new fb first so that we can bail out if we* fail.*/if (plane->fb != fb) {ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);if (ret) {mutex_unlock(&dev->struct_mutex);return ret;}}i915_gem_track_fb(old_obj, obj,INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));if (intel_crtc->primary_enabled)intel_disable_primary_hw_plane(plane, crtc);if (plane->fb != fb)if (plane->fb)intel_unpin_fb_obj(old_obj);
if (intel_crtc_has_pending_flip(crtc)) {
DRM_ERROR("pipe is still busy with an old pageflip\n");return -EBUSY;}
if (plane->fb != fb) {
mutex_lock(&dev->struct_mutex);ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);if (ret == 0)i915_gem_track_fb(old_obj, obj,INTEL_FRONTBUFFER_PRIMARY(pipe));mutex_unlock(&dev->struct_mutex);
- } else {
if (intel_crtc && intel_crtc->active &&intel_crtc->primary_enabled) {/** FBC does not work on some platforms for rotated* planes, so disable it when rotation is not 0 and* update it when rotation is set back to 0.** FIXME: This is redundant with the fbc update done in* the primary plane enable function except that that* one is done too late. We eventually need to unify* this.*/if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&dev_priv->fbc.plane == intel_crtc->plane &&intel_plane->rotation != BIT(DRM_ROTATE_0)) {intel_disable_fbc(dev);}}ret = intel_pipe_set_base(crtc, src->x1, src->y1, fb);if (ret)
if (ret != 0) {DRM_DEBUG_KMS("pin & fence failed\n"); return ret;
if (!intel_crtc->primary_enabled)intel_enable_primary_hw_plane(plane, crtc);
}}
crtc->primary->fb = fb;
crtc->x = src->x1;
crtc->y = src->y1;
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);
@@ -11888,6 +11862,59 @@ intel_commit_primary_plane(struct drm_plane *plane, intel_plane->src_h = drm_rect_height(&state->orig_src); intel_plane->obj = obj;
- if (intel_crtc->active) {
/** FBC does not work on some platforms for rotated* planes, so disable it when rotation is not 0 and* update it when rotation is set back to 0.** FIXME: This is redundant with the fbc update done in* the primary plane enable function except that that* one is done too late. We eventually need to unify* this.*/if (intel_crtc->primary_enabled &&INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&dev_priv->fbc.plane == intel_crtc->plane &&intel_plane->rotation != BIT(DRM_ROTATE_0)) {intel_disable_fbc(dev);}if (state->visible) {/* FIXME: kill this fastboot hack */intel_update_pipe_size(intel_crtc);
Need to set primary_enabled=true here.
dev_priv->display.update_primary_plane(crtc, plane->fb,crtc->x, crtc->y);
And you need to duplicate the BDW wait_for_vblank hack from intel_enable_primary_hw_plane() here. But that wait should only be done if the plane was previosuly disabled.
} else {/** If clipping results in a non-visible primary plane,* we'll disable the primary plane. Note that this is* a bit different than what happens if userspace* explicitly disables the plane by passing fb=0* because plane->fb still gets set and pinned.*/intel_disable_primary_hw_plane(plane, crtc);}intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));mutex_lock(&dev->struct_mutex);intel_update_fbc(dev);mutex_unlock(&dev->struct_mutex);- }
- if (old_fb) {
Still could unify the old_fb!=fb checks with this one.
if (old_fb && old_fb != fb) {
if (intel_crtc->active && old_fb != fb)intel_wait_for_vblank(dev, intel_crtc->pipe);if (old_fb != fb) {mutex_lock(&dev->struct_mutex);intel_unpin_fb_obj(old_obj);mutex_unlock(&dev->struct_mutex);}- }
- return 0;
}
-- 1.9.3
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Gustavo Padovan -
Ville Syrjälä