Re: [PATCH v2 1/3] drm/rockchip: vop2: Use drm_format_info_min_pitch() for plane offsets

[sashiko-bot@kernel.org]

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 <chaoyi.chen@rock-chips.com>

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/20261029081429.862-1-kernel@airkyi.com?part=1