Hi Daniel,
On 2021/11/18 23:05, Laurent Pinchart wrote:
Hi Daniel,
On Thu, Nov 18, 2021 at 01:02:11PM +0000, Daniel Stone wrote:
Hi all,
Thanks for this Laurent. Esaki-san, could you please CC dri-devel@ on
discussions like this?
On Thu, 18 Nov 2021 at 12:32, Laurent Pinchart wrote:
On Sat, May 11, 2019 at 09:10:27PM +0300, Laurent Pinchart wrote:
On Thu, May 09, 2019 at 06:25:19PM +0900, Esaki Tomohito wrote:
Weston compositor (v5.0.0 or later) uses the DRM API to get the
supported modifiers and determines if the sprite plane can be used by
comparing the modifiers with the client specified modifier.
In currently driver, since the weston doesn't know supported modifiers,
that cannot determine if the received dmabuf can be passed through to
sprite plane.
Since there are R-Car GPU which support linear modifier, the sprite
plane cannot be used in a compositor similar to the weston if client
specify linear modifier.
I don't think the right solution is to expose the linear modifier from
all drivers that don't otherwise support modifiers. We should instead
fix it either in Weston, and treat drivers that don't support the
modifiers API as supporting the linear modifier only, or in the DRM/KMS
core by reporting the linear modifier for drivers that don't explicitly
support modifiers.
I've been pointed to
https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/3350#note_1161827,
and we had a discussion on the #dri-devel IRC channel today on this
topic. It turns out I was wrong, not specifying modifiers in userspace
is different than specifying a linear modifier. This is true for some
legacy drivers only (e.g. radeon) that pre-date the modifiers API, and
which select a tiling format internally based on some heuristics.
I still don't like this patch though, as it would need to be replicated
in most drivers. It would be better if we could handle this in the DRM
core. Daniel kindly offered to summarize the IRC discussion in a reply
to this e-mail.
Just quickly, I believe the check for the linear modifier in fb_create
is unnecessary, as this should already be checked in the core through
format_mod_supported().
There is indeed a difference between LINEAR and INVALID. Linear is an
explicit declaration of the layout; INVALID (i.e. no modifier) means
'I don't know what this is, so you should guess'. Guessing is
obviously not reliable, so Weston only passes buffers with no modifier
to KMS in two cases. The first case is when we allocate a dumb buffer
and the driver does not support modifiers; this is safe since it's the
same driver. The second case is when either the GPU driver or KMS
driver do not support modifiers and we allocate a buffer via GBM with
USE_SCANOUT; in this case, it is GBM's responsibility to select the
'right' layout.
We will never create a DRM framebuffer with no modifiers when the
original buffer comes from a client. If the client does not support
modifiers but the KMS driver does, then we do not know that the client
has allocated a suitable layout, so this is not safe. If the client
does explicitly declare a modifier but the KMS driver does not support
modifiers, then we also do not know that this is safe to use. So
unless both sides (client/GPU and KMS) support modifiers, we do not do
direct scanout from client buffers.
This patch would enable this usecase by declaring support for the
linear modifier from KMS; when used with a PVR driver which explicitly
declares the linear modifier, we know it is safe to pass that client
buffer to KMS.
Laurent's concern is that the DRM core should handle this rather than
open-coding in every driver, which I agree with. Some drivers (e.g.
radeon, maybe legacy NV?) do not support modifiers, and _also_ do
magic inference of the actual layout of the underlying buffer.
However, these drivers are legacy and we do not accept any new
addition of inferring layout without modifiers.
I think the best way forward here is:
- add a new mode_config.cannot_support_modifiers flag, and enable
this in radeon (plus any other drivers in the same boat)
Is there an easy way to identify the drivers that need this ?
Should I find a driver that has not use drm_plane_funcs?
- change drm_universal_plane_init() to advertise the LINEAR modifier
when NULL is passed as the modifier list (including installing a
default .format_mod_supported hook)
- remove the mode_config.allow_fb_modifiers hook and always
advertise modifier support, unless
mode_config.cannot_support_modifiers is set
Looks good to me.
I agree with this way, I'll try to create a patches.
FWIW, the effective modifier API and also valid usage is documented
here, which should be finished and merged shortly:
https://lore.kernel.org/dri-devel/20210905122742.86029-1-daniels@xxxxxxxxxxxxx/
--
Best Regards
Tomohito Esaki
