[Intel-gfx] [PATCH 7/7] drm/i915: Consolidate plane 'cleanup' operations (v3)
Ander Conselvan de Oliveira
conselvan2 at gmail.com
Wed Dec 3 01:52:24 PST 2014
Reviewed-by: Ander Conselvan de Oliveira <conselvan2 at gmail.com>
On 12/02/2014 05:45 PM, Matt Roper wrote:
> All plane update functions need to unpin the old framebuffer when
> flipping to a new one. Pull this logic into a separate function to ease
> the integration with atomic plane helpers.
>
> v2: Don't wait for vblank if we don't have an old fb to cleanup (Ander)
>
> v3: Really don't wait for vblank if we don't have an old fb to cleanup.
> Previous version only handled this for primary planes; we need the
> same change on cursors/sprites too! (Ander)
>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 58 ++++++++++++++++++++++++------------
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_sprite.c | 26 +++++-----------
> 3 files changed, 49 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f48faaa..86c4686 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11701,6 +11701,31 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> return ret;
> }
>
> +/**
> + * intel_cleanup_plane_fb - Cleans up an fb after plane use
> + * @plane: drm plane to clean up for
> + * @fb: old framebuffer that was on plane
> + *
> + * Cleans up a framebuffer that has just been removed from a plane.
> + */
> +void
> +intel_cleanup_plane_fb(struct drm_plane *plane,
> + struct drm_framebuffer *fb)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> +
> + if (WARN_ON(!obj))
> + return;
> +
> + if (plane->type != DRM_PLANE_TYPE_CURSOR ||
> + !INTEL_INFO(dev)->cursor_needs_physical) {
> + mutex_lock(&dev->struct_mutex);
> + intel_unpin_fb_obj(obj);
> + mutex_unlock(&dev->struct_mutex);
> + }
> +}
> +
> static int
> intel_check_primary_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> @@ -11728,9 +11753,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
> 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);
> struct drm_rect *src = &state->src;
>
> @@ -11801,15 +11824,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
> intel_update_fbc(dev);
> mutex_unlock(&dev->struct_mutex);
> }
> -
> - if (old_fb && old_fb != fb) {
> - if (intel_crtc->active)
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> -
> - mutex_lock(&dev->struct_mutex);
> - intel_unpin_fb_obj(old_obj);
> - mutex_unlock(&dev->struct_mutex);
> - }
> }
>
> static int
> @@ -11819,6 +11833,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h)
> {
> + struct drm_device *dev = plane->dev;
> struct drm_framebuffer *old_fb = plane->fb;
> struct intel_plane_state state;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -11866,6 +11881,12 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>
> intel_commit_primary_plane(plane, &state);
>
> + if (fb != old_fb && old_fb) {
> + if (intel_crtc->active)
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + intel_cleanup_plane_fb(plane, old_fb);
> + }
> +
> return 0;
> }
>
> @@ -12048,14 +12069,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
> else
> addr = obj->phys_handle->busaddr;
>
> - if (intel_crtc->cursor_bo) {
> - if (!INTEL_INFO(dev)->cursor_needs_physical) {
> - mutex_lock(&dev->struct_mutex);
> - intel_unpin_fb_obj(intel_crtc->cursor_bo);
> - mutex_unlock(&dev->struct_mutex);
> - }
> - }
> -
> intel_crtc->cursor_addr = addr;
> intel_crtc->cursor_bo = obj;
> update:
> @@ -12080,6 +12093,7 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h)
> {
> + struct drm_device *dev = plane->dev;
> struct drm_framebuffer *old_fb = plane->fb;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_plane_state state;
> @@ -12120,6 +12134,12 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
>
> intel_commit_cursor_plane(plane, &state);
>
> + if (fb != old_fb && old_fb) {
> + if (intel_crtc->active)
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + intel_cleanup_plane_fb(plane, old_fb);
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a36f696..38cb553 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -924,6 +924,8 @@ void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
> void intel_check_page_flip(struct drm_device *dev, int pipe);
> int intel_prepare_plane_fb(struct drm_plane *plane,
> struct drm_framebuffer *fb);
> +void intel_cleanup_plane_fb(struct drm_plane *plane,
> + struct drm_framebuffer *fb);
>
> /* shared dpll functions */
> struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 5d8c2e0..031f95e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1268,7 +1268,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> enum pipe pipe = intel_crtc->pipe;
> struct drm_framebuffer *fb = state->base.fb;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> - 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;
> @@ -1326,23 +1325,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> if (!primary_was_enabled && primary_enabled)
> intel_post_enable_primary(crtc);
> }
> -
> - /* Unpin old obj after new one is active to avoid ugliness */
> - if (old_obj && old_obj != obj) {
> -
> - /*
> - * It's fairly common to simply update the position of
> - * an existing object. In that case, we don't need to
> - * wait for vblank to avoid ugliness, we only need to
> - * do the pin & ref bookkeeping.
> - */
> - if (intel_crtc->active)
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> -
> - mutex_lock(&dev->struct_mutex);
> - intel_unpin_fb_obj(old_obj);
> - mutex_unlock(&dev->struct_mutex);
> - }
> }
>
> static int
> @@ -1352,6 +1334,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h)
> {
> + struct drm_device *dev = plane->dev;
> struct drm_framebuffer *old_fb = plane->fb;
> struct intel_plane_state state;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -1390,6 +1373,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> }
>
> intel_commit_sprite_plane(plane, &state);
> +
> + if (fb != old_fb && old_fb) {
> + if (intel_crtc->active)
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + intel_cleanup_plane_fb(plane, old_fb);
> + }
> +
> return 0;
> }
>
>
More information about the Intel-gfx
mailing list