[PATCH V4] drm/ast: Swap framebuffer writes on big-endian machines

René Rebe posted 1 patch 1 month, 3 weeks ago
drivers/gpu/drm/ast/ast_cursor.c | 11 ++++++++---
drivers/gpu/drm/ast/ast_mode.c   | 11 +++++++++--
2 files changed, 17 insertions(+), 5 deletions(-)
[PATCH V4] drm/ast: Swap framebuffer writes on big-endian machines
Posted by René Rebe 1 month, 3 weeks ago
Swap the pixel data when writing to framebuffer memory on big-endian
machines. Fixes incorrect output. Aspeed graphics does not appear to
support big-endian framebuffers after AST2400, although the feature
has been documented.

There's a lengthy discussion at [1].

Signed-off-by: René Rebe <rene@exactco.de>
Link: https://lore.kernel.org/dri-devel/20251202.170626.2134482663677806825.rene@exactco.de/ # [1]
---
The ARGB4444 cursor not yet 100% correct, but that might be another layer/helper bug.
Tested on sparc64 T4-1 running T2/Linux.
---
 drivers/gpu/drm/ast/ast_cursor.c | 11 ++++++++---
 drivers/gpu/drm/ast/ast_mode.c   | 11 +++++++++--
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 2d3ad7610c2e..d4620171d845 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -92,12 +92,17 @@ 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);
-	u32 csum;
-
-	csum = ast_cursor_calculate_checksum(src, width, height);
+	u32 csum = ast_cursor_calculate_checksum(src, width, height);
 
 	/* write pixel data */
+#if defined(__BIG_ENDIAN)
+	unsigned int i;
+
+	for (i = 0; i < AST_HWC_SIZE; i += 2)
+		writew(swab16(*(const __be16 *)&src[i]), &dst[i]);
+#else
 	memcpy_toio(dst, src, AST_HWC_SIZE);
+#endif
 
 	/* 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..57c6fbc3232b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -526,12 +526,18 @@ 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));
 
 	iosys_map_incr(&dst, drm_fb_clip_offset(fb->pitches[0], fb->format, clip));
+
+#if defined(__BIG_ENDIAN)
+	drm_fb_swab(&dst, fb->pitches, src, fb, clip, !src[0].is_iomem, fmtcnv_state);
+#else
 	drm_fb_memcpy(&dst, fb->pitches, src, fb, clip);
+#endif
 }
 
 static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
@@ -561,7 +567,8 @@ static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
 	if (drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE) == 0) {
 		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);
-- 
2.52.0

-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe
Re: [PATCH V4] drm/ast: Swap framebuffer writes on big-endian machines
Posted by kernel test robot 1 month, 2 weeks ago
Hi René,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm/drm-next drm-i915/for-linux-next drm-i915/for-linux-next-fixes drm-tip/drm-tip linus/master v6.19-rc1 next-20251219]
[cannot apply to daeinki-drm-exynos/exynos-drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ren-Rebe/drm-ast-Swap-framebuffer-writes-on-big-endian-machines/20251213-040713
base:   https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next
patch link:    https://lore.kernel.org/r/20251212.210504.1355099120650239629.rene%40exactco.de
patch subject: [PATCH V4] drm/ast: Swap framebuffer writes on big-endian machines
config: openrisc-randconfig-r122-20251217 (https://download.01.org/0day-ci/archive/20251221/202512210011.dKKTJkNT-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251221/202512210011.dKKTJkNT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512210011.dKKTJkNT-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/ast/ast_cursor.c:103:24: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned short [usertype] val @@     got restricted __be16 const [usertype] @@
   drivers/gpu/drm/ast/ast_cursor.c:103:24: sparse:     expected unsigned short [usertype] val
   drivers/gpu/drm/ast/ast_cursor.c:103:24: sparse:     got restricted __be16 const [usertype]
>> drivers/gpu/drm/ast/ast_cursor.c:103:24: sparse: sparse: cast from restricted __be16
>> drivers/gpu/drm/ast/ast_cursor.c:103:24: sparse: sparse: cast from restricted __be16

vim +103 drivers/gpu/drm/ast/ast_cursor.c

    91	
    92	static void ast_set_cursor_image(struct ast_device *ast, const u8 *src,
    93					 unsigned int width, unsigned int height)
    94	{
    95		u8 __iomem *dst = ast_plane_vaddr(&ast->cursor_plane.base);
    96		u32 csum = ast_cursor_calculate_checksum(src, width, height);
    97	
    98		/* write pixel data */
    99	#if defined(__BIG_ENDIAN)
   100		unsigned int i;
   101	
   102		for (i = 0; i < AST_HWC_SIZE; i += 2)
 > 103			writew(swab16(*(const __be16 *)&src[i]), &dst[i]);
   104	#else
   105		memcpy_toio(dst, src, AST_HWC_SIZE);
   106	#endif
   107	
   108		/* write checksum + signature */
   109		dst += AST_HWC_SIZE;
   110		writel(csum, dst);
   111		writel(width, dst + AST_HWC_SIGNATURE_SizeX);
   112		writel(height, dst + AST_HWC_SIGNATURE_SizeY);
   113		writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX);
   114		writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY);
   115	}
   116	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH V4] drm/ast: Swap framebuffer writes on big-endian machines
