Wrong colors on ARAnyM

Wrong colors on ARAnyM

[Geert Uytterhoeven]

Hi all,

While working on an Atari DRM driver for Linux, I discovered a problem
with video modes using 2 bitplanes: the display only shows tints of
blue. 1, 4, and 8 bitplanes work fine.

With debug code added to ARAnyM, it turns out the bad behavior
happens when ARAnyM uses the STe palette registers instead of the
Falcon palette registers. And apparently the issue can also be
reproduced with atafb, and with other color depths:

 1. Boot Linux (I'm using video=atafb:tthigh)

    Everything looks good:

      refreshPalette:319: st_shift = 0
      refreshPalette:354: Fal color 0 = 0x00000000 => 00/00/00
      refreshPalette:354: Fal color 1 = 0x000000a8 => 00/00/aa
      refreshPalette:354: Fal color 2 = 0x00a80000 => 00/aa/00
      refreshPalette:354: Fal color 3 = 0x00a800a8 => 00/aa/aa

 2. Change the color depth a few times:

    $ fbset -depth 1
    $ fbset -depth 2
    $ fbset -depth 4
      ...

    If good: console is white(grey) on black

      refreshPalette:319: st_shift = 0
      refreshPalette:354: Fal color 0 = 0x00000000 => 00/00/00
      refreshPalette:354: Fal color 1 = 0xa8a800a8 => aa/aa/aa
      refreshPalette:354: Fal color 2 = 0x54540054 => 55/55/55
      refreshPalette:354: Fal color 3 = 0xfcfc00fc => ff/ff/ff

    If bad: console is blue on black

      refreshPalette:319: st_shift = 256
      refreshPalette:335: STe color 0 = 0x0000 => 00/00/00
      refreshPalette:335: STe color 1 = 0x0117 => 22/22/ee
      refreshPalette:335: STe color 2 = 0x0000 => 00/00/00
      refreshPalette:335: STe color 3 = 0x0000 => 00/00/00

As a workaround, I fixed ARAnyM to force numColors to 256, so
it will always use the Falcon palette.
https://github.com/aranym/aranym/blob/master/src/videl.cpp#L319

Is this a bug in ARAnyM, or a bug in atafb?
As the problem is not 100% reproducible, it looks like a race condition
in ARAnyM, or a wrong initialization order in atafb.

Thanks!

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: Wrong colors on ARAnyM

[Geert Uytterhoeven]

Hi Michael,

(replying in plain text mode ;-)

On Sun, Feb 13, 2022 at 9:29 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 13/02/22 03:59, Geert Uytterhoeven wrote:
> While working on an Atari DRM driver for Linux, I discovered a problem
> with video modes using 2 bitplanes: the display only shows tints of
> blue. 1, 4, and 8 bitplanes work fine.
>
> With debug code added to ARAnyM, it turns out the bad behavior
> happens when ARAnyM uses the STe palette registers instead of the
> Falcon palette registers. And apparently the issue can also be
> reproduced with atafb, and with other color depths:
>
>  1. Boot Linux (I'm using video=atafb:tthigh)
>
>     Everything looks good:
>
>       refreshPalette:319: st_shift = 0
>       refreshPalette:354: Fal color 0 = 0x00000000 => 00/00/00
>       refreshPalette:354: Fal color 1 = 0x000000a8 => 00/00/aa
>       refreshPalette:354: Fal color 2 = 0x00a80000 => 00/aa/00
>       refreshPalette:354: Fal color 3 = 0x00a800a8 => 00/aa/aa
>
>  2. Change the color depth a few times:
>
>     $ fbset -depth 1
>     $ fbset -depth 2
>
> Depth 2 forces STe palette mode (in decode_var; activated in the Videl in the vbl switcher).
>
>     $ fbset -depth 4
>       ...
>
>     If good: console is white(grey) on black
>
>       refreshPalette:319: st_shift = 0
>       refreshPalette:354: Fal color 0 = 0x00000000 => 00/00/00
>       refreshPalette:354: Fal color 1 = 0xa8a800a8 => aa/aa/aa
>       refreshPalette:354: Fal color 2 = 0x54540054 => 55/55/55
>       refreshPalette:354: Fal color 3 = 0xfcfc00fc => ff/ff/ff
>
>     If bad: console is blue on black
>
>       refreshPalette:319: st_shift = 256
>
> That's still depth 2.
>
>       refreshPalette:335: STe color 0 = 0x0000 => 00/00/00
>       refreshPalette:335: STe color 1 = 0x0117 => 22/22/ee
>       refreshPalette:335: STe color 2 = 0x0000 => 00/00/00
>       refreshPalette:335: STe color 3 = 0x0000 => 00/00/00
>
> As a workaround, I fixed ARAnyM to force numColors to 256, so
> it will always use the Falcon palette.
> https://github.com/aranym/aranym/blob/master/src/videl.cpp#L319
>
> Is this a bug in ARAnyM, or a bug in atafb?
>
> It _looks_ as though changes to f_shift and st_shift are not pushed to hardware in the VBL interrupt. Does ARAnyM run the VBL interrupt regularly?

Yes it does.

> But maybe the kernel and ARAnyM disagree because useStPalette() has this:
>
> int hreg = handleReadW(HW + 0x82); // Too lame!
> // Better way how to make out ST LOW mode wanted
> if (hreg == 0x10 || hreg == 0x17 || hreg == 0x3e) {
> useStPal = true;
>
> }
>
> for st_shift == 0

Linux' falcon_setcolreg() writes to both the Falcon palette registers
and shifter_tt.color_reg[], so shouldn't the STE color palette contain
the correct colors, too?

> As the problem is not 100% reproducible, it looks like a race condition
> in ARAnyM, or a wrong initialization order in atafb.
>
> I'd have to test this on hardware but haven't got fbset on the Falcon. I'll try to locate a binary.

Sorry, you do not need fbset to reproduce this:

    echo N > /sys/class/graphics/fb0/bits_per_pixel

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: Wrong colors on ARAnyM

[Michael Schmitz]

Hi Geert,

On 14/02/22 20:41, Geert Uytterhoeven wrote:
> Hi Michael,
>
> (replying in plain text mode ;-)
Sorry about that - hopefully fixed now.
>
> On Sun, Feb 13, 2022 at 9:29 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
>>
>>   2. Change the color depth a few times:
>>
>>      $ fbset -depth 1
>>      $ fbset -depth 2
>>
>> Depth 2 forces STe palette mode (in decode_var; activated in the Videl in the vbl switcher).
>>
>> [...]
>> It _looks_ as though changes to f_shift and st_shift are not pushed to hardware in the VBL interrupt. Does ARAnyM run the VBL interrupt regularly?
> Yes it does.
>
>> But maybe the kernel and ARAnyM disagree because useStPalette() has this:
>>
>> int hreg = handleReadW(HW + 0x82); // Too lame!
>> // Better way how to make out ST LOW mode wanted
>> if (hreg == 0x10 || hreg == 0x17 || hreg == 0x3e) {
>> useStPal = true;
>>
>> }
>>
>> for st_shift == 0
> Linux' falcon_setcolreg() writes to both the Falcon palette registers
> and shifter_tt.color_reg[], so shouldn't the STE color palette contain
> the correct colors, too?


I don't think it will:

The STe palette is set like this:

                 shifter_tt.color_reg[regno] =
                         (((red & 0xe) >> 1) | ((red & 1) << 3) << 8) |
                         (((green & 0xe) >> 1) | ((green & 1) << 3) << 4) |
                         ((blue & 0xe) >> 1) | ((blue & 1) << 3);

with all colours downshifted by 12 bits already. Rewriting that in the 
same form as is used in falcon_setcolreg(), i.e. without the downshift:

                 shifter_tt.color_reg[regno] =
                         (((red & 0xe000) >> 13) | ((red & 1000) >> 9) 
<< 8) |
                         (((green & 0xe000) >> 13) | ((green & 1000) >> 
9) << 4) |
                         ((blue & 0xe000) >> 13) | ((blue & 1000) >> 9);


Now look at falcon_setcolreg:

                 shifter_tt.color_reg[regno] =
                         (((red & 0xe000) >> 13) | ((red & 0x1000) >> 
12) << 8) |
                         (((green & 0xe000) >> 13) | ((green & 0x1000) 
 >> 12) << 4) |
                         ((blue & 0xe000) >> 13) | ((blue & 0x1000) >> 12);


My guess would be the two ought to be identical, assuming the STe 
palette registers provide backwards hardware compatibility.

Either way, I think we've got operator precedence wrong here. Shift 
takes precedence over bitwise and / or? With that precedence, what we 
have in summary is

                 shifter_tt.color_reg[regno] =
                         (((red & 0xe000) >> 13) | ((red & 0x1000) >> 4)) |
                         (((green & 0xe000) >> 13) | ((green & 0x1000) 
 >> 8)) |
                         ((blue & 0xe000) >> 13) | ((blue & 0x1000) >> 12);

Only the blue bits ever entirely seem to end up where they ought to. 
Testing this hypothesis on ARAnyM (without getting it to boot all the 
way, for some odd reason likely to do with my current .config) shows 
that the current code results in the shifter_tt.color_reg entries either 
5 or 0x17, 0x113 or 0x117. Adding the missing parentheses results in 
what I'd expect to see.

This bug has been present since at least 2.4.30. I do recall seeing this 
odd blue console before on the Falcon, so it's been a real bug for ages. 
Did the operator precedence change sometime since?

>
>> As the problem is not 100% reproducible, it looks like a race condition
>> in ARAnyM, or a wrong initialization order in atafb.
>>
>> I'd have to test this on hardware but haven't got fbset on the Falcon. I'll try to locate a binary.
> Sorry, you do not need fbset to reproduce this:
>
>      echo N > /sys/class/graphics/fb0/bits_per_pixel

Thanks, that's easy enough to try.

Playing around with that command a bit, it does appear that the second 
switch after selecting depth 2 turns the console blue (regardless of 
what depth is set in that step). And the second switch after that (even 
if setting depth 2) restores normal colours. No race that I can see 
there, it's quite reproducible.

Now what would cause us to miss every other palette update or depth change?

I'll verify that the bug is gone once I've rebuilt with a sane .config. 
But I think we've got enough to fix at least the annoying STe palette 
bug. Unless you want to keep that alive until we've found out what 
causes us to miss odd updates...

Cheers,

     Michael


>
> 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: Wrong colors on ARAnyM

[Michael Schmitz]

Hi Geert,

On 15/02/22 09:50, Michael Schmitz wrote:
>
> Either way, I think we've got operator precedence wrong here. Shift 
> takes precedence over bitwise and / or? With that precedence, what we 
> have in summary is
>
>                 shifter_tt.color_reg[regno] =
>                         (((red & 0xe000) >> 13) | ((red & 0x1000) >> 
> 4)) |
>                         (((green & 0xe000) >> 13) | ((green & 0x1000) 
> >> 8)) |
>                         ((blue & 0xe000) >> 13) | ((blue & 0x1000) >> 
> 12);
>
> Only the blue bits ever entirely seem to end up where they ought to. 
> Testing this hypothesis on ARAnyM (without getting it to boot all the 
> way, for some odd reason likely to do with my current .config) shows 
> that the current code results in the shifter_tt.color_reg entries 
> either 5 or 0x17, 0x113 or 0x117. Adding the missing parentheses 
> results in what I'd expect to see.
>
I can confirm that the bug is gone with additional parentheses to apply 
the shifts (<<8 for red, <<4 for green) to the result of the bit-wise 
or, not just the right-hand portion of it.

Do you want me to prepare a patch, or would you prefer to do it yourself?

Cheers,

     Michael

Re: Wrong colors on ARAnyM

[Geert Uytterhoeven]

Hi Michael,

On Mon, Feb 14, 2022 at 9:50 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 14/02/22 20:41, Geert Uytterhoeven wrote:
> > On Sun, Feb 13, 2022 at 9:29 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
> >>
> >>   2. Change the color depth a few times:
> >>
> >>      $ fbset -depth 1
> >>      $ fbset -depth 2
> >>
> >> Depth 2 forces STe palette mode (in decode_var; activated in the Videl in the vbl switcher).
> >>
> >> [...]
> >> It _looks_ as though changes to f_shift and st_shift are not pushed to hardware in the VBL interrupt. Does ARAnyM run the VBL interrupt regularly?
> > Yes it does.
> >
> >> But maybe the kernel and ARAnyM disagree because useStPalette() has this:
> >>
> >> int hreg = handleReadW(HW + 0x82); // Too lame!
> >> // Better way how to make out ST LOW mode wanted
> >> if (hreg == 0x10 || hreg == 0x17 || hreg == 0x3e) {
> >> useStPal = true;
> >>
> >> }
> >>
> >> for st_shift == 0
> > Linux' falcon_setcolreg() writes to both the Falcon palette registers
> > and shifter_tt.color_reg[], so shouldn't the STE color palette contain
> > the correct colors, too?
>
> I don't think it will:
>
> The STe palette is set like this:
>
>                  shifter_tt.color_reg[regno] =
>                          (((red & 0xe) >> 1) | ((red & 1) << 3) << 8) |
>                          (((green & 0xe) >> 1) | ((green & 1) << 3) << 4) |
>                          ((blue & 0xe) >> 1) | ((blue & 1) << 3);
>
> with all colours downshifted by 12 bits already. Rewriting that in the
> same form as is used in falcon_setcolreg(), i.e. without the downshift:
>
>                  shifter_tt.color_reg[regno] =
>                          (((red & 0xe000) >> 13) | ((red & 1000) >> 9)
> << 8) |
>                          (((green & 0xe000) >> 13) | ((green & 1000) >>
> 9) << 4) |
>                          ((blue & 0xe000) >> 13) | ((blue & 1000) >> 9);
>
>
> Now look at falcon_setcolreg:
>
>                  shifter_tt.color_reg[regno] =
>                          (((red & 0xe000) >> 13) | ((red & 0x1000) >>
> 12) << 8) |
>                          (((green & 0xe000) >> 13) | ((green & 0x1000)
>  >> 12) << 4) |
>                          ((blue & 0xe000) >> 13) | ((blue & 0x1000) >> 12);
>
>
> My guess would be the two ought to be identical, assuming the STe
> palette registers provide backwards hardware compatibility.
>
> Either way, I think we've got operator precedence wrong here. Shift
> takes precedence over bitwise and / or? With that precedence, what we
> have in summary is
>
>                  shifter_tt.color_reg[regno] =
>                          (((red & 0xe000) >> 13) | ((red & 0x1000) >> 4)) |
>                          (((green & 0xe000) >> 13) | ((green & 0x1000)
>  >> 8)) |
>                          ((blue & 0xe000) >> 13) | ((blue & 0x1000) >> 12);
>
> Only the blue bits ever entirely seem to end up where they ought to.
> Testing this hypothesis on ARAnyM (without getting it to boot all the
> way, for some odd reason likely to do with my current .config) shows
> that the current code results in the shifter_tt.color_reg entries either
> 5 or 0x17, 0x113 or 0x117. Adding the missing parentheses results in
> what I'd expect to see.

Oops.

> This bug has been present since at least 2.4.30. I do recall seeing this
> odd blue console before on the Falcon, so it's been a real bug for ages.

I recall seeing an odd blue console before, too.
I always dismissed it as some artefact of redrawing the screen in
a different color mode, like we used to have when switching from a
16-color to a monochrome text console (remember the underlined text?).

Did/do you see it on the real Falcon, too?
Initially, I suspected the real Falcon always using the Falcon palette,
so the bug was never seen on real hardware. But the following comment
makes me think the colors have always been wrong if bpp == 2:

        /* 2 planes must use STE compatibility mode */
        par->hw.falcon.ste_mode = bpp == 2;

> Did the operator precedence change sometime since?

;-)

> >> As the problem is not 100% reproducible, it looks like a race condition
> >> in ARAnyM, or a wrong initialization order in atafb.
> >>
> >> I'd have to test this on hardware but haven't got fbset on the Falcon. I'll try to locate a binary.
> > Sorry, you do not need fbset to reproduce this:
> >
> >      echo N > /sys/class/graphics/fb0/bits_per_pixel
>
> Thanks, that's easy enough to try.
>
> Playing around with that command a bit, it does appear that the second
> switch after selecting depth 2 turns the console blue (regardless of
> what depth is set in that step). And the second switch after that (even
> if setting depth 2) restores normal colours. No race that I can see
> there, it's quite reproducible.

Yes, it is quite reproducible, but it doesn't always happen every
second switch. As my Atari DRM driver cannot switch mode on the fly,
I have to initialize the driver with the mode I want to use. Booting
with a mode with 2 bitplanes gives me the wrong colors in ca. 20%
of the cases. So it is random.

> Now what would cause us to miss every other palette update or depth change?

That's a good question! Why does ARAnyM sometimes decide to use the
Falcon palette, and sometimes the STe palette, and not only with
bpp == 2, but also with other color depths?

> I'll verify that the bug is gone once I've rebuilt with a sane .config.
> But I think we've got enough to fix at least the annoying STe palette
> bug. Unless you want to keep that alive until we've found out what
> causes us to miss odd updates...

And:

> I can confirm that the bug is gone with additional parentheses to apply
> the shifts (<<8 for red, <<4 for green) to the result of the bit-wise
> or, not just the right-hand portion of it.
>
> Do you want me to prepare a patch, or would you prefer to do it yourself?

Feel free to send a patch.
If you don't get to it, I'll make one, eventually.
Thanks!

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: Wrong colors on ARAnyM

[Petr Stehlík]

On Út, 2022-02-15 at 09:11 +0100, Geert Uytterhoeven wrote:
> > Now what would cause us to miss every other palette update or depth
> > change?
> 
> That's a good question! Why does ARAnyM sometimes decide to use the
> Falcon palette, and sometimes the STe palette, and not only with
> bpp == 2, but also with other color depths?

just wanted to let you know that I'm following you. If you happen to
identify any bug in ARAnyM I'll try to fix it ASAP.

Thanks,

Petr

Re: Wrong colors on ARAnyM

[Michael Schmitz]

Hi Geert,

On 15/02/22 21:11, Geert Uytterhoeven wrote:
>>> Linux' falcon_setcolreg() writes to both the Falcon palette registers
>>> and shifter_tt.color_reg[], so shouldn't the STE color palette contain
>>> the correct colors, too?
>> I don't think it will:
>>
>> The STe palette is set like this:
>>
>>                   shifter_tt.color_reg[regno] =
>>                           (((red & 0xe) >> 1) | ((red & 1) << 3) << 8) |
>>                           (((green & 0xe) >> 1) | ((green & 1) << 3) << 4) |
>>                           ((blue & 0xe) >> 1) | ((blue & 1) << 3);
>>
>> with all colours downshifted by 12 bits already. Rewriting that in the
>> same form as is used in falcon_setcolreg(), i.e. without the downshift:
>>
>>                   shifter_tt.color_reg[regno] =
>>                           (((red & 0xe000) >> 13) | ((red & 1000) >> 9)
>> << 8) |
>>                           (((green & 0xe000) >> 13) | ((green & 1000) >>
>> 9) << 4) |
>>                           ((blue & 0xe000) >> 13) | ((blue & 1000) >> 9);
>>
>>
>> Now look at falcon_setcolreg:
>>
>>                   shifter_tt.color_reg[regno] =
>>                           (((red & 0xe000) >> 13) | ((red & 0x1000) >>
>> 12) << 8) |
>>                           (((green & 0xe000) >> 13) | ((green & 0x1000)
>>   >> 12) << 4) |
>>                           ((blue & 0xe000) >> 13) | ((blue & 0x1000) >> 12);
>>
>>
>> My guess would be the two ought to be identical, assuming the STe
>> palette registers provide backwards hardware compatibility.

The atafb driver references a 'linux/tools/hardware.txt' file for a 
description of the Videl registers. Does anyone still have a copy of 
that file?

Failing that - how would I go about setting specific text colours for 
the console (or draw a test pattern showing the result of the four 
palette entries)?

>>
>> Either way, I think we've got operator precedence wrong here. Shift
>> takes precedence over bitwise and / or? With that precedence, what we
>> have in summary is
>>
>>                   shifter_tt.color_reg[regno] =
>>                           (((red & 0xe000) >> 13) | ((red & 0x1000) >> 4)) |
>>                           (((green & 0xe000) >> 13) | ((green & 0x1000)
>>   >> 8)) |
>>                           ((blue & 0xe000) >> 13) | ((blue & 0x1000) >> 12);
>>
>> Only the blue bits ever entirely seem to end up where they ought to.
>> Testing this hypothesis on ARAnyM (without getting it to boot all the
>> way, for some odd reason likely to do with my current .config) shows
>> that the current code results in the shifter_tt.color_reg entries either
>> 5 or 0x17, 0x113 or 0x117. Adding the missing parentheses results in
>> what I'd expect to see.
> Oops.
>
>> This bug has been present since at least 2.4.30. I do recall seeing this
>> odd blue console before on the Falcon, so it's been a real bug for ages.
> I recall seeing an odd blue console before, too.
> I always dismissed it as some artefact of redrawing the screen in
> a different color mode, like we used to have when switching from a
> 16-color to a monochrome text console (remember the underlined text?).
Can't say I do ... but I don't use the console much at all (keyboard 
beginning to act up).
> Did/do you see it on the real Falcon, too?
Didn't try yet, sorry.
> Initially, I suspected the real Falcon always using the Falcon palette,
> so the bug was never seen on real hardware. But the following comment
> makes me think the colors have always been wrong if bpp == 2:
>
>          /* 2 planes must use STE compatibility mode */
>          par->hw.falcon.ste_mode = bpp == 2;
>
>> Did the operator precedence change sometime since?
> ;-)
>
>>>> As the problem is not 100% reproducible, it looks like a race condition
>>>> in ARAnyM, or a wrong initialization order in atafb.
>>>>
>>>> I'd have to test this on hardware but haven't got fbset on the Falcon. I'll try to locate a binary.
>>> Sorry, you do not need fbset to reproduce this:
>>>
>>>       echo N > /sys/class/graphics/fb0/bits_per_pixel
>> Thanks, that's easy enough to try.
>>
>> Playing around with that command a bit, it does appear that the second
>> switch after selecting depth 2 turns the console blue (regardless of
>> what depth is set in that step). And the second switch after that (even
>> if setting depth 2) restores normal colours. No race that I can see
>> there, it's quite reproducible.
> Yes, it is quite reproducible, but it doesn't always happen every
> second switch. As my Atari DRM driver cannot switch mode on the fly,
> I have to initialize the driver with the mode I want to use. Booting
> with a mode with 2 bitplanes gives me the wrong colors in ca. 20%
> of the cases. So it is random.
Yes, the 'every second switch' does appear to be a little random, too. 
One boot I get that behavior, the next it's behaving as it should.
>> Now what would cause us to miss every other palette update or depth change?
> That's a good question! Why does ARAnyM sometimes decide to use the
> Falcon palette, and sometimes the STe palette, and not only with
> bpp == 2, but also with other color depths?

