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

[Thomas Zimmermann]

Hi

Am 26.03.25 um 14:04 schrieb 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>

Thanks for reconsidering. I'll send out an updated series with an 
expanded commit message.

Best regards
Thomas

>
> take care,
>    Gerd
>

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