* [PATCH v2] drm: rcar-du: track dma-buf fences @ 2018-04-04 11:03 Emre Ucan 2018-04-23 21:47 ` Laurent Pinchart 0 siblings, 1 reply; 5+ messages in thread From: Emre Ucan @ 2018-04-04 11:03 UTC (permalink / raw) To: dri-devel; +Cc: laurent.pinchart 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. 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> #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); fail: while (i--) { -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm: rcar-du: track dma-buf fences 2018-04-04 11:03 [PATCH v2] drm: rcar-du: track dma-buf fences Emre Ucan @ 2018-04-23 21:47 ` Laurent Pinchart 2018-04-30 8:40 ` Ucan, Emre (ADITG/ESB) 0 siblings, 1 reply; 5+ messages in thread From: Laurent Pinchart @ 2018-04-23 21:47 UTC (permalink / raw) To: Emre Ucan; +Cc: dri-devel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2] drm: rcar-du: track dma-buf fences 2018-04-23 21:47 ` Laurent Pinchart @ 2018-04-30 8:40 ` Ucan, Emre (ADITG/ESB) 2018-04-30 11:43 ` Laurent Pinchart 0 siblings, 1 reply; 5+ messages in thread From: Ucan, Emre (ADITG/ESB) @ 2018-04-30 8:40 UTC (permalink / raw) To: Laurent Pinchart; +Cc: dri-devel@lists.freedesktop.org Hi Laurent, Sorry for late reply. > -----Original Message----- > From: Laurent Pinchart [mailto:laurent.pinchart@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 <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--) { > > -- > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm: rcar-du: track dma-buf fences 2018-04-30 8:40 ` Ucan, Emre (ADITG/ESB) @ 2018-04-30 11:43 ` Laurent Pinchart 2018-04-30 12:25 ` Ucan, Emre (ADITG/ESB) 0 siblings, 1 reply; 5+ messages in thread From: Laurent Pinchart @ 2018-04-30 11:43 UTC (permalink / raw) To: Ucan, Emre (ADITG/ESB); +Cc: dri-devel@lists.freedesktop.org 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2] drm: rcar-du: track dma-buf fences 2018-04-30 11:43 ` Laurent Pinchart @ 2018-04-30 12:25 ` Ucan, Emre (ADITG/ESB) 0 siblings, 0 replies; 5+ messages in thread From: Ucan, Emre (ADITG/ESB) @ 2018-04-30 12:25 UTC (permalink / raw) To: Laurent Pinchart; +Cc: dri-devel@lists.freedesktop.org Hi Laurent, (snip) > > 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 > > IMHO, it is not a new feature, but It is a bug. Because explicit fencing is a recent kernel feature. As Daniel Vetter stated, most X and wayland setups still rely on implicit fencing. Anyway this patch must be integrated first to Linus' tree, before I can propose to the stable release. It would be great if it can be part of 4.17. Best Regards, Emre Ucan _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-04-30 12:25 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-04-04 11:03 [PATCH v2] drm: rcar-du: track dma-buf fences Emre Ucan 2018-04-23 21:47 ` Laurent Pinchart 2018-04-30 8:40 ` Ucan, Emre (ADITG/ESB) 2018-04-30 11:43 ` Laurent Pinchart 2018-04-30 12:25 ` Ucan, Emre (ADITG/ESB)
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.