On Tue, 25 Mar 2025, Thomas Zimmermann <tzimmerm...@suse.de> wrote:
> For ease of implementation, existing line-conversion functions
> for 16-bit formats write each pixel individually. Optimize the
> performance by writing mulitple pixels in single 64-bit and 32-bit
> stores.
>
> Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
> ---
>  drivers/gpu/drm/drm_format_helper.c | 40 ++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_format_helper.c 
> b/drivers/gpu/drm/drm_format_helper.c
> index b9c9c712aa9c..66137df85725 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -262,10 +262,48 @@ static __always_inline void 
> drm_fb_xfrm_line_32to16(void *dbuf, const void *sbuf
>                                                   unsigned int pixels,
>                                                   u32 (*xfrm_pixel)(u32))
>  {
> -     __le16 *dbuf16 = dbuf;
> +     __le64 *dbuf64 = dbuf;
> +     __le32 *dbuf32;
> +     __le16 *dbuf16;
>       const __le32 *sbuf32 = sbuf;
>       const __le32 *send32 = sbuf32 + pixels;
>  
> +#if defined(CONFIG_64BIT)
> +     /* write 4 pixels at once */
> +     send32 -= pixels & GENMASK(1, 0);
> +     while (sbuf32 < send32) {

I find the adjusting of send32 before and after the loop with different
masks a bit confusing. Would it not suffice to:

        while (sbuf32 < ALIGN_DOWN(send32, 4))

and leave send32 untouched? With different alignments for 2 pixels at a
time.


BR,
Jani.


> +             u32 pix[4] = {
> +                     le32_to_cpup(sbuf32++),
> +                     le32_to_cpup(sbuf32++),
> +                     le32_to_cpup(sbuf32++),
> +                     le32_to_cpup(sbuf32++),
> +             };
> +             /* write output bytes in reverse order for little endianness */
> +             u64 val64 = ((u64)xfrm_pixel(pix[0])) |
> +                         ((u64)xfrm_pixel(pix[1]) << 16) |
> +                         ((u64)xfrm_pixel(pix[2]) << 32) |
> +                         ((u64)xfrm_pixel(pix[3]) << 48);
> +             *dbuf64++ = cpu_to_le64(val64);
> +     }
> +     send32 += pixels & GENMASK(1, 1);
> +#endif
> +
> +     /* write 2 pixels at once */
> +     dbuf32 = (__le32 __force *)dbuf64;
> +     while (sbuf32 < send32) {
> +             u32 pix[2] = {
> +                     le32_to_cpup(sbuf32++),
> +                     le32_to_cpup(sbuf32++),
> +             };
> +             /* write output bytes in reverse order for little endianness */
> +             u32 val32 = xfrm_pixel(pix[0]) |
> +                        (xfrm_pixel(pix[1]) << 16);
> +             *dbuf32++ = cpu_to_le32(val32);
> +     }
> +     send32 += pixels & GENMASK(0, 0);
> +
> +     /* write trailing pixel */
> +     dbuf16 = (__le16 __force *)dbuf32;
>       while (sbuf32 < send32)
>               *dbuf16++ = cpu_to_le16(xfrm_pixel(le32_to_cpup(sbuf32++)));
>  }

-- 
Jani Nikula, Intel

Reply via email to