[PATCH] drm: rcar-du: track dma-buf fences

[Daniel Vetter]

On Tue, Apr 3, 2018 at 8:53 PM, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
> Hello Emre,
>
> Thank you for the patch.
>
> On Tuesday, 3 April 2018 12:14:33 EEST Emre Ucan wrote:
>> We have to check dma-buf reservation objects
>> of our framebuffers before we use them.
>> Otherwise, another driver might be writing
>> on the same buffer which we are using.
>> This would cause visible tearing effects
>> on display.
>>
>> We can use existing atomic helper functions
>> to solve this problem.
>>
>> Signed-off-by: Emre Ucan <eucan at de.adit-jv.com>
>> ---
>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c |  2 ++
>>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 20 ++++++++++++++++++++
>>  2 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 0329b35..f3da3d1 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> @@ -255,6 +255,8 @@ static void rcar_du_atomic_commit_tail(struct
>> drm_atomic_state *old_state) {
>>       struct drm_device *dev = old_state->dev;
>>
>> +     drm_atomic_helper_wait_for_fences(dev, old_state, false);
>> +
>
> The commit_tail() function in drm_atomic_helper.c, which calls our
> atomic_commit_tail() implementation, already calls
> drm_atomic_helper_wait_for_fences(). Why is there a need to duplicate the call
> here ?
>
>>       /* Apply the atomic update. */
>>       drm_atomic_helper_commit_modeset_disables(dev, old_state);
>>       drm_atomic_helper_commit_planes(dev, old_state,
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 2c260c3..482e23c 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>> @@ -18,12 +18,16 @@
>>  #include <drm/drm_fb_cma_helper.h>
>>  #include <drm/drm_gem_cma_helper.h>
>>  #include <drm/drm_plane_helper.h>
>> +#include <drm/drm_atomic.h>
>>
>>  #include <linux/bitops.h>
>>  #include <linux/dma-mapping.h>
>> +#include <linux/dma-fence.h>
>> +#include <linux/dma-buf.h>
>>  #include <linux/of_platform.h>
>>  #include <linux/scatterlist.h>
>>  #include <linux/videodev2.h>
>> +#include <linux/reservation.h>
>>
>>  #include <media/vsp1.h>
>>
>> @@ -203,6 +207,20 @@ static void rcar_du_vsp_plane_setup(struct
>> rcar_du_vsp_plane *plane) plane->index, &cfg);
>>  }
>>
>> +static void rcar_du_vsp_set_fence_for_plane(struct drm_plane_state *state)
>> +{
>> +     struct drm_gem_cma_object *gem;
>> +     struct dma_buf *dma_buf;
>> +     struct dma_fence *fence;
>> +
>> +     gem = drm_fb_cma_get_gem_obj(state->fb, 0);
>> +     dma_buf = gem->base.dma_buf;
>> +     if (dma_buf) {
>> +             fence = reservation_object_get_excl_rcu(dma_buf->resv);
>> +             drm_atomic_set_fence_for_plane(state, fence);
>
> Unless I'm mistaken this is used for implicit fencing only. What is your use
> case, wouldn't it be better for userspace to use explicit fencing as that is
> the fence model that has been selected for display ?

Implicit fencing is very much still a thing on most X and wayland
setups. There's a push to eventually make everything explicit, but
it'll take a while I think.
-Daniel

>
>> +     }
>> +}
>
> This looks very similar to drm_gem_fb_prepare_fb(), couldn't you use that
> function instead ?
>
>>  static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
>>                                       struct drm_plane_state *state)
>>  {
>> @@ -237,6 +255,8 @@ static int rcar_du_vsp_plane_prepare_fb(struct drm_plane
>> *plane, }
>>       }
>>
>> +     rcar_du_vsp_set_fence_for_plane(state);
>> +
>>       return 0;
>>
>>  fail:
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch