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

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, May 06, 2016 at 03:01:37PM +0200, Noralf Trønnes wrote:
> 
> 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@ideasonboard.com
> >>Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >>---
> >>  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.

Oh right, cma fb helpers aren't pulled int at all. Would be great if you
can fix 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

Just add it somewhere to this list of helpers where you think it fits.
We're not that structured yet.

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

Yeah, in upstream it's terrible. But if you base your patch on top of
drm-intel-nightly, or pull topic/kerneldoc in, and install asciidoc, then
it should be fairly pretty. Note: That asciidoc support is pretty hacked
up, it takes 10-20 minutes to build the gpu docs with that. We're working
on something much better.
-Daniel

> 
> 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
> >>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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