diff mbox

[RFC,v3,7/8] drm/vc4: update cursors asynchronously through atomic

Message ID 20170512191054.10074-8-gustavo@padovan.org (mailing list archive)
State New, archived
Headers show

Commit Message

Gustavo Padovan May 12, 2017, 7:10 p.m. UTC 
From: Gustavo Padovan <gustavo.padovan@collabora.com>

Add support to async updates of cursors by using the new atomic
interface for that. Basically what this commit does is do what
vc4_update_plane() did but through atomic.

v3: move size checks back to drivers (Ville Syrjälä)

v2: move fb setting to core and use new state (Eric Anholt)

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
---
 drivers/gpu/drm/vc4/vc4_plane.c | 99 +++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 64 deletions(-)

Comments

Robert Foss May 18, 2017, 10:54 p.m. UTC | #1 
Hey,

I just tested this patch on a RPi2, and the cursor behaves correctly with no 
unexpected errors.

Tested-by: Robert Foss <robert.foss@collabora.com>

On 2017-05-12 03:10 PM, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Add support to async updates of cursors by using the new atomic
> interface for that. Basically what this commit does is do what
> vc4_update_plane() did but through atomic.
> 
> v3: move size checks back to drivers (Ville Syrjälä)
> 
> v2: move fb setting to core and use new state (Eric Anholt)
> 
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> ---
>   drivers/gpu/drm/vc4/vc4_plane.c | 99 +++++++++++++++--------------------------
>   1 file changed, 35 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index d34cd53..05e9f5f 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -735,70 +735,38 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb)
>   	vc4_state->dlist[vc4_state->ptr0_offset] = addr;
>   }
>   
> -static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
> -	.atomic_check = vc4_plane_atomic_check,
> -	.atomic_update = vc4_plane_atomic_update,
> -};
> -
> -static void vc4_plane_destroy(struct drm_plane *plane)
> -{
> -	drm_plane_helper_disable(plane);
> -	drm_plane_cleanup(plane);
> -}
> -
> -/* Implements immediate (non-vblank-synced) updates of the cursor
> - * position, or falls back to the atomic helper otherwise.
> - */
> -static int
> -vc4_update_plane(struct drm_plane *plane,
> -		 struct drm_crtc *crtc,
> -		 struct drm_framebuffer *fb,
> -		 int crtc_x, int crtc_y,
> -		 unsigned int crtc_w, unsigned int crtc_h,
> -		 uint32_t src_x, uint32_t src_y,
> -		 uint32_t src_w, uint32_t src_h,
> -		 struct drm_modeset_acquire_ctx *ctx)
> +static int vc4_plane_atomic_async_check(struct drm_plane *plane,
> +					struct drm_plane_state *state)
>   {
> -	struct drm_plane_state *plane_state;
> -	struct vc4_plane_state *vc4_state;
> -
> -	if (plane != crtc->cursor)
> -		goto out;
> -
> -	plane_state = plane->state;
> -	vc4_state = to_vc4_plane_state(plane_state);
> +	if (plane != state->crtc->cursor)
> +		return -EINVAL;
>   
> -	if (!plane_state)
> -		goto out;
> +	if (!plane->state)
> +		return -EINVAL;
>   
>   	/* No configuring new scaling in the fast path. */
> -	if (crtc_w != plane_state->crtc_w ||
> -	    crtc_h != plane_state->crtc_h ||
> -	    src_w != plane_state->src_w ||
> -	    src_h != plane_state->src_h) {
> -		goto out;
> +	if (state->crtc_w != plane->state->crtc_w ||
> +	    state->crtc_h != plane->state->crtc_h ||
> +	    state->src_w != plane->state->src_w ||
> +	    state->src_h != plane->state->src_h) {
> +		return -EINVAL;
>   	}
>   
> -	if (fb != plane_state->fb) {
> -		drm_atomic_set_fb_for_plane(plane->state, fb);
> -		vc4_plane_async_set_fb(plane, fb);
> -	}
> +	return 0;
> +}
>   
> -	/* Set the cursor's position on the screen.  This is the
> -	 * expected change from the drm_mode_cursor_universal()
> -	 * helper.
> -	 */
> -	plane_state->crtc_x = crtc_x;
> -	plane_state->crtc_y = crtc_y;
> +static void vc4_plane_atomic_async_update(struct drm_plane *plane,
> +					  struct drm_plane_state *new_state)
> +{
> +	struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state);
>   
> -	/* Allow changing the start position within the cursor BO, if
> -	 * that matters.
> -	 */
> -	plane_state->src_x = src_x;
> -	plane_state->src_y = src_y;
> +	plane->state->src_x = new_state->src_x;
> +	plane->state->src_y = new_state->src_y;
> +	plane->state->crtc_x = new_state->crtc_x;
> +	plane->state->crtc_y = new_state->crtc_y;
>   
> -	/* Update the display list based on the new crtc_x/y. */
> -	vc4_plane_atomic_check(plane, plane_state);
> +	if (plane->state->fb != new_state->fb)
> +		vc4_plane_async_set_fb(plane, new_state->fb);
>   
>   	/* Note that we can't just call vc4_plane_write_dlist()
>   	 * because that would smash the context data that the HVS is
> @@ -810,20 +778,23 @@ vc4_update_plane(struct drm_plane *plane,
>   	       &vc4_state->hw_dlist[vc4_state->pos2_offset]);
>   	writel(vc4_state->dlist[vc4_state->ptr0_offset],
>   	       &vc4_state->hw_dlist[vc4_state->ptr0_offset]);
> +}
>   
> -	return 0;
> +static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
> +	.atomic_check = vc4_plane_atomic_check,
> +	.atomic_update = vc4_plane_atomic_update,
> +	.atomic_async_check = vc4_plane_atomic_async_check,
> +	.atomic_async_update = vc4_plane_atomic_async_update,
> +};
>   
> -out:
> -	return drm_atomic_helper_update_plane(plane, crtc, fb,
> -					      crtc_x, crtc_y,
> -					      crtc_w, crtc_h,
> -					      src_x, src_y,
> -					      src_w, src_h,
> -					      ctx);
> +static void vc4_plane_destroy(struct drm_plane *plane)
> +{
> +	drm_plane_helper_disable(plane);
> +	drm_plane_cleanup(plane);
>   }
>   
>   static const struct drm_plane_funcs vc4_plane_funcs = {
> -	.update_plane = vc4_update_plane,
> +	.update_plane = drm_atomic_helper_update_plane,
>   	.disable_plane = drm_atomic_helper_disable_plane,
>   	.destroy = vc4_plane_destroy,
>   	.set_property = NULL,
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index d34cd53..05e9f5f 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -735,70 +735,38 @@  void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb)
 	vc4_state->dlist[vc4_state->ptr0_offset] = addr;
 }
 
-static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
-	.atomic_check = vc4_plane_atomic_check,
-	.atomic_update = vc4_plane_atomic_update,
-};
-
-static void vc4_plane_destroy(struct drm_plane *plane)
-{
-	drm_plane_helper_disable(plane);
-	drm_plane_cleanup(plane);
-}
-
-/* Implements immediate (non-vblank-synced) updates of the cursor
- * position, or falls back to the atomic helper otherwise.
- */
-static int
-vc4_update_plane(struct drm_plane *plane,
-		 struct drm_crtc *crtc,
-		 struct drm_framebuffer *fb,
-		 int crtc_x, int crtc_y,
-		 unsigned int crtc_w, unsigned int crtc_h,
-		 uint32_t src_x, uint32_t src_y,
-		 uint32_t src_w, uint32_t src_h,
-		 struct drm_modeset_acquire_ctx *ctx)
+static int vc4_plane_atomic_async_check(struct drm_plane *plane,
+					struct drm_plane_state *state)
 {
-	struct drm_plane_state *plane_state;
-	struct vc4_plane_state *vc4_state;
-
-	if (plane != crtc->cursor)
-		goto out;
-
-	plane_state = plane->state;
-	vc4_state = to_vc4_plane_state(plane_state);
+	if (plane != state->crtc->cursor)
+		return -EINVAL;
 
-	if (!plane_state)
-		goto out;
+	if (!plane->state)
+		return -EINVAL;
 
 	/* No configuring new scaling in the fast path. */
-	if (crtc_w != plane_state->crtc_w ||
-	    crtc_h != plane_state->crtc_h ||
-	    src_w != plane_state->src_w ||
-	    src_h != plane_state->src_h) {
-		goto out;
+	if (state->crtc_w != plane->state->crtc_w ||
+	    state->crtc_h != plane->state->crtc_h ||
+	    state->src_w != plane->state->src_w ||
+	    state->src_h != plane->state->src_h) {
+		return -EINVAL;
 	}
 
-	if (fb != plane_state->fb) {
-		drm_atomic_set_fb_for_plane(plane->state, fb);
-		vc4_plane_async_set_fb(plane, fb);
-	}
+	return 0;
+}
 
-	/* Set the cursor's position on the screen.  This is the
-	 * expected change from the drm_mode_cursor_universal()
-	 * helper.
-	 */
-	plane_state->crtc_x = crtc_x;
-	plane_state->crtc_y = crtc_y;
+static void vc4_plane_atomic_async_update(struct drm_plane *plane,
+					  struct drm_plane_state *new_state)
+{
+	struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state);
 
-	/* Allow changing the start position within the cursor BO, if
-	 * that matters.
-	 */
-	plane_state->src_x = src_x;
-	plane_state->src_y = src_y;
+	plane->state->src_x = new_state->src_x;
+	plane->state->src_y = new_state->src_y;
+	plane->state->crtc_x = new_state->crtc_x;
+	plane->state->crtc_y = new_state->crtc_y;
 
-	/* Update the display list based on the new crtc_x/y. */
-	vc4_plane_atomic_check(plane, plane_state);
+	if (plane->state->fb != new_state->fb)
+		vc4_plane_async_set_fb(plane, new_state->fb);
 
 	/* Note that we can't just call vc4_plane_write_dlist()
 	 * because that would smash the context data that the HVS is
@@ -810,20 +778,23 @@  vc4_update_plane(struct drm_plane *plane,
 	       &vc4_state->hw_dlist[vc4_state->pos2_offset]);
 	writel(vc4_state->dlist[vc4_state->ptr0_offset],
 	       &vc4_state->hw_dlist[vc4_state->ptr0_offset]);
+}
 
-	return 0;
+static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
+	.atomic_check = vc4_plane_atomic_check,
+	.atomic_update = vc4_plane_atomic_update,
+	.atomic_async_check = vc4_plane_atomic_async_check,
+	.atomic_async_update = vc4_plane_atomic_async_update,
+};
 
-out:
-	return drm_atomic_helper_update_plane(plane, crtc, fb,
-					      crtc_x, crtc_y,
-					      crtc_w, crtc_h,
-					      src_x, src_y,
-					      src_w, src_h,
-					      ctx);
+static void vc4_plane_destroy(struct drm_plane *plane)
+{
+	drm_plane_helper_disable(plane);
+	drm_plane_cleanup(plane);
 }
 
 static const struct drm_plane_funcs vc4_plane_funcs = {
-	.update_plane = vc4_update_plane,
+	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = vc4_plane_destroy,
 	.set_property = NULL,