[PATCH 1/2] drm/fb_cma_helper: Add drm_fb_cma_setup_fence() helper

[Daniel Vetter]

On Wed, Sep 28, 2016 at 10:08:21PM +0200, Marek Vasut wrote:
> On 09/28/2016 03:55 PM, Lucas Stach wrote:
> > Hi Marek,
> > 
> > Am Montag, den 26.09.2016, 15:01 +0200 schrieb Marek Vasut:
> >> Add new drm_fb_cma_setup_fence() helper function extracted from the
> >> imx-drm driver. This function checks if the plane has DMABUF attached
> >> to it and if so, sets up the fence on which the atomic helper can wait.
> >>
> >> Signed-off-by: Marek Vasut <marex at denx.de>
> >> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> Cc: Lucas Stach <l.stach at pengutronix.de>
> >> ---
> >>  drivers/gpu/drm/drm_fb_cma_helper.c | 26 ++++++++++++++++++++++++++
> >>  include/drm/drm_fb_cma_helper.h     |  3 +++
> >>  2 files changed, 29 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> >> index 1fd6eac..2441707 100644
> >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> >> @@ -23,8 +23,10 @@
> >>  #include <drm/drm_crtc_helper.h>
> >>  #include <drm/drm_gem_cma_helper.h>
> >>  #include <drm/drm_fb_cma_helper.h>
> >> +#include <linux/dma-buf.h>
> >>  #include <linux/dma-mapping.h>
> >>  #include <linux/module.h>
> >> +#include <linux/reservation.h>
> >>  
> >>  #define DEFAULT_FBDEFIO_DELAY_MS 50
> >>  
> >> @@ -265,6 +267,30 @@ struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
> >>  }
> >>  EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
> >>  
> >> +/**
> >> + * drm_fb_cma_setup_fence() - Set up the fence for atomic helper to wait on
> > 
> > I don't really like the naming of the helper. It's not setting up any
> > fence, it's either extracting it from the plane and/or attaching it to
> > the planestate, so I would have expected the name to include extract or
> > attach.

Imo for helpers that should be put into a specific vhook like this here,
it'd go with that vhooks name. So here drm_fb_cma_prepare_fb.

And then in the kerneldoc also mention where it should be put (both for
drivers using atomic helpers, and those using simple pipe helpers), e.g.
"This should be put into the prepare_fb hook of struct
&drm_plane_helper_funcs."

Please run $ make htmldocs to make sure the docs look pretty and the
generated links are correct.
-Daniel

> > 
> >> + * @plane: Which plane
> >> + * @state: Plane state to check
> > 
> > s/check/attach fence to
> 
> Both fixed in V2, thanks.
> 
> >> + *
> >> + * If the plane fb has an dma-buf attached, fish out the exclusive
> >> + * fence for the atomic helper to wait on.
> >> + */
> >> +void drm_fb_cma_setup_fence(struct drm_plane *plane,
> >> +			    struct drm_plane_state *state)
> >> +{
> >> +	struct dma_buf *dma_buf;
> >> +
> >> +	if ((plane->state->fb == state->fb) || !state->fb)
> >> +		return;
> >> +
> >> +	dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf;
> >> +	if (!dma_buf)
> >> +		return;
> >> +
> >> +	state->fence = reservation_object_get_excl_rcu(dma_buf->resv);
> >> +}
> >> +EXPORT_SYMBOL_GPL(drm_fb_cma_setup_fence);
> >> +
> >>  #ifdef CONFIG_DEBUG_FS
> >>  static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
> >>  {
> >> diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
> >> index f313211..fc122d3 100644
> >> --- a/include/drm/drm_fb_cma_helper.h
> >> +++ b/include/drm/drm_fb_cma_helper.h
> >> @@ -41,6 +41,9 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
> >>  struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
> >>  	unsigned int plane);
> >>  
> >> +void drm_fb_cma_setup_fence(struct drm_plane *plane,
> >> +			    struct drm_plane_state *state);
> >> +
> >>  #ifdef CONFIG_DEBUG_FS
> >>  struct seq_file;
> >>  
> > 
> > 
> 
> 
> -- 
> Best regards,
> Marek Vasut

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch