[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3e46c10b-79db-4c11-9047-cd33e94ff5e0@suse.de>
Date: Thu, 11 Dec 2025 15:03:48 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: René Rebe <rene@...ctco.de>
Cc: tpearson@...torengineering.com, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, airlied@...hat.com
Subject: Re: [PATCH] drm/ast: Fix big-endian support
Hi,
Am 11.12.25 um 13:43 schrieb René Rebe:
[...]
>> The code for the primary plane should be fine now. But we also need
>> something for the cursor plane as well. There's a
>> ast_set_cursor_image() with a memcpy_toio() [1] and several additional
>> writes. IIUC they all have to be swapped as well.
> Of course, any obvious style issue or endianess swapping linux-kernel
> would like to see differently? You did not answer if I should just
> conditionalize on the chip id. I used a bool to avoid intermangled
> #ifdef conditionals to hopefully match kernel style.
> Btw. checkpatch.pl warns:
>
> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
>
> I could add this if desired while at it.
>
> Only compile tested, will do a final hw test once patch is approved in
> general.
It's all a bit excessive. There's a patch attached that will hopefully
fix the issues.
If you could test it, I'll send it out for official review. The easiest
way of testing cursor support is to run Xorg and see if the cursor looks
correct.
The Co-developed-by tag requires your Signed-off-by.
Best regards
Thomas
>
> Thanks!
> René
> ---
> drivers/gpu/drm/ast/ast_cursor.c | 20 +++++++++++++++++-
> drivers/gpu/drm/ast/ast_mode.c | 35 +++++++++++++++++++++++++++++---
> drivers/gpu/drm/ast/ast_reg.h | 2 ++
> 3 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
> index 2d3ad7610c2e..a16745cff83e 100644
> --- a/drivers/gpu/drm/ast/ast_cursor.c
> +++ b/drivers/gpu/drm/ast/ast_cursor.c
> @@ -92,12 +92,30 @@ static void ast_set_cursor_image(struct ast_device *ast, const u8 *src,
> unsigned int width, unsigned int height)
> {
> u8 __iomem *dst = ast_plane_vaddr(&ast->cursor_plane.base);
> + bool sw_swab = false;
> + int i;
> u32 csum;
>
> csum = ast_cursor_calculate_checksum(src, width, height);
>
> +#ifdef __BIG_ENDIAN
> + /* HW swap bytes on big-endian formats, if possible */
> + if (ast->chip < AST2400)
> + ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xa2, 0x3f, AST_IO_VGACRA2_BE_MODE_16);
> + else
> + sw_swab = true;
> +#endif
> +
> /* write pixel data */
> - memcpy_toio(dst, src, AST_HWC_SIZE);
> + if (!sw_swab)
> + memcpy_toio(dst, src, AST_HWC_SIZE);
> + else {
> + for (i = 0; i < AST_HWC_SIZE / 2; i += 2) {
> + const u16 *src16 = (const u16 *)(src + i);
> +
> + writel(*src16, dst + i);
> + }
> + }
>
> /* write checksum + signature */
> dst += AST_HWC_SIZE;
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index cd08990a10f9..9a18f0dc1a99 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -526,12 +526,24 @@ static int ast_primary_plane_helper_atomic_check(struct drm_plane *plane,
>
> static void ast_handle_damage(struct ast_plane *ast_plane, struct iosys_map *src,
> struct drm_framebuffer *fb,
> - const struct drm_rect *clip)
> + const struct drm_rect *clip,
> + struct drm_format_conv_state *fmtcnv_state)
> {
> struct iosys_map dst = IOSYS_MAP_INIT_VADDR_IOMEM(ast_plane_vaddr(ast_plane));
> + bool sw_swab = false;
>
> iosys_map_incr(&dst, drm_fb_clip_offset(fb->pitches[0], fb->format, clip));
> - drm_fb_memcpy(&dst, fb->pitches, src, fb, clip);
> +
> +#ifdef __BIG_ENDIAN
> + /* Swap bytes on big-endian formats */
> + if (ast->chip >= AST2400)
> + sw_swab = true;
> +#endif
> +
> + if (sw_swab)
> + drm_fb_swab(&dst, fb->pitches, src, fb, clip, false, fmtcnv_state);
> + else
> + drm_fb_memcpy(&dst, fb->pitches, src, fb, clip);
> }
>
> static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
> @@ -559,9 +571,26 @@ static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
>
> /* if the buffer comes from another device */
> if (drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE) == 0) {
> +#ifdef __BIG_ENDIAN
> + /* HW swap bytes on big-endian formats, if possible */
> + if (ast->chip < AST2400) {
> + switch (fb->format->format) {
> + case DRM_FORMAT_RGB565:
> + ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xa2, 0x3f,
> + AST_IO_VGACRA2_BE_MODE_16);
> + break;
> + case DRM_FORMAT_XRGB8888:
> + ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xa2, 0x3f,
> + AST_IO_VGACRA2_BE_MODE);
> + break;
> + }
> + }
> +#endif
> +
> drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
> drm_atomic_for_each_plane_damage(&iter, &damage) {
> - ast_handle_damage(ast_plane, shadow_plane_state->data, fb, &damage);
> + ast_handle_damage(ast_plane, shadow_plane_state->data, fb, &damage,
> + &shadow_plane_state->fmtcnv_state);
> }
>
> drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
> index 30578e3b07e4..bcd39d7438b9 100644
> --- a/drivers/gpu/drm/ast/ast_reg.h
> +++ b/drivers/gpu/drm/ast/ast_reg.h
> @@ -34,6 +34,8 @@
> #define AST_IO_VGACR99_VGAMEM_RSRV_MASK GENMASK(1, 0)
> #define AST_IO_VGACRA1_VGAIO_DISABLED BIT(1)
> #define AST_IO_VGACRA1_MMIO_ENABLED BIT(2)
> +#define AST_IO_VGACRA2_BE_MODE BIT(7)
> +#define AST_IO_VGACRA2_BE_MODE_16 (BIT(7) | BIT(6))
> #define AST_IO_VGACRA3_DVO_ENABLED BIT(7)
> #define AST_IO_VGACRAA_VGAMEM_SIZE_MASK GENMASK(1, 0)
> #define AST_IO_VGACRB6_HSYNC_OFF BIT(0)
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
View attachment "0001-drm-ast-Swap-framebuffer-writes-on-big-endian-machin.patch" of type "text/x-patch" (4473 bytes)
Powered by blists - more mailing lists