Hi,
thanks for the patch.
Am 02.12.25 um 17:06 schrieb René Rebe:
The Aspeed ast drm driver has the frame-buffer RGBX swapped on
It is XRGB.
big-endian RISC systems. Fix by enabling byte swapping for any
__BIG_ENDIAN config.
Is this the RISC support that Linux famously shot down? :)
Anyway, I have another BE fix for PPC64, which I didn't take. I'd rather
merge your fix with some changes.
[1]
https://lore.kernel.org/dri-devel/407388289.1798972.1760725035958.javamail.zim...@raptorengineeringinc.com/
Fixes: 12fec1405dd5 ("drm: Initial KMS driver for AST (ASpeed Technologies) 2000
series (v2)")
I'd leave out the Fixes tag. We never claimed that the drivers supports
BE, so it's not really a fix.
Signed-off-by: René Rebe <[email protected]>
---
Tested on Oracle T4-1 running sparc64 T2/Linux.
---
drivers/gpu/drm/ast/ast_mode.c | 14 ++++++++++++++
drivers/gpu/drm/ast/ast_reg.h | 6 ++++++
2 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 30b011ed0a05..155ae35470d9 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -708,6 +708,20 @@ static void ast_crtc_helper_mode_set_nofb(struct drm_crtc
*crtc)
ast_set_dclk_reg(ast, adjusted_mode, vmode);
ast_set_crtthd_reg(ast);
ast_set_sync_reg(ast, adjusted_mode, vmode);
+
+#ifdef __BIG_ENDIAN
+ /* Big-endian byte-swapping */
+ switch (ast_crtc_state->format->format) {
This function sets the display mode. But the color format can change per
frame. So it's not the right place.
Then, we also have a cursor plane that always scans out in ARGB4444
format. How does this change interact with the cursor? AFAIU it should
mix up the pixels if set to 32-bit BE.
Therefore, I think we need to set the BE mode in each plane's atomic
update before it updates the video memory. At [2], for the primary
plane, it has to be located between the color-update code and the damage
handling. At [3], for the cursor plane, it can be within the if-damage
branch. The cursor update needs unconditional 16-but swapping.
[2]
https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/drm-misc-next-2025-12-01-1/drivers/gpu/drm/ast/ast_mode.c?ref_type=tags#L559
[3]
https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/drm-misc-next-2025-12-01-1/drivers/gpu/drm/ast/ast_cursor.c?ref_type=tags#L209
+ case DRM_FORMAT_RGB565:
+ ast_set_index_reg_mask(ast, AST_IO_VGACRI, AST_IO_VGACRA2,
0x3f, 0x40);
+ break;
+ case DRM_FORMAT_XRGB8888:
+ ast_set_index_reg_mask(ast, AST_IO_VGACRI, AST_IO_VGACRA2,
0x3f, 0x80);
Where did you get these bits from? According to the docs I have, 0x80
enables BE swapping in general and 0x40 selects 16-bit vs 32-bit swaps.
So the 16-bit case would rather be 0xc0. But I might be wrong, as the
docs are vague.
Did you test 16-bit output?
+ break;
+ default:
+ break;
+ }
+#endif
}
static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index 30578e3b07e4..5c8c0fd2e229 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -75,4 +75,10 @@
#define AST_IO_VGAIR1_R (0x5A)
#define AST_IO_VGAIR1_VREFRESH BIT(3)
+/*
+ * PCI Control
+ */
+
No separate block please. Just put the define between VGACRA1 and
VGACRA3 above.
Please also add constants for setting the bits:
#define AST_IO_VGACRA2_BE_MODE BIT(7)
#define AST_IO_VGACRA2_BE_MODE_16 BIT(6)
Best regards
Thomas
+#define AST_IO_VGACRA2 (0xA2) /* PCI control & big-endian */
+
#endif
--
--
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)
