[PATCH] drm/ast: Fix big-endian support

[PATCH] drm/ast: Fix big-endian support

[René Rebe]

The Aspeed ast drm driver has the frame-buffer RGBX swapped on
big-endian RISC systems. Fix by enabling byte swapping for any
__BIG_ENDIAN config.

Fixes: 12fec1405dd5 ("drm: Initial KMS driver for AST (ASpeed Technologies) 2000 series (v2)")
Signed-off-by: René Rebe <rene@exactco.de>
---
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) {
+	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);
+		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
+ */
+
+#define AST_IO_VGACRA2			(0xA2) /* PCI control & big-endian */
+
 #endif
-- 
2.52.0


-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[Thomas Zimmermann]

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.zimbra@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 <rene@exactco.de>
> ---
> 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)

Re: [PATCH] drm/ast: Fix big-endian support

[René Rebe]

(Resend w/o HTML, sorry!)
Hi,

thank you for the review!

> On 3. Dec 2025, at 10:40, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
> 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.

indeed.

>> big-endian RISC systems. Fix by enabling byte swapping for any
>> __BIG_ENDIAN config.
> 
> Is this the RISC support that Linux famously shot down? :)

Linux shut down RISC? Or Linus the non existing big-endian RISCV? ;-)

> 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.zimbra@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.

Well, in practice this machines are broken for years, would probably be nice to finally get this to production datacenter running Power 9, … or the last SPARC users.

>> Signed-off-by: René Rebe <rene@exactco.de>
>> ---
>> 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.

Ah.

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

The cursor was correct in X, though I tested w/ the mag driver (which I also fixed some more). I’ll try the modesetting driver, too.

> 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

I was not considering buffers in different modes. Can there be different VRAM access of different formats with this simple driver? Because 16 and 32-bit planes / buffers will not work concurrently with global byte-swapping at the same time, ...

>> + 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.

From the spec I found with Google, but I think you are indeed correct, I somehow missed that bit that day.

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

Sure will update, I was just using the style already present with reg magic numbers all over the place :-/

Thanks!
René

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

-- 
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[Thomas Zimmermann]

Hi

Am 03.12.25 um 11:14 schrieb René Rebe:
> (Resend w/o HTML, sorry!)
> Hi,
>
> thank you for the review!
>
>> On 3. Dec 2025, at 10:40, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> 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.
> indeed.
>
>>> big-endian RISC systems. Fix by enabling byte swapping for any
>>> __BIG_ENDIAN config.
>> Is this the RISC support that Linux famously shot down? :)
> Linux shut down RISC? Or Linus the non existing big-endian RISCV? ;-)

Yeah, that's what I meant.

>
>> 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.zimbra@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.
> Well, in practice this machines are broken for years, would probably be nice to finally get this to production datacenter running Power 9, … or the last SPARC users.

OK, leave it in then.

>
>>> Signed-off-by: René Rebe <rene@exactco.de>
>>> ---
>>> 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.
> Ah.
>
>> 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.
> The cursor was correct in X, though I tested w/ the mag driver (which I also fixed some more). I’ll try the modesetting driver, too.

The Matrox driver doesn't have hardware cursor planes. It only provides 
a primary plane and the cursor image is drawn in software by the compositor.

>
>> 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
> I was not considering buffers in different modes. Can there be different VRAM access of different formats with this simple driver? Because 16 and 32-bit planes / buffers will not work concurrently with global byte-swapping at the same time, ...

Yes, primary plane and cursor plane both use a shadow buffer in system 
memory that they copy into video memory on each display update. 
Essentially a memcpy with optimizations.  For the cursor plane, the 
driver also converts to ARGB4444, which is the only useful color format 
that the hardware supports. ARGB4444 is 16-bit wide per pixel, so we 
need to swap accordingly.

If the primary plane uses XRGB8888, it requires 32-bit swapping. Hence 
we have to set the correct mode in each plane's atomic_update helper 
before it updates video memory. It should work if you do it at the 
locations [2] and [3] that I pointed to.

Best regards
Thomas

>
>>> + 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.
>  From the spec I found with Google, but I think you are indeed correct, I somehow missed that bit that day.
>
>> 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)
> Sure will update, I was just using the style already present with reg magic numbers all over the place :-/
>
> Thanks!
> René
>
>> 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)
>>
>>

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

Re: [PATCH] drm/ast: Fix big-endian support

[René Rebe]

Hello Thomas,

On Wed, 3 Dec 2025 10:40:17 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:

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

While working on it I discovered that the Big-Endian byte-swapping
bits do apparently not just-work on a newer AST2400 in our Power 8
while my initial patch did work as tested with an AST2200 in the Sun
T4-1 :-/

Maybe that is what Timothy meant with "This is due to a ppc64 hardware
quirk, which when combined with a hardware design fault in the AST2500
VGA controller results in a need to use software-based red-blue
channel swapping." [1]

Is there a way to simply specify the frame-buffer as BGRX8888? In a
quick test the drm layer complaint about "not supported" and "no
compatible format found"?

Thanks,
	René

[1] https://lore.kernel.org/dri-devel/407388289.1798972.1760725035958.JavaMail.zimbra@raptorengineeringinc.com/

-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[Timothy Pearson]

----- Original Message -----
> From: "René Rebe" <rene@exactco.de>
> To: tzimmermann@suse.de
> Cc: "dri-devel" <dri-devel@lists.freedesktop.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Dave Airlie"
> <airlied@redhat.com>, "Timothy Pearson" <tpearson@raptorengineering.com>
> Sent: Friday, December 5, 2025 9:14:59 AM
> Subject: Re: [PATCH] drm/ast: Fix big-endian support

