| 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
| ||
|
Message-ID: <k7vorukb6rockoanjdebfjw3iio4l5nu3u6hdqyi4ke4cp3h66@omwo5bjxbt4t>
Date: Thu, 4 Sep 2025 23:19:14 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Jessica Zhang <jessica.zhang@....qualcomm.com>
Cc: Rob Clark <robin.clark@....qualcomm.com>,
Dmitry Baryshkov <lumag@...nel.org>,
Abhinav Kumar <abhinav.kumar@...ux.dev>, Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Jordan Crouse <jordan@...micpenguin.net>,
linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 12/12] drm/msm/dpu: use standard functions in
_dpu_format_populate_plane_sizes_ubwc()
On Thu, Sep 04, 2025 at 11:38:16AM -0700, Jessica Zhang wrote:
>
>
> On 7/4/2025 7:47 PM, Dmitry Baryshkov wrote:
> > The _dpu_format_populate_plane_sizes_ubwc() used MSM_MEDIA_ALIGN() and
> > MSM_MEDIA_ROUNDUP(), macros inherited from the previous implementation,
> > msm_media_info.h. Replace them with the standard Linux macros,
> > round_up() and DIV_ROUND_UP() respectively.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 62 ++++++++++++-----------------
> > 1 file changed, 26 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > index 67bc5a6eeb43dcf113dea9eccdb778cd52b1ad40..6a0426ed1460c5af4822844d7a7b0c51739df875 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> > @@ -10,16 +10,6 @@
> > #include "dpu_kms.h"
> > #include "dpu_formats.h"
> > -#ifndef MSM_MEDIA_ALIGN
> > -#define MSM_MEDIA_ALIGN(__sz, __align) (((__align) & ((__align) - 1)) ?\
> > - ((((__sz) + (__align) - 1) / (__align)) * (__align)) :\
> > - (((__sz) + (__align) - 1) & (~((__align) - 1))))
> > -#endif
> > -
> > -#ifndef MSM_MEDIA_ROUNDUP
> > -#define MSM_MEDIA_ROUNDUP(__sz, __r) (((__sz) + ((__r) - 1)) / (__r))
> > -#endif
> > -
> > #define DPU_UBWC_PLANE_SIZE_ALIGNMENT 4096
> > /*
> > @@ -80,57 +70,57 @@ static int _dpu_format_populate_plane_sizes_ubwc(
> > fmt->pixel_format == DRM_FORMAT_P010) {
> > if (MSM_FORMAT_IS_DX(fmt)) {
> > if (fmt->flags & MSM_FORMAT_FLAG_UNPACK_TIGHT) {
> > - stride = MSM_MEDIA_ALIGN(fb->width, 192);
> > - stride = MSM_MEDIA_ALIGN(stride * 4 / 3, 256);
> > + stride = round_up(fb->width, 192);
>
> Hi Dmitry,
>
> It seems like the usage of round_up() here might be incorrect -- the docs
> say "round up to next specified power of 2".
>
> Maybe we should use roundup() instead here?
Indeed, two macros with very simlar names. Thanks for spotting it!
>
> The rest of the patch LGTM.
>
> Thanks,
>
> Jessica Zhang
>
> > + stride = round_up(stride * 4 / 3, 256);
> > y_tile_width = 48;
> > } else {
> > - stride = MSM_MEDIA_ALIGN(fb->width * 2, 256);
> > + stride = round_up(fb->width * 2, 256);
> > y_tile_width = 32;
> > }
> > - sclines = MSM_MEDIA_ALIGN(fb->height, 16);
> > + sclines = round_up(fb->height, 16);
> > y_tile_height = 4;
> > } else {
> > - stride = MSM_MEDIA_ALIGN(fb->width, 128);
> > + stride = round_up(fb->width, 128);
> > y_tile_width = 32;
> > - sclines = MSM_MEDIA_ALIGN(fb->height, 32);
> > + sclines = round_up(fb->height, 32);
> > y_tile_height = 8;
> > }
> > }
> > layout->plane_pitch[0] = stride;
> > - layout->plane_size[0] = MSM_MEDIA_ALIGN(layout->plane_pitch[0] *
> > + layout->plane_size[0] = round_up(layout->plane_pitch[0] *
> > sclines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
> > layout->plane_pitch[1] = stride;
> > - layout->plane_size[1] = MSM_MEDIA_ALIGN(layout->plane_pitch[1] *
> > + layout->plane_size[1] = round_up(layout->plane_pitch[1] *
> > sclines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
> > if (!meta)
> > return 0;
> > - y_meta_stride = MSM_MEDIA_ROUNDUP(fb->width, y_tile_width);
> > - layout->plane_pitch[2] = MSM_MEDIA_ALIGN(y_meta_stride, 64);
> > + y_meta_stride = DIV_ROUND_UP(fb->width, y_tile_width);
> > + layout->plane_pitch[2] = round_up(y_meta_stride, 64);
> > - y_meta_scanlines = MSM_MEDIA_ROUNDUP(fb->height, y_tile_height);
> > - y_meta_scanlines = MSM_MEDIA_ALIGN(y_meta_scanlines, 16);
> > - layout->plane_size[2] = MSM_MEDIA_ALIGN(layout->plane_pitch[2] *
> > + y_meta_scanlines = DIV_ROUND_UP(fb->height, y_tile_height);
> > + y_meta_scanlines = round_up(y_meta_scanlines, 16);
> > + layout->plane_size[2] = round_up(layout->plane_pitch[2] *
> > y_meta_scanlines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
> > - uv_meta_stride = MSM_MEDIA_ROUNDUP((fb->width+1)>>1, y_tile_width / 2);
> > - layout->plane_pitch[3] = MSM_MEDIA_ALIGN(uv_meta_stride, 64);
> > + uv_meta_stride = DIV_ROUND_UP((fb->width+1)>>1, y_tile_width / 2);
> > + layout->plane_pitch[3] = round_up(uv_meta_stride, 64);
> > - uv_meta_scanlines = MSM_MEDIA_ROUNDUP((fb->height+1)>>1, y_tile_height);
> > - uv_meta_scanlines = MSM_MEDIA_ALIGN(uv_meta_scanlines, 16);
> > - layout->plane_size[3] = MSM_MEDIA_ALIGN(layout->plane_pitch[3] *
> > + uv_meta_scanlines = DIV_ROUND_UP((fb->height+1)>>1, y_tile_height);
> > + uv_meta_scanlines = round_up(uv_meta_scanlines, 16);
> > + layout->plane_size[3] = round_up(layout->plane_pitch[3] *
> > uv_meta_scanlines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
> > } else {
> > unsigned int rgb_scanlines, rgb_meta_scanlines, rgb_meta_stride;
> > - layout->plane_pitch[0] = MSM_MEDIA_ALIGN(fb->width * fmt->bpp, 256);
> > - rgb_scanlines = MSM_MEDIA_ALIGN(fb->height, 16);
> > - layout->plane_size[0] = MSM_MEDIA_ALIGN(layout->plane_pitch[0] *
> > + layout->plane_pitch[0] = round_up(fb->width * fmt->bpp, 256);
> > + rgb_scanlines = round_up(fb->height, 16);
> > + layout->plane_size[0] = round_up(layout->plane_pitch[0] *
> > rgb_scanlines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
> > if (!meta)
> > @@ -139,13 +129,13 @@ static int _dpu_format_populate_plane_sizes_ubwc(
> > /* uAPI leaves plane[1] empty and plane[2] as meta */
> > layout->num_planes += 1;
> > - rgb_meta_stride = MSM_MEDIA_ROUNDUP(fb->width, 16);
> > - layout->plane_pitch[2] = MSM_MEDIA_ALIGN(rgb_meta_stride, 64);
> > + rgb_meta_stride = DIV_ROUND_UP(fb->width, 16);
> > + layout->plane_pitch[2] = round_up(rgb_meta_stride, 64);
> > - rgb_meta_scanlines = MSM_MEDIA_ROUNDUP(fb->height, 4);
> > - rgb_meta_scanlines = MSM_MEDIA_ALIGN(rgb_meta_scanlines, 16);
> > + rgb_meta_scanlines = DIV_ROUND_UP(fb->height, 4);
> > + rgb_meta_scanlines = round_up(rgb_meta_scanlines, 16);
> > - layout->plane_size[2] = MSM_MEDIA_ALIGN(layout->plane_pitch[2] *
> > + layout->plane_size[2] = round_up(layout->plane_pitch[2] *
> > rgb_meta_scanlines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
> > }
> >
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists