linux-kernel - Re: [PATCH v3 2/4] drm/atomic: rename async_{update,check} to amend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bda69249-f193-9dbc-5186-7139fb2c365a@collabora.com>
Date:   Fri, 12 Apr 2019 11:06:13 -0300
From:   Helen Koike <helen.koike@...labora.com>
To:     Boris Brezillon <boris.brezillon@...labora.com>
Cc:     dri-devel@...ts.freedesktop.org, David Airlie <airlied@...ux.ie>,
        dnicoara@...omium.org, daniels@...labora.com,
        alexandros.frantzis@...labora.com, daniel.vetter@...ll.ch,
        linux-kernel@...r.kernel.org, tomasz Figa <tfiga@...omium.org>,
        tina.zhang@...el.com, Sean Paul <seanpaul@...gle.com>,
        kernel@...labora.com, nicholas.kazlauskas@....com,
        Stéphane Marchesin <marcheu@...gle.com>,
        Gustavo Padovan <gustavo.padovan@...labora.com>,
        Sean Paul <sean@...rly.run>, Sandy Huang <hjc@...k-chips.com>,
        linux-doc@...r.kernel.org, Thomas Zimmermann <tzimmermann@...e.de>,
        Jonathan Corbet <corbet@....net>,
        Harry Wentland <harry.wentland@....com>,
        Alex Deucher <alexander.deucher@....com>,
        Bhawanpreet Lakha <Bhawanpreet.Lakha@....com>,
        "David (ChunMing) Zhou" <David1.Zhou@....com>,
        Anthony Koo <Anthony.Koo@....com>,
        Russell King <rmk+kernel@...linux.org.uk>,
        linux-rockchip@...ts.infradead.org,
        Ville Syrjälä <ville.syrjala@...ux.intel.com>,
        Rob Clark <robdclark@...il.com>,
        Heiko Stübner <heiko@...ech.de>,
        Eric Anholt <eric@...olt.net>, Leo Li <sunpeng.li@....com>,
        linux-arm-msm@...r.kernel.org,
        Christian König <christian.koenig@....com>,
        linux-arm-kernel@...ts.infradead.org,
        David Francis <David.Francis@....com>,
        Mikita Lipski <mikita.lipski@....com>,
        amd-gfx@...ts.freedesktop.org,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <maxime.ripard@...tlin.com>,
        freedreno@...ts.freedesktop.org,
        Mamta Shukla <mamtashukla555@...il.com>,
        Daniel Vetter <daniel@...ll.ch>
Subject: Re: [PATCH v3 2/4] drm/atomic: rename async_{update,check} to
 amend_{update,check}

Hi Boris,

On 4/12/19 10:49 AM, Boris Brezillon wrote:
> Hi Helen,
> 
> On Fri, 12 Apr 2019 09:58:25 -0300
> Helen Koike <helen.koike@...labora.com> wrote:
> 
>> Asynchronous update is the ability change the hw state at any time, not
>> only during vblank.
>>
>> Amend mode is the ability to perform 1000 commits to be applied as soon
>> as possible without waiting for 1000 vblanks.
>>
>> async updates can be seen as amend, but the opposite is not true.
>>
>> &drm_plane_helper_funcs.atomic_async_{update,check}() was being used by
>> drivers to implement amend and not async. So rename them to amend.
>>
>> Also improve docs explaining the difference.
>>
>> If asynchronous is required, normal page flip can be performed using
>> DRM_MODE_PAGE_FLIP_ASYNC flag.
>>
>> Signed-off-by: Helen Koike <helen.koike@...labora.com>
>>
>> ---
>> Hello,
>>
>> I would like to officially clarify what async update means by adding it
>> in the docs.
>> Please correct me if I am wrong, but the current async_{update,check}
>> callbacks are being used to do amend (the legacy cursor behavior), i.e.
>> to allow 1000 updates without waiting for 1000 vblanks.
> 
> Right now, the semantic of async update is driver dependent. Some
> drivers will amend the last commit touching that plane (amend semantic),
> others will update the plane buffer immediately which might cause
> tearing (async semantic).

In my pov, async updates holds the properties of an amend update, so all
async updates we have are amend, but the opposite is not true.

> 
>>
>> So I would like to clarify this in the docs and rename the current
>> callbacks to reflect this behaviour.
> 
> I'm all for this clarification, but I don't think renaming the async
> hooks is a good idea, since some drivers will not do real 'amend'. So,
> you're changing the name, but it's still confusing :-).
> 
> How about adding new hooks (and/or flags) for the AMEND case, and
> keeping the async path untouched. We can then let drivers that
> currently implement async as amend implement the amend hooks instead.
> Once you've done that, you can hook that up to the legacy cursor update
> path so that it first tries one then the other and finally falls back
> to a sync update if none of ASYNC/AMEND is possible.

I kinda did this (I re-introduced async in the last patch in the
series). I know this order is confusing, but as rockchip doesn't
implement true async, I would have to do a bunch of modifs at once to
keep the commits consistent, but I can re-work on that if it makes
things clearer.

> 
>>
>> I also see that for real async updates, the flag
>> DRM_MODE_PAGE_FLIP_ASYNC can be used in a normal sync update (it is
>> already being used by some drivers actually, in the atomic path, not only
>> in the legacy page flip, at least is what I understood from
>> amdgpu_dm_atomic_commit_tail() implementation).
> 
> Yes, right now, async does not necessarily imply non-block, but
> Daniel seemed to think that most users want non-block when they do an
> async page flip, so maybe it should be clarified too.

users could combine NONBLOCK flag with PAGE_FLIP_ASYNC, no? (we need to
add code for it of course).

Thanks
Helen

> 
> Regards,
> 
> Boris
>