Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756292AbcJ1HdG (ORCPT ); Fri, 28 Oct 2016 03:33:06 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:35046 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751549AbcJ1HdE (ORCPT ); Fri, 28 Oct 2016 03:33:04 -0400 Date: Fri, 28 Oct 2016 09:33:00 +0200 From: Daniel Vetter To: Gustavo Padovan , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Daniel Stone , Rob Clark , Greg Hackmann , John Harrison , laurent.pinchart@ideasonboard.com, seanpaul@google.com, marcheu@google.com, m.chehab@samsung.com, Sumit Semwal , Maarten Lankhorst , Brian Starkey , Gustavo Padovan Subject: Re: [PATCH v6 1/6] drm/atomic: add drm_atomic_set_fence_for_plane() Message-ID: <20161028073300.ih4rx5bq7dckthfq@phenom.ffwll.local> Mail-Followup-To: Gustavo Padovan , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Daniel Stone , Rob Clark , Greg Hackmann , John Harrison , laurent.pinchart@ideasonboard.com, seanpaul@google.com, marcheu@google.com, m.chehab@samsung.com, Sumit Semwal , Maarten Lankhorst , Brian Starkey , Gustavo Padovan References: <1477597031-5139-1-git-send-email-gustavo@padovan.org> <1477597031-5139-2-git-send-email-gustavo@padovan.org> <20161028073026.wgv5guzqspoa5vi2@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161028073026.wgv5guzqspoa5vi2@phenom.ffwll.local> X-Operating-System: Linux phenom 4.6.0-1-amd64 User-Agent: NeoMutt/20161014 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4159 Lines: 106 On Fri, Oct 28, 2016 at 09:30:26AM +0200, Daniel Vetter wrote: > On Thu, Oct 27, 2016 at 05:37:06PM -0200, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > This new function should be used by drivers when setting a implicit > > fence for the plane. It abstracts the fact that the user might have > > chosen explicit fencing instead. > > > > Signed-off-by: Gustavo Padovan > > Reviewed-by: Daniel Vetter > > > --- > > drivers/gpu/drm/drm_atomic.c | 30 ++++++++++++++++++++++++++++++ > > include/drm/drm_atomic.h | 2 ++ > > include/drm/drm_plane.h | 2 +- > > 3 files changed, 33 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index c32fb3c..5e73954 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -1147,6 +1147,36 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state, > > EXPORT_SYMBOL(drm_atomic_set_fb_for_plane); > > > > /** > > + * drm_atomic_set_fence_for_plane - set fence for plane > > + * @plane_state: atomic state object for the plane > > + * @fence: dma_fence to use for the plane > > + * > > + * Helper to setup the plane_state fence in case it is not set yet. > > + * By using this drivers doesn't need to worry if the user choose > > + * implicit or explicit fencing. > > + * > > + * This function will not set the fence to the state if it was set > > + * via explicit fencing interfaces on the atomic ioctl. It will > > + * all drope the reference to the fence as we not storing it > > + * anywhere. > > + * > > + * Otherwise, if plane_state->fence is not set this function we > > + * just set it with the received implict fence. > > + */ > > +void > > +drm_atomic_set_fence_for_plane(struct drm_plane_state *plane_state, > > + struct dma_fence *fence) > > +{ > > + if (plane_state->fence) { > > + dma_fence_put(fence); > > + return; > > + } > > + > > + plane_state->fence = fence; > > +} > > +EXPORT_SYMBOL(drm_atomic_set_fence_for_plane); > > + > > +/** > > * drm_atomic_set_crtc_for_connector - set crtc for connector > > * @conn_state: atomic state object for the connector > > * @crtc: crtc to use for the connector > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > > index fc8af53..2d1e9c9 100644 > > --- a/include/drm/drm_atomic.h > > +++ b/include/drm/drm_atomic.h > > @@ -345,6 +345,8 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state, > > struct drm_crtc *crtc); > > void drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state, > > struct drm_framebuffer *fb); > > +void drm_atomic_set_fence_for_plane(struct drm_plane_state *plane_state, > > + struct dma_fence *fence); > > int __must_check > > drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, > > struct drm_crtc *crtc); > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > > index c5e8a0d..68f6d22 100644 > > --- a/include/drm/drm_plane.h > > +++ b/include/drm/drm_plane.h > > @@ -59,7 +59,7 @@ struct drm_plane_state { > > > > struct drm_crtc *crtc; /* do not write directly, use drm_atomic_set_crtc_for_plane() */ > > struct drm_framebuffer *fb; /* do not write directly, use drm_atomic_set_fb_for_plane() */ > > - struct dma_fence *fence; > > + struct dma_fence *fence; /* do not write directly, use drm_atomic_set_fence_for_plane() */ Ok, small bikeshed maybe: When you feel the need to add more comments in-line in a struct, then please switch to the in-line kernel-doc member documentation style, and merge the existing kerneldoc together with these additional comments. Would be great to do that with all the others here too. Follow-up patch to address this would be great. -Daniel > > > > /* Signed dest location allows it to be partially off screen */ > > int32_t crtc_x, crtc_y; > > -- > > 2.5.5 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch