On Fri, 25 Aug 2023 10:55:35 +0200 Jocelyn Falempe <jfalempe@xxxxxxxxxx> wrote: > After discussions on IRC, the consensus is that the DRM drivers should > avoid software color conversion, and only advertise the formats supported > by hardware. > Update the doc accordingly so that the rule and exceptions are clear for > everyone. > > Signed-off-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx> > --- > drivers/gpu/drm/drm_plane.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > Hi, thanks for revising the patch! > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index 24e7998d1731..7215521afddd 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -140,6 +140,25 @@ > * DRM_FORMAT_MOD_LINEAR. Before linux kernel release v5.1 there have been > * various bugs in this area with inconsistencies between the capability > * flag and per-plane properties. > + * > + * All drivers must support XRGB8888, even if the hardware cannot support Maybe "should" rather than "must"? If a driver that never supported XRGB8888 before continues to not support it, I would not count it as a bug. > + * it. This has become the de-facto standard and a lot of user-space assume > + * it will be present. If XRGB8888 is not natively supported, then it > + * shouldn't be the default for preferred depth or fbdev emulation. > + * > + * DRM drivers should not do software color conversion in dumb buffers, and This seems to be the opposite of what I was trying to say. In my opinion: Dumb buffers are the only type of buffers where software color conversion could be expected. I would never expect software color conversion with non-dumb buffers or buffers imported from another device. But it has a catch: if software color conversion is needed, the dumb buffers are better be allocated in sysram. The conversion performance would likely be abysmal if dumb buffers were allocated from discrete VRAM. > + * only advertise the formats they support in hardware. This is for > + * performance reason, and to avoid multiple conversions in userspace and > + * kernel space. Maybe also add: KMS page flips are generally expected to be very cheap operations. > + * But there are two exceptions: > + * * 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. An example > + * would be to drop the padding component from a format to save some memory > + * bandwidth. This is fine as long as no-one can mistake this to refer to in-place conversion. The driver must not modify the userspace submitted buffer's contents. > + * Extra care should be taken when doing software conversion with > + * DRM_CAP_DUMB_PREFER_SHADOW, there are more detailed explanation here: > + * https://lore.kernel.org/dri-devel/20230818162415.2185f8e3@eldfell/ > */ > > static unsigned int drm_num_planes(struct drm_device *dev) > > base-commit: 82d750e9d2f5d0594c8f7057ce59127e701af781 Thanks, pq
Attachment:
pgpInEhOi7s9U.pgp
Description: OpenPGP digital signature