I guess we'll have to wait and see whether I see that random behaviour 
on the actual hardware.

Looking at getBpp() in the ARAnyM source, I see that ST shifter modes 
are used if bits 10, 8 and 4 in f_shift are clear, but the selection of 
mono vs. STe 2 bpp modes is different from what's used in the kernel driver:

The kernel uses videl.st_shift & 0x300 == 0x100 for STe mode, and == 
0x200 for mono. ARAnyM uses == 0x01 for STe mode in getBpp(). In 
useStPalette(), == 0x100 is used. Both read from the same register 
offset. The kernel never sets 0x01 for st_shift, so the code in getBpp() 
can't work as far as I can see.

>
>> I can confirm that the bug is gone with additional parentheses to apply
>> the shifts (<<8 for red, <<4 for green) to the result of the bit-wise
>> or, not just the right-hand portion of it.
>>
>> Do you want me to prepare a patch, or would you prefer to do it yourself?
> Feel free to send a patch.
> If you don't get to it, I'll make one, eventually.
> Thanks!

My pleasure - checkpatch may grumble a litte over the additonal line 
length but I think we can ignore that!

Cheers,

     Michael

>
> 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: Wrong colors on ARAnyM

[Michael Schmitz]

Hi Geert,

Am 15.02.2022 um 21:11 schrieb Geert Uytterhoeven:
>> Only the blue bits ever entirely seem to end up where they ought to.
>> Testing this hypothesis on ARAnyM (without getting it to boot all the
>> way, for some odd reason likely to do with my current .config) shows
>> that the current code results in the shifter_tt.color_reg entries either
>> 5 or 0x17, 0x113 or 0x117. Adding the missing parentheses results in
>> what I'd expect to see.
>
> Oops.
>
>> This bug has been present since at least 2.4.30. I do recall seeing this
>> odd blue console before on the Falcon, so it's been a real bug for ages.
>
> I recall seeing an odd blue console before, too.
> I always dismissed it as some artefact of redrawing the screen in
> a different color mode, like we used to have when switching from a
> 16-color to a monochrome text console (remember the underlined text?).
>
> Did/do you see it on the real Falcon, too?