> Hello Thomas,
> 
> On Wed, 3 Dec 2025 10:40:17 +0100, Thomas Zimmermann <tzimmermann@suse.de>
> wrote:
> 
>> [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
> 
> While working on it I discovered that the Big-Endian byte-swapping
> bits do apparently not just-work on a newer AST2400 in our Power 8
> while my initial patch did work as tested with an AST2200 in the Sun
> T4-1 :-/
> 
> Maybe that is what Timothy meant with "This is due to a ppc64 hardware
> quirk, which when combined with a hardware design fault in the AST2500
> VGA controller results in a need to use software-based red-blue
> channel swapping." [1]
> 
> Is there a way to simply specify the frame-buffer as BGRX8888? In a
> quick test the drm layer complaint about "not supported" and "no
> compatible format found"?

I've been all around that loop.  You can't do that -- the fb code has no idea how to drive such a framebuffer, and elsewhere in the kernel it's made clear that the GPU driver *must* provide a RGBX8888 linear framebuffer if the Linux fb code is going to be able to display a console.

Does the Sun T4 CPU perform automatic byte swapping on PCI[e] data transactions?  That might be the difference; POWER performs the byte swapping, and since the ASpeed device is broken in BE mode we can't swap back by setting the BE register bit in the AST GPU hardware.

Fun fact -- it'll sorta work on the framebuffer side, but we lose the entire control BAR in the process.  ASpeed seems OK with this, they just say something along the lines of "oh, BE is not supported despite our documentation" :facepalm:

Re: [PATCH] drm/ast: Fix big-endian support

[Thomas Zimmermann]

Hi

Am 05.12.25 um 19:31 schrieb Timothy Pearson:
>
> ----- Original Message -----
>> From: "René Rebe" <rene@exactco.de>
>> To: tzimmermann@suse.de
>> Cc: "dri-devel" <dri-devel@lists.freedesktop.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Dave Airlie"
>> <airlied@redhat.com>, "Timothy Pearson" <tpearson@raptorengineering.com>
>> Sent: Friday, December 5, 2025 9:14:59 AM
>> Subject: Re: [PATCH] drm/ast: Fix big-endian support
>> Hello Thomas,
>>
>> On Wed, 3 Dec 2025 10:40:17 +0100, Thomas Zimmermann <tzimmermann@suse.de>
>> wrote:
>>
>>> [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
>> While working on it I discovered that the Big-Endian byte-swapping
>> bits do apparently not just-work on a newer AST2400 in our Power 8
>> while my initial patch did work as tested with an AST2200 in the Sun
>> T4-1 :-/

In the upcoming v6.19-rc1, ast will support per-chip quirks. So we can 
control this by chip version, if necessary

>>
>> Maybe that is what Timothy meant with "This is due to a ppc64 hardware
>> quirk, which when combined with a hardware design fault in the AST2500
>> VGA controller results in a need to use software-based red-blue
>> channel swapping." [1]
>>
>> Is there a way to simply specify the frame-buffer as BGRX8888? In a
>> quick test the drm layer complaint about "not supported" and "no
>> compatible format found"?
> I've been all around that loop.  You can't do that -- the fb code has no idea how to drive such a framebuffer, and elsewhere in the kernel it's made clear that the GPU driver *must* provide a RGBX8888 linear framebuffer if the Linux fb code is going to be able to display a console.
>
> Does the Sun T4 CPU perform automatic byte swapping on PCI[e] data transactions?  That might be the difference; POWER performs the byte swapping, and since the ASpeed device is broken in BE mode we can't swap back by setting the BE register bit in the AST GPU hardware.
>
> Fun fact -- it'll sorta work on the framebuffer side, but we lose the entire control BAR in the process.  ASpeed seems OK with this, they just say something along the lines of "oh, BE is not supported despite our documentation" :facepalm:

On the 2400-and-onwards models, ast could set 
drm_device.mode_config.quirk_addfb_prefer_host_byte_order. If set, the 
format lookup will select a different format on BE machines. [1] For 
example requesting XRGB8888 returns BGRX8888 instead. If ast later sees 
such a format in the atomic_update code, it could transparently swap 
bytes when writing out pixels to the video memory.  IIRC this works 
transparently to DRM clients and fbcon.  I think this would be the 
preferred way of fixing the issue.

[1] 
https://elixir.bootlin.com/linux/v6.18/source/drivers/gpu/drm/drm_fourcc.c#L123

For the pre-2400 chips, I suggest to fix this problem with the hardware 
byte swapping if possible. That seems like the correct approach.

Best regards
Thomas


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

Re: [PATCH] drm/ast: Fix big-endian support

[René Rebe]

Hi,

> On 5. Dec 2025, at 20:46, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
> Hi
> 
> Am 05.12.25 um 19:31 schrieb Timothy Pearson:
>> 
>> ----- Original Message -----
>>> From: "René Rebe" <rene@exactco.de>
>>> To: tzimmermann@suse.de
>>> Cc: "dri-devel" <dri-devel@lists.freedesktop.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Dave Airlie"
>>> <airlied@redhat.com>, "Timothy Pearson" <tpearson@raptorengineering.com>
>>> Sent: Friday, December 5, 2025 9:14:59 AM
>>> Subject: Re: [PATCH] drm/ast: Fix big-endian support
>>> Hello Thomas,
>>> 
>>> On Wed, 3 Dec 2025 10:40:17 +0100, Thomas Zimmermann <tzimmermann@suse.de>
>>> wrote:
>>> 
>>>> [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
>>> While working on it I discovered that the Big-Endian byte-swapping
>>> bits do apparently not just-work on a newer AST2400 in our Power 8
>>> while my initial patch did work as tested with an AST2200 in the Sun
>>> T4-1 :-/
> 
> In the upcoming v6.19-rc1, ast will support per-chip quirks. So we can control this by chip version, if necessary
> 
>>> 
>>> Maybe that is what Timothy meant with "This is due to a ppc64 hardware
>>> quirk, which when combined with a hardware design fault in the AST2500
>>> VGA controller results in a need to use software-based red-blue
>>> channel swapping." [1]
>>> 
>>> Is there a way to simply specify the frame-buffer as BGRX8888? In a
>>> quick test the drm layer complaint about "not supported" and "no
>>> compatible format found"?
>> I've been all around that loop.  You can't do that -- the fb code has no idea how to drive such a framebuffer, and elsewhere in the kernel it's made clear that the GPU driver *must* provide a RGBX8888 linear framebuffer if the Linux fb code is going to be able to display a console.
>> 
>> Does the Sun T4 CPU perform automatic byte swapping on PCI[e] data transactions?  That might be the difference; POWER performs the byte swapping, and since the ASpeed device is broken in BE mode we can't swap back by setting the BE register bit in the AST GPU hardware.
>> 
>> Fun fact -- it'll sorta work on the framebuffer side, but we lose the entire control BAR in the process.  ASpeed seems OK with this, they just say something along the lines of "oh, BE is not supported despite our documentation" :facepalm:
> 
> On the 2400-and-onwards models, ast could set drm_device.mode_config.quirk_addfb_prefer_host_byte_order. If set, the format lookup will select a different format on BE machines. [1] For example requesting XRGB8888 returns BGRX8888 instead. If ast later sees such a format in the atomic_update code, it could transparently swap bytes when writing out pixels to the video memory.  IIRC this works transparently to DRM clients and fbcon.  I think this would be the preferred way of fixing the issue.

Uff, I get better than nothing ;-)

> [1] https://elixir.bootlin.com/linux/v6.18/source/drivers/gpu/drm/drm_fourcc.c#L123
> 
> For the pre-2400 chips, I suggest to fix this problem with the hardware byte swapping if possible. That seems like the correct approach.

I had re-done the code as you suggested, should I send a v2 as tested on the sparc64 t4-1 and we quirk later, non working chips or ppc64 later?

	René

> Best regards
> Thomas

-- 
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[Thomas Zimmermann]

Hi

Am 05.12.25 um 20:50 schrieb René Rebe:
> Hi,
>
>> On 5. Dec 2025, at 20:46, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 05.12.25 um 19:31 schrieb Timothy Pearson:
>>> ----- Original Message -----
>>>> From: "René Rebe" <rene@exactco.de>
>>>> To: tzimmermann@suse.de
>>>> Cc: "dri-devel" <dri-devel@lists.freedesktop.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Dave Airlie"
>>>> <airlied@redhat.com>, "Timothy Pearson" <tpearson@raptorengineering.com>
>>>> Sent: Friday, December 5, 2025 9:14:59 AM
>>>> Subject: Re: [PATCH] drm/ast: Fix big-endian support
>>>> Hello Thomas,
>>>>
>>>> On Wed, 3 Dec 2025 10:40:17 +0100, Thomas Zimmermann <tzimmermann@suse.de>
>>>> wrote:
>>>>
>>>>> [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
>>>> While working on it I discovered that the Big-Endian byte-swapping
>>>> bits do apparently not just-work on a newer AST2400 in our Power 8
>>>> while my initial patch did work as tested with an AST2200 in the Sun
>>>> T4-1 :-/
>> In the upcoming v6.19-rc1, ast will support per-chip quirks. So we can control this by chip version, if necessary
>>
>>>> Maybe that is what Timothy meant with "This is due to a ppc64 hardware
>>>> quirk, which when combined with a hardware design fault in the AST2500
>>>> VGA controller results in a need to use software-based red-blue
>>>> channel swapping." [1]
>>>>
>>>> Is there a way to simply specify the frame-buffer as BGRX8888? In a
>>>> quick test the drm layer complaint about "not supported" and "no
>>>> compatible format found"?
>>> I've been all around that loop.  You can't do that -- the fb code has no idea how to drive such a framebuffer, and elsewhere in the kernel it's made clear that the GPU driver *must* provide a RGBX8888 linear framebuffer if the Linux fb code is going to be able to display a console.
>>>
>>> Does the Sun T4 CPU perform automatic byte swapping on PCI[e] data transactions?  That might be the difference; POWER performs the byte swapping, and since the ASpeed device is broken in BE mode we can't swap back by setting the BE register bit in the AST GPU hardware.
>>>
>>> Fun fact -- it'll sorta work on the framebuffer side, but we lose the entire control BAR in the process.  ASpeed seems OK with this, they just say something along the lines of "oh, BE is not supported despite our documentation" :facepalm:
>> On the 2400-and-onwards models, ast could set drm_device.mode_config.quirk_addfb_prefer_host_byte_order. If set, the format lookup will select a different format on BE machines. [1] For example requesting XRGB8888 returns BGRX8888 instead. If ast later sees such a format in the atomic_update code, it could transparently swap bytes when writing out pixels to the video memory.  IIRC this works transparently to DRM clients and fbcon.  I think this would be the preferred way of fixing the issue.
> Uff, I get better than nothing ;-)

Well, you can set the quirk in mode config. And then in 
ast_handle_damage(), you'll require a switch for the big-endian formats. [1]

ast_handle_damage(...)
{
     ...

     switch (fb->format->format) {
         default:
             drm_fb_memcyp()
             break;
         case DRM_FORMAT_BGRX8888:
         case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
             /* Swap bytes on big-endian formats */
             drm_fb_swab(dst, fb->pitches, src, fb, clip, false, 
fmtcnv_state);
             break;
     }
}

You can get that final argument fmtcnv_state from the DMR shadow-plane 
state. [2]

[1] 
https://elixir.bootlin.com/linux/v6.18/source/drivers/gpu/drm/ast/ast_mode.c#L549
[2] 
https://elixir.bootlin.com/linux/v6.18/source/drivers/gpu/drm/ast/ast_mode.c#L558

Does that fix the color corruption?

>
>> [1] https://elixir.bootlin.com/linux/v6.18/source/drivers/gpu/drm/drm_fourcc.c#L123
>>
>> For the pre-2400 chips, I suggest to fix this problem with the hardware byte swapping if possible. That seems like the correct approach.
> I had re-done the code as you suggested, should I send a v2 as tested on the sparc64 t4-1 and we quirk later, non working chips or ppc64 later?

Not sure what you mean. If splitting by chip model is too complicated, 
we can also do only the software variant that works with all chips.

Thanks for sticking with it.

Best regards
Thomas

>
> 	René
>
>> Best regards
>> Thomas

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

Re: [PATCH] drm/ast: Fix big-endian support

[René Rebe]

Hi,

On Mon, 8 Dec 2025 09:44:23 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:

> >> On the 2400-and-onwards models, ast could set
> >> drm_device.mode_config.quirk_addfb_prefer_host_byte_order. If set, the
> >> format lookup will select a different format on BE machines. [1] For
> >> example requesting XRGB8888 returns BGRX8888 instead. If ast later
> >> sees such a format in the atomic_update code, it could transparently
> >> swap bytes when writing out pixels to the video memory.  IIRC this
> >> works transparently to DRM clients and fbcon.  I think this would be
> >> the preferred way of fixing the issue.
> > Uff, I get better than nothing ;-)
> 
> Well, you can set the quirk in mode config. And then in
> ast_handle_damage(), you'll require a switch for the big-endian
> formats. [1]
> 
> ast_handle_damage(...)
> {
>     ...
> 
>     switch (fb->format->format) {
>         default:
>             drm_fb_memcyp()
>             break;
>         case DRM_FORMAT_BGRX8888:
>         case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
>             /* Swap bytes on big-endian formats */
>             drm_fb_swab(dst, fb->pitches, src, fb, clip, false,
> fmtcnv_state);
>             break;
>     }
> }
> 
> You can get that final argument fmtcnv_state from the DMR shadow-plane
> state. [2]
> 
> [1]
> https://elixir.bootlin.com/linux/v6.18/source/drivers/gpu/drm/ast/ast_mode.c#L549
> [2]
> https://elixir.bootlin.com/linux/v6.18/source/drivers/gpu/drm/ast/ast_mode.c#L558
> 
> Does that fix the color corruption?

Following your suggestions conversion does not want to just work:

root@XCODE_SPARC_T4_1:~# dmesg  | tail
[  105.444761] ast 0000:0a:00.0: AST 2200 detected
[  105.444947] ast 0000:0a:00.0: [drm] dram MCLK=266 Mhz type=2 bus_width=32
[  105.444963] ast 0000:0a:00.0: [drm] Using analog VGA
[  105.445470] [drm] Initialized ast 0.1.0 for 0000:0a:00.0 on minor 0
[  105.673289] ast 0000:0a:00.0: [drm] format BX24 little-endian (0x34325842) not supported
[  105.673302] ast 0000:0a:00.0: [drm] No compatible format found
[  105.673348] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup emulation (ret=-22)
[  105.901306] ast 0000:0a:00.0: [drm] format BX24 little-endian (0x34325842) not supported
[  105.901319] ast 0000:0a:00.0: [drm] No compatible format found
[  105.901350] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup emulation (ret=-22)

WIP w/ BIG_ENDIAN temp commented out to test the code-path on the
otherwise function big-endian byte-swapping SPARC64 AST:

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 2d3ad7610c2e..3f17aa263bdb 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -227,6 +227,12 @@ static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
 			}
 			break;
 		}
