Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/rockchip: vop2: 64-bit division by hsub in vop2_plane_atomic_update() --
commit 6053e1180bcd99e2d0019c04165b9318bc1a9fc3 Author: Chaoyi Chen <[email protected]> drm/rockchip: vop2: Use drm_format_info_min_pitch() for plane offsets This commit replaces fb->format->cpp[] with drm_format_info_min_pitch() when computing horizontal plane offsets in vop2_plane_atomic_update() to support packed 10-bit formats such as NV15, NV20, and NV30. > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > index bfb3841425687..adee018c6ef07 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c [ ... ] > @@ -1309,7 +1309,7 @@ static void vop2_plane_atomic_update(struct drm_plane > *plane, > int hsub = fb->format->hsub; > int vsub = fb->format->vsub; > > - offset = (src->x1 >> 16) * fb->format->cpp[1] / hsub; > + offset = drm_format_info_min_pitch(fb->format, 1, src->x1 >> > 16) / hsub; [Severity: Medium] Does dividing the return value of drm_format_info_min_pitch() by hsub in vop2_plane_atomic_update() cause a 64-bit division linker regression on 32-bit architectures? drm_format_info_min_pitch() returns a uint64_t: drivers/gpu/drm/drm_fourcc.c:drm_format_info_min_pitch() { if (!info || plane < 0 || plane >= info->num_planes) return 0; return DIV_ROUND_UP_ULL((u64)buffer_width * info->char_per_block[plane], drm_format_info_block_width(info, plane) * drm_format_info_block_height(info, plane)); } Because hsub is a runtime int variable, dividing the uint64_t result directly with '/' promotes the expression to a 64-bit unsigned division before assigning to unsigned long offset. Since CONFIG_ROCKCHIP_VOP2 can be built on 32-bit targets (via 32-bit ARM ARCH_ROCKCHIP or COMPILE_TEST), this emits an unresolved call to __aeabi_uldivmod or __udivdi3 at link time. Additionally, drm_format_info.block_w[1] is defined in units of subsampled chroma pixels (for example, block_w is { 4, 2, 0 } for NV15 and NV20), so drm_format_info_min_pitch() expects buffer_width for plane 1 to already be subsampled. Dividing by hsub after DIV_ROUND_UP_ULL() rounds up before subsampling and then truncates. Should (src->x1 >> 16) / hsub be passed as the buffer_width argument to drm_format_info_min_pitch(), or should offset be assigned before dividing by hsub as is done in vop_plane_atomic_update() in rockchip_drm_vop.c? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