I do see the blue text colour bug, but not the random behaviour in 
switching to 2 bpp, on the actual hardware. So there appear to be two 
distinct bugs - the STe colour map entries tend towards the blue, and 
switching to that palette does not always happen when it should.

Still no idea what causes ARAnyM to miss palette updates or use the 
wrong palette at times ...

>> I can confirm that the bug is gone with additional parentheses to apply
>> the shifts (<<8 for red, <<4 for green) to the result of the bit-wise
>> or, not just the right-hand portion of it.
>>
>> Do you want me to prepare a patch, or would you prefer to do it yourself?
>
> Feel free to send a patch.

Done (now that I've tested on hardware).

Cheers,

	Michael


> If you don't get to it, I'll make one, eventually.
> Thanks!
>
> 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: Wrong colors on ARAnyM

[Geert Uytterhoeven]

Hi Michael,

On Tue, Feb 15, 2022 at 9:49 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 15/02/22 21:11, Geert Uytterhoeven wrote:
> >>> Linux' falcon_setcolreg() writes to both the Falcon palette registers
> >>> and shifter_tt.color_reg[], so shouldn't the STE color palette contain
> >>> the correct colors, too?
> >> I don't think it will:
> >>
> >> The STe palette is set like this:
> >>
> >>                   shifter_tt.color_reg[regno] =
> >>                           (((red & 0xe) >> 1) | ((red & 1) << 3) << 8) |
> >>                           (((green & 0xe) >> 1) | ((green & 1) << 3) << 4) |
> >>                           ((blue & 0xe) >> 1) | ((blue & 1) << 3);
> >>
> >> with all colours downshifted by 12 bits already. Rewriting that in the
> >> same form as is used in falcon_setcolreg(), i.e. without the downshift:
> >>
> >>                   shifter_tt.color_reg[regno] =
> >>                           (((red & 0xe000) >> 13) | ((red & 1000) >> 9)
> >> << 8) |
> >>                           (((green & 0xe000) >> 13) | ((green & 1000) >>
> >> 9) << 4) |
> >>                           ((blue & 0xe000) >> 13) | ((blue & 1000) >> 9);
> >>
> >>
> >> Now look at falcon_setcolreg:
> >>
> >>                   shifter_tt.color_reg[regno] =
> >>                           (((red & 0xe000) >> 13) | ((red & 0x1000) >>
> >> 12) << 8) |
> >>                           (((green & 0xe000) >> 13) | ((green & 0x1000)
> >>   >> 12) << 4) |
> >>                           ((blue & 0xe000) >> 13) | ((blue & 0x1000) >> 12);
> >>
> >>
> >> My guess would be the two ought to be identical, assuming the STe
> >> palette registers provide backwards hardware compatibility.
>
> The atafb driver references a 'linux/tools/hardware.txt' file for a
> description of the Videl registers. Does anyone still have a copy of
> that file?

At least full-history-linux doesn't have it.

> Failing that - how would I go about setting specific text colours for
> the console (or draw a test pattern showing the result of the four
> palette entries)?

I typically use fbtest for that.

I also have a script to show the 16 colors (assumed there are 16):

#!/bin/bash
printf "\e[7m"
printf "\e[30m                    BLACK                     \n"
printf "\e[31m                    RED                       \n"
printf "\e[32m                    GREEN                     \n"
printf "\e[33m                    YELLOW                    \n"
printf "\e[34m                    BLUE                      \n"
printf "\e[35m                    MAGENTA                   \n"
printf "\e[36m                    CYAN                      \n"
printf "\e[37m                    WHITE                     \n"
printf "\e[1m"
printf "\e[30m                    BLACK                     \n"
printf "\e[31m                    RED                       \n"
printf "\e[32m                    GREEN                     \n"
printf "\e[33m                    YELLOW                    \n"
printf "\e[34m                    BLUE                      \n"
printf "\e[35m                    MAGENTA                   \n"
printf "\e[36m                    CYAN                      \n"
printf "\e[37m                    WHITE                     \n"
printf "\e[0m"

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: Wrong colors on ARAnyM

[Michael Schmitz]

Hi Geert,

On 16/02/22 21:24, Geert Uytterhoeven wrote:
>
>> The atafb driver references a 'linux/tools/hardware.txt' file for a
>> description of the Videl registers. Does anyone still have a copy of
>> that file?
> At least full-history-linux doesn't have it.
Might be a reference to the old FTP archive at Erlangen then ...
>> Failing that - how would I go about setting specific text colours for
>> the console (or draw a test pattern showing the result of the four
>> palette entries)?
> I typically use fbtest for that.
One more reason to hunt for that in the dino age package archives :-)
>
> I also have a script to show the 16 colors (assumed there are 16):
>
> #!/bin/bash
> printf "\e[7m"
> printf "\e[30m                    BLACK                     \n"
> printf "\e[31m                    RED                       \n"
> printf "\e[32m                    GREEN                     \n"
> printf "\e[33m                    YELLOW                    \n"
> printf "\e[34m                    BLUE                      \n"
> printf "\e[35m                    MAGENTA                   \n"
> printf "\e[36m                    CYAN                      \n"
> printf "\e[37m                    WHITE                     \n"
> printf "\e[1m"
> printf "\e[30m                    BLACK                     \n"
> printf "\e[31m                    RED                       \n"
> printf "\e[32m                    GREEN                     \n"
> printf "\e[33m                    YELLOW                    \n"
> printf "\e[34m                    BLUE                      \n"
> printf "\e[35m                    MAGENTA                   \n"
> printf "\e[36m                    CYAN                      \n"
> printf "\e[37m                    WHITE                     \n"
> printf "\e[0m"
>
Thanks, that ought to do - I vaguely remembered there should be control 
sequences to set colour (having worked on VT100 and VT2xx emulators, 
though not the actual hardware terminals) but wasn't too sure.

Cheers,

     Michael


> 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: Wrong colors on ARAnyM

[Michael Schmitz]

Hi Petr,

all good with the palette handling.

There is another issue I ran into while testing atafb's handling of 16 
bit video modes though:

When specifying non-default video modes, the kernel parameter

video=atafb:R320;240;16

is used (as an example). Using this in ARAnyM, the command line ends on

video=atafb:R320

and anything specified past the video option on the command line is lost 
(along with xres and depth).

The same mode option is passed on to the kernel correctly by ataboot on 
my Falcon.

Could you look into what in ARAnyMs parsing of the config file or the 
'Args' config item causes truncation of the kernel options?

This can be trivially worked around by using another token separator, 
and Geert is the only one who ever tried 16 bit video modes as far as 
I've seen, so nothing urgent ...

Cheers,

	Michael


Am 15.02.2022 um 22:16 schrieb Petr Stehlík:
> On Út, 2022-02-15 at 09:11 +0100, Geert Uytterhoeven wrote:
>>> Now what would cause us to miss every other palette update or depth
>>> change?
>>
>> That's a good question! Why does ARAnyM sometimes decide to use the
>> Falcon palette, and sometimes the STe palette, and not only with
>> bpp == 2, but also with other color depths?
>
> just wanted to let you know that I'm following you. If you happen to
> identify any bug in ARAnyM I'll try to fix it ASAP.
>
> Thanks,
>
> Petr
>
>

