linux-kernel - Re: [RFC PATCH] drm/ssd130x: Allocate buffer in the CRTC's .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]

Message-ID: <87h6oe5jsu.fsf@minerva.mail-host-address-is-not-set>
Date:   Fri, 01 Sep 2023 11:23:13 +0200
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        Maxime Ripard <mripard@...nel.org>
Cc:     linux-kernel@...r.kernel.org,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...il.com>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [RFC PATCH] drm/ssd130x: Allocate buffer in the CRTC's
 .atomic_check() callback

Geert Uytterhoeven <geert@...ux-m68k.org> writes:

> Hi Maxime,
>
> On Fri, Sep 1, 2023 at 10:22 AM Maxime Ripard <mripard@...nel.org> wrote:
>> On Wed, Aug 30, 2023 at 08:25:08AM +0200, Javier Martinez Canillas wrote:
>> > The commit 45b58669e532 ("drm/ssd130x: Allocate buffer in the plane's
>> > .atomic_check() callback") moved the allocation of the intermediate and
>> > HW buffers from the encoder's .atomic_enable callback to primary plane's
>> > .atomic_check callback.
>> >
>> > This was suggested by Maxime Ripard because drivers aren't allowed to fail
>> > after drm_atomic_helper_swap_state() has been called, and the encoder's
>> > .atomic_enable happens after the new atomic state has been swapped.
>> >
>> > But that change caused a performance regression in very slow platforms,
>> > since now the allocation happens for every plane's atomic state commit.
>> > For example, Geert Uytterhoeven reports that is the case on a VexRiscV
>> > softcore (RISC-V CPU implementation on an FPGA).
>>
>> I'd like to have numbers on that. It's a bit surprising to me that,
>> given how many objects we already allocate during a commit, two small
>> additional allocations affect performances so dramatically, even on a
>> slow platform.
>
> To be fair, I didn't benchmark that.  Perhaps it's just too slow due to
> all these other allocations (and whatever else happens).
>
> I just find it extremely silly to allocate a buffer over and over again,
> while we know that buffer is needed for each and every display update.
>

Is not efficient that's true, but on the other hand we have other objects
that are destroyed and created on each atomic update.

In the ssd1307fb driver, the buffer is allocated on ssd1307fb_update_rect()
(by calling the ssd1307fb_alloc_array() function), so it also happens for
every display update in the fbdev driver.

> Gr{oetje,eeting}s,
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat