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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Emre,

Sorry for the late reply.

On Wednesday, 4 April 2018 14:03:57 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.

I've always found the 72 columns limit in commit messages to be quite 
limiting, especially for the subject line. Out of curiosity, is there any 
specific reason why you limit it further ?

> v2 changes:
> - Remove drm_atomic_helper_wait_for_fences()
>   call in rcar_du_kms.c. The commit_tail()
>   function in drm_atomic_helper.c, which calls
>   our atomic_commit_tail() implementation,
>   already calls it.
> - Remove proposed rcar_du_vsp_set_fence_for_plane()
>   function. Call drm_gem_fb_prepare_fb(), which
>   calls drm_atomic_set_fence_for_plane().
> 
> Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 2c260c3..fbad616 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -18,6 +18,7 @@
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_plane_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>

Nitpicking, could you please keep the headers alphabetically sorted ? It helps 
locating duplicates quickly.

>  #include <linux/bitops.h>
>  #include <linux/dma-mapping.h>
> @@ -237,7 +238,7 @@ static int rcar_du_vsp_plane_prepare_fb(struct drm_plane
> *plane, }
>  	}
> 
> -	return 0;
> +	return drm_gem_fb_prepare_fb(plane, state);

If drm_gem_fb_prepare_fb() fails we need to clean up the operations performed 
earlier in this function. I think the following would be enough.

        ret = drm_gem_fb_prepare_fb(plane, state);
        if (ret)
                goto fail;

        return 0;

Could you please test this ?

>  fail:
>  	while (i--) {

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel