Hi Laurent,

Sorry for late reply. 

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
> Sent: Montag, 23. April 2018 23:47
> To: Ucan, Emre (ADITG/ESB)
> Cc: dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH v2] drm: rcar-du: track dma-buf fences
> 
> 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 ?

I tried to keep lines short. But I did not check if they are shorter than 72.
I will improve it.

> 
> > 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 <eu...@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.

No problem. I will fix it.

> 
> >  #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 ?

No problem.

> 
> >  fail:
> >     while (i--) {
> 
> --
> Regards,
> 
> Laurent Pinchart
> 
> 

I will send a new patch with these changes after I tested it. Is it possible 
that improved patch lands on 4.17 and 4.14 stable versions?
What is the process to propose a patch to a stable release ? 

Best Regards,

Emre Ucan

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

Reply via email to