Please send drm/i915 patches also to intel-...@lists.freedesktop.org to
trigger CI on the patches among other things.

BR,
Jani.


On Thu, 07 Mar 2019, Helen Koike <helen.ko...@collabora.com> wrote:
> From: Gustavo Padovan <gustavo.pado...@collabora.com>
>
> Add implementation for async plane update callbacks
>
> Signed-off-by: Gustavo Padovan <gustavo.pado...@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balle...@collabora.com>
> Signed-off-by: Tina Zhang <tina.zh...@intel.com>
> Signed-off-by: Helen Koike <helen.ko...@collabora.com>
>
> ---
> Hi,
>
> This patch just adds the callbacks for atomic async update, the next in
> the series will replace the legacy cursor implementation with these
> callbacks.
>
> I tested with igt (ksm_cursor_legacy and plane_cursor_legacy).
>
> This patch depends on "[PATCH] drm: don't block fb changes for async plane 
> updates",
> otherwise there will be a regression on igt tests:
>
>         cursor-vs-flip-atomic-transitions-varying-size
>         cursor-vs-flip-toggle
>         cursor-vs-flip-varying-size
>
> with errors of type:
>
> "CRITICAL: completed 97 cursor updated in a period of 30 flips, we
> expect to complete approximately 15360 updates, with the threshold set
> at 7680"
>
> This happens because what should be async updates are being turned into
> syncronous updates.
>
> Thanks
> Helen
>
> Changes in v8:
>  - v7: https://lkml.org/lkml/2018/6/8/168
>  - v7 was splited in two, one that adds the async callbacks and another
>  that updates the cursor.
>  - rebase with drm-intel
>  - allow async update in all types of planes, not only cursor
>  - add watermark checks in async update
>  - remove bypass of intel_prepare_plane_fb() in case of async update
>  - add missing drm_atomic_helper_cleanup_planes(dev, state) call in
>  intel_atomic_commit().
>  - use swap() function in async update to set the old_fb in the
>  new_state object.
>  - use helpers intel_update_plane()/intel_disable_plane()
>
> Changes in v7:
> - Rebase on top of drm-intel repository. Hopefully now will play
>   nicely with autobuilders.
>
> Changes in v6:
> - Rework the intel_plane_atomic_async_update due driver changed from
>   last time.
> - Removed the mutex_lock/unlock as causes a deadlock.
>
> Changes in v5:
> - Call drm_atomic_helper_async_check() from the check hook
>
> Changes in v4:
> - Set correct vma to new state for cleanup
> - Move size checks back to drivers (Ville Syrjälä)
>
> Changes in v3:
> - Move fb setting to core and use new state (Eric Anholt)
>
>  drivers/gpu/drm/i915/intel_atomic_plane.c | 71 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c      |  9 +++
>  2 files changed, 80 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
> b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 7961cf0e6951..f516b227dba9 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -318,10 +318,81 @@ void i9xx_update_planes_on_crtc(struct 
> intel_atomic_state *state,
>       }
>  }
>  
> +static int intel_plane_atomic_async_check(struct drm_plane *plane,
> +                                       struct drm_plane_state *state)
> +{
> +     struct drm_crtc_state *crtc_state;
> +
> +     crtc_state = drm_atomic_get_existing_crtc_state(state->state,
> +                                                     state->crtc);
> +     if (WARN_ON(!crtc_state))
> +             return -EINVAL;
> +
> +     /*
> +      * When crtc is inactive or there is a modeset pending,
> +      * wait for it to complete in the slowpath
> +      */
> +     if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
> +             return -EINVAL;
> +
> +     /*
> +      * If any parameters change that may affect watermarks,
> +      * take the slowpath. Only changing fb or position should be
> +      * in the fastpath.
> +      */
> +     if (plane->state->crtc != state->crtc ||
> +         plane->state->src_w != state->src_w ||
> +         plane->state->src_h != state->src_h ||
> +         plane->state->crtc_w != state->crtc_w ||
> +         plane->state->crtc_h != state->crtc_h ||
> +         !plane->state->fb != !state->fb)
> +             return -EINVAL;
> +
> +     return 0;
> +}
> +
> +static void intel_plane_atomic_async_update(struct drm_plane *plane,
> +                                         struct drm_plane_state *new_state)
> +{
> +     struct intel_atomic_state *intel_new_state =
> +             to_intel_atomic_state(new_state->state);
> +     struct intel_plane *intel_plane = to_intel_plane(plane);
> +     struct drm_crtc *crtc = plane->state->crtc;
> +     struct intel_crtc_state *new_crtc_state;
> +     struct intel_crtc *intel_crtc;
> +     int i;
> +
> +     for_each_new_intel_crtc_in_state(intel_new_state, intel_crtc,
> +                                      new_crtc_state, i)
> +             WARN_ON(new_crtc_state->wm.need_postvbl_update ||
> +                     new_crtc_state->update_wm_post);
> +
> +     i915_gem_track_fb(intel_fb_obj(plane->state->fb),
> +                       intel_fb_obj(new_state->fb),
> +                       intel_plane->frontbuffer_bit);
> +
> +     plane->state->src_x = new_state->src_x;
> +     plane->state->src_y = new_state->src_y;
> +     plane->state->crtc_x = new_state->crtc_x;
> +     plane->state->crtc_y = new_state->crtc_y;
> +
> +     swap(plane->state->fb, new_state->fb);
> +
> +     if (plane->state->visible)
> +             intel_update_plane(intel_plane,
> +                                to_intel_crtc_state(crtc->state),
> +                                to_intel_plane_state(plane->state));
> +     else
> +             intel_disable_plane(intel_plane,
> +                                 to_intel_crtc_state(crtc->state));
> +}
> +
>  const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
>       .prepare_fb = intel_prepare_plane_fb,
>       .cleanup_fb = intel_cleanup_plane_fb,
>       .atomic_check = intel_plane_atomic_check,
> +     .atomic_async_check = intel_plane_atomic_async_check,
> +     .atomic_async_update = intel_plane_atomic_async_update,
>  };
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 415d8968f2c5..244e1c94277d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13464,6 +13464,15 @@ static int intel_atomic_commit(struct drm_device 
> *dev,
>       struct drm_i915_private *dev_priv = to_i915(dev);
>       int ret = 0;
>  
> +     if (state->async_update) {
> +             ret = drm_atomic_helper_prepare_planes(dev, state);
> +             if (ret)
> +                     return ret;
> +             drm_atomic_helper_async_commit(dev, state);
> +             drm_atomic_helper_cleanup_planes(dev, state);
> +             return 0;
> +     }
> +
>       drm_atomic_state_get(state);
>       i915_sw_fence_init(&intel_state->commit_ready,
>                          intel_atomic_commit_ready);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to