linux-kernel - Re: [PATCH v3] drm/rockchip: update cursors asynchronously through atomic.

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:   Mon, 26 Nov 2018 12:36:03 -0800
From:   Eric Anholt <eric@...olt.net>
To:     Michael Zoran <mzoran@...wfest.net>,
        Tomasz Figa <tfiga@...omium.org>, helen.koike@...labora.com
Cc:     Sandy Huang <hjc@...k-chips.com>,
        Heiko Stübner <heiko@...ech.de>,
        David Airlie <airlied@...ux.ie>,
        "list\@263.net\:IOMMU DRIVERS \<iommu\@lists.linux-foundation.org\>\,
        Joerg Roedel \<joro\@8bytes.org\>\," 
        <iommu@...ts.linux-foundation.org>,
        "list\@263.net\:IOMMU DRIVERS \<iommu\@lists.linux-foundation.org\>\,
        Joerg Roedel \<joro\@8bytes.org\>\," <joro@...tes.org>,
        "list\@263.net\:IOMMU DRIVERS \<iommu\@lists.linux-foundation.org\>\,
        Joerg Roedel \<joro\@8bytes.org\>\," 
        <linux-arm-kernel@...ts.infradead.org>,
        Enric Balletbo i Serra <enric.balletbo@...labora.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        "open list\:ARM\/Rockchip SoC..." 
        <linux-rockchip@...ts.infradead.org>,
        Gustavo Padovan <gustavo.padovan@...labora.com>,
        Sean Paul <seanpaul@...gle.com>, kernel@...labora.com,
        Stéphane Marchesin <marcheu@...gle.com>,
        Boris Brezillon <boris.brezillon@...tlin.com>
Subject: Re: [PATCH v3] drm/rockchip: update cursors asynchronously through atomic.

Michael Zoran <mzoran@...wfest.net> writes:

> On Fri, 2018-11-23 at 11:27 +0900, Tomasz Figa wrote:
>> 
>> The point here is not about setting and resetting the plane->fb
>> pointer. It's about what happens inside
>> drm_atomic_set_fb_for_plane().
>> 
>> It calls drm_framebuffer_get() for the new fb and
>> drm_framebuffer_put() for the old fb. In result, if the fb changes,
>> the old fb, which had its reference count incremented in the atomic
>> commit that set it to the plane before, has its reference count
>> decremented. Moreover, if the new reference count becomes 0,
>> drm_framebuffer_put() will immediately free the buffer.
>> 
>> Freeing a buffer when the hardware is still scanning out of it isn't
>> a
>> good idea, is it?
>
> No, it's not.  But the board I submitted the patch for doesn't have
> anything like hot swapable ram.  The ram access is still going to work,
> just it might display something it shouldn't. Say for example if that
> frame buffer got reused by somethig else and filled with new data in
> the very small window.
>
> But yes, I agree the best solution would be to not release the buffer
> until the next vblank.
>
> Perhaps a good solution would be for the DRM api to have the concept of
> a deferred release?  Meaning if the put() call just added the frame
> buffer to a list that DRM core could walk during the vblank.  That
> might be better then every single driver trying to work up a custom
> solution.
>
>> The vc4 driver seems to be able to program the hardware to switch the
>> scanout to the new buffer immediately:
>> 
>> https://elixir.bootlin.com/linux/v4.20-rc3/source/drivers/gpu/drm/vc4/vc4_plane.c#L794
>> 
>> Although I wonder if there isn't still a tiny race there - the
>> hardware may have just started refilling the FIFO from the old
>> address. Still, if the FIFO is small, the FIFO refill operation may
>> be
>> much shorter than it takes for the kernel code to actually free the
>> buffer. Eric and Michael, could you confirm?
>> 
>
> I don't have those boards anymore, and I don't have access to any
> technical documentation on the GPU so I can't really add much here.  
> Eric can probably provide the best information.

I don't think I understood my scanout hardware well enough when I
started on the async update stuff for rpi.  vc4 probably needs to wait
until the HW starts scanning out a new line before letting the old BO
get freed.

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)