[3/5] drm: Move more framebuffer doc from docbook to kerneldoc
diff mbox

Message ID 1449564561-3896-3-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter Dec. 8, 2015, 8:49 a.m. UTC
I missed a few paragraphs in the docbook that need to be pulled into
the fbdev vfunc docs.

Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/DocBook/gpu.tmpl | 72 +-----------------------------------------
 include/drm/drm_crtc.h         | 18 ++++++++++-
 2 files changed, 18 insertions(+), 72 deletions(-)

Comments

Thierry Reding Dec. 14, 2015, 3:58 p.m. UTC | #1
On Tue, Dec 08, 2015 at 09:49:19AM +0100, Daniel Vetter wrote:
[...]
> @@ -187,6 +189,9 @@ struct drm_framebuffer_funcs {
>  	 * copying the current screen contents to a private buffer and blending
>  	 * between that and the new contents.
>  	 *
> +	 * GEM based drivers should call drm_gem_handle_create() to create the
> +	 * handle.
> +	 *
>  	 * RETURNS:
>  	 *
>  	 * 0 on success or a negative error code on failure.
> @@ -1727,6 +1732,17 @@ struct drm_mode_config_funcs {
>  	 * requested metadata, but most of that is left to the driver. See
>  	 * struct &drm_mode_fb_cmd2 for details.
>  	 *
> +	 * If the parameters are deemed valid and the backing storage objects in
> +	 * the underlying memory manager all exists then the drivers to allocate

"... all exist, then the driver allocates a new &drm_framebuffer structure
..."?

> +	 * a new &drm_framebuffer structure, subclassed to contain
> +	 * driver-specific information (like the internal native buffer object
> +	 * references). It also needs to fill out all relevant metadata, which
> +	 * should by done by calling drm_helper_mode_fill_fb_struct().

"should be done"

> +	 *
> +	 * The initializing is finalized by calling drm_framebuffer_init(),

"The initialization"

Other than that, looks good:

Reviewed-by: Thierry Reding <treding@nvidia.com>
Daniel Vetter Dec. 14, 2015, 5:25 p.m. UTC | #2
On Mon, Dec 14, 2015 at 04:58:13PM +0100, Thierry Reding wrote:
> On Tue, Dec 08, 2015 at 09:49:19AM +0100, Daniel Vetter wrote:
> [...]
> > @@ -187,6 +189,9 @@ struct drm_framebuffer_funcs {
> >  	 * copying the current screen contents to a private buffer and blending
> >  	 * between that and the new contents.
> >  	 *
> > +	 * GEM based drivers should call drm_gem_handle_create() to create the
> > +	 * handle.
> > +	 *
> >  	 * RETURNS:
> >  	 *
> >  	 * 0 on success or a negative error code on failure.
> > @@ -1727,6 +1732,17 @@ struct drm_mode_config_funcs {
> >  	 * requested metadata, but most of that is left to the driver. See
> >  	 * struct &drm_mode_fb_cmd2 for details.
> >  	 *
> > +	 * If the parameters are deemed valid and the backing storage objects in
> > +	 * the underlying memory manager all exists then the drivers to allocate
> 
> "... all exist, then the driver allocates a new &drm_framebuffer structure
> ..."?
> 
> > +	 * a new &drm_framebuffer structure, subclassed to contain
> > +	 * driver-specific information (like the internal native buffer object
> > +	 * references). It also needs to fill out all relevant metadata, which
> > +	 * should by done by calling drm_helper_mode_fill_fb_struct().
> 
> "should be done"
> 
> > +	 *
> > +	 * The initializing is finalized by calling drm_framebuffer_init(),
> 
> "The initialization"
> 
> Other than that, looks good:
> 
> Reviewed-by: Thierry Reding <treding@nvidia.com>

All fixed up on both this and the previous patch applied, thanks a lot for
your review.
-Daniel

Patch
diff mbox

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 5698c93dae8b..0c02ba0d0750 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -986,10 +986,7 @@  int max_width, max_height;</synopsis>
 !Idrivers/gpu/drm/drm_atomic.c
     </sect2>
     <sect2>
-      <title>Frame Buffer Creation</title>
-      <synopsis>struct drm_framebuffer *(*fb_create)(struct drm_device *dev,
-				     struct drm_file *file_priv,
-				     struct drm_mode_fb_cmd2 *mode_cmd);</synopsis>
+      <title>Frame Buffer Abstraction</title>
       <para>
         Frame buffers are abstract memory objects that provide a source of
         pixels to scanout to a CRTC. Applications explicitly request the
@@ -1008,73 +1005,6 @@  int max_width, max_height;</synopsis>
 	and so expects TTM handles in the create ioctl and not GEM handles.
       </para>
       <para>
-        Drivers must first validate the requested frame buffer parameters passed
-        through the mode_cmd argument. In particular this is where invalid
-        sizes, pixel formats or pitches can be caught.
-      </para>
-      <para>
-        If the parameters are deemed valid, drivers then create, initialize and
-        return an instance of struct <structname>drm_framebuffer</structname>.
-        If desired the instance can be embedded in a larger driver-specific
-	structure. Drivers must fill its <structfield>width</structfield>,
-	<structfield>height</structfield>, <structfield>pitches</structfield>,
-        <structfield>offsets</structfield>, <structfield>depth</structfield>,
-        <structfield>bits_per_pixel</structfield> and
-        <structfield>pixel_format</structfield> fields from the values passed
-        through the <parameter>drm_mode_fb_cmd2</parameter> argument. They
-        should call the <function>drm_helper_mode_fill_fb_struct</function>
-        helper function to do so.
-      </para>
-
-      <para>
-	The initialization of the new framebuffer instance is finalized with a
-	call to <function>drm_framebuffer_init</function> which takes a pointer
-	to DRM frame buffer operations (struct
-	<structname>drm_framebuffer_funcs</structname>). Note that this function
-	publishes the framebuffer and so from this point on it can be accessed
-	concurrently from other threads. Hence it must be the last step in the
-	driver's framebuffer initialization sequence. Frame buffer operations
-	are
-        <itemizedlist>
-          <listitem>
-            <synopsis>int (*create_handle)(struct drm_framebuffer *fb,
-		     struct drm_file *file_priv, unsigned int *handle);</synopsis>
-            <para>
-              Create a handle to the frame buffer underlying memory object. If
-              the frame buffer uses a multi-plane format, the handle will
-              reference the memory object associated with the first plane.
-            </para>
-            <para>
-              Drivers call <function>drm_gem_handle_create</function> to create
-              the handle.
-            </para>
-          </listitem>
-          <listitem>
-            <synopsis>void (*destroy)(struct drm_framebuffer *framebuffer);</synopsis>
-            <para>
-              Destroy the frame buffer object and frees all associated
-              resources. Drivers must call
-              <function>drm_framebuffer_cleanup</function> to free resources
-              allocated by the DRM core for the frame buffer object, and must
-              make sure to unreference all memory objects associated with the
-              frame buffer. Handles created by the
-              <methodname>create_handle</methodname> operation are released by
-              the DRM core.
-            </para>
-          </listitem>
-          <listitem>
-            <synopsis>int (*dirty)(struct drm_framebuffer *framebuffer,
-	     struct drm_file *file_priv, unsigned flags, unsigned color,
-	     struct drm_clip_rect *clips, unsigned num_clips);</synopsis>
-            <para>
-              This optional operation notifies the driver that a region of the
-              frame buffer has changed in response to a DRM_IOCTL_MODE_DIRTYFB
-              ioctl call.
-            </para>
-          </listitem>
-        </itemizedlist>
-      </para>
-      <para>
 	The lifetime of a drm framebuffer is controlled with a reference count,
 	drivers can grab additional references with
 	<function>drm_framebuffer_reference</function>and drop them
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 4f587a5bc88f..6fe14a773def 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -172,7 +172,9 @@  struct drm_framebuffer_funcs {
 	 * Clean up framebuffer resources, specifically also unreference the
 	 * backing storage. The core guarantees to call this function for every
 	 * framebuffer successfully created by ->fb_create() in
-	 * &drm_mode_config_funcs.
+	 * &drm_mode_config_funcs. Drivers must also call
+	 * drm_framebuffer_cleanup() to release DRM core resources for this
+	 * framebuffer.
 	 */
 	void (*destroy)(struct drm_framebuffer *framebuffer);
 
@@ -187,6 +189,9 @@  struct drm_framebuffer_funcs {
 	 * copying the current screen contents to a private buffer and blending
 	 * between that and the new contents.
 	 *
+	 * GEM based drivers should call drm_gem_handle_create() to create the
+	 * handle.
+	 *
 	 * RETURNS:
 	 *
 	 * 0 on success or a negative error code on failure.
@@ -1727,6 +1732,17 @@  struct drm_mode_config_funcs {
 	 * requested metadata, but most of that is left to the driver. See
 	 * struct &drm_mode_fb_cmd2 for details.
 	 *
+	 * If the parameters are deemed valid and the backing storage objects in
+	 * the underlying memory manager all exists then the drivers to allocate
+	 * a new &drm_framebuffer structure, subclassed to contain
+	 * driver-specific information (like the internal native buffer object
+	 * references). It also needs to fill out all relevant metadata, which
+	 * should by done by calling drm_helper_mode_fill_fb_struct().
+	 *
+	 * The initializing is finalized by calling drm_framebuffer_init(),
+	 * which registers the framebuffer and makes it accessible to other
+	 * threads.
+	 *
 	 * RETURNS:
 	 *
 	 * A new framebuffer with an initial reference count of 1 or a negative