LinuxTV Patchwork [v6,17/18] drm: rcar-du: vsp: Extract framebuffer (un)mapping to separate functions

login
register
mail settings
Submitter Laurent Pinchart
Date March 13, 2019, 12:05 a.m.
Message ID <20190313000532.7087-18-laurent.pinchart+renesas@ideasonboard.com>
Download mbox | patch
Permalink /patch/55029/
State Superseded
Headers show

Comments

Laurent Pinchart - March 13, 2019, 12:05 a.m.
The rcar_du_vsp_plane_prepare_fb() and rcar_du_vsp_plane_cleanup_fb()
functions implement the DRM plane .prepare_fb() and .cleanup_fb()
operations. They map and unmap the framebuffer to/from the VSP
internally, which will be useful to implement writeback support. Split
the mapping and unmapping out to separate functions.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 68 ++++++++++++++++-----------
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 17 +++++++
 2 files changed, 58 insertions(+), 27 deletions(-)
Kieran Bingham - March 13, 2019, 11:54 a.m.
Hi Laurent,

On 13/03/2019 00:05, Laurent Pinchart wrote:
> The rcar_du_vsp_plane_prepare_fb() and rcar_du_vsp_plane_cleanup_fb()
> functions implement the DRM plane .prepare_fb() and .cleanup_fb()
> operations. They map and unmap the framebuffer to/from the VSP
> internally, which will be useful to implement writeback support. Split
> the mapping and unmapping out to separate functions.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Looks good to me, Just a refactor and nothing controversial.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>



> ---
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 68 ++++++++++++++++-----------
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 17 +++++++
>  2 files changed, 58 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index 29a08f7b0761..0806a69c4679 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -173,26 +173,16 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
>  			      plane->index, &cfg);
>  }
>  
> -static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
> -					struct drm_plane_state *state)
> +int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb,
> +		       struct sg_table sg_tables[3])
>  {
> -	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
> -	struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
>  	struct rcar_du_device *rcdu = vsp->dev;
>  	unsigned int i;
>  	int ret;
>  
> -	/*
> -	 * There's no need to prepare (and unprepare) the framebuffer when the
> -	 * plane is not visible, as it will not be displayed.
> -	 */
> -	if (!state->visible)
> -		return 0;
> -
> -	for (i = 0; i < rstate->format->planes; ++i) {
> -		struct drm_gem_cma_object *gem =
> -			drm_fb_cma_get_gem_obj(state->fb, i);
> -		struct sg_table *sgt = &rstate->sg_tables[i];
> +	for (i = 0; i < fb->format->num_planes; ++i) {
> +		struct drm_gem_cma_object *gem = drm_fb_cma_get_gem_obj(fb, i);
> +		struct sg_table *sgt = &sg_tables[i];
>  
>  		ret = dma_get_sgtable(rcdu->dev, sgt, gem->vaddr, gem->paddr,
>  				      gem->base.size);
> @@ -207,15 +197,11 @@ static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
>  		}
>  	}
>  
> -	ret = drm_gem_fb_prepare_fb(plane, state);
> -	if (ret)
> -		goto fail;
> -
>  	return 0;
>  
>  fail:
>  	while (i--) {
> -		struct sg_table *sgt = &rstate->sg_tables[i];
> +		struct sg_table *sgt = &sg_tables[i];
>  
>  		vsp1_du_unmap_sg(vsp->vsp, sgt);
>  		sg_free_table(sgt);
> @@ -224,22 +210,50 @@ static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
>  	return ret;
>  }
>  
> +static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
> +					struct drm_plane_state *state)
> +{
> +	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
> +	struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
> +	int ret;
> +
> +	/*
> +	 * There's no need to prepare (and unprepare) the framebuffer when the
> +	 * plane is not visible, as it will not be displayed.
> +	 */
> +	if (!state->visible)
> +		return 0;

What about writeback?

- Never mind - that doesn't call through here.

> +
> +	ret = rcar_du_vsp_map_fb(vsp, state->fb, rstate->sg_tables);
> +	if (ret < 0)
> +		return ret;
> +
> +	return drm_gem_fb_prepare_fb(plane, state);
> +}
> +
> +void rcar_du_vsp_unmap_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb,
> +			  struct sg_table sg_tables[3])
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < fb->format->num_planes; ++i) {
> +		struct sg_table *sgt = &sg_tables[i];
> +
> +		vsp1_du_unmap_sg(vsp->vsp, sgt);
> +		sg_free_table(sgt);
> +	}
> +}
> +
>  static void rcar_du_vsp_plane_cleanup_fb(struct drm_plane *plane,
>  					 struct drm_plane_state *state)
>  {
>  	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
>  	struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
> -	unsigned int i;
>  
>  	if (!state->visible)
>  		return;
>  
> -	for (i = 0; i < rstate->format->planes; ++i) {
> -		struct sg_table *sgt = &rstate->sg_tables[i];
> -
> -		vsp1_du_unmap_sg(vsp->vsp, sgt);
> -		sg_free_table(sgt);
> -	}
> +	rcar_du_vsp_unmap_fb(vsp, state->fb, rstate->sg_tables);
>  }
>  
>  static int rcar_du_vsp_plane_atomic_check(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> index db232037f24a..9b4724159378 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
> @@ -12,8 +12,10 @@
>  
>  #include <drm/drm_plane.h>
>  
> +struct drm_framebuffer;
>  struct rcar_du_format_info;
>  struct rcar_du_vsp;
> +struct sg_table;
>  
>  struct rcar_du_vsp_plane {
>  	struct drm_plane plane;
> @@ -60,6 +62,10 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc);
>  void rcar_du_vsp_disable(struct rcar_du_crtc *crtc);
>  void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
>  void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc);
> +int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb,
> +		       struct sg_table sg_tables[3]);
> +void rcar_du_vsp_unmap_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb,
> +			  struct sg_table sg_tables[3]);
>  #else
>  static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp,
>  				   struct device_node *np,
> @@ -71,6 +77,17 @@ static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
>  static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
>  static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { };
>  static inline void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc) { };
> +static inline int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp,
> +				     struct drm_framebuffer *fb,
> +				     struct sg_table sg_tables[3])
> +{
> +	return -ENXIO;
> +}
> +static inline void rcar_du_vsp_unmap_fb(struct rcar_du_vsp *vsp,
> +					struct drm_framebuffer *fb,
> +					struct sg_table sg_tables[3])
> +{
> +}
>  #endif
>  
>  #endif /* __RCAR_DU_VSP_H__ */
>

