Re: [PATCH v5 05/18] drm/rockchip: vop2: Set AXI id for rk3588

[Detlev Casanova <detlev.casanova@collabora.com>]

Hi Andy,

On Monday, 9 December 2024 07:29:18 EST Andy Yan wrote:
> From: Andy Yan <andy.yan@rock-chips.com>
> 
> There are two AXI bus in vop2, windows attached on the same bus must
> have a unique channel YUV and RGB channel ID.
> 
> The default IDs will conflict with each other on the rk3588, so they
> need to be reassigned.
> 
> Fixes: 5a028e8f062f ("drm/rockchip: vop2: Add support for rk3588")
> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> Tested-by: Derek Foreman <derek.foreman@collabora.com>
> 
> ---
> 
> Changes in v5:
> - Added in V5
> 
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 +++++++++++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  9 +++++++
>  drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 26 +++++++++++++++++++-
>  3 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c index
> dc4edd65bc9e..8b9ca046eeeb 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -1426,6 +1426,12 @@ static void vop2_plane_atomic_update(struct drm_plane
> *plane, &fb->format->format,
>  		afbc_en ? "AFBC" : "", &yrgb_mst);
> 
> +	if (vop2->data->soc_id > 3568) {

Shouldn't this be done only for rk3588, as specified in the comments below ? 
The test we did before showed that it is failing on rk3576 and 3576 is > 3588.

I suggest

	if (vop2->data->soc_id == 3588) {

Regards,
Detlev

> +		vop2_win_write(win, VOP2_WIN_AXI_BUS_ID, win->data-
>axi_bus_id);
> +		vop2_win_write(win, VOP2_WIN_AXI_YRGB_R_ID, win->data-
>axi_yrgb_r_id);
> +		vop2_win_write(win, VOP2_WIN_AXI_UV_R_ID, win->data-
>axi_uv_r_id);
> +	}
> +
>  	if (vop2_cluster_window(win))
>  		vop2_win_write(win, VOP2_WIN_AFBC_HALF_BLOCK_EN, 
half_block_en);
> 
> @@ -3354,6 +3360,10 @@ static struct reg_field
> vop2_cluster_regs[VOP2_WIN_MAX_REG] = { [VOP2_WIN_Y2R_EN] =
> REG_FIELD(RK3568_CLUSTER_WIN_CTRL0, 8, 8),
>  	[VOP2_WIN_R2Y_EN] = REG_FIELD(RK3568_CLUSTER_WIN_CTRL0, 9, 9),
>  	[VOP2_WIN_CSC_MODE] = REG_FIELD(RK3568_CLUSTER_WIN_CTRL0, 10, 11),
> +	[VOP2_WIN_AXI_YRGB_R_ID] = REG_FIELD(RK3568_CLUSTER_WIN_CTRL2, 0, 
3),
> +	[VOP2_WIN_AXI_UV_R_ID] = REG_FIELD(RK3568_CLUSTER_WIN_CTRL2, 5, 8),
> +	/* RK3588 only, reserved bit on rk3568*/
> +	[VOP2_WIN_AXI_BUS_ID] = REG_FIELD(RK3568_CLUSTER_CTRL, 13, 13),
> 
>  	/* Scale */
>  	[VOP2_WIN_SCALE_YRGB_X] = 
REG_FIELD(RK3568_CLUSTER_WIN_SCL_FACTOR_YRGB, 0,
> 15), @@ -3446,6 +3456,10 @@ static struct reg_field
> vop2_esmart_regs[VOP2_WIN_MAX_REG] = { [VOP2_WIN_YMIRROR] =
> REG_FIELD(RK3568_SMART_CTRL1, 31, 31),
>  	[VOP2_WIN_COLOR_KEY] = REG_FIELD(RK3568_SMART_COLOR_KEY_CTRL, 0, 
29),
>  	[VOP2_WIN_COLOR_KEY_EN] = REG_FIELD(RK3568_SMART_COLOR_KEY_CTRL, 
31, 31),
> +	[VOP2_WIN_AXI_YRGB_R_ID] = REG_FIELD(RK3568_SMART_CTRL1, 4, 8),
> +	[VOP2_WIN_AXI_UV_R_ID] = REG_FIELD(RK3568_SMART_CTRL1, 12, 16),
> +	/* RK3588 only, reserved register on rk3568 */
> +	[VOP2_WIN_AXI_BUS_ID] = REG_FIELD(RK3588_SMART_AXI_CTRL, 1, 1),
> 
>  	/* Scale */
>  	[VOP2_WIN_SCALE_YRGB_X] = 
REG_FIELD(RK3568_SMART_REGION0_SCL_FACTOR_YRGB,
> 0, 15), diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h index
> 8786c2ba48df..0a3b22e8d14e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> @@ -78,6 +78,9 @@ enum vop2_win_regs {
>  	VOP2_WIN_COLOR_KEY,
>  	VOP2_WIN_COLOR_KEY_EN,
>  	VOP2_WIN_DITHER_UP,
> +	VOP2_WIN_AXI_BUS_ID,
> +	VOP2_WIN_AXI_YRGB_R_ID,
> +	VOP2_WIN_AXI_UV_R_ID,
> 
>  	/* scale regs */
>  	VOP2_WIN_SCALE_YRGB_X,
> @@ -149,6 +152,10 @@ struct vop2_win_data {
>  	unsigned int layer_sel_id;
>  	uint64_t feature;
> 
> +	uint8_t axi_bus_id;
> +	uint8_t axi_yrgb_r_id;
> +	uint8_t axi_uv_r_id;
> +
>  	unsigned int max_upscale_factor;
>  	unsigned int max_downscale_factor;
>  	const u8 dly[VOP2_DLY_MODE_MAX];
> @@ -319,6 +326,7 @@ enum dst_factor_mode {
> 
>  #define RK3568_CLUSTER_WIN_CTRL0		0x00
>  #define RK3568_CLUSTER_WIN_CTRL1		0x04
> +#define RK3568_CLUSTER_WIN_CTRL2		0x08
>  #define RK3568_CLUSTER_WIN_YRGB_MST		0x10
>  #define RK3568_CLUSTER_WIN_CBR_MST		0x14
>  #define RK3568_CLUSTER_WIN_VIR			0x18
> @@ -341,6 +349,7 @@ enum dst_factor_mode {
>  /* (E)smart register definition, offset relative to window base */
>  #define RK3568_SMART_CTRL0			0x00
>  #define RK3568_SMART_CTRL1			0x04
> +#define RK3588_SMART_AXI_CTRL			0x08
>  #define RK3568_SMART_REGION0_CTRL		0x10
>  #define RK3568_SMART_REGION0_YRGB_MST		0x14
>  #define RK3568_SMART_REGION0_CBR_MST		0x18
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c index
> f7d3350594fa..37c65138012d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> @@ -395,7 +395,7 @@ static const struct vop2_video_port_data
> rk3588_vop_video_ports[] = { * AXI1 is a read only bus.
>   *
>   * Every window on a AXI bus must assigned two unique
> - * read id(yrgb_id/uv_id, valid id are 0x1~0xe).
> + * read id(yrgb_r_id/uv_r_id, valid id are 0x1~0xe).
>   *
>   * AXI0:
>   * Cluster0/1, Esmart0/1, WriteBack
> @@ -415,6 +415,9 @@ static const struct vop2_win_data rk3588_vop_win_data[]
> = { .layer_sel_id = 0,
>  		.supported_rotations = DRM_MODE_ROTATE_90 | 
DRM_MODE_ROTATE_270 |
>  				       DRM_MODE_REFLECT_X | 
DRM_MODE_REFLECT_Y,
> +		.axi_bus_id = 0,
> +		.axi_yrgb_r_id = 2,
> +		.axi_uv_r_id = 3,
>  		.max_upscale_factor = 4,
>  		.max_downscale_factor = 4,
>  		.dly = { 4, 26, 29 },
> @@ -431,6 +434,9 @@ static const struct vop2_win_data rk3588_vop_win_data[]
> = { .supported_rotations = DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270 |
> DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y,
>  		.type = DRM_PLANE_TYPE_PRIMARY,
> +		.axi_bus_id = 0,
> +		.axi_yrgb_r_id = 6,
> +		.axi_uv_r_id = 7,
>  		.max_upscale_factor = 4,
>  		.max_downscale_factor = 4,
>  		.dly = { 4, 26, 29 },
> @@ -446,6 +452,9 @@ static const struct vop2_win_data rk3588_vop_win_data[]
> = { .supported_rotations = DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270 |
> DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y,
>  		.type = DRM_PLANE_TYPE_PRIMARY,
> +		.axi_bus_id = 1,
> +		.axi_yrgb_r_id = 2,
> +		.axi_uv_r_id = 3,
>  		.max_upscale_factor = 4,
>  		.max_downscale_factor = 4,
>  		.dly = { 4, 26, 29 },
> @@ -461,6 +470,9 @@ static const struct vop2_win_data rk3588_vop_win_data[]
> = { .supported_rotations = DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270 |
> DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y,
>  		.type = DRM_PLANE_TYPE_PRIMARY,
> +		.axi_bus_id = 1,
> +		.axi_yrgb_r_id = 6,
> +		.axi_uv_r_id = 7,
>  		.max_upscale_factor = 4,
>  		.max_downscale_factor = 4,
>  		.dly = { 4, 26, 29 },
> @@ -475,6 +487,9 @@ static const struct vop2_win_data rk3588_vop_win_data[]
> = { .layer_sel_id = 2,
>  		.supported_rotations = DRM_MODE_REFLECT_Y,
>  		.type = DRM_PLANE_TYPE_OVERLAY,
> +		.axi_bus_id = 0,
> +		.axi_yrgb_r_id = 0x0a,
> +		.axi_uv_r_id = 0x0b,
>  		.max_upscale_factor = 8,
>  		.max_downscale_factor = 8,
>  		.dly = { 23, 45, 48 },
> @@ -488,6 +503,9 @@ static const struct vop2_win_data rk3588_vop_win_data[]
> = { .layer_sel_id = 3,
>  		.supported_rotations = DRM_MODE_REFLECT_Y,
>  		.type = DRM_PLANE_TYPE_OVERLAY,
> +		.axi_bus_id = 0,
> +		.axi_yrgb_r_id = 0x0c,
> +		.axi_uv_r_id = 0x01,
>  		.max_upscale_factor = 8,
>  		.max_downscale_factor = 8,
>  		.dly = { 23, 45, 48 },
> @@ -501,6 +519,9 @@ static const struct vop2_win_data rk3588_vop_win_data[]
> = { .layer_sel_id = 6,
>  		.supported_rotations = DRM_MODE_REFLECT_Y,
>  		.type = DRM_PLANE_TYPE_OVERLAY,
> +		.axi_bus_id = 1,
> +		.axi_yrgb_r_id = 0x0a,
> +		.axi_uv_r_id = 0x0b,
>  		.max_upscale_factor = 8,
>  		.max_downscale_factor = 8,
>  		.dly = { 23, 45, 48 },
> @@ -514,6 +535,9 @@ static const struct vop2_win_data rk3588_vop_win_data[]
> = { .layer_sel_id = 7,
>  		.supported_rotations = DRM_MODE_REFLECT_Y,
>  		.type = DRM_PLANE_TYPE_OVERLAY,
> +		.axi_bus_id = 1,
> +		.axi_yrgb_r_id = 0x0c,
> +		.axi_uv_r_id = 0x0d,
>  		.max_upscale_factor = 8,
>  		.max_downscale_factor = 8,
>  		.dly = { 23, 45, 48 },