Re: [PATCH 1/4] drm/fb-cma-helper: Add function drm_fb_cma_create_with_funcs()

[Noralf TrÃnnes]

Den 05.05.2016 18:27, skrev Daniel Vetter:
> On Thu, May 05, 2016 at 03:24:31PM +0200, Noralf Trønnes wrote:
> > Add drm_fb_cma_create_with_funcs() for drivers that need to set the
> > dirty() callback.
> >
> > Cc: laurent.pinchart@xxxxxxxxxxxxxxxx
> > Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/drm_fb_cma_helper.c | 29 +++++++++++++++++++++++------
> >   include/drm/drm_fb_cma_helper.h     |  3 +++
> >   2 files changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> > index 086f600..7165209 100644
> > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > @@ -161,13 +161,16 @@ static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
> >   }
> >   
> >   /**
> > - * drm_fb_cma_create() - (struct drm_mode_config_funcs *)->fb_create callback function
> > + * drm_fb_cma_create_with_funcs() - helper function for the
> > + *                                  &drm_mode_config_funcs ->fb_create
> > + *                                  callback function
> >    *
> > - * If your hardware has special alignment or pitch requirements these should be
> > - * checked before calling this function.
> > + * This can be used to set &drm_framebuffer_funcs for drivers that need the
> > + * dirty() callback.
> Please reference the other function in your kerneldoc using
> drm_fb_cma_create() syntax. That will create a hyperlink. With such sets
> of functions it's always good to cross link them and explain exactly when
> another one is more appropriate. E.g. here "If your driver does not need a
> custom &drm_framebuffer_funcs then just use drm_fb_cma_create() directly."
>
> Similar, but other way round for the existing one.
>
> Again please check with make htmldocs that it all looks good.

Ok, I didn't understand this htmldocs stuff, I thought it picked up the docs
by magic or something.
It turns out that drm_fb_cma_helper isn't mentioned in gpu.tmpl so I have
to make a patch for that.
Is there an order to things where I should put it in gpu.tmpl?
(drm_simple_kms_helper also)

This is the current order:

Mode Setting Helper Functions

Atomic Modeset Helper Functions Reference
Modeset Helper Reference for Common Vtables
Legacy CRTC/Modeset Helper Functions Reference
Output Probing Helper Functions Reference
fbdev Helper Functions Reference
Display Port Helper Functions Reference
Display Port MST Helper Functions Reference
MIPI DSI Helper Functions Reference
EDID Helper Functions Reference
Rectangle Utilities Reference
Flip-work Helper Reference
HDMI Infoframes Helper Reference
Plane Helper Reference
Tile group
Bridges


And the code example I put in drm_fb_cma_helper DOC: looks
terrible, maybe it looks better in the intel augmented version?

Noralf.

> Otherwise lgtm.
> -Daniel
>
> >    */
> > -struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
> > -	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
> > +struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
> > +	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
> > +	struct drm_framebuffer_funcs *funcs)
> >   {
> >   	struct drm_fb_cma *fb_cma;
> >   	struct drm_gem_cma_object *objs[4];
> > @@ -204,7 +207,7 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
> >   		objs[i] = to_drm_gem_cma_obj(obj);
> >   	}
> >   
> > -	fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, &drm_fb_cma_funcs);
> > +	fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs);
> >   	if (IS_ERR(fb_cma)) {
> >   		ret = PTR_ERR(fb_cma);
> >   		goto err_gem_object_unreference;
> > @@ -217,6 +220,20 @@ err_gem_object_unreference:
> >   		drm_gem_object_unreference_unlocked(&objs[i]->base);
> >   	return ERR_PTR(ret);
> >   }
> > +EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
> > +
> > +/**
> > + * drm_fb_cma_create() - &drm_mode_config_funcs ->fb_create callback function
> > + *
> > + * If your hardware has special alignment or pitch requirements these should be
> > + * checked before calling this function.
> > + */
> > +struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
> > +	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
> > +{
> > +	return drm_fb_cma_create_with_funcs(dev, file_priv, mode_cmd,
> > +					    &drm_fb_cma_funcs);
> > +}
> >   EXPORT_SYMBOL_GPL(drm_fb_cma_create);
> >   
> >   /**
> > diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
> > index c6d9c9c..1f9a8bc 100644
> > --- a/include/drm/drm_fb_cma_helper.h
> > +++ b/include/drm/drm_fb_cma_helper.h
> > @@ -31,6 +31,9 @@ void drm_fb_cma_destroy(struct drm_framebuffer *fb);
> >   int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
> >   	struct drm_file *file_priv, unsigned int *handle);
> >   
> > +struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
> > +	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
> > +	struct drm_framebuffer_funcs *funcs);
> >   struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
> >   	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd);
> >   
> > --
> > 2.2.2