+
+#if 0 //def __BIG_ENDIAN
+		/* Big-endian byte-swapping */
+		ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xa2, 0x3f, AST_IO_VGACRA2_BE_MODE_16);
+#endif
+
 		ast_set_cursor_image(ast, argb4444, fb->width, fb->height);
 		ast_set_cursor_base(ast, dst_off);
 	}
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index cd08990a10f9..1065f481ec5f 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -526,12 +526,23 @@ 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));
-	drm_fb_memcpy(&dst, fb->pitches, src, fb, clip);
+
+	switch (fb->format->format) {
+	default:
+		drm_fb_memcpy(&dst, fb->pitches, src, fb, clip);
+		break;
+	case DRM_FORMAT_BGRX8888:
+	case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
+		/* Swap bytes on big-endian formats */
+		drm_fb_swab(&dst, fb->pitches, src, fb, clip, false, fmtcnv_state);
+		break;
+	}
 }
 
 static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
@@ -557,11 +568,25 @@ static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
 		ast_set_vbios_color_reg(ast, fb->format, ast_crtc_state->vmode);
 	}
 
+
 	/* if the buffer comes from another device */
 	if (drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE) == 0) {
+#if 0 // def __BIG_ENDIAN
+		/* Big-endian byte-swapping */
+		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);
@@ -1020,6 +1045,11 @@ int ast_mode_config_init(struct ast_device *ast)
 		dev->mode_config.max_height = 1200;
 	}
 
