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@xxxxxxxxx> 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@xxxxxxxxxxxxxx
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