Patch

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 29a08f7b0761..0806a69c4679 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -173,26 +173,16 @@  static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 			      plane->index, &cfg);
 }
 
-static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
-					struct drm_plane_state *state)
+int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb,
+		       struct sg_table sg_tables[3])
 {
-	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
-	struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
 	struct rcar_du_device *rcdu = vsp->dev;
 	unsigned int i;
 	int ret;
 
-	/*
-	 * There's no need to prepare (and unprepare) the framebuffer when the
-	 * plane is not visible, as it will not be displayed.
-	 */
-	if (!state->visible)
-		return 0;
-
-	for (i = 0; i < rstate->format->planes; ++i) {
-		struct drm_gem_cma_object *gem =
-			drm_fb_cma_get_gem_obj(state->fb, i);
-		struct sg_table *sgt = &rstate->sg_tables[i];
+	for (i = 0; i < fb->format->num_planes; ++i) {
+		struct drm_gem_cma_object *gem = drm_fb_cma_get_gem_obj(fb, i);
+		struct sg_table *sgt = &sg_tables[i];
 
 		ret = dma_get_sgtable(rcdu->dev, sgt, gem->vaddr, gem->paddr,
 				      gem->base.size);
@@ -207,15 +197,11 @@  static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
 		}
 	}
 
-	ret = drm_gem_fb_prepare_fb(plane, state);
-	if (ret)
-		goto fail;
-
 	return 0;
 
 fail:
 	while (i--) {
-		struct sg_table *sgt = &rstate->sg_tables[i];
+		struct sg_table *sgt = &sg_tables[i];
 
 		vsp1_du_unmap_sg(vsp->vsp, sgt);
 		sg_free_table(sgt);
@@ -224,22 +210,50 @@  static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
 	return ret;
 }
 
+static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
+					struct drm_plane_state *state)
+{
+	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
+	struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
+	int ret;
+
+	/*
+	 * There's no need to prepare (and unprepare) the framebuffer when the
+	 * plane is not visible, as it will not be displayed.
+	 */
+	if (!state->visible)
+		return 0;
+
+	ret = rcar_du_vsp_map_fb(vsp, state->fb, rstate->sg_tables);
+	if (ret < 0)
+		return ret;
+
+	return drm_gem_fb_prepare_fb(plane, state);
+}
+
+void rcar_du_vsp_unmap_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb,
+			  struct sg_table sg_tables[3])
+{
+	unsigned int i;
+
+	for (i = 0; i < fb->format->num_planes; ++i) {
+		struct sg_table *sgt = &sg_tables[i];
+
+		vsp1_du_unmap_sg(vsp->vsp, sgt);
+		sg_free_table(sgt);
+	}
+}
+
 static void rcar_du_vsp_plane_cleanup_fb(struct drm_plane *plane,
 					 struct drm_plane_state *state)
 {
 	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
 	struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
-	unsigned int i;
 
 	if (!state->visible)
 		return;
 
-	for (i = 0; i < rstate->format->planes; ++i) {
-		struct sg_table *sgt = &rstate->sg_tables[i];
-
-		vsp1_du_unmap_sg(vsp->vsp, sgt);
-		sg_free_table(sgt);
-	}
+	rcar_du_vsp_unmap_fb(vsp, state->fb, rstate->sg_tables);
 }
 
 static int rcar_du_vsp_plane_atomic_check(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
index db232037f24a..9b4724159378 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
@@ -12,8 +12,10 @@ 
 
 #include <drm/drm_plane.h>
 
+struct drm_framebuffer;
 struct rcar_du_format_info;
 struct rcar_du_vsp;
+struct sg_table;
 
 struct rcar_du_vsp_plane {
 	struct drm_plane plane;
@@ -60,6 +62,10 @@  void rcar_du_vsp_enable(struct rcar_du_crtc *crtc);
 void rcar_du_vsp_disable(struct rcar_du_crtc *crtc);
 void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
 void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc);
+int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb,
+		       struct sg_table sg_tables[3]);
+void rcar_du_vsp_unmap_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb,
+			  struct sg_table sg_tables[3]);
 #else
 static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp,
 				   struct device_node *np,
@@ -71,6 +77,17 @@  static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
 static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
 static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { };
 static inline void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc) { };
+static inline int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp,
+				     struct drm_framebuffer *fb,
+				     struct sg_table sg_tables[3])
+{
+	return -ENXIO;
+}
+static inline void rcar_du_vsp_unmap_fb(struct rcar_du_vsp *vsp,
+					struct drm_framebuffer *fb,
+					struct sg_table sg_tables[3])
+{
+}
 #endif
 
 #endif /* __RCAR_DU_VSP_H__ */

Privacy Policy