[PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Linmao Li]

The SuperBlitter operations derive an integer byte count per pixel.  The
accelerated fill path handles only one-, two- and four-byte pixels.
However, the operations are currently installed for every external
framebuffer in SuperVidel RAM, including planar 1/2/4/8-bpp and 24-bpp
truecolor modes accepted by the external video parser.

For 1/2/4-bpp modes, the byte count becomes zero, so accelerated copies do
nothing and fills fall through to 32-bit stores.  Planar 8-bpp uses an
incompatible memory layout.  For 24-bpp modes, fills also use 32-bit stores
despite advancing addresses by three bytes per pixel.  These cases can
corrupt the framebuffer beyond the requested rectangle.

Enable the SuperBlitter operations only for the layouts they implement:
8-bpp packed pixels and 16/32-bpp truecolor.  Keep the existing software
operations for all other external formats.

Fixes: d463633d63e6 ("fbdev: atafb: Add support for SuperVidel's SuperBlitter")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/video/fbdev/atafb.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
index 5bca34c45cef3..c3011b61a94b9 100644
--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -3360,7 +3360,11 @@ static int __init atafb_probe(struct platform_device *pdev)
 		memset (screen_base, 0, external_len);
 
 		/* framebuffer in SV RAM: enable the SuperBlitter */
