Re: [RFC 7/7] drm/vc4: update cursors asynchronously through atomic

[Robert Foss <robert.foss@collabora.com>]

Sorry about the spam, but the previous reply was intended for
[RFC v3 7/8] drm/vc4: update cursors asynchronously through atomic


On 2017-05-18 06:52 PM, Robert Foss wrote:
> Hey,
> 
> I ran the series on a RPi2 and the cursor seems to behave correctly.
> 
> Tested-by: Robert Foss <robert.foss@collabora.com>
> 
> On 2017-04-09 08:24 PM, Gustavo Padovan wrote:
>> From: Gustavo Padovan <gustavo.padovan@collabora.com>
>>
>> Add support to async updates of cursors by using the new atomic
>> interface for that. Basically what this commit does is do what
>> vc4_update_plane() did but through atomic.
>>
>> Cc: Eric Anholt <eric@anholt.net>
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
>> ---
>>   drivers/gpu/drm/vc4/vc4_plane.c | 94 ++++++++++++-----------------------------
>>   1 file changed, 27 insertions(+), 67 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
>> index d34cd53..e33c75b 100644
>> --- a/drivers/gpu/drm/vc4/vc4_plane.c
>> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
>> @@ -735,70 +735,27 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, 
>> struct drm_framebuffer *fb)
>>       vc4_state->dlist[vc4_state->ptr0_offset] = addr;
>>   }
>> -static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
>> -    .atomic_check = vc4_plane_atomic_check,
>> -    .atomic_update = vc4_plane_atomic_update,
>> -};
>> -
>> -static void vc4_plane_destroy(struct drm_plane *plane)
>> -{
>> -    drm_plane_helper_disable(plane);
>> -    drm_plane_cleanup(plane);
>> -}
>> -
>> -/* Implements immediate (non-vblank-synced) updates of the cursor
>> - * position, or falls back to the atomic helper otherwise.
>> - */
>> -static int
>> -vc4_update_plane(struct drm_plane *plane,
>> -         struct drm_crtc *crtc,
>> -         struct drm_framebuffer *fb,
>> -         int crtc_x, int crtc_y,
>> -         unsigned int crtc_w, unsigned int crtc_h,
>> -         uint32_t src_x, uint32_t src_y,
>> -         uint32_t src_w, uint32_t src_h,
>> -         struct drm_modeset_acquire_ctx *ctx)
>> +static int vc4_plane_atomic_async_check(struct drm_plane *plane,
>> +                    struct drm_plane_state *state)
>>   {
>> -    struct drm_plane_state *plane_state;
>> -    struct vc4_plane_state *vc4_state;
>> -
>> -    if (plane != crtc->cursor)
>> -        goto out;
>> -
>> -    plane_state = plane->state;
>> -    vc4_state = to_vc4_plane_state(plane_state);
>> -
>> -    if (!plane_state)
>> -        goto out;
>> +    if (plane != state->crtc->cursor)
>> +        return -EINVAL;
>> -    /* No configuring new scaling in the fast path. */
>> -    if (crtc_w != plane_state->crtc_w ||
>> -        crtc_h != plane_state->crtc_h ||
>> -        src_w != plane_state->src_w ||
>> -        src_h != plane_state->src_h) {
>> -        goto out;
>> -    }
>> +    if (!plane->state)
>> +        return -EINVAL;
>> -    if (fb != plane_state->fb) {
>> -        drm_atomic_set_fb_for_plane(plane->state, fb);
>> -        vc4_plane_async_set_fb(plane, fb);
>> -    }
>> +    return 0;
>> +}
>> -    /* Set the cursor's position on the screen.  This is the
>> -     * expected change from the drm_mode_cursor_universal()
>> -     * helper.
>> -     */
>> -    plane_state->crtc_x = crtc_x;
>> -    plane_state->crtc_y = crtc_y;
>> +static void vc4_plane_atomic_async_update(struct drm_plane *plane,
>> +                      struct drm_plane_state *new_state)
>> +{
>> +    struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state);
>> -    /* Allow changing the start position within the cursor BO, if
>> -     * that matters.
>> -     */
>> -    plane_state->src_x = src_x;
>> -    plane_state->src_y = src_y;
>> +    if (plane->state->fb != new_state->fb)
>> +        vc4_plane_async_set_fb(plane, new_state->fb);
>> -    /* Update the display list based on the new crtc_x/y. */
>> -    vc4_plane_atomic_check(plane, plane_state);
>> +    plane->state->fb = new_state->fb;
>>       /* Note that we can't just call vc4_plane_write_dlist()
>>        * because that would smash the context data that the HVS is
>> @@ -810,20 +767,23 @@ vc4_update_plane(struct drm_plane *plane,
>>              &vc4_state->hw_dlist[vc4_state->pos2_offset]);
>>       writel(vc4_state->dlist[vc4_state->ptr0_offset],
>>              &vc4_state->hw_dlist[vc4_state->ptr0_offset]);
>> +}
>> -    return 0;
>> +static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
>> +    .atomic_check = vc4_plane_atomic_check,
>> +    .atomic_update = vc4_plane_atomic_update,
>> +    .atomic_async_check = vc4_plane_atomic_async_check,
>> +    .atomic_async_update = vc4_plane_atomic_async_update,
>> +};
>> -out:
>> -    return drm_atomic_helper_update_plane(plane, crtc, fb,
>> -                          crtc_x, crtc_y,
>> -                          crtc_w, crtc_h,
>> -                          src_x, src_y,
>> -                          src_w, src_h,
>> -                          ctx);
>> +static void vc4_plane_destroy(struct drm_plane *plane)
>> +{
>> +    drm_plane_helper_disable(plane);
>> +    drm_plane_cleanup(plane);
>>   }
>>   static const struct drm_plane_funcs vc4_plane_funcs = {
>> -    .update_plane = vc4_update_plane,
>> +    .update_plane = drm_atomic_helper_update_plane,
>>       .disable_plane = drm_atomic_helper_disable_plane,
>>       .destroy = vc4_plane_destroy,
>>       .set_property = NULL,
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel