Re: [PATCH 20/28] drm: Add kerneldoc for drm_framebuffer_funcs

Thierry Reding
2015-12-07T12:37:44Z

[-- Attachment #1.1: Type: text/plain, Size: 3215 bytes --]

On Fri, Dec 04, 2015 at 09:46:01AM +0100, Daniel Vetter wrote:
> While typing these I noticed that ->dirty is a bit a can of worms
> and even supports blt/fill semantics ... shocked me a bit.
>
> Oh well it's defined in a way that nothing bad (just a bit of inefficiency)
> will happen for drivers which supports this. So I didn't bother copying
> the detailed spec into the new kerneldoc.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> include/drm/drm_crtc.h | 50 +++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 72a7e096dd42..551484fb55e5 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -162,23 +162,55 @@ struct drm_tile_group {
> u8 group_data[8];
> };
>
> +/**
> + * struct drm_framebuffer_funcs - framebuffer hooks
> + */
> struct drm_framebuffer_funcs {
> - /* note: use drm_framebuffer_remove() */
> + /**
> + * @destroy:
> + *
> + * Clean up framebuffer resources, specifically also unreference the
> + * backing storage. The core guarantees to call this function for every
> + * framebuffer succesfully created by fb_create in

"successfully" and "->fb_create()"?

> + /**
> + * @create_handle:
> + *
> + * Create a buffer handle in the driver-specific buffer manager (either
> + * GEM or TTM) valid for the passed-in struct &drm_file. This is used by
> + * the core to implement the GETFB ioctl, which returns (for

s/ioctl/IOCTL/?

> + * sufficiently priviledged user) also a native buffer handle. This can

"users"

> + * be used for seamless transitions between modesetting clients by
> + * copying the current screen contents to a private buffer and blending
> + * between that and the new contents.
> + *
> + * RETURNS:
> + *
> + * 0 on success or a negative error code on failure.
> + */
> int (*create_handle)(struct drm_framebuffer *fb,
> struct drm_file *file_priv,
> unsigned int *handle);
> - /*
> + /**
> + * @dirty:
> + *
> * Optional callback for the dirty fb ioctl.
> *
> - * Userspace can notify the driver via this callback
> - * that a area of the framebuffer has changed and should
> - * be flushed to the display hardware.
> + * Userspace can notify the driver via this callback that a area of the

"an area"

> + * framebuffer has changed and should be flushed to the display
> + * hardware. This can also be used internally, e.g. by the fbdev
> + * emulation, though that's not (yet) the case currently.

Nit: I'd drop "(yet) " because of "currently".

> + *
> + * See documentation in drm_mode.h for the struct drm_mode_fb_dirty_cmd
> + * for more information as all the semantics and arguments have a one to
> + * one mapping on this function.

Perhaps "for more information on struct drm_mode_fb_dirty_cmd as all the
...". Oh and I guess also &drm_mode_fb_dirty_cmd for the cross-reference?

Again, fine if this is all fixed-up in a follow-up patch, since this is
all from moving the documentation:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]