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

[Robert Foss]

Hey,

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


Tested-by: Robert Foss 

On 2017-05-12 03:10 PM, Gustavo Padovan wrote:

From: Gustavo Padovan 

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 
Signed-off-by: Gustavo Padovan 
---
  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,
   _state->hw_dlist[vc4_state->pos2_offset]);
writel(vc4_state->dlist[vc4_state->ptr0_offset],
   _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 

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

[Gustavo Padovan]

From: Gustavo Padovan 

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 
Signed-off-by: Gustavo Padovan 
---
 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,
   _state->hw_dlist[vc4_state->pos2_offset]);
writel(vc4_state->dlist[vc4_state->ptr0_offset],
   _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,
-