-		if (external_addr >= 0xa0000000) {
+		if (external_addr >= 0xa0000000 &&
+		    ((external_pmode == FB_TYPE_PACKED_PIXELS &&
+		      external_depth == 8) ||
+		     (external_pmode == -1 &&
+		      (external_depth == 16 || external_depth == 32)))) {
 			svblit_regs = ioremap(SVBLIT_REGS_PHYS, 0x100);
 			if (svblit_regs) {
 				svblit_fw = svblit_rd(SVBLIT_VERSION) & 0x1ff;
-- 
2.25.1

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Michael Schmitz]

Hi Linmao,

thanks for your patch!

Am 27.08.2026 um 21:39 schrieb Linmao Li:
> The SuperBlitter operations derive an integer byte count per pixel.  The
> accelerated fill path handles only one-, two- and four-byte pixels.
> However, the operations are currently installed for every external
> framebuffer in SuperVidel RAM, including planar 1/2/4/8-bpp and 24-bpp
> truecolor modes accepted by the external video parser.
>
> For 1/2/4-bpp modes, the byte count becomes zero, so accelerated copies do
> nothing and fills fall through to 32-bit stores.  Planar 8-bpp uses an
> incompatible memory layout.  For 24-bpp modes, fills also use 32-bit stores
> despite advancing addresses by three bytes per pixel.  These cases can
> corrupt the framebuffer beyond the requested rectangle.

I believe 24 bpp mode can be rescued using something like this (entirely 
untested):

--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -2463,6 +2463,11 @@ static void svblit_fillrect(struct fb_info *info,
         case 2:
                 memset16((u16 *)line, pix, rect->width);
                 break;
+       case 3:
+               memset(line, pix, ((rect->width * bytespp) % 4));
+               line += ((rect->width * bytespp) % 4);
+               memset32((u32 *)line, pix, (rect->width * bytespp) / 4);
+               break;
         default:
                 memset32((u32 *)line, pix, rect->width);
                 break;

Can't test this on hardware, and would need Miro to confirm it works as 
intended.

>
> Enable the SuperBlitter operations only for the layouts they implement:
> 8-bpp packed pixels and 16/32-bpp truecolor.  Keep the existing software
> operations for all other external formats.
>
> Fixes: d463633d63e6 ("fbdev: atafb: Add support for SuperVidel's SuperBlitter")
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> ---
>  drivers/video/fbdev/atafb.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
> index 5bca34c45cef3..c3011b61a94b9 100644
> --- a/drivers/video/fbdev/atafb.c
> +++ b/drivers/video/fbdev/atafb.c
> @@ -3360,7 +3360,11 @@ static int __init atafb_probe(struct platform_device *pdev)
>  		memset (screen_base, 0, external_len);
>
>  		/* framebuffer in SV RAM: enable the SuperBlitter */
> -		if (external_addr >= 0xa0000000) {
> +		if (external_addr >= 0xa0000000 &&
> +		    ((external_pmode == FB_TYPE_PACKED_PIXELS &&
> +		      external_depth == 8) ||
> +		     (external_pmode == -1 &&
> +		      (external_depth == 16 || external_depth == 32)))) {
>  			svblit_regs = ioremap(SVBLIT_REGS_PHYS, 0x100);
>  			if (svblit_regs) {
>  				svblit_fw = svblit_rd(SVBLIT_VERSION) & 0x1ff;
>

Otherwise, LGTM.

@Geert: can you remember if pixel format or bit depth of an external 
framebuffer can be changed at runtime using fbset??

Reviewed-by: Michael Schmitz <schmitzmic@gmail.com>

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Geert Uytterhoeven]

Hi Michael,

CC linux-m68k

On Sun, 6 Sept 2026 at 03:49, Michael Schmitz <schmitzmic@gmail.com> wrote:
> Am 27.08.2026 um 21:39 schrieb Linmao Li:
> > The SuperBlitter operations derive an integer byte count per pixel.  The
> > accelerated fill path handles only one-, two- and four-byte pixels.
> > However, the operations are currently installed for every external
> > framebuffer in SuperVidel RAM, including planar 1/2/4/8-bpp and 24-bpp
> > truecolor modes accepted by the external video parser.
> >
> > For 1/2/4-bpp modes, the byte count becomes zero, so accelerated copies do
> > nothing and fills fall through to 32-bit stores.  Planar 8-bpp uses an
> > incompatible memory layout.  For 24-bpp modes, fills also use 32-bit stores
> > despite advancing addresses by three bytes per pixel.  These cases can
> > corrupt the framebuffer beyond the requested rectangle.
>
> I believe 24 bpp mode can be rescued using something like this (entirely
> untested):
>
> --- a/drivers/video/fbdev/atafb.c
> +++ b/drivers/video/fbdev/atafb.c
> @@ -2463,6 +2463,11 @@ static void svblit_fillrect(struct fb_info *info,
>          case 2:
>                  memset16((u16 *)line, pix, rect->width);
>                  break;
> +       case 3:
> +               memset(line, pix, ((rect->width * bytespp) % 4));
> +               line += ((rect->width * bytespp) % 4);
> +               memset32((u32 *)line, pix, (rect->width * bytespp) / 4);

This uses unaligned writes for the largest part.
You can avoid that by doing the memset32() first.

> +               break;
>          default:
>                  memset32((u32 *)line, pix, rect->width);
>                  break;
>
> Can't test this on hardware, and would need Miro to confirm it works as
> intended.
>
> >
> > Enable the SuperBlitter operations only for the layouts they implement:
> > 8-bpp packed pixels and 16/32-bpp truecolor.  Keep the existing software
> > operations for all other external formats.
> >
> > Fixes: d463633d63e6 ("fbdev: atafb: Add support for SuperVidel's SuperBlitter")
> > Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> > ---
> >  drivers/video/fbdev/atafb.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
> > index 5bca34c45cef3..c3011b61a94b9 100644
> > --- a/drivers/video/fbdev/atafb.c
> > +++ b/drivers/video/fbdev/atafb.c
> > @@ -3360,7 +3360,11 @@ static int __init atafb_probe(struct platform_device *pdev)
> >               memset (screen_base, 0, external_len);
> >
> >               /* framebuffer in SV RAM: enable the SuperBlitter */
> > -             if (external_addr >= 0xa0000000) {
> > +             if (external_addr >= 0xa0000000 &&
> > +                 ((external_pmode == FB_TYPE_PACKED_PIXELS &&
> > +                   external_depth == 8) ||
> > +                  (external_pmode == -1 &&
> > +                   (external_depth == 16 || external_depth == 32)))) {
> >                       svblit_regs = ioremap(SVBLIT_REGS_PHYS, 0x100);
> >                       if (svblit_regs) {
> >                               svblit_fw = svblit_rd(SVBLIT_VERSION) & 0x1ff;
> >
>
> Otherwise, LGTM.
>
> @Geert: can you remember if pixel format or bit depth of an external
> framebuffer can be changed at runtime using fbset??

I am no Atari expert, but IIRC all external framebuffers rely on
being fully set-up before Linux boots?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Miro Kropáček]

Hi Linmao, hi Michael,

On Sun, 6 Sept 2026 at 11:49, Michael Schmitz <schmitzmic@gmail.com> wrote:
>
> Hi Linmao,
>
> thanks for your patch!
>
> Am 27.08.2026 um 21:39 schrieb Linmao Li:
> > The SuperBlitter operations derive an integer byte count per pixel.  The
> > accelerated fill path handles only one-, two- and four-byte pixels.
> > However, the operations are currently installed for every external
> > framebuffer in SuperVidel RAM, including planar 1/2/4/8-bpp and 24-bpp
> > truecolor modes accepted by the external video parser.
> >
> > For 1/2/4-bpp modes, the byte count becomes zero, so accelerated copies do
> > nothing and fills fall through to 32-bit stores.  Planar 8-bpp uses an
> > incompatible memory layout.  For 24-bpp modes, fills also use 32-bit stores
> > despite advancing addresses by three bytes per pixel.  These cases can
> > corrupt the framebuffer beyond the requested rectangle.
>
> I believe 24 bpp mode can be rescued using something like this (entirely
> untested):
>
> --- a/drivers/video/fbdev/atafb.c
> +++ b/drivers/video/fbdev/atafb.c
> @@ -2463,6 +2463,11 @@ static void svblit_fillrect(struct fb_info *info,
>          case 2:
>                  memset16((u16 *)line, pix, rect->width);
>                  break;
> +       case 3:
> +               memset(line, pix, ((rect->width * bytespp) % 4));
> +               line += ((rect->width * bytespp) % 4);
> +               memset32((u32 *)line, pix, (rect->width * bytespp) / 4);
> +               break;
>          default:
>                  memset32((u32 *)line, pix, rect->width);
>                  break;
>
> Can't test this on hardware, and would need Miro to confirm it works as
> intended.

Linmao's patch: I ran it on a Falcon with SuperVidel at
1920x1080x32bpp, all good.

Michael's 24bpp fill: I cannot test it because the SuperVidel has no
24bpp video mode. It was planned but never implemented. Its mode
register offers 1/2/4/8-bit bitplane, 8-bit chunky, 16-bit highcolor
and 32-bit truecolor (24-bit RGB plus an alpha byte), with the
remaining code reserved - and TOS offers no 24bpp mode to set.

Unless I overlooked something, the fill also looks wrong on paper: the
first line is filled with a repeated byte and then with repeated
32-bit words of the same value, which cannot reproduce a 3-byte pixel
pattern unless all three bytes are equal.

On the fbset question: no, the pixel format, depth and geometry of an
external framebuffer cannot be changed at runtime here. fbset -depth
16 and fbset -xres 1024 -yres 768 both return success and change
nothing, because ext_decode_var() only validates against the values
from the boot argument, ext_encode_var() re-derives every field from
the external_* globals and ext_set_par() is empty. Only a reboot with
a different atafb:external: argument changes the mode.

-- 
http://mikro.atari.org

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Michael Schmitz]

Hi Miro,

On 7/09/26 23:14, Miro Kropáček wrote:
> Hi Linmao, hi Michael,
>
> On Sun, 6 Sept 2026 at 11:49, Michael Schmitz <schmitzmic@gmail.com> wrote:
>> Hi Linmao,
>>
>> thanks for your patch!
>>
>> Am 27.08.2026 um 21:39 schrieb Linmao Li:
>>> The SuperBlitter operations derive an integer byte count per pixel.  The
>>> accelerated fill path handles only one-, two- and four-byte pixels.
>>> However, the operations are currently installed for every external
>>> framebuffer in SuperVidel RAM, including planar 1/2/4/8-bpp and 24-bpp
>>> truecolor modes accepted by the external video parser.
>>>
>>> For 1/2/4-bpp modes, the byte count becomes zero, so accelerated copies do
>>> nothing and fills fall through to 32-bit stores.  Planar 8-bpp uses an
>>> incompatible memory layout.  For 24-bpp modes, fills also use 32-bit stores
>>> despite advancing addresses by three bytes per pixel.  These cases can
>>> corrupt the framebuffer beyond the requested rectangle.
>> I believe 24 bpp mode can be rescued using something like this (entirely
>> untested):
>>
>> --- a/drivers/video/fbdev/atafb.c
>> +++ b/drivers/video/fbdev/atafb.c
>> @@ -2463,6 +2463,11 @@ static void svblit_fillrect(struct fb_info *info,
>>           case 2:
>>                   memset16((u16 *)line, pix, rect->width);
>>                   break;
>> +       case 3:
>> +               memset(line, pix, ((rect->width * bytespp) % 4));
>> +               line += ((rect->width * bytespp) % 4);
>> +               memset32((u32 *)line, pix, (rect->width * bytespp) / 4);
>> +               break;
>>           default:
>>                   memset32((u32 *)line, pix, rect->width);
>>                   break;
>>
>> Can't test this on hardware, and would need Miro to confirm it works as
>> intended.
> Linmao's patch: I ran it on a Falcon with SuperVidel at
> 1920x1080x32bpp, all good.
>
> Michael's 24bpp fill: I cannot test it because the SuperVidel has no
> 24bpp video mode. It was planned but never implemented. Its mode
> register offers 1/2/4/8-bit bitplane, 8-bit chunky, 16-bit highcolor
> and 32-bit truecolor (24-bit RGB plus an alpha byte), with the
> remaining code reserved - and TOS offers no 24bpp mode to set.
Without a 24 bpp mode, the 24 bpp fill is pointless, so no point trying 
to keep that mode alive.
> Unless I overlooked something, the fill also looks wrong on paper: the
> first line is filled with a repeated byte and then with repeated
> 32-bit words of the same value, which cannot reproduce a 3-byte pixel
> pattern unless all three bytes are equal.

You're correct - we'd need a memset24() to account for that. Without 
hardware support this is an academic exercise that I don't have time for :-)

> On the fbset question: no, the pixel format, depth and geometry of an
> external framebuffer cannot be changed at runtime here. fbset -depth
> 16 and fbset -xres 1024 -yres 768 both return success and change
> nothing, because ext_decode_var() only validates against the values
> from the boot argument, ext_encode_var() re-derives every field from
> the external_* globals and ext_set_par() is empty. Only a reboot with
> a different atafb:external: argument changes the mode.

All good then - do you need my Acked-by for Linmao's patch, Helge?

Cheers,

Michael

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Michael Schmitz]

Hi Geert,

On 7/09/26 20:38, Geert Uytterhoeven wrote:
> Hi Michael,
>
> CC linux-m68k
>
> On Sun, 6 Sept 2026 at 03:49, Michael Schmitz <schmitzmic@gmail.com> wrote:
>> Am 27.08.2026 um 21:39 schrieb Linmao Li:
>>> The SuperBlitter operations derive an integer byte count per pixel.  The
>>> accelerated fill path handles only one-, two- and four-byte pixels.
>>> However, the operations are currently installed for every external
>>> framebuffer in SuperVidel RAM, including planar 1/2/4/8-bpp and 24-bpp
>>> truecolor modes accepted by the external video parser.
>>>
>>> For 1/2/4-bpp modes, the byte count becomes zero, so accelerated copies do
>>> nothing and fills fall through to 32-bit stores.  Planar 8-bpp uses an
>>> incompatible memory layout.  For 24-bpp modes, fills also use 32-bit stores
>>> despite advancing addresses by three bytes per pixel.  These cases can
>>> corrupt the framebuffer beyond the requested rectangle.
>> I believe 24 bpp mode can be rescued using something like this (entirely
>> untested):
>>
>> --- a/drivers/video/fbdev/atafb.c
>> +++ b/drivers/video/fbdev/atafb.c
>> @@ -2463,6 +2463,11 @@ static void svblit_fillrect(struct fb_info *info,
>>           case 2:
>>                   memset16((u16 *)line, pix, rect->width);
>>                   break;
>> +       case 3:
>> +               memset(line, pix, ((rect->width * bytespp) % 4));
>> +               line += ((rect->width * bytespp) % 4);
>> +               memset32((u32 *)line, pix, (rect->width * bytespp) / 4);
> This uses unaligned writes for the largest part.
> You can avoid that by doing the memset32() first.

True enough (iff line happens to be aligned, which is only true if 
rect->dx is even).

But as Miro pointed out, 24 bpp can't actually be used (and without TOS 
setting such a mode before Linux boot, it can't ever happen).

Cheers,

     Michael


>
>> +               break;
>>           default:
>>                   memset32((u32 *)line, pix, rect->width);
>>                   break;
>>
>> Can't test this on hardware, and would need Miro to confirm it works as
>> intended.
>>
>>> Enable the SuperBlitter operations only for the layouts they implement:
>>> 8-bpp packed pixels and 16/32-bpp truecolor.  Keep the existing software
>>> operations for all other external formats.
>>>
>>> Fixes: d463633d63e6 ("fbdev: atafb: Add support for SuperVidel's SuperBlitter")
>>> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
>>> ---
>>>   drivers/video/fbdev/atafb.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
>>> index 5bca34c45cef3..c3011b61a94b9 100644
>>> --- a/drivers/video/fbdev/atafb.c
>>> +++ b/drivers/video/fbdev/atafb.c
>>> @@ -3360,7 +3360,11 @@ static int __init atafb_probe(struct platform_device *pdev)
>>>                memset (screen_base, 0, external_len);
>>>
>>>                /* framebuffer in SV RAM: enable the SuperBlitter */
>>> -             if (external_addr >= 0xa0000000) {
>>> +             if (external_addr >= 0xa0000000 &&
>>> +                 ((external_pmode == FB_TYPE_PACKED_PIXELS &&
>>> +                   external_depth == 8) ||
>>> +                  (external_pmode == -1 &&
>>> +                   (external_depth == 16 || external_depth == 32)))) {
>>>                        svblit_regs = ioremap(SVBLIT_REGS_PHYS, 0x100);
>>>                        if (svblit_regs) {
>>>                                svblit_fw = svblit_rd(SVBLIT_VERSION) & 0x1ff;
>>>
>> Otherwise, LGTM.
>>
>> @Geert: can you remember if pixel format or bit depth of an external
>> framebuffer can be changed at runtime using fbset??
> I am no Atari expert, but IIRC all external framebuffers rely on
> being fully set-up before Linux boots?
>
> Gr{oetje,eeting}s,
>
>                          Geert
>

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Geert Uytterhoeven]

Hi Michael,

On Mon, 7 Sept 2026 at 22:14, Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 7/09/26 23:14, Miro Kropáček wrote:
> > On Sun, 6 Sept 2026 at 11:49, Michael Schmitz <schmitzmic@gmail.com> wrote:
> >> Hi Linmao,
> >>
> >> thanks for your patch!
> >>
> >> Am 27.08.2026 um 21:39 schrieb Linmao Li:
> >>> The SuperBlitter operations derive an integer byte count per pixel.  The
> >>> accelerated fill path handles only one-, two- and four-byte pixels.
> >>> However, the operations are currently installed for every external
> >>> framebuffer in SuperVidel RAM, including planar 1/2/4/8-bpp and 24-bpp
> >>> truecolor modes accepted by the external video parser.
> >>>
> >>> For 1/2/4-bpp modes, the byte count becomes zero, so accelerated copies do
> >>> nothing and fills fall through to 32-bit stores.  Planar 8-bpp uses an
> >>> incompatible memory layout.  For 24-bpp modes, fills also use 32-bit stores
> >>> despite advancing addresses by three bytes per pixel.  These cases can
> >>> corrupt the framebuffer beyond the requested rectangle.
> >> I believe 24 bpp mode can be rescued using something like this (entirely
> >> untested):
> >>
> >> --- a/drivers/video/fbdev/atafb.c
> >> +++ b/drivers/video/fbdev/atafb.c
> >> @@ -2463,6 +2463,11 @@ static void svblit_fillrect(struct fb_info *info,
> >>           case 2:
> >>                   memset16((u16 *)line, pix, rect->width);
> >>                   break;
> >> +       case 3:
> >> +               memset(line, pix, ((rect->width * bytespp) % 4));
> >> +               line += ((rect->width * bytespp) % 4);
> >> +               memset32((u32 *)line, pix, (rect->width * bytespp) / 4);
> >> +               break;
> >>           default:
> >>                   memset32((u32 *)line, pix, rect->width);
> >>                   break;
> >>
> >> Can't test this on hardware, and would need Miro to confirm it works as
> >> intended.
> > Linmao's patch: I ran it on a Falcon with SuperVidel at
> > 1920x1080x32bpp, all good.
> >
> > Michael's 24bpp fill: I cannot test it because the SuperVidel has no
> > 24bpp video mode. It was planned but never implemented. Its mode
> > register offers 1/2/4/8-bit bitplane, 8-bit chunky, 16-bit highcolor
> > and 32-bit truecolor (24-bit RGB plus an alpha byte), with the
> > remaining code reserved - and TOS offers no 24bpp mode to set.
> Without a 24 bpp mode, the 24 bpp fill is pointless, so no point trying
> to keep that mode alive.
> > Unless I overlooked something, the fill also looks wrong on paper: the
> > first line is filled with a repeated byte and then with repeated
> > 32-bit words of the same value, which cannot reproduce a 3-byte pixel
> > pattern unless all three bytes are equal.
>
> You're correct - we'd need a memset24() to account for that. Without

Unless the frame buffer would be using a DIRECT_COLOR visual (which
is not the case), then you could draw pixels in the 16 console colors
by replicating the color index in all 3 bytes ;-)

> hardware support this is an academic exercise that I don't have time for :-)

True. And as this code is used only to fill the first line (the blitter
replicates it into the other lines), the performance difference between
a simple byte loop and a true memset24() won't be that large.


Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Helge Deller]

On 9/7/26 22:14, Michael Schmitz wrote:
> Hi Miro,
> 
> On 7/09/26 23:14, Miro Kropáček wrote:
>> Hi Linmao, hi Michael,
>>
>> On Sun, 6 Sept 2026 at 11:49, Michael Schmitz <schmitzmic@gmail.com> wrote:
>>> Hi Linmao,
>>>
>>> thanks for your patch!
>>>
>>> Am 27.08.2026 um 21:39 schrieb Linmao Li:
>>>> The SuperBlitter operations derive an integer byte count per pixel.  The
>>>> accelerated fill path handles only one-, two- and four-byte pixels.
>>>> However, the operations are currently installed for every external
>>>> framebuffer in SuperVidel RAM, including planar 1/2/4/8-bpp and 24-bpp
>>>> truecolor modes accepted by the external video parser.
>>>>
>>>> For 1/2/4-bpp modes, the byte count becomes zero, so accelerated copies do
>>>> nothing and fills fall through to 32-bit stores.  Planar 8-bpp uses an
>>>> incompatible memory layout.  For 24-bpp modes, fills also use 32-bit stores
>>>> despite advancing addresses by three bytes per pixel.  These cases can
>>>> corrupt the framebuffer beyond the requested rectangle.
>>> I believe 24 bpp mode can be rescued using something like this (entirely
>>> untested):
>>>
>>> --- a/drivers/video/fbdev/atafb.c
>>> +++ b/drivers/video/fbdev/atafb.c
>>> @@ -2463,6 +2463,11 @@ static void svblit_fillrect(struct fb_info *info,
>>>           case 2:
>>>                   memset16((u16 *)line, pix, rect->width);
>>>                   break;
>>> +       case 3:
>>> +               memset(line, pix, ((rect->width * bytespp) % 4));
>>> +               line += ((rect->width * bytespp) % 4);
>>> +               memset32((u32 *)line, pix, (rect->width * bytespp) / 4);
>>> +               break;
>>>           default:
>>>                   memset32((u32 *)line, pix, rect->width);
>>>                   break;
>>>
>>> Can't test this on hardware, and would need Miro to confirm it works as
>>> intended.
>> Linmao's patch: I ran it on a Falcon with SuperVidel at
>> 1920x1080x32bpp, all good.
>>
>> Michael's 24bpp fill: I cannot test it because the SuperVidel has no
>> 24bpp video mode. It was planned but never implemented. Its mode
>> register offers 1/2/4/8-bit bitplane, 8-bit chunky, 16-bit highcolor
>> and 32-bit truecolor (24-bit RGB plus an alpha byte), with the
>> remaining code reserved - and TOS offers no 24bpp mode to set.
> Without a 24 bpp mode, the 24 bpp fill is pointless, so no point trying to keep that mode alive.
>> Unless I overlooked something, the fill also looks wrong on paper: the
>> first line is filled with a repeated byte and then with repeated
>> 32-bit words of the same value, which cannot reproduce a 3-byte pixel
>> pattern unless all three bytes are equal.
> 
> You're correct - we'd need a memset24() to account for that. Without hardware support this is an academic exercise that I don't have time for :-)
> 
>> On the fbset question: no, the pixel format, depth and geometry of an
>> external framebuffer cannot be changed at runtime here. fbset -depth
>> 16 and fbset -xres 1024 -yres 768 both return success and change
>> nothing, because ext_decode_var() only validates against the values
>> from the boot argument, ext_encode_var() re-derives every field from
>> the external_* globals and ext_set_par() is empty. Only a reboot with
>> a different atafb:external: argument changes the mode.
> 
> All good then - do you need my Acked-by for Linmao's patch, Helge?

No, I think your Reviewed-by is sufficient.

But I added a
Tested-by: Miro Kropáček <miro.kropacek@gmail.com>
tag, if that's Ok for you, Miro?
It's always good to know if a patch has been tested.

With that, I just added the patch to the fbdev git tree.

Thanks!
Helge

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Miro Kropáček]

On Fri, 11 Sept 2026 at 06:39, Helge Deller <deller@gmx.de> wrote:
> But I added a
> Tested-by: Miro Kropáček <miro.kropacek@gmail.com>
> tag, if that's Ok for you, Miro?
No problem.

-- 
http://mikro.atari.org

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Michael Schmitz]

Thanks Helge,

Am 11.09.2026 um 08:39 schrieb Helge Deller:
>> All good then - do you need my Acked-by for Linmao's patch, Helge?
>
> No, I think your Reviewed-by is sufficient.
>
> But I added a
> Tested-by: Miro Kropáček <miro.kropacek@gmail.com>
> tag, if that's Ok for you, Miro?
> It's always good to know if a patch has been tested.
>
> With that, I just added the patch to the fbdev git tree.

All good then, much appreciated!

Cheers,

	Michael

>
> Thanks!
> Helge

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Michael Schmitz]

Hi Geert,

Am 08.09.2026 um 19:18 schrieb Geert Uytterhoeven:
>>> Unless I overlooked something, the fill also looks wrong on paper: the
>>> first line is filled with a repeated byte and then with repeated
>>> 32-bit words of the same value, which cannot reproduce a 3-byte pixel
>>> pattern unless all three bytes are equal.
>>
>> You're correct - we'd need a memset24() to account for that. Without
>
> Unless the frame buffer would be using a DIRECT_COLOR visual (which
> is not the case), then you could draw pixels in the 16 console colors
> by replicating the color index in all 3 bytes ;-)

Sorry, I don't get that - are the console colors chosen judiciously to 
allow for that?

>> hardware support this is an academic exercise that I don't have time for :-)
>
> True. And as this code is used only to fill the first line (the blitter
> replicates it into the other lines), the performance difference between
> a simple byte loop and a true memset24() won't be that large.

I wasn't worried about performance as the blitter takes care of most of 
the work (though we may schedule away from svblit_fillrect() while the 
blitter is active). Getting the 'simple byte loop' right is more 
important. My sample code got everything wrong in fact.

There's a reason why we don't work with 24 bit sized objects usually, 
and probably a reason why 24 bpp was never implemented for the 
SuperVidel :-)

Cheers,

	Michael

>
>
> Gr{oetje,eeting}s,
>
>                         Geert
>

Re: [PATCH] fbdev: atafb: Restrict SuperBlitter to supported formats

[Geert Uytterhoeven]

Hi Michael,

On Fri, 11 Sept 2026 at 06:53, Michael Schmitz <schmitzmic@gmail.com> wrote:
> Am 08.09.2026 um 19:18 schrieb Geert Uytterhoeven:
> >>> Unless I overlooked something, the fill also looks wrong on paper: the
> >>> first line is filled with a repeated byte and then with repeated
> >>> 32-bit words of the same value, which cannot reproduce a 3-byte pixel
> >>> pattern unless all three bytes are equal.
> >>
> >> You're correct - we'd need a memset24() to account for that. Without
> >
> > Unless the frame buffer would be using a DIRECT_COLOR visual (which
> > is not the case), then you could draw pixels in the 16 console colors
> > by replicating the color index in all 3 bytes ;-)
>
> Sorry, I don't get that - are the console colors chosen judiciously to
> allow for that?

Sorry, I don't remember if we still do that.

> >> hardware support this is an academic exercise that I don't have time for :-)
> >
> > True. And as this code is used only to fill the first line (the blitter
> > replicates it into the other lines), the performance difference between
> > a simple byte loop and a true memset24() won't be that large.
>
> I wasn't worried about performance as the blitter takes care of most of
> the work (though we may schedule away from svblit_fillrect() while the
> blitter is active). Getting the 'simple byte loop' right is more
> important. My sample code got everything wrong in fact.
>
> There's a reason why we don't work with 24 bit sized objects usually,
> and probably a reason why 24 bpp was never implemented for the
> SuperVidel :-)

Yeah, 24bpp is tricky.  And in this case we don't even have to deal with a
different endianness ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds