Re: [PATCH v3] drm/plane: Add documentation about software color conversion.

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

 



Hi Javier

Am 08.09.23 um 15:46 schrieb Javier Martinez Canillas:
Thomas Zimmermann <tzimmermann@xxxxxxx> writes:

Hello Thomas,

Hi Maxime

Am 08.09.23 um 12:58 schrieb Maxime Ripard:
Hi,

On Fri, Sep 08, 2023 at 11:21:51AM +0200, Thomas Zimmermann wrote:
Am 25.08.23 um 16:04 schrieb Jocelyn Falempe:
[...]
+ *
+ *     But there are two exceptions only for dumb buffers:
+ *     * To support XRGB8888 if it's not supported by the hardware.


+ *     * Any driver is free to modify its internal representation of the format,
+ *       as long as it doesn't alter the visible content in any way, and doesn't
+ *       modify the user-provided buffer. An example would be to drop the
+ *       padding component from a format to save some memory bandwidth.

I have strong objections to this point, _especially_ as you're apparently
trying to sneak this in after our discussion.

I think it's an unfair characterization. This was discussed on
#dri-devel, and went through several rounds over the mailing lists, with
you in Cc for each. How is that sneaking something in?

A few months ago, we had a flamewar'ish IRC discussion on format
conversion within the kernel. The general sentiment was that the kernel
drivers should use what ever is provided by userspace without further
processing. The short argument was 'userspace knows better'. The only
exception is for supporting XRGB8888 on hardware that would otherwise
not support it. After some consideration, I agree with all that. (Back
then I didn't.)

A few weeks ago I received a patch to do an implicit conversion from
XRGB8888 to RGB888 within mgag200. [1] I don't have a link to the
discussion, but I NAK'ed that patch pretty hard on IRC by following that
other discussion.

And know I find that this patch (even in its v1) contains language that
retroactively legitimizes the mgag200 patch. I wrote 'apparently' I my
reply, as I assume that there's more to it, but how does it not look
like an attempt to sneak in something that is known to be controversial?


While is true that the motivation for Jocelyn's patch was to make explicit
what are the rules with regard to drivers emulating formats (other than
"we had a flamewar on IRC a while back" which is quite ambiguous), it was
not attempt to sneak something that is known to be controversial.

In fact, it is an attempt to dispel the controversy and document what is
acceptable and what is not for a driver.

It might have been better to discuss the question separately on the
dri-devel ML. Maybe we can do this here.


This was discussed in the #dri-devel IRC channel, I believe you were on
PTO at the time and probably that's why you missed. I found the logs here:

https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&date=2023-08-04

As you can see there, most people agreed that what Jocelyn wrote in his
doc patch is the most pragmatic compromise.

Thanks for the pointer to the URL. Quoting Daniel (sima) from that discussion.

"imo just document that for hw/drivers where XRGB8888 is not support or has a very bad cost in terms of upload/scanout bw it's ok to emulate it in the kernel driver, but _only_ for that format "

This seems the overall sentiment. I disagree with the "has very bad cost" part, though. The cost/benefit ratio should be determined by userspace IMHO. Please see my reply to Pekka for some details.

I don't have a pointer to that other IRC discussion, but IIRC there where quite a lot more people involved, including from userspace. Many of those are absent here.

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)

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux