Patchwork [v3,1/7] drm: Add consistency check for page-flipping

login
register
mail settings
Submitter Thierry Reding
Date Feb. 13, 2013, 4:05 p.m.
Message ID <1360771506-17849-2-git-send-email-thierry.reding@avionic-design.de>
Download mbox | patch
Permalink /patch/220186/
State Not Applicable, archived
Headers show

Comments

Thierry Reding - Feb. 13, 2013, 4:05 p.m.
Driver implementations of the drm_crtc's .page_flip() function are
required to update the crtc->fb field on success to reflect that the new
framebuffer is now in use. This is important to keep reference counting
on the framebuffers balanced.

While at it, document this requirement to keep others from falling into
the same trap.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
 Documentation/DocBook/drm.tmpl | 6 ++++++
 drivers/gpu/drm/drm_crtc.c     | 7 +++++++
 2 files changed, 13 insertions(+)
Daniel Vetter - Feb. 13, 2013, 7:20 p.m.
On Wed, Feb 13, 2013 at 05:05:00PM +0100, Thierry Reding wrote:
> Driver implementations of the drm_crtc's .page_flip() function are
> required to update the crtc->fb field on success to reflect that the new
> framebuffer is now in use. This is important to keep reference counting
> on the framebuffers balanced.
> 
> While at it, document this requirement to keep others from falling into
> the same trap.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/DocBook/drm.tmpl | 6 ++++++
>  drivers/gpu/drm/drm_crtc.c     | 7 +++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index fae8018..8dfaeb0 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -1161,6 +1161,12 @@ int max_width, max_height;</synopsis>
>              any new rendering to the frame buffer until the page flip completes.
>            </para>
>            <para>
> +            If a page flip can be successfully scheduled the driver must set the
> +            <code>drm_crtc-&lt;fb</code> field to the new framebuffer pointed to
> +            by <code>fb</code>. This is important so that the reference counting
> +            on framebuffers stays balanced.
> +          </para>
> +          <para>
>              If a page flip is already pending, the
>              <methodname>page_flip</methodname> operation must return
>              -<errorname>EBUSY</errorname>.
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 838c9b6..d86edc1 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3766,6 +3766,13 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>  		/* Keep the old fb, don't unref it. */
>  		old_fb = NULL;
>  	} else {
> +		/*
> +		 * Warn if the driver hasn't properly updated the crtc->fb
> +		 * field to reflect that the new framebuffer is now used.
> +		 * Failing to do so will screw with the reference counting
> +		 * on framebuffers.
> +		 */
> +		WARN_ON(crtc->fb != fb);
>  		/* Unref only the old framebuffer. */
>  		fb = NULL;
>  	}
> -- 
> 1.8.1.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Patch

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index fae8018..8dfaeb0 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -1161,6 +1161,12 @@  int max_width, max_height;</synopsis>
             any new rendering to the frame buffer until the page flip completes.
           </para>
           <para>
+            If a page flip can be successfully scheduled the driver must set the
+            <code>drm_crtc-&lt;fb</code> field to the new framebuffer pointed to
+            by <code>fb</code>. This is important so that the reference counting
+            on framebuffers stays balanced.
+          </para>
+          <para>
             If a page flip is already pending, the
             <methodname>page_flip</methodname> operation must return
             -<errorname>EBUSY</errorname>.
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 838c9b6..d86edc1 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3766,6 +3766,13 @@  int drm_mode_page_flip_ioctl(struct drm_device *dev,
 		/* Keep the old fb, don't unref it. */
 		old_fb = NULL;
 	} else {
+		/*
+		 * Warn if the driver hasn't properly updated the crtc->fb
+		 * field to reflect that the new framebuffer is now used.
+		 * Failing to do so will screw with the reference counting
+		 * on framebuffers.
+		 */
+		WARN_ON(crtc->fb != fb);
 		/* Unref only the old framebuffer. */
 		fb = NULL;
 	}