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 <tzimmerm...@suse.de>
>

Thanks!

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

Reply via email to