[PATCH 3/4] drm/cirrus-qemu: Use framebuffer format as-is, drop adjustments

[Thomas Zimmermann]

Hi,

first of all, what about the other patches?

- Patch 1 is a bugfix.
- Patch 4 depends on this one.
- Patch 2 should be given consideration.

Am 26.03.25 um 11:35 schrieb Gerd Hoffmann:
> On Tue, Mar 25, 2025 at 06:12:51PM +0100, Thomas Zimmermann wrote:
>> Remove internal adjustments to framebuffer format from cirrus-qemu
>> driver. The driver did this to support higher resolutions by reducing
>> the per-pixel memory consumption.
>>
>> DRM has a policy of exporting formats as they are implemented in
>> hardware. So avoid internal adjustments if possible.
> Well.  While this policy makes sense for modern hardware this is IMHO
> not the case for the cirrus.
>
> First, because there is almost no userspace which can handle the ancient
> 24 bpp format (DRM_FORMAT_RGB888).

True, there's really just 32 bpp or 16.

>
> Second, because there is no way to communicate the hardware constrains
> of the cirrus.  userspace can query the formats, and userspace can query
> the resolutions, but there is no way to tell userspace that not all
> combinations are valid and that you have to go for the DRM_FORMAT_RGB565
> format if you want higher resolutions.

The viable strategy for user space is to allocate a variety of different 
configs and check them one by one, thus filtering out the ones that 
work. Weston failed to do this for me while I experimented with such 
low-end scenarios. It tried to run an atomic state with a resolution at 
32 bpp, which did not work. Weston just ignored the problem and the 
display went stale. Xorg was clever enough to pick 16-bpp colors.

>
> Essentially the format conversations allows the driver to hide the
> oddities of the prehistoric hardware from userspace, so things are
> more smooth when running wayland on the cirrus.

I'm aware of the situation. We've had similar discussions about other 
low-end hardware, but generally went with the hardware limits.

Please note that there is a trade-off here: the effect of this series is 
that the maximum resolution will be limited to 800x600. If user space 
would appropriately validate atomic states, lower bpp could still 
support higher resolutions. But converting color formats on the fly 
isn't free. I recently did some simple measurements in a different 
context and converting from 32 bpp to 16 bpp took 3 times as long as 
memcpy'ing the raw pixels. See the cover letter of [1]. AFAICT cirrus is 
currently paying CPU overhead for display resolution.

[1] https://patchwork.freedesktop.org/series/146722/#rev1

>
> take care,
>    Gerd
>
> PS: https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> still applies of course.

It's been 10 years since you wrote that. So maybe it's time to 
re-consider cirrus' exceptions and just go for a 'dumb implementation'. 
Anyone can easily switch to better alternatives.

Best regards
Thomas


>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)