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

[Gerd Hoffmann]

On Wed, Mar 26, 2025 at 01:30:13PM +0100, Thomas Zimmermann wrote:
> 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.

Looks reasonable to me.  Don't feel like giving a reviewed-by due to not
following drm development very closely any more, so

Acked-by: Gerd Hoffmann <kraxel at redhat.com>

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

Last time I checked (which has been a few years) alot of existing
software didn't do that.  Maybe that changed with atomic becoming
more mature though.

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

Ah, ok, this is how you deal with the problem, go with the max
resolution the cirrus can support at 32 bpp.

Could be more explicit in the commit message, especially the 800x600
limit, there is a high chance people search for that when their display
resolution changes after a kernel update.

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

Ouch.  That is alot.

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

Fair enough.

With an improved commit message:
Acked-by: Gerd Hoffmann <kraxel at redhat.com>

take care,
  Gerd