From: Esaki Tomohito <etom@igel.co.jp>
To: Daniel Stone <daniel@fooishbar.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
linux-renesas-soc@vger.kernel.org,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: rcar-du: add modifiers support
Date: Wed, 24 Nov 2021 11:49:09 +0900 [thread overview]
Message-ID: <d1dbd860-47f7-c96c-0455-a877e75f118c@igel.co.jp> (raw)
In-Reply-To: <CAPj87rNPiLve14H85ZBbn-oozqgeS26DGUCg2oJ8LupAdYcEqA@mail.gmail.com>
Hello Daniel
Thanks for the summary of the discussion.
On 2021/11/18 22:02, Daniel Stone wrote:
> Hi all,
> Thanks for this Laurent. Esaki-san, could you please CC dri-devel@ on
> discussions like this?
I'm sorry.
I will check using get_maintainer.pl next time.
> On Thu, 18 Nov 2021 at 12:32, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> 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)
> - 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
>
> 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@collabora.com/
>
> Cheers,
> Daniel
>
--
Best Regards
Tomohito Esaki
prev parent reply other threads:[~2021-11-24 2:49 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-09 5:45 [PATCH] drm: rcar-du: add modifiers support Tomohito Esaki
2019-05-09 7:14 ` Laurent Pinchart
2019-05-09 9:25 ` Esaki Tomohito
2019-05-11 18:10 ` Laurent Pinchart
2019-05-17 7:45 ` Esaki Tomohito
2021-11-18 12:31 ` Laurent Pinchart
2021-11-18 13:02 ` Daniel Stone
2021-11-18 14:05 ` Laurent Pinchart
2021-11-30 8:44 ` Esaki Tomohito
2021-11-30 13:20 ` Daniel Stone
2021-12-01 5:47 ` Esaki Tomohito
2021-11-24 2:49 ` Esaki Tomohito [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d1dbd860-47f7-c96c-0455-a877e75f118c@igel.co.jp \
--to=etom@igel.co.jp \
--cc=daniel@fooishbar.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-renesas-soc@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).