+#ifdef __BIG_ENDIAN
+	//if (ast->chip >= AST2400)
+		dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
+#endif
+
 	dev->mode_config.helper_private = &ast_mode_config_helper_funcs;
 
 	ret = ast_primary_plane_init(ast);
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index 30578e3b07e4..4e11ece9fce7 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	(AST_IO_VGACRA2_BE_MODE | 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)
-- 
2.52.0

> Thanks for sticking with it.

Of course!

   René

-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[Thomas Zimmermann]

[-- Attachment #1: Type: text/plain, Size: 6930 bytes --]

Hi

Am 09.12.25 um 13:13 schrieb René Rebe:
> Hi,
>
> On Mon, 8 Dec 2025 09:44:23 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>>>> On the 2400-and-onwards models, ast could set
>>>> drm_device.mode_config.quirk_addfb_prefer_host_byte_order. If set, the
>>>> format lookup will select a different format on BE machines. [1] For
>>>> example requesting XRGB8888 returns BGRX8888 instead. If ast later
>>>> sees such a format in the atomic_update code, it could transparently
>>>> swap bytes when writing out pixels to the video memory.  IIRC this
>>>> works transparently to DRM clients and fbcon.  I think this would be
>>>> the preferred way of fixing the issue.
>>> Uff, I get better than nothing ;-)
>> Well, you can set the quirk in mode config. And then in
>> ast_handle_damage(), you'll require a switch for the big-endian
>> formats. [1]
>>
>> ast_handle_damage(...)
>> {
>>      ...
>>
>>      switch (fb->format->format) {
>>          default:
>>              drm_fb_memcyp()
>>              break;
>>          case DRM_FORMAT_BGRX8888:
>>          case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
>>              /* Swap bytes on big-endian formats */
>>              drm_fb_swab(dst, fb->pitches, src, fb, clip, false,
>> fmtcnv_state);
>>              break;
>>      }
>> }
>>
>> You can get that final argument fmtcnv_state from the DMR shadow-plane
>> state. [2]
>>
>> [1]
>> https://elixir.bootlin.com/linux/v6.18/source/drivers/gpu/drm/ast/ast_mode.c#L549
>> [2]
>> https://elixir.bootlin.com/linux/v6.18/source/drivers/gpu/drm/ast/ast_mode.c#L558
>>
>> Does that fix the color corruption?
> Following your suggestions conversion does not want to just work:
>
> root@XCODE_SPARC_T4_1:~# dmesg  | tail
> [  105.444761] ast 0000:0a:00.0: AST 2200 detected
> [  105.444947] ast 0000:0a:00.0: [drm] dram MCLK=266 Mhz type=2 bus_width=32
> [  105.444963] ast 0000:0a:00.0: [drm] Using analog VGA
> [  105.445470] [drm] Initialized ast 0.1.0 for 0000:0a:00.0 on minor 0
> [  105.673289] ast 0000:0a:00.0: [drm] format BX24 little-endian (0x34325842) not supported
> [  105.673302] ast 0000:0a:00.0: [drm] No compatible format found
> [  105.673348] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup emulation (ret=-22)
> [  105.901306] ast 0000:0a:00.0: [drm] format BX24 little-endian (0x34325842) not supported
> [  105.901319] ast 0000:0a:00.0: [drm] No compatible format found
> [  105.901350] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup emulation (ret=-22)

Oh well...

There's a very simple patch attach. Does it fix the problem?

Best regards
Thomas

>
> WIP w/ BIG_ENDIAN temp commented out to test the code-path on the
> otherwise function big-endian byte-swapping SPARC64 AST:
>
> diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
> index 2d3ad7610c2e..3f17aa263bdb 100644
> --- a/drivers/gpu/drm/ast/ast_cursor.c
> +++ b/drivers/gpu/drm/ast/ast_cursor.c
> @@ -227,6 +227,12 @@ static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
>   			}
>   			break;
>   		}
> +
> +#if 0 //def __BIG_ENDIAN
> +		/* Big-endian byte-swapping */
> +		ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xa2, 0x3f, AST_IO_VGACRA2_BE_MODE_16);
> +#endif
> +
>   		ast_set_cursor_image(ast, argb4444, fb->width, fb->height);
>   		ast_set_cursor_base(ast, dst_off);
>   	}
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index cd08990a10f9..1065f481ec5f 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -526,12 +526,23 @@ 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));
> -	drm_fb_memcpy(&dst, fb->pitches, src, fb, clip);
> +
> +	switch (fb->format->format) {
> +	default:
> +		drm_fb_memcpy(&dst, fb->pitches, src, fb, clip);
> +		break;
> +	case DRM_FORMAT_BGRX8888:
> +	case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
> +		/* Swap bytes on big-endian formats */
> +		drm_fb_swab(&dst, fb->pitches, src, fb, clip, false, fmtcnv_state);
> +		break;
> +	}
>   }
>   
>   static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
> @@ -557,11 +568,25 @@ static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
>   		ast_set_vbios_color_reg(ast, fb->format, ast_crtc_state->vmode);
>   	}
>   
> +
>   	/* if the buffer comes from another device */
>   	if (drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE) == 0) {
> +#if 0 // def __BIG_ENDIAN
> +		/* Big-endian byte-swapping */
> +		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);
> @@ -1020,6 +1045,11 @@ int ast_mode_config_init(struct ast_device *ast)
>   		dev->mode_config.max_height = 1200;
>   	}
>   
> +#ifdef __BIG_ENDIAN
> +	//if (ast->chip >= AST2400)
> +		dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
> +#endif
> +
>   	dev->mode_config.helper_private = &ast_mode_config_helper_funcs;
>   
>   	ret = ast_primary_plane_init(ast);
> diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
> index 30578e3b07e4..4e11ece9fce7 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	(AST_IO_VGACRA2_BE_MODE | 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)


