lkml.org 
[lkml]   [2019]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v3 1/5] drm/rockchip: fix fb references in async update
    Date
    In the case of async update, modifications are done in place, i.e. in the
    current plane state, so the new_state is prepared and the new_state is
    cleaned up (instead of the old_state, unlike what happens in a
    normal sync update).
    To cleanup the old_fb properly, it needs to be placed in the new_state
    in the end of async_update, so cleanup call will unreference the old_fb
    correctly.

    Also, the previous code had a:

    plane_state = plane->funcs->atomic_duplicate_state(plane);
    ...
    swap(plane_state, plane->state);

    if (plane->state->fb && plane->state->fb != new_state->fb) {
    ...
    }

    Which was wrong, as the fb were just assigned to be equal, so this if
    statement nevers evaluates to true.

    Another details is that the function drm_crtc_vblank_get() can only be
    called when vop->is_enabled is true, otherwise it has no effect and
    trows a WARN_ON().

    Calling drm_atomic_set_fb_for_plane() (which get a referent of the new
    fb and pus the old fb) is not required, as it is taken care by
    drm_mode_cursor_universal() when calling
    drm_atomic_helper_update_plane().

    Signed-off-by: Helen Koike <helen.koike@collabora.com>

    ---
    Hello,

    I tested on the rockchip ficus v1.1 using igt plane_cursor_legacy and
    kms_cursor_legacy and I didn't see any regressions.

    Changes in v3:
    - use swap() to swap old and new framebuffers in async_update
    - get the reference to old_fb and set the worker after vop_plane_atomic_update()
    - add a FIXME tag for when we have multiple fbs to be released when
    vblank happens.
    - update commit message

    Changes in v2: None

    drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 51 +++++++++++----------
    1 file changed, 26 insertions(+), 25 deletions(-)

    diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
    index c7d4c6073ea5..08f5dcd738a7 100644
    --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
    +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
    @@ -912,29 +912,17 @@ static void vop_plane_atomic_async_update(struct drm_plane *plane,
    struct drm_plane_state *new_state)
    {
    struct vop *vop = to_vop(plane->state->crtc);
    - struct drm_plane_state *plane_state;
    -
    - plane_state = plane->funcs->atomic_duplicate_state(plane);
    - plane_state->crtc_x = new_state->crtc_x;
    - plane_state->crtc_y = new_state->crtc_y;
    - plane_state->crtc_h = new_state->crtc_h;
    - plane_state->crtc_w = new_state->crtc_w;
    - plane_state->src_x = new_state->src_x;
    - plane_state->src_y = new_state->src_y;
    - plane_state->src_h = new_state->src_h;
    - plane_state->src_w = new_state->src_w;
    -
    - if (plane_state->fb != new_state->fb)
    - drm_atomic_set_fb_for_plane(plane_state, new_state->fb);
    -
    - swap(plane_state, plane->state);
    -
    - if (plane->state->fb && plane->state->fb != new_state->fb) {
    - drm_framebuffer_get(plane->state->fb);
    - WARN_ON(drm_crtc_vblank_get(plane->state->crtc) != 0);
    - drm_flip_work_queue(&vop->fb_unref_work, plane->state->fb);
    - set_bit(VOP_PENDING_FB_UNREF, &vop->pending);
    - }
    + struct drm_framebuffer *old_fb = plane->state->fb;
    +
    + plane->state->crtc_x = new_state->crtc_x;
    + plane->state->crtc_y = new_state->crtc_y;
    + plane->state->crtc_h = new_state->crtc_h;
    + plane->state->crtc_w = new_state->crtc_w;
    + plane->state->src_x = new_state->src_x;
    + plane->state->src_y = new_state->src_y;
    + plane->state->src_h = new_state->src_h;
    + plane->state->src_w = new_state->src_w;
    + swap(plane->state->fb, new_state->fb);

    if (vop->is_enabled) {
    rockchip_drm_psr_inhibit_get_state(new_state->state);
    @@ -943,9 +931,22 @@ static void vop_plane_atomic_async_update(struct drm_plane *plane,
    vop_cfg_done(vop);
    spin_unlock(&vop->reg_lock);
    rockchip_drm_psr_inhibit_put_state(new_state->state);
    - }

    - plane->funcs->atomic_destroy_state(plane, plane_state);
    + /*
    + * A scanout can still be occurring, so we can't drop the
    + * reference to the old framebuffer. To solve this we get a
    + * reference to old_fb and set a worker to release it later.
    + * FIXME: if we perform 500 async_update calls before the
    + * vblank, then we can have 500 different framebuffers waiting
    + * to be released.
    + */
    + if (old_fb && plane->state->fb != old_fb) {
    + drm_framebuffer_get(old_fb);
    + WARN_ON(drm_crtc_vblank_get(plane->state->crtc) != 0);
    + drm_flip_work_queue(&vop->fb_unref_work, old_fb);
    + set_bit(VOP_PENDING_FB_UNREF, &vop->pending);
    + }
    + }
    }

    static const struct drm_plane_helper_funcs plane_helper_funcs = {
    --
    2.20.1
    \
     
     \ /
      Last update: 2019-03-14 01:21    [W:4.365 / U:0.676 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site