tech-x11 archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Adjusting rasops_info for colourmap size



tl;dr We are implicitly deriving colourmap size in a few places from
bit depth in rasops_info, but that is not always correct, we should
add an explicit bit depth to rasops_info

Some context: (Note: this message repeatedly mentions
wsdisplayio_fbinfo and wsdisplay_fbinfo, which differ only by the
extra 'io' in the middle. Casual readers be aware :)

So, rasops_info is used in various display drivers to hold state.
- https://nxr.netbsd.org/xref/src/sys/dev/rasops/rasops.h#rasops_info

One use of this state is to provision wsdisplayio_fbinfo and the older
wsdisplay_fbinfo to userland via the WSDISPLAYIO_GET_FBINFO and
WSDISPLAYIO_GINFO ioctl calls, which are often used by X servers
- https://nxr.netbsd.org/xref/src/sys/dev/wscons/wsconsio.h#wsdisplayio_fbinfo
- https://nxr.netbsd.org/xref/src/sys/dev/wscons/wsconsio.h#wsdisplay_fbinfo

There is even a convenience method for populating wsdisplayio_fbinfo
from rasops_info
- https://nxr.netbsd.org/xref/src/sys/dev/wscons/wsdisplay_util.c#wsdisplayio_get_fbinfo

However, there is no matching method for the older wsdisplay_fbinfo,
both because it's simpler, and because each usage has an explicit
cmsize, rather than using rasops_info
https://nxr.netbsd.org/xref/src/sys/dev/tc/mfb.c#335

The mfb driver above was picked with malice aforethought - it's a mono
framebuffer with each pixel is stored as a single bit in a byte, so
the cmsize returned by WSDISPLAYIO_GINFO's wsdisplay_fbinfo is
explicitly set to 0. The issue is conceptually similar to that handled
by rasops_info's ri_stride value, but for individual pixels rather
than pixel rows.

However, WSDISPLAYIO_GET_FBINFO's wsdisplayio_fbinfo has a different
value of 256 set via its implicit
"fbi->fbi_subtype.fbi_cmapinfo.cmap_entries = 1 << ri->ri_depth;",
which is in this case incorrect

WSDISPLAYIO_GET_FBINFO's logic treats all 8 bit and lower displays as
colourmap capable, which misrepresents greyscale (or drivers unable to
set colourmaps)

(Having waded through some display drivers I'm a huge fan of
WSDISPLAYIO_GET_FBINFO
and the wsdisplayio_get_fbinfo(), I just think it needs to be tweaked
to handle this case)

My initial thought is to add a cmdepth to rasops_info, which
rasops_init_devcmap() can use to populate the colourmap data, and
wsdisplayio_get_fbinfo() can use to populate wsdisplayio_fbinfo.
- https://nxr.netbsd.org/xref/src/sys/dev/rasops/rasops.c#rasops_init_devcmap
I'm happy enough to take a pass through the tree and update any drivers

I could make it cmsize, which then allow crazy devices to say they
have 8 bit pixel depth, (paint colour thoughts appreciated)

I'd then possibly add a wsdisplayio_ginfo() helper method and ensure
that drivers that have rasops_info are populating either of them using
the same logic (but that's definitely a separate pass and is likely to
get bogged down in a maze of twisty framebuffer drivers, each of them
oh so very different in any number of ways)

David


Home | Main Index | Thread Index | Old Index