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