linux-kernel - Re: [RFC] drm/atomic+msm: add helper to implement legacy dirtyfb

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]

Date:   Wed, 4 Apr 2018 20:52:14 +0200
From:   Noralf Trønnes <noralf@...nnes.org>
To:     Daniel Vetter <daniel@...ll.ch>
Cc:     Rob Clark <robdclark@...il.com>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        David Airlie <airlied@...ux.ie>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Deepak Singh Rawat <drawat@...are.com>,
        freedreno <freedreno@...ts.freedesktop.org>,
        Lukasz Spintzyk <lukasz.spintzyk@...playlink.com>
Subject: Re: [RFC] drm/atomic+msm: add helper to implement legacy dirtyfb


Den 04.04.2018 19.56, skrev Daniel Vetter:
> On Wed, Apr 4, 2018 at 7:41 PM, Noralf Trønnes <noralf@...nnes.org> wrote:
>>
>> Den 04.04.2018 00.42, skrev Rob Clark:
>>> Add an atomic helper to implement dirtyfb support.  This is needed to
>>> support DSI command-mode panels with x11 userspace (ie. when we can't
>>> rely on pageflips to trigger a flush to the panel).
>>>
>>> To signal to the driver that the async atomic update needs to
>>> synchronize with fences, even though the fb didn't change, the
>>> drm_atomic_state::dirty flag is added.
>>>
>>> Signed-off-by: Rob Clark <robdclark@...il.com>
>>> ---
>>> Background: there are a number of different folks working on getting
>>> upstream kernel working on various different phones/tablets with qcom
>>> SoC's.. many of them have command mode panels, so we kind of need a
>>> way to support the legacy dirtyfb ioctl for x11 support.
>>>
>>> I know there is work on a proprer non-legacy atomic property for
>>> userspace to communicate dirty-rect(s) to the kernel, so this can
>>> be improved from triggering a full-frame flush once that is in
>>> place.  But we kinda needa a stop-gap solution.
>>>
>>> I had considered an in-driver solution for this, but things get a
>>> bit tricky if userspace ands up combining dirtyfb ioctls with page-
>>> flips, because we need to synchronize setting various CTL.FLUSH bits
>>> with setting the CTL.START bit.  (ie. really all we need to do for
>>> cmd mode panels is bang CTL.START, but is this ends up racing with
>>> pageflips setting FLUSH bits, then bad things.)  The easiest soln
>>> is to wrap this up as an atomic commit and rely on the worker to
>>> serialize things.  Hence adding an atomic dirtyfb helper.
>>>
>>> I guess at least the helper, with some small addition to translate
>>> and pass-thru the dirty rect(s) is useful to the final atomic dirty-
>>> rect property solution.  Depending on how far off that is, a stop-
>>> gap solution could be useful.
>>
>> I have been waiting for someone to address this since I'm not skilled
>> enough myself to tackle the atomic machinery. It would be be nice to do
>> all flushing during commit since that would mean that the tinydrm drivers
>> could be driven solely through drm_simple_display_pipe_funcs. I wouldn't
>> have to wire through the dirty callback like I do now.
>>
>> I see that you use a nonblocking commit. What happens if a new dirtyfb
>> comes in before the previous is done?
> We could reuse the same trick we're doing in the fbdev code, of
> pushing the commit to a worker and doing it in a blocking fashion. Or
> at least wait for it to finish (can be done with the crtc_state->event
> stuff). That way userspace can pile us up in dirtyfb calls, but we'd
> do at most one per frame. More isn't really useful anyway.
>
>> If we could save the dirty clips, then I could use this in tinydrm.
>> A full flush over SPI takes ~50ms so I need to shave off where I can.
>>
>> This works for me in my tinydrm world:
> One thing I thought you've had around somewhere is some additional
> tracking code, so we don't have to take all the plane mutexes in the
> ->dirtyfb callback. Does that exist, or was that just a figment of my
> imagination?

I haven't looked at this at all since I know way too little about how
atomic works and after the discussion started with damage on page flips,
I knew I just had to wait for someone other than me to do this. And I
hardly know anything about how the multitude of userspace operates.
So I'm sorry, but I can't add much to this discussion, I fall off rather
quickly when the details and corner cases are discussed.
All I can do is state the needs of tinydrm :-)

Noralf.