[-- Attachment #2: 0001-ast-swap-bytes-on-BE-machines.patch --]
[-- Type: text/x-patch, Size: 1819 bytes --]

From ea2cbca712d6e6e5b3114b89fc41fe51a8376862 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Wed, 10 Dec 2025 09:52:23 +0100
Subject: [PATCH] ast: swap bytes on BE machines

---
 drivers/gpu/drm/ast/ast_mode.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

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

Re: [PATCH] drm/ast: Fix big-endian support

[René Rebe]

Hallo,

On Wed, 10 Dec 2025 09:55:50 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:
...
> >> Does that fix the color corruption?
> > Following your suggestions conversion does not want to just work:
> >
> > root@XCODE_SPARC_T4_1:~# dmesg  | tail
> > [  105.444761] ast 0000:0a:00.0: AST 2200 detected
> > [ 105.444947] ast 0000:0a:00.0: [drm] dram MCLK=266 Mhz type=2
> > bus_width=32
> > [  105.444963] ast 0000:0a:00.0: [drm] Using analog VGA
> > [  105.445470] [drm] Initialized ast 0.1.0 for 0000:0a:00.0 on minor 0
> > [ 105.673289] ast 0000:0a:00.0: [drm] format BX24 little-endian
> > (0x34325842) not supported
> > [  105.673302] ast 0000:0a:00.0: [drm] No compatible format found
> > [ 105.673348] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup
> > emulation (ret=-22)
> > [ 105.901306] ast 0000:0a:00.0: [drm] format BX24 little-endian
> > (0x34325842) not supported
> > [  105.901319] ast 0000:0a:00.0: [drm] No compatible format found
> > [ 105.901350] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup
> > emulation (ret=-22)
> 
> Oh well...
> 
> There's a very simple patch attach. Does it fix the problem?

Yes, only leaving the hardcoded swapping from my patch liek this fixes
the byte-swapped output as expected on the sparc64 Sun T4.

How would you like me to go from here? Just use the chip_id to force
swapping and enable hw swapper for pre-AST2400 chips or fix the
generic format selection to work as you had suggested?

Does the ast_primary_plane_formats need to byte swapped formats for it
to work?

Thanks for sticking with it!

       René

> Best regards
> Thomas

-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[Thomas Zimmermann]

Hi

Am 10.12.25 um 16:33 schrieb René Rebe:
> Hallo,
>
> On Wed, 10 Dec 2025 09:55:50 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> ...
>>>> Does that fix the color corruption?
>>> Following your suggestions conversion does not want to just work:
>>>
>>> root@XCODE_SPARC_T4_1:~# dmesg  | tail
>>> [  105.444761] ast 0000:0a:00.0: AST 2200 detected
>>> [ 105.444947] ast 0000:0a:00.0: [drm] dram MCLK=266 Mhz type=2
>>> bus_width=32
>>> [  105.444963] ast 0000:0a:00.0: [drm] Using analog VGA
>>> [  105.445470] [drm] Initialized ast 0.1.0 for 0000:0a:00.0 on minor 0
>>> [ 105.673289] ast 0000:0a:00.0: [drm] format BX24 little-endian
>>> (0x34325842) not supported
>>> [  105.673302] ast 0000:0a:00.0: [drm] No compatible format found
>>> [ 105.673348] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup
>>> emulation (ret=-22)
>>> [ 105.901306] ast 0000:0a:00.0: [drm] format BX24 little-endian
>>> (0x34325842) not supported
>>> [  105.901319] ast 0000:0a:00.0: [drm] No compatible format found
>>> [ 105.901350] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup
>>> emulation (ret=-22)
>> Oh well...
>>
>> There's a very simple patch attach. Does it fix the problem?
> Yes, only leaving the hardcoded swapping from my patch liek this fixes
> the byte-swapped output as expected on the sparc64 Sun T4.

Great.

>
> How would you like me to go from here? Just use the chip_id to force
> swapping and enable hw swapper for pre-AST2400 chips or fix the
> generic format selection to work as you had suggested?
>
> Does the ast_primary_plane_formats need to byte swapped formats for it
> to work?

I'll send out a full patch that implements the byte swapping. Once 
reviewed, it can be merged quickly. Can I add your Tested-by tag to the 
patch?

Best regards
Thomas

>
> Thanks for sticking with it!
>
>         René
>
>> Best regards
>> Thomas

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

Re: [PATCH] drm/ast: Fix big-endian support

[René Rebe]

Hi,

On Wed, 10 Dec 2025 16:41:50 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Hi
> 
> Am 10.12.25 um 16:33 schrieb René Rebe:
> > Hallo,
> >
> > On Wed, 10 Dec 2025 09:55:50 +0100, Thomas Zimmermann
> > <tzimmermann@suse.de> wrote:
> > ...
> >>>> Does that fix the color corruption?
> >>> Following your suggestions conversion does not want to just work:
> >>>
> >>> root@XCODE_SPARC_T4_1:~# dmesg  | tail
> >>> [  105.444761] ast 0000:0a:00.0: AST 2200 detected
> >>> [ 105.444947] ast 0000:0a:00.0: [drm] dram MCLK=266 Mhz type=2
> >>> bus_width=32
> >>> [  105.444963] ast 0000:0a:00.0: [drm] Using analog VGA
> >>> [  105.445470] [drm] Initialized ast 0.1.0 for 0000:0a:00.0 on minor 0
> >>> [ 105.673289] ast 0000:0a:00.0: [drm] format BX24 little-endian
> >>> (0x34325842) not supported
> >>> [  105.673302] ast 0000:0a:00.0: [drm] No compatible format found
> >>> [ 105.673348] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup
> >>> emulation (ret=-22)
> >>> [ 105.901306] ast 0000:0a:00.0: [drm] format BX24 little-endian
> >>> (0x34325842) not supported
> >>> [  105.901319] ast 0000:0a:00.0: [drm] No compatible format found
> >>> [ 105.901350] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup
> >>> emulation (ret=-22)
> >> Oh well...
> >>
> >> There's a very simple patch attach. Does it fix the problem?
> > Yes, only leaving the hardcoded swapping from my patch liek this fixes
> > the byte-swapped output as expected on the sparc64 Sun T4.
> 
> Great.
> 
> >
> > How would you like me to go from here? Just use the chip_id to force
> > swapping and enable hw swapper for pre-AST2400 chips or fix the
> > generic format selection to work as you had suggested?
> >
> > Does the ast_primary_plane_formats need to byte swapped formats for it
> > to work?
> 
> I'll send out a full patch that implements the byte swapping. Once
> reviewed, it can be merged quickly. Can I add your Tested-by tag to
> the patch?

I'd be happy to finish my work. But if you want to put the last touch
on it now you can add Co-developed-by, too ... and I'll test the final
version.

   René

-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[Thomas Zimmermann]

Hi

Am 10.12.25 um 17:56 schrieb René Rebe:
> Hi,
>
> On Wed, 10 Dec 2025 16:41:50 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>> Hi
>>
>> Am 10.12.25 um 16:33 schrieb René Rebe:
>>> Hallo,
>>>
>>> On Wed, 10 Dec 2025 09:55:50 +0100, Thomas Zimmermann
>>> <tzimmermann@suse.de> wrote:
>>> ...
>>>>>> Does that fix the color corruption?
>>>>> Following your suggestions conversion does not want to just work:
>>>>>
>>>>> root@XCODE_SPARC_T4_1:~# dmesg  | tail
>>>>> [  105.444761] ast 0000:0a:00.0: AST 2200 detected
>>>>> [ 105.444947] ast 0000:0a:00.0: [drm] dram MCLK=266 Mhz type=2
>>>>> bus_width=32
>>>>> [  105.444963] ast 0000:0a:00.0: [drm] Using analog VGA
>>>>> [  105.445470] [drm] Initialized ast 0.1.0 for 0000:0a:00.0 on minor 0
>>>>> [ 105.673289] ast 0000:0a:00.0: [drm] format BX24 little-endian
>>>>> (0x34325842) not supported
>>>>> [  105.673302] ast 0000:0a:00.0: [drm] No compatible format found
>>>>> [ 105.673348] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup
>>>>> emulation (ret=-22)
>>>>> [ 105.901306] ast 0000:0a:00.0: [drm] format BX24 little-endian
>>>>> (0x34325842) not supported
>>>>> [  105.901319] ast 0000:0a:00.0: [drm] No compatible format found
>>>>> [ 105.901350] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup
>>>>> emulation (ret=-22)
>>>> Oh well...
>>>>
>>>> There's a very simple patch attach. Does it fix the problem?
>>> Yes, only leaving the hardcoded swapping from my patch liek this fixes
>>> the byte-swapped output as expected on the sparc64 Sun T4.
>> Great.
>>
>>> How would you like me to go from here? Just use the chip_id to force
>>> swapping and enable hw swapper for pre-AST2400 chips or fix the
>>> generic format selection to work as you had suggested?
>>>
>>> Does the ast_primary_plane_formats need to byte swapped formats for it
>>> to work?
>> I'll send out a full patch that implements the byte swapping. Once
>> reviewed, it can be merged quickly. Can I add your Tested-by tag to
>> the patch?
> I'd be happy to finish my work.

Of course, no problem.

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.

[1] 
https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/drm-misc-next/drivers/gpu/drm/ast/ast_cursor.c#L101

Best regards
Thomas

>   But if you want to put the last touch
> on it now you can add Co-developed-by, too ... and I'll test the final
> version.
>
>     René
>

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

Re: [PATCH] drm/ast: Fix big-endian support

[René Rebe]

Good morning,

On Thu, 11 Dec 2025 08:22:12 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Hi
> 
> Am 10.12.25 um 17:56 schrieb René Rebe:
> > Hi,
> >
> > On Wed, 10 Dec 2025 16:41:50 +0100, Thomas Zimmermann
> > <tzimmermann@suse.de> wrote:
> >
> >> Hi
> >>
> >> Am 10.12.25 um 16:33 schrieb René Rebe:
> >>> Hallo,
> >>>
> >>> On Wed, 10 Dec 2025 09:55:50 +0100, Thomas Zimmermann
> >>> <tzimmermann@suse.de> wrote:
> >>> ...
> >>>>>> Does that fix the color corruption?
> >>>>> Following your suggestions conversion does not want to just work:
> >>>>>
> >>>>> root@XCODE_SPARC_T4_1:~# dmesg  | tail
> >>>>> [  105.444761] ast 0000:0a:00.0: AST 2200 detected
> >>>>> [ 105.444947] ast 0000:0a:00.0: [drm] dram MCLK=266 Mhz type=2
> >>>>> bus_width=32
> >>>>> [  105.444963] ast 0000:0a:00.0: [drm] Using analog VGA
> >>>>> [  105.445470] [drm] Initialized ast 0.1.0 for 0000:0a:00.0 on minor 0
> >>>>> [ 105.673289] ast 0000:0a:00.0: [drm] format BX24 little-endian
> >>>>> (0x34325842) not supported
> >>>>> [  105.673302] ast 0000:0a:00.0: [drm] No compatible format found
> >>>>> [ 105.673348] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup
> >>>>> emulation (ret=-22)
> >>>>> [ 105.901306] ast 0000:0a:00.0: [drm] format BX24 little-endian
> >>>>> (0x34325842) not supported
> >>>>> [  105.901319] ast 0000:0a:00.0: [drm] No compatible format found
> >>>>> [ 105.901350] ast 0000:0a:00.0: [drm] *ERROR* fbdev: Failed to setup
> >>>>> emulation (ret=-22)
> >>>> Oh well...
> >>>>
> >>>> There's a very simple patch attach. Does it fix the problem?
> >>> Yes, only leaving the hardcoded swapping from my patch liek this fixes
> >>> the byte-swapped output as expected on the sparc64 Sun T4.
> >> Great.
> >>
> >>> How would you like me to go from here? Just use the chip_id to force
> >>> swapping and enable hw swapper for pre-AST2400 chips or fix the
> >>> generic format selection to work as you had suggested?
> >>>
> >>> Does the ast_primary_plane_formats need to byte swapped formats for it
> >>> to work?
> >> I'll send out a full patch that implements the byte swapping. Once
> >> reviewed, it can be merged quickly. Can I add your Tested-by tag to
> >> the patch?
> > I'd be happy to finish my work.
> 
> Of course, no problem.
> 
> 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.

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



> [1]
> https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/drm-misc-next/drivers/gpu/drm/ast/ast_cursor.c#L101
> 
> Best regards
> Thomas

-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[Thomas Zimmermann]

[-- Attachment #1: Type: text/plain, Size: 5667 bytes --]

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)


[-- Attachment #2: 0001-drm-ast-Swap-framebuffer-writes-on-big-endian-machin.patch --]
[-- Type: text/x-patch, Size: 4473 bytes --]

From 2777ad050a50091f189df9e1a71b3e6412568890 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Wed, 10 Dec 2025 09:52:23 +0100
Subject: [PATCH] drm/ast: Swap framebuffer writes on big-endian machines
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Swap the pixel data when writing to framebuffer memory on big-endian
machines. Fixes incorrect output. Aspeed graphics does not appear to
support little-endian framebuffers after AST2400, although the feature
has been documented.

There's a lengthy discussion at [1].

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Co-developed-by: René Rebe <rene@exactco.de>
Link: https://lore.kernel.org/dri-devel/20251202.170626.2134482663677806825.rene@exactco.de/ # [1]
---
 drivers/gpu/drm/ast/ast_cursor.c | 27 +++++++++++++++++++++------
 drivers/gpu/drm/ast/ast_mode.c   | 11 +++++++++--
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 30b62d3f0151..02e019eb97c5 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -90,23 +90,37 @@ 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)
+				 unsigned int width, unsigned int height,
+				 struct drm_format_conv_state *fmtcnv_state)
 {
 	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 */
-	memcpy_toio(dst, src, AST_HWC_SIZE);
+#if defined(__BIG_ENDIAN)
+	unsigned int i;
+
+	for (i = 0; i < AST_HWC_SIZE; i += 2)
+		writew(swab16(*(const __be16 *)&src[i]), &dst[i]);
+	dst += AST_HWC_SIZE;
 
 	/* write checksum + signature */
+	writel(swab32(csum), dst);
+	writel(swab32(width), dst + AST_HWC_SIGNATURE_SizeX);
+	writel(swab32(height), dst + AST_HWC_SIGNATURE_SizeY);
+	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTX);
+	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTY);
+#else
+	memcpy_toio(dst, src, AST_HWC_SIZE);
 	dst += AST_HWC_SIZE;
