On Fri, 8 Sep 2023 11:21:51 +0200 Thomas Zimmermann <tzimmerm...@suse.de> wrote:
> Hi > > 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. NAK on this > part from my side. > > If you want userspace to be able to use a certain format, then export > the corresponding 4cc code. Then let userspace decide what to do about > it. If userspace pick a certain format, go with it. What is the reason for your objection, exactly? > Hence, no implicit conversion from XRGB888 to RGB888, just because it's > possible. For the particular driver in question though, the conversion allows using a display resolution that is otherwise not possible. I also hear it improves performance since 25% less data needs to travel across a slow bus. There is also so little VRAM, than all dumb buffers need to be allocated from sysram instead anyway, so a copy is always necessary. Since XRGB8888 is the one format that is recommended to be supported by all drivers, I don't see a problem here. Did you test on your incredibly slow g200 test rig if the conversion ends up hurting instead of helping performance there? If it hurts, then I see that you have a good reason to NAK this. It's hard to imagine how it would hurt, since you always need a copy from sysram dumb buffers to VRAM - or do you? Thanks, pq > > + * On most hardware, VRAM read access are slow, so when doing the > > software > > + * conversion, the dumb buffer should be allocated in system RAM in > > order to > > + * have decent performance. > > + * Extra care should be taken when doing software conversion with > > + * DRM_CAP_DUMB_PREFER_SHADOW, there are more detailed explanations > > here: > > + * https://lore.kernel.org/dri-devel/20230818162415.2185f8e3@eldfell/ > > */ > > > > static unsigned int drm_num_planes(struct drm_device *dev) > > > > base-commit: 82d750e9d2f5d0594c8f7057ce59127e701af781 >
pgp6yO4iDygjw.pgp
Description: OpenPGP digital signature