Re: Wrong colours on ARAnyM

[Thorsten Glaser]

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

Dixi quod…

> On Sat, 12 Feb 2022, Geert Uytterhoeven wrote:
> 
> > While working on an Atari DRM driver for Linux, I discovered a problem
> > with video modes using 2 bitplanes: the display only shows tints of
> > blue. 1, 4, and 8 bitplanes work fine.
> 
> Adrian pointed me to this thread with a graphics problem I just
> noticed; I’m not sure it’s related.
> 
> aranym:amd64 (= 1.1.0-2) from bullseye.
> 
> I updated my VM from vmlinux-5.4.0-3-m68k to vmlinux-5.16.0-3-m68k
> and get broken display; it boots fine (ssh works) though.
> 
> Screenshot attached.

Resending with screenshot cropped due to mailing list size limits.

bye,
//mirabilos
-- 
«MyISAM tables -will- get corrupted eventually. This is a fact of life. »
“mysql is about as much database as ms access” – “MSSQL at least descends
from a database” “it's a rebranded SyBase” “MySQL however was born from a
flatfile and went downhill from there” – “at least jetDB doesn’t claim to
be a database”	(#nosec)    ‣‣‣ Please let MySQL and MariaDB finally die!

[-- Attachment #2: Type: image/png, Size: 47757 bytes --]