[PATCH 2/3] drm/ast: Remove vaddr field from struct ast_plane
Jocelyn Falempe
jfalempe at redhat.com
Tue Mar 25 09:01:20 UTC 2025
On 24/03/2025 10:44, Thomas Zimmermann wrote:
> The vaddr field in struct ast_plane serves no purpose. Its value
> can be calculated easily from the VRAM base plus the plane offset.
> Do so and remove the field.
>
> In ast_primary_plane_helper_get_scanout_buffer(), remove the test
> for vaddr being NULL. This cannot legally happen.
Thanks, it looks good to me.
Reviewed-by: Jocelyn Falempe <jfalempe at redhat.com>
>
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
> drivers/gpu/drm/ast/ast_cursor.c | 8 +++-----
> drivers/gpu/drm/ast/ast_drv.h | 4 ++--
> drivers/gpu/drm/ast/ast_mode.c | 19 ++++++++++++-------
> 3 files changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
> index 5ee724bfd682..2d3ad7610c2e 100644
> --- a/drivers/gpu/drm/ast/ast_cursor.c
> +++ b/drivers/gpu/drm/ast/ast_cursor.c
> @@ -91,7 +91,7 @@ static u32 ast_cursor_calculate_checksum(const void *src, unsigned int width, un
> static void ast_set_cursor_image(struct ast_device *ast, const u8 *src,
> unsigned int width, unsigned int height)
> {
> - u8 __iomem *dst = ast->cursor_plane.base.vaddr;
> + u8 __iomem *dst = ast_plane_vaddr(&ast->cursor_plane.base);
> u32 csum;
>
> csum = ast_cursor_calculate_checksum(src, width, height);
> @@ -193,7 +193,7 @@ static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
> struct ast_device *ast = to_ast_device(plane->dev);
> struct drm_rect damage;
> u64 dst_off = ast_plane->offset;
> - u8 __iomem *dst = ast_plane->vaddr; /* TODO: Use mapping abstraction properly */
> + u8 __iomem *dst = ast_plane_vaddr(ast_plane); /* TODO: Use mapping abstraction properly */
> u8 __iomem *sig = dst + AST_HWC_SIZE; /* TODO: Use mapping abstraction properly */
> unsigned int offset_x, offset_y;
> u16 x, y;
> @@ -291,7 +291,6 @@ int ast_cursor_plane_init(struct ast_device *ast)
> struct ast_plane *ast_plane = &ast_cursor_plane->base;
> struct drm_plane *cursor_plane = &ast_plane->base;
> unsigned long size;
> - void __iomem *vaddr;
> long offset;
> int ret;
>
> @@ -299,9 +298,8 @@ int ast_cursor_plane_init(struct ast_device *ast)
> offset = ast_cursor_vram_offset(ast);
> if (offset < 0)
> return offset;
> - vaddr = ast->vram + offset;
>
> - ret = ast_plane_init(dev, ast_plane, vaddr, offset, size,
> + ret = ast_plane_init(dev, ast_plane, offset, size,
> 0x01, &ast_cursor_plane_funcs,
> ast_cursor_plane_formats, ARRAY_SIZE(ast_cursor_plane_formats),
> NULL, DRM_PLANE_TYPE_CURSOR);
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index d9da2328d46b..2ee402096cd9 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -122,7 +122,6 @@ enum ast_config_mode {
> struct ast_plane {
> struct drm_plane base;
>
> - void __iomem *vaddr;
> u64 offset;
> unsigned long size;
> };
> @@ -443,11 +442,12 @@ int ast_astdp_output_init(struct ast_device *ast);
> /* ast_mode.c */
> int ast_mode_config_init(struct ast_device *ast);
> int ast_plane_init(struct drm_device *dev, struct ast_plane *ast_plane,
> - void __iomem *vaddr, u64 offset, unsigned long size,
> + u64 offset, unsigned long size,
> uint32_t possible_crtcs,
> const struct drm_plane_funcs *funcs,
> const uint32_t *formats, unsigned int format_count,
> const uint64_t *format_modifiers,
> enum drm_plane_type type);
> +void __iomem *ast_plane_vaddr(struct ast_plane *ast);
>
> #endif
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 20fbea11b710..d3ed27faefd1 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -457,7 +457,7 @@ static void ast_wait_for_vretrace(struct ast_device *ast)
> */
>
> int ast_plane_init(struct drm_device *dev, struct ast_plane *ast_plane,
> - void __iomem *vaddr, u64 offset, unsigned long size,
> + u64 offset, unsigned long size,
> uint32_t possible_crtcs,
> const struct drm_plane_funcs *funcs,
> const uint32_t *formats, unsigned int format_count,
> @@ -466,7 +466,6 @@ int ast_plane_init(struct drm_device *dev, struct ast_plane *ast_plane,
> {
> struct drm_plane *plane = &ast_plane->base;
>
> - ast_plane->vaddr = vaddr;
> ast_plane->offset = offset;
> ast_plane->size = size;
>
> @@ -475,6 +474,13 @@ int ast_plane_init(struct drm_device *dev, struct ast_plane *ast_plane,
> type, NULL);
> }
>
> +void __iomem *ast_plane_vaddr(struct ast_plane *ast_plane)
> +{
> + struct ast_device *ast = to_ast_device(ast_plane->base.dev);
> +
> + return ast->vram + ast_plane->offset;
> +}
> +
> /*
> * Primary plane
> */
> @@ -521,7 +527,7 @@ static void ast_handle_damage(struct ast_plane *ast_plane, struct iosys_map *src
> struct drm_framebuffer *fb,
> const struct drm_rect *clip)
> {
> - struct iosys_map dst = IOSYS_MAP_INIT_VADDR_IOMEM(ast_plane->vaddr);
> + struct iosys_map dst = IOSYS_MAP_INIT_VADDR_IOMEM(ast_plane_vaddr(ast_plane));
>
> iosys_map_incr(&dst, drm_fb_clip_offset(fb->pitches[0], fb->format, clip));
> drm_fb_memcpy(&dst, fb->pitches, src, fb, clip);
> @@ -594,12 +600,12 @@ static int ast_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane,
> {
> struct ast_plane *ast_plane = to_ast_plane(plane);
>
> - if (plane->state && plane->state->fb && ast_plane->vaddr) {
> + if (plane->state && plane->state->fb) {
> sb->format = plane->state->fb->format;
> sb->width = plane->state->fb->width;
> sb->height = plane->state->fb->height;
> sb->pitch[0] = plane->state->fb->pitches[0];
> - iosys_map_set_vaddr_iomem(&sb->map[0], ast_plane->vaddr);
> + iosys_map_set_vaddr_iomem(&sb->map[0], ast_plane_vaddr(ast_plane));
> return 0;
> }
> return -ENODEV;
> @@ -626,12 +632,11 @@ static int ast_primary_plane_init(struct ast_device *ast)
> struct drm_device *dev = &ast->base;
> struct ast_plane *ast_primary_plane = &ast->primary_plane;
> struct drm_plane *primary_plane = &ast_primary_plane->base;
> - void __iomem *vaddr = ast->vram;
> u64 offset = ast_fb_vram_offset();
> unsigned long size = ast_fb_vram_size(ast);
> int ret;
>
> - ret = ast_plane_init(dev, ast_primary_plane, vaddr, offset, size,
> + ret = ast_plane_init(dev, ast_primary_plane, offset, size,
> 0x01, &ast_primary_plane_funcs,
> ast_primary_plane_formats, ARRAY_SIZE(ast_primary_plane_formats),
> NULL, DRM_PLANE_TYPE_PRIMARY);
More information about the dri-devel
mailing list