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

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

Hi Emre,

On Monday, 30 April 2018 11:40:51 EEST Ucan, Emre (ADITG/ESB) wrote:
> Hi Laurent,
> 
> Sorry for late reply.

No worries.

> On Montag, 23. April 2018 23:47 Laurent Pinchart wrote:
> > 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 <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.
> 
> 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--) {
> 
> 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 ?

The process is pretty simple, it's documented in Documentation/process/stable-
kernel-rules.rst. The file states the following requirement for a patch to be 
eligible for stable backporting:

 - It must fix a problem that causes a build error (but not for things
   marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
   security issue, or some "oh, that's not good" issue.  In short, something
   critical

I'll let you decide whether this patch qualifies, as it could be argued that 
it implements a new feature.

-- 
Regards,

Laurent Pinchart



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