linux-kernel - Re: [PATCH v3 2/7] drm/format-helper: Add drm_fb_{xrgb8888, gray8}_to_mono

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: <3b6fe640-47e5-8765-3a09-8f9f2f0a6329@redhat.com>
Date:   Wed, 9 Feb 2022 16:30:58 +0100
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     Thomas Zimmermann <tzimmermann@...e.de>,
        linux-kernel@...r.kernel.org
Cc:     linux-fbdev@...r.kernel.org, David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        dri-devel@...ts.freedesktop.org,
        Noralf Trønnes <noralf@...nnes.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Maxime Ripard <maxime@...no.tech>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Sam Ravnborg <sam@...nborg.org>
Subject: Re: [PATCH v3 2/7] drm/format-helper: Add drm_fb_{xrgb8888,
 gray8}_to_mono_reversed()

On 2/9/22 16:21, Thomas Zimmermann wrote:

[snip]

>>
>> It could be taken from this patch-set anyways if someone wants to wire the
>> needed support for R8.
> 
> I think, policy is to not keep unused code around.
>

Ok, I'll drop it then. We can include it again when adding R8 formats.
 
[snip]

>>> If might be faster to allocate both buffers in one step and set the
>>> pointers into the allocated buffer.
>>>
>>
>> Not sure I got this. Do you mean to have a single buffer with length
>> linepixels + len_src32 and point src32 and gray8 to the same buffer ?
> 
> That's the idea. I don't know the exact overhead for kalloc(), but at 
> least the in userspace, malloc() in hot code paths is not a good idea. 
> There's usually some searching for free space involved.
>

Sure, let's do it in one allocation then and I'll add some comments to
make easier for people to follow the code.
 
> In the long term, we could add a field in struct drm_framebuffer to keep 
> such buffers around for reuse.
> 
>>
>>>> +
>>>> +	/*
>>>> +	 * For damage handling, it is possible that only parts of the source
>>>> +	 * buffer is copied and this could lead to start and end pixels that
>>>> +	 * are not aligned to multiple of 8.
>>>> +	 *
>>>> +	 * Calculate if the start and end pixels are not aligned and set the
>>>> +	 * offsets for the reversed mono line conversion function to adjust.
>>>> +	 */
>>>> +	start_offset = clip->x1 % 8;
>>>> +	end_offset = clip->x2 % 8;
>>>
>>> end_len, again. If you have 1 single bit set in the final byte, the
>>> offset is 0, but the length is 1.
>>>
>>
>> Agreed, will change it too.
> 
> Feel free to add my
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@...e.de>
>

Thanks!

Best regards, -- 
Javier Martinez Canillas
Linux Engineering
Red Hat