+
+	/* write checksum + signature */
 	writel(csum, dst);
 	writel(width, dst + AST_HWC_SIGNATURE_SizeX);
 	writel(height, dst + AST_HWC_SIGNATURE_SizeY);
 	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX);
 	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY);
+#endif
 }
 
 static void ast_set_cursor_base(struct ast_device *ast, u64 address)
@@ -267,7 +281,8 @@ static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
 								   &damage);
 
 		if (argb4444)
-			ast_set_cursor_image(ast, argb4444, fb->width, fb->height);
+			ast_set_cursor_image(ast, argb4444, fb->width, fb->height,
+					     &shadow_plane_state->fmtcnv_state);
 
 		ast_set_cursor_base(ast, dst_off);
 	}
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

Re: [PATCH] drm/ast: Fix big-endian support

[René Rebe]

Hi,

On Thu, 11 Dec 2025 15:03:48 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:

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

Ok, so you are not a fan of using the hw swapping. I think I asked two
emails ago if having both pathes is acceptable. To be honest this
driver is for some reason already annoyingly slow. Buf of course we
can just keep the sw swapping for now.

>  	/* write checksum + signature */
> +	writel(swab32(csum), dst);
> +	writel(swab32(width), dst + AST_HWC_SIGNATURE_SizeX);
> +	writel(swab32(height), dst + AST_HWC_SIGNATURE_SizeY);
> +	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTX);
> +	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTY);
> +#else
> +	memcpy_toio(dst, src, AST_HWC_SIZE);
>  	dst += AST_HWC_SIZE;
> +
> +	/* write checksum + signature */
>  	writel(csum, dst);
>  	writel(width, dst + AST_HWC_SIGNATURE_SizeX);
>  	writel(height, dst + AST_HWC_SIGNATURE_SizeY);
>  	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX);
>  	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY);
> +#endif

