[PATCH] drm/repaper: Do not access GEM-DMA vaddr directly

[PATCH] drm/repaper: Do not access GEM-DMA vaddr directly

[Thomas Zimmermann]

Use DRM's shadow-plane helper to map and access the GEM object's buffer
within kernel address space. Encasulates the vmap logic in the GEM-DMA
helpers.

The repaper driver currently reads the vaddr field from the GME buffer
object directly. This only works because GEM code 'automagically' sets
vaddr.

Shadow-plane helpers perform the same steps, but with correct abstraction
behind drm_gem_vmap(). The shadow-plane state provides the buffer address
in kernel address space and the format-conversion state.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/repaper.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index 5c3b51eb0a97..4824f863fdba 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -510,13 +510,12 @@ static void repaper_get_temperature(struct repaper_epd *epd)
 	epd->factored_stage_time = epd->stage_time * factor10x / 10;
 }
 
-static int repaper_fb_dirty(struct drm_framebuffer *fb,
+static int repaper_fb_dirty(struct drm_framebuffer *fb, const struct iosys_map *vmap,
 			    struct drm_format_conv_state *fmtcnv_state)
 {
-	struct drm_gem_dma_object *dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
 	struct repaper_epd *epd = drm_to_epd(fb->dev);
 	unsigned int dst_pitch = 0;
-	struct iosys_map dst, vmap;
+	struct iosys_map dst;
 	struct drm_rect clip;
 	int idx, ret = 0;
 	u8 *buf = NULL;
@@ -546,8 +545,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
 		goto out_free;
 
 	iosys_map_set_vaddr(&dst, buf);
-	iosys_map_set_vaddr(&vmap, dma_obj->vaddr);
-	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, &vmap, fb, &clip, fmtcnv_state);
+	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, &clip, fmtcnv_state);
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
@@ -832,16 +830,15 @@ static void repaper_pipe_update(struct drm_simple_display_pipe *pipe,
 				struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = pipe->plane.state;
-	struct drm_format_conv_state fmtcnv_state = DRM_FORMAT_CONV_STATE_INIT;
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
 	struct drm_rect rect;
 
 	if (!pipe->crtc.state->active)
 		return;
 
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
-		repaper_fb_dirty(state->fb, &fmtcnv_state);
-
-	drm_format_conv_state_release(&fmtcnv_state);
+		repaper_fb_dirty(state->fb, shadow_plane_state->data,
+				 &shadow_plane_state->fmtcnv_state);
 }
 
 static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
@@ -849,6 +846,7 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
 	.enable = repaper_pipe_enable,
 	.disable = repaper_pipe_disable,
 	.update = repaper_pipe_update,
+	DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS,
 };
 
 static int repaper_connector_get_modes(struct drm_connector *connector)
-- 
2.50.0

Re: [PATCH] drm/repaper: Do not access GEM-DMA vaddr directly

[Thomas Zimmermann]

ping for a review

Am 27.06.25 um 17:24 schrieb Thomas Zimmermann:
> Use DRM's shadow-plane helper to map and access the GEM object's buffer
> within kernel address space. Encasulates the vmap logic in the GEM-DMA
> helpers.
>
> The repaper driver currently reads the vaddr field from the GME buffer
> object directly. This only works because GEM code 'automagically' sets
> vaddr.
>
> Shadow-plane helpers perform the same steps, but with correct abstraction
> behind drm_gem_vmap(). The shadow-plane state provides the buffer address
> in kernel address space and the format-conversion state.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/tiny/repaper.c | 16 +++++++---------
>   1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
> index 5c3b51eb0a97..4824f863fdba 100644
> --- a/drivers/gpu/drm/tiny/repaper.c
> +++ b/drivers/gpu/drm/tiny/repaper.c
> @@ -510,13 +510,12 @@ static void repaper_get_temperature(struct repaper_epd *epd)
>   	epd->factored_stage_time = epd->stage_time * factor10x / 10;
>   }
>   
> -static int repaper_fb_dirty(struct drm_framebuffer *fb,
> +static int repaper_fb_dirty(struct drm_framebuffer *fb, const struct iosys_map *vmap,
>   			    struct drm_format_conv_state *fmtcnv_state)
>   {
> -	struct drm_gem_dma_object *dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
>   	struct repaper_epd *epd = drm_to_epd(fb->dev);
>   	unsigned int dst_pitch = 0;
> -	struct iosys_map dst, vmap;
> +	struct iosys_map dst;
>   	struct drm_rect clip;
>   	int idx, ret = 0;
>   	u8 *buf = NULL;
> @@ -546,8 +545,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
>   		goto out_free;
>   
>   	iosys_map_set_vaddr(&dst, buf);
> -	iosys_map_set_vaddr(&vmap, dma_obj->vaddr);
> -	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, &vmap, fb, &clip, fmtcnv_state);
> +	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, &clip, fmtcnv_state);
>   
>   	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>   
> @@ -832,16 +830,15 @@ static void repaper_pipe_update(struct drm_simple_display_pipe *pipe,
>   				struct drm_plane_state *old_state)
>   {
>   	struct drm_plane_state *state = pipe->plane.state;
> -	struct drm_format_conv_state fmtcnv_state = DRM_FORMAT_CONV_STATE_INIT;
> +	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
>   	struct drm_rect rect;
>   
>   	if (!pipe->crtc.state->active)
>   		return;
>   
>   	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> -		repaper_fb_dirty(state->fb, &fmtcnv_state);
> -
> -	drm_format_conv_state_release(&fmtcnv_state);
> +		repaper_fb_dirty(state->fb, shadow_plane_state->data,
> +				 &shadow_plane_state->fmtcnv_state);
>   }
>   
>   static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
> @@ -849,6 +846,7 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
>   	.enable = repaper_pipe_enable,
>   	.disable = repaper_pipe_disable,
>   	.update = repaper_pipe_update,
> +	DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS,
>   };
>   
>   static int repaper_connector_get_modes(struct drm_connector *connector)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH] drm/repaper: Do not access GEM-DMA vaddr directly

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Use DRM's shadow-plane helper to map and access the GEM object's buffer
> within kernel address space. Encasulates the vmap logic in the GEM-DMA
> helpers.
>
> The repaper driver currently reads the vaddr field from the GME buffer
> object directly. This only works because GEM code 'automagically' sets
> vaddr.
>
> Shadow-plane helpers perform the same steps, but with correct abstraction
> behind drm_gem_vmap(). The shadow-plane state provides the buffer address
> in kernel address space and the format-conversion state.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat