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

From: Daniel Vetter
Date: Fri May 06 2016 - 09:13:31 EST


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@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.

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

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