Re: [PATCH 8/8] drm/rockchip: vop2: Fix the AFBC offsets of a source that starts inside a superblock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] drm/rockchip: vop2: wrong 32x8 AFBC x mask in
  vop2_afbc_transform_offset()
- [Low] drm/rockchip: vop2: missing Fixes trailer for commit
  604be85547ce
--

commit 1aeff8145623ebb58cb0a14969b5391365ee2e1c
Author: Arun Iyengar <aiyengar1990@gmail.com>

> drm/rockchip: vop2: Fix the AFBC offsets of a source that starts inside a superblock
>
> The AFBC decoder of a window takes the position of the source in the
> framebuffer (AFBC_PIC_OFFSET), the number of superblock columns to
> decode (AFBC_TILE_NUM) and the offset of the first pixel inside its
> superblock (the transform offset). For a source that doesn't start at
> x = 0:
>
> - The tile number only counts the superblocks of the source width, one
>   short when the source starts inside a superblock and ends inside
>   another one.
> - The transform offset is the whole x position, truncated to the 8 bit
>   field, instead of the position in the 16 pixel wide superblock.
>
> Count the superblock columns from the first one the source touches, and
> take the x offset in the superblock, like the vendor kernel does for the
> not mirrored cases. The X mirrored and rotated cases are left as they are.
>
> Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>

[Severity: Low]
Since this fixes a bug introduced with the initial VOP2 driver, should it
include:

  Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 9ad138457204f..dfb76bfbc358f 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -486,7 +486,7 @@ static u32 vop2_afbc_transform_offset(struct drm_plane_state *pstate,
>  		ty = act_yoffset;
>  		break;
>  	case DRM_MODE_REFLECT_Y:
> -		tx = act_xoffset;
> +		tx = act_xoffset & 0xf;
>  		ty = bottom_crop_line_num - act_yoffset;
>  		break;
>  	case DRM_MODE_ROTATE_90:
> @@ -498,7 +498,7 @@ static u32 vop2_afbc_transform_offset(struct drm_plane_state *pstate,
>  		ty = vir_width - width - act_xoffset;
>  		break;
>  	case 0:
> -		tx = act_xoffset;
> +		tx = act_xoffset & 0xf;
>  		ty = act_yoffset;
>  		break;
>  	}

[Severity: Medium]
Does masking act_xoffset with 0xf break 32x8 AFBC superblocks when
(act_xoffset % 32) is between 16 and 31?

In vop2_plane_atomic_update(), block_w is 32 when
AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 is set (such as on RK3576 cluster windows),
and afbc_tile_num is computed using (src->x1 >> 16) % block_w.  Hardcoding
0xf here truncates tx to 0..15 instead of 0..31, shifting the decoded
plane horizontally by 16 pixels whenever bit 4 of act_xoffset is set.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-vop2-fixes-v1-0-d336e9dd90d3@gmail.com?part=8