[Intel-gfx] [PATCH 12/20] drm/doc: Update drm_framebuffer docs

Daniel Vetter daniel at ffwll.ch
Fri Aug 12 20:09:31 UTC 2016


On Wed, Aug 10, 2016 at 06:15:56PM +0300, Ville Syrjälä wrote:
> 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 at intel.com>
> > ---
> >  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.

offset is imo only for linear offset when you put multiple planes into the
same buffer. Anything which isn't tile-aligned (or whatever the hw execpts
for tiled layout) doesn't make sense imo. I'll add a few more words to
clarify that this is _not_ meant to add a x/y pixel offset to the buffer,
we already have that in the plane state.
-Daniel

> 
> > +	 */
> >  	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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

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


More information about the Intel-gfx mailing list