I'm pretty sure this will break the cursor, as the position was
working correctly and I only had to swap the cursor image data. The
csum will also not be identical anyway, as the checksum function
computes it in native byte order. Theoretically that would have to be
changed. However, I do not see where it is really used, maybe only
some special remote desktop vendor protocol that I'm not using. Maybe
the exact checksum does not even matter and is only used as
optimization to not resend an unchanged cursor image.

I'll send a final version after validating it w/ HW later.

Thanks,
	René

-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[Thomas Zimmermann]

Hi

Am 11.12.25 um 15:31 schrieb René Rebe:
> Hi,
>
> On Thu, 11 Dec 2025 15:03:48 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>> 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.
> Ok, so you are not a fan of using the hw swapping. I think I asked two
> emails ago if having both pathes is acceptable. To be honest this
> driver is for some reason already annoyingly slow. Buf of course we
> can just keep the sw swapping for now.

I know. But it's all just a niche thing and I'm not sure if there's much 
benefit for the HW swapper if Aspeed doesn't even care. This has been 
going back and forth for some time. I'd really like to get it done ASAP.

Performance is mostly slow because of software rendering. Really 
improving the copying in the kernel requires a larger commitment and 
refactoring that is out of the scope of the patch. We can talk about 
that, but changes would touch a lot of drivers.

>
>>   	/* write checksum + signature */
>> +	writel(swab32(csum), dst);
>> +	writel(swab32(width), dst + AST_HWC_SIGNATURE_SizeX);
>> +	writel(swab32(height), dst + AST_HWC_SIGNATURE_SizeY);
>> +	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTX);
>> +	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTY);
>> +#else
>> +	memcpy_toio(dst, src, AST_HWC_SIZE);
>>   	dst += AST_HWC_SIZE;
>> +
>> +	/* write checksum + signature */
>>   	writel(csum, dst);
>>   	writel(width, dst + AST_HWC_SIGNATURE_SizeX);
>>   	writel(height, dst + AST_HWC_SIGNATURE_SizeY);
>>   	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX);
>>   	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY);
>> +#endif
> I'm pretty sure this will break the cursor, as the position was
> working correctly and I only had to swap the cursor image data. The
> csum will also not be identical anyway, as the checksum function
> computes it in native byte order. Theoretically that would have to be
> changed. However, I do not see where it is really used, maybe only
> some special remote desktop vendor protocol that I'm not using. Maybe
> the exact checksum does not even matter and is only used as
> optimization to not resend an unchanged cursor image.

Oh well! I though that the bus does implicit byte swaps? Or does 
writel() already swap to little endian, which the AST chip expects? I'm 
confused.

Best regards
Thomas

>
> I'll send a final version after validating it w/ HW later.
>
> Thanks,
> 	René
>

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

Re: [PATCH] drm/ast: Fix big-endian support

[Michel Dänzer]

On 12/11/25 15:56, Thomas Zimmermann wrote:
> Am 11.12.25 um 15:31 schrieb René Rebe:
>> On Thu, 11 Dec 2025 15:03:48 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>>>       /* write checksum + signature */
>>> +    writel(swab32(csum), dst);
>>> +    writel(swab32(width), dst + AST_HWC_SIGNATURE_SizeX);
>>> +    writel(swab32(height), dst + AST_HWC_SIGNATURE_SizeY);
>>> +    writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTX);
>>> +    writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTY);
>>> +#else
>>> +    memcpy_toio(dst, src, AST_HWC_SIZE);
>>>       dst += AST_HWC_SIZE;
>>> +
>>> +    /* write checksum + signature */
>>>       writel(csum, dst);
>>>       writel(width, dst + AST_HWC_SIGNATURE_SizeX);
>>>       writel(height, dst + AST_HWC_SIGNATURE_SizeY);
>>>       writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX);
>>>       writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY);
>>> +#endif
>> I'm pretty sure this will break the cursor, as the position was
>> working correctly and I only had to swap the cursor image data. The
>> csum will also not be identical anyway, as the checksum function
>> computes it in native byte order. Theoretically that would have to be
>> changed. However, I do not see where it is really used, maybe only
>> some special remote desktop vendor protocol that I'm not using. Maybe
>> the exact checksum does not even matter and is only used as
>> optimization to not resend an unchanged cursor image.
> 
> Oh well! I though that the bus does implicit byte swaps? Or does writel() already swap to little endian, which the AST chip expects? I'm confused.

FWIW, writel indeed converts from native byte order to little endian, see include/asm-generic/io.h.

