On Tue, Aug 09, 2016 at 03:41:23PM +0200, Daniel Vetter wrote:
> - Move the intro section into a DOC comment, and update it slightly.
> - kernel-doc for struct drm_framebuffer!
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> ---
> Documentation/gpu/drm-kms.rst | 26 +----------
> drivers/gpu/drm/drm_framebuffer.c | 35 +++++++++++++++
> include/drm/drm_framebuffer.h | 94 +++++++++++++++++++++++++++++++++------
> 3 files changed, 118 insertions(+), 37 deletions(-)
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 8264a88a8695..d244e03658cc 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -39,30 +39,8 @@ Atomic Mode Setting Function Reference
> Frame Buffer Abstraction
> ========================
>
> -Frame buffers are abstract memory objects that provide a source of
> -pixels to scanout to a CRTC. Applications explicitly request the
> -creation of frame buffers through the DRM_IOCTL_MODE_ADDFB(2) ioctls
> -and receive an opaque handle that can be passed to the KMS CRTC control,
> -plane configuration and page flip functions.
> -
> -Frame buffers rely on the underneath memory manager for low-level memory
> -operations. When creating a frame buffer applications pass a memory
> -handle (or a list of memory handles for multi-planar formats) through
> -the ``drm_mode_fb_cmd2`` argument. For drivers using GEM as their
> -userspace buffer management interface this would be a GEM handle.
> -Drivers are however free to use their own backing storage object
> -handles, e.g. vmwgfx directly exposes special TTM handles to userspace
> -and so expects TTM handles in the create ioctl and not GEM handles.
> -
> -The lifetime of a drm framebuffer is controlled with a reference count,
> -drivers can grab additional references with
> -:c:func:`drm_framebuffer_reference()`and drop them again with
> -:c:func:`drm_framebuffer_unreference()`. For driver-private
> -framebuffers for which the last reference is never dropped (e.g. for the
> -fbdev framebuffer when the struct :c:type:`struct drm_framebuffer
> -<drm_framebuffer>` is embedded into the fbdev helper struct)
> -drivers can manually clean up a framebuffer at module unload time with
> -:c:func:`drm_framebuffer_unregister_private()`.
> +.. kernel-doc:: drivers/gpu/drm/drm_framebuffer.c
> + :doc: overview
>
> Frame Buffer Functions Reference
> --------------------------------
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index c7a8a623b336..f2f4928c7262 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -28,6 +28,41 @@
> #include "drm_crtc_internal.h"
>
> /**
> + * DOC: overview
> + *
> + * Frame buffers are abstract memory objects that provide a source of pixels to
> + * scanout to a CRTC. Applications explicitly request the creation of frame
> + * buffers through the DRM_IOCTL_MODE_ADDFB(2) ioctls and receive an opaque
> + * handle that can be passed to the KMS CRTC control, plane configuration and
> + * page flip functions.
> + *
> + * Frame buffers rely on the underlying memory manager for allocating backing
> + * storage. When creating a frame buffer applications pass a memory handle
> + * (or a list of memory handles for multi-planar formats) through the
> + * struct &drm_mode_fb_cmd2 argument. For drivers using GEM as their userspace
> + * buffer management interface this would be a GEM handle. Drivers are however
> + * free to use their own backing storage object handles, e.g. vmwgfx directly
> + * exposes special TTM handles to userspace and so expects TTM handles in the
> + * create ioctl and not GEM handles.
> + *
> + * Framebuffers are tracked with struct &drm_framebuffer. They are published
> + * using drm_framebuffer_init() - after calling that function userspace can use
> + * and access the framebuffer object. The helper function
> + * drm_helper_mode_fill_fb_struct() can be used to pre-fill the required
> + * metadata fields.
> + *
> + * The lifetime of a drm framebuffer is controlled with a reference count,
> + * drivers can grab additional references with drm_framebuffer_reference() and
> + * drop them again with drm_framebuffer_unreference(). For driver-private
> + * framebuffers for which the last reference is never dropped (e.g. for the
> + * fbdev framebuffer when the struct struct &drm_framebuffer is embedded into
> + * the fbdev helper struct) drivers can manually clean up a framebuffer at
> + * module unload time with drm_framebuffer_unregister_private(). But doing this
> + * is not recommended, and it's better to have a normal free-standing struct
> + * &drm_framebuffer.
> + */
> +
> +/**
> * drm_mode_addfb - add an FB to the graphics configuration
> * @dev: drm device for the ioctl
> * @data: data pointer for the ioctl
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index 46abdace8fa5..685e062e57d7 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -92,37 +92,105 @@ struct drm_framebuffer_funcs {
> unsigned num_clips);
> };
>
> +/**
> + * struct drm_framebuffer - frame buffer object
> + *
> + * Note that the fb is refcounted for the benefit of driver internals,
> + * for example some hw, disabling a CRTC/plane is asynchronous, and
> + * scanout does not actually complete until the next vblank. So some
> + * cleanup (like releasing the reference(s) on the backing GEM bo(s))
> + * should be deferred. In cases like this, the driver would like to
> + * hold a ref to the fb even though it has already been removed from
> + * userspace perspective. See drm_framebuffer_reference() and
> + * drm_framebuffer_unreference().
> + *
> + * The refcount is stored inside the mode object @base.
> + */
> struct drm_framebuffer {
> + /**
> + * @dev: DRM device this framebuffer belongs to
> + */
> struct drm_device *dev;
> - /*
> - * Note that the fb is refcounted for the benefit of driver internals,
> - * for example some hw, disabling a CRTC/plane is asynchronous, and
> - * scanout does not actually complete until the next vblank. So some
> - * cleanup (like releasing the reference(s) on the backing GEM bo(s))
> - * should be deferred. In cases like this, the driver would like to
> - * hold a ref to the fb even though it has already been removed from
> - * userspace perspective.
> - * The refcount is stored inside the mode object.
> - */
> - /*
> - * Place on the dev->mode_config.fb_list, access protected by
> + /**
> + * @head: Place on the dev->mode_config.fb_list, access protected by
> * dev->mode_config.fb_lock.
> */
> struct list_head head;
> +
> + /**
> + * @base: base modeset object structure, contains the reference count.
> + */
> struct drm_mode_object base;
> + /**
> + * @funcs: framebuffer vfunc table
> + */
> const struct drm_framebuffer_funcs *funcs;
> + /**
> + * @pitches: Line stride per buffer. For userspace created object this
> + * is copied from drm_mode_fb_cmd2.
in bytes
> + */
> unsigned int pitches[4];
> + /**
> + * @offsets: Offset from buffer start to the actual pixel data in bytes,
> + * per buffer. For userspace created object this is copied from
> + * drm_mode_fb_cmd2.
Should we try to standardise what this actually means? Can we? For
i915 we're going with the linear offset approach, as in the byte
offset is calculated from x/y offsets as y*pitch+x*cpp. Not sure
that will work for everyone, or would someone have to specify the
offset in a different way?
With linear buffers it's all the same anyway, but with tiling this
distiction becomes rather important if you need to specify non
tile row aligned offsets. Obviously any non-linear offset approach
would be tiling format specific, and you wouldn't be able to do
a x/y -> offsets[] conversion without knowing the tile layout.
Which is why I chose to go with linear offsets for i915.
> + */
> unsigned int offsets[4];
> + /**
> + * @modifier: Data layout modifier, per buffer. This is used to describe
> + * tiling, or also special layouts (like compression) of auxiliary
> + * buffers. For userspace created object this is copied from
> + * drm_mode_fb_cmd2.
> + */
> uint64_t modifier[4];
> + /**
> + * @width: Logical width of the visible area of the framebuffer, in
> + * pixels.
> + */
> unsigned int width;
> + /**
> + * @height: Logical width of the visible area of the framebuffer, in
> + * pixels.
> + */
> unsigned int height;
> - /* depth can be 15 or 16 */
> + /**
> + * @depth: Depth in bits per pixel for RGB formats. 0 for everything
> + * else. Legacy information derived from @pixel_format, it's suggested to use
> + * the DRM FOURCC codes and helper functions directly instead.
> + */
> unsigned int depth;
> + /**
> + * @bits_per_pixel: Storage used bits per pixel for RGB formats. 0 for
> + * everything else. Legacy information derived from @pixel_format, it's
> + * suggested to use the DRM FOURCC codes and helper functions directly
> + * instead.
> + */
> int bits_per_pixel;
> + /**
> + * @flags: Framebuffer flags like DRM_MODE_FB_INTERLACED or
> + * DRM_MODE_FB_MODIFIERS.
> + */
> int flags;
> + /**
> + * @pixel_format: DRM FOURCC code describing the pixel format.
> + */
> uint32_t pixel_format; /* fourcc format */
> + /**
> + * @hot_x: X coordinate of the cursor hotspot. Used by the legacy cursor
> + * IOCTL when the driver supports cursor through a DRM_PLANE_TYPE_CURSOR
> + * universal plane.
> + */
> int hot_x;
> + /**
> + * @hot_y: Y coordinate of the cursor hotspot. Used by the legacy cursor
> + * IOCTL when the driver supports cursor through a DRM_PLANE_TYPE_CURSOR
> + * universal plane.
> + */
> int hot_y;
> + /**
> + * @filp_head: Placed on struct &drm_file fbs list_head, protected by
> + * fbs_lock in the same structure.
> + */
> struct list_head filp_head;
> };
>
> --
> 2.8.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