Posted by Thomas Zimmermann 1 month, 3 weeks ago

Am 12.12.25 um 21:05 schrieb René Rebe:
> Swap the pixel data when writing to framebuffer memory on big-endian
> machines. Fixes incorrect output. Aspeed graphics does not appear to
> support big-endian framebuffers after AST2400, although the feature
> has been documented.
>
> There's a lengthy discussion at [1].
>
> Signed-off-by: René Rebe <rene@exactco.de>
> Link: https://lore.kernel.org/dri-devel/20251202.170626.2134482663677806825.rene@exactco.de/ # [1]

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks for the updated patch.

> ---
> The ARGB4444 cursor not yet 100% correct, but that might be another layer/helper bug.
> Tested on sparc64 T4-1 running T2/Linux.
> ---
>   drivers/gpu/drm/ast/ast_cursor.c | 11 ++++++++---
>   drivers/gpu/drm/ast/ast_mode.c   | 11 +++++++++--
>   2 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
> index 2d3ad7610c2e..d4620171d845 100644
> --- a/drivers/gpu/drm/ast/ast_cursor.c
> +++ b/drivers/gpu/drm/ast/ast_cursor.c
> @@ -92,12 +92,17 @@ 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);
> -	u32 csum;
> -
> -	csum = ast_cursor_calculate_checksum(src, width, height);
> +	u32 csum = ast_cursor_calculate_checksum(src, width, height);
>   
>   	/* write pixel data */
> +#if defined(__BIG_ENDIAN)
> +	unsigned int i;
> +
> +	for (i = 0; i < AST_HWC_SIZE; i += 2)
> +		writew(swab16(*(const __be16 *)&src[i]), &dst[i]);
> +#else
>   	memcpy_toio(dst, src, AST_HWC_SIZE);
> +#endif
>   
>   	/* 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..57c6fbc3232b 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -526,12 +526,18 @@ 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));
>   
>   	iosys_map_incr(&dst, drm_fb_clip_offset(fb->pitches[0], fb->format, clip));
> +
> +#if defined(__BIG_ENDIAN)
> +	drm_fb_swab(&dst, fb->pitches, src, fb, clip, !src[0].is_iomem, fmtcnv_state);
> +#else
>   	drm_fb_memcpy(&dst, fb->pitches, src, fb, clip);
> +#endif
>   }
>   
>   static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
> @@ -561,7 +567,8 @@ static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
>   	if (drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE) == 0) {
>   		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);

-- 
--
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)