Can't help with your other questions though.


-- 
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast

Re: [PATCH] drm/ast: Fix big-endian support

[René Rebe]

Hi,

On Fri, 12 Dec 2025 16:14:37 +0100, Michel Dänzer <michel.daenzer@mailbox.org> wrote:

> On 12/11/25 15:56, Thomas Zimmermann wrote:
> > Am 11.12.25 um 15:31 schrieb René Rebe:
> >> On Thu, 11 Dec 2025 15:03:48 +0100, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >>>       /* write checksum + signature */
> >>> +    writel(swab32(csum), dst);
> >>> +    writel(swab32(width), dst + AST_HWC_SIGNATURE_SizeX);
> >>> +    writel(swab32(height), dst + AST_HWC_SIGNATURE_SizeY);
> >>> +    writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTX);
> >>> +    writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTY);
> >>> +#else
> >>> +    memcpy_toio(dst, src, AST_HWC_SIZE);
> >>>       dst += AST_HWC_SIZE;
> >>> +
> >>> +    /* write checksum + signature */
> >>>       writel(csum, dst);
> >>>       writel(width, dst + AST_HWC_SIGNATURE_SizeX);
> >>>       writel(height, dst + AST_HWC_SIGNATURE_SizeY);
> >>>       writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX);
> >>>       writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY);
> >>> +#endif
> >> I'm pretty sure this will break the cursor, as the position was
> >> working correctly and I only had to swap the cursor image data. The
> >> csum will also not be identical anyway, as the checksum function
> >> computes it in native byte order. Theoretically that would have to be
> >> changed. However, I do not see where it is really used, maybe only
> >> some special remote desktop vendor protocol that I'm not using. Maybe
> >> the exact checksum does not even matter and is only used as
> >> optimization to not resend an unchanged cursor image.
> > 
> > Oh well! I though that the bus does implicit byte swaps? Or does writel() already swap to little endian, which the AST chip expects? I'm confused.
> 
> FWIW, writel indeed converts from native byte order to little endian, see include/asm-generic/io.h.
> 
> Can't help with your other questions though.

It turns out this writes are not uses for VGA out, I suspect this is
only for a RDP thing. The VGA cursor control is in
ast_cursor_plane_helper_atomic_update [1].

I'm testing a final patch without that and will send it ASAP.

Thanks,
	René

[1] https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/drm-misc-next/drivers/gpu/drm/ast/ast_cursor.c#L279

-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[René Rebe]

Hi,

On Thu, 11 Dec 2025 15:31:01 +0100 (CET), René Rebe <rene@exactco.de> wrote:

> >  	/* write checksum + signature */
> > +	writel(swab32(csum), dst);
> > +	writel(swab32(width), dst + AST_HWC_SIGNATURE_SizeX);
> > +	writel(swab32(height), dst + AST_HWC_SIGNATURE_SizeY);
> > +	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTX);
> > +	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTY);
> > +#else
> > +	memcpy_toio(dst, src, AST_HWC_SIZE);
> >  	dst += AST_HWC_SIZE;
> > +
> > +	/* write checksum + signature */
> >  	writel(csum, dst);
> >  	writel(width, dst + AST_HWC_SIGNATURE_SizeX);
> >  	writel(height, dst + AST_HWC_SIGNATURE_SizeY);
> >  	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX);
> >  	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY);
> > +#endif
> 
> I'm pretty sure this will break the cursor, as the position was
> working correctly and I only had to swap the cursor image data. The
> csum will also not be identical anyway, as the checksum function
> computes it in native byte order. Theoretically that would have to be
> changed. However, I do not see where it is really used, maybe only
> some special remote desktop vendor protocol that I'm not using. Maybe
> the exact checksum does not even matter and is only used as
> optimization to not resend an unchanged cursor image.
> 
> I'll send a final version after validating it w/ HW later.

I just sent a more minimally tested V4 removing the superflous unused
fmt_cnv_state to ast_set_cursor_image you somehow had added.

As the additional writen cursor RDP service writes are untested, I
left them out.

There is still something suspect or buggy: the 2-bit X11 cursor is
filled with transparency, while the ARGB RGB channel work now. Modern
ARGB Xcursor theme also look strange. Not sure if that is just due to
loosing 4-bit precision and thus half of the dynamic range with all
the shadows. To me at least the 2-bit transparent X cursor looks like
a fmt conversion bug in some layer that we would need to continue
debugging another day. At least the framebuffer / installer text would
be more readble upstream now, too ;-)

Thanks,
	René

-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe

Re: [PATCH] drm/ast: Fix big-endian support

[Thomas Zimmermann]

Hi

Am 12.12.25 um 21:15 schrieb René Rebe:
> Hi,
>
> On Thu, 11 Dec 2025 15:31:01 +0100 (CET), René Rebe <rene@exactco.de> wrote:
>
>>>   	/* write checksum + signature */
>>> +	writel(swab32(csum), dst);
>>> +	writel(swab32(width), dst + AST_HWC_SIGNATURE_SizeX);
>>> +	writel(swab32(height), dst + AST_HWC_SIGNATURE_SizeY);
>>> +	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTX);
>>> +	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTY);
>>> +#else
>>> +	memcpy_toio(dst, src, AST_HWC_SIZE);
>>>   	dst += AST_HWC_SIZE;
>>> +
>>> +	/* write checksum + signature */
>>>   	writel(csum, dst);
>>>   	writel(width, dst + AST_HWC_SIGNATURE_SizeX);
>>>   	writel(height, dst + AST_HWC_SIGNATURE_SizeY);
>>>   	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX);
>>>   	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY);
>>> +#endif
>> I'm pretty sure this will break the cursor, as the position was
>> working correctly and I only had to swap the cursor image data. The
>> csum will also not be identical anyway, as the checksum function
>> computes it in native byte order. Theoretically that would have to be
>> changed. However, I do not see where it is really used, maybe only
>> some special remote desktop vendor protocol that I'm not using. Maybe
>> the exact checksum does not even matter and is only used as
>> optimization to not resend an unchanged cursor image.
>>
>> I'll send a final version after validating it w/ HW later.
> I just sent a more minimally tested V4 removing the superflous unused
> fmt_cnv_state to ast_set_cursor_image you somehow had added.

That is a leftover form earlier drm_fb_swab() code.

>
> As the additional writen cursor RDP service writes are untested, I
> left them out.
>
> There is still something suspect or buggy: the 2-bit X11 cursor is
> filled with transparency, while the ARGB RGB channel work now. Modern
> ARGB Xcursor theme also look strange. Not sure if that is just due to
> loosing 4-bit precision and thus half of the dynamic range with all
> the shadows. To me at least the 2-bit transparent X cursor looks like
> a fmt conversion bug in some layer that we would need to continue
> debugging another day. At least the framebuffer / installer text would
> be more readble upstream now, too ;-)

The kernel's ast driver doesn't support 2-bit cursors. It's likely a 
problem somewhere in Xorg or the rendering libraries.

Best regards
Thomas

>
> Thanks,
> 	René
>

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