Adjusting rasops_info for colourmap size
David Brownlee <[email protected]> Fri, 10 Dec 2021 21:58:06 +0000
| Newsgroups | gmane.os.netbsd.devel.x11 |
|---|---|
| Message-ID | <CAGN_6pY2e1zTh1tVSWFOdrDP0KO+Mm+nU2n4WUaPxA_o1qQAVA@mail.gmail.com> |
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