| Message ID | 20250324094520.192974-3-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ast: Various fixes | expand |
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@redhat.com> > > Signed-off-by: Thomas Zimmermann <tzimmermann@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);
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);
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. Signed-off-by: Thomas Zimmermann <tzimmermann@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(-)