1. Patchew
  2. linux
  3. [PATCH v5 00/18] VOP Support for rk3576
  4. View patch

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

Andy Yan posted 18 patches 1 year ago
There is a newer version of this series
[PATCH v5 05/18] drm/rockchip: vop2: Set AXI id for rk3588
Posted by Andy Yan 1 year ago 
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) {
+		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 },
-- 
2.34.1
Re: [PATCH v5 05/18] drm/rockchip: vop2: Set AXI id for rk3588
Posted by Detlev Casanova 1 year ago 
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 },
Re:Re: [PATCH v5 05/18] drm/rockchip: vop2: Set AXI id for rk3588
Posted by Andy Yan 1 year ago 
Hi Detlev,

At 2024-12-11 02:40:14, "Detlev Casanova" <detlev.casanova@collabora.com> wrote:
>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 think this is because you tested before with the patch I gave Derek  without axi id 
assigned for rk3576.
I assigned axi id for rk3576 in this version 18/18（The new ID assignment can adapt to more application scenarios.）,
can you test it with the whole V5 version again?

>
>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 },
>
>
>
>
Re: [PATCH v5 05/18] drm/rockchip: vop2: Set AXI id for rk3588
Posted by Detlev Casanova 1 year ago 
On Wednesday, 11 December 2024 01:34:34 EST Andy Yan wrote:
> Hi Detlev,
> 
> At 2024-12-11 02:40:14, "Detlev Casanova" <detlev.casanova@collabora.com> 
wrote:
> >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 think this is because you tested before with the patch I gave Derek 
> without axi id assigned for rk3576.
> I assigned axi id for rk3576 in this version 18/18（The new ID assignment can
> adapt to more application scenarios.）, can you test it with the whole V5
> version again?

Yes, I will do that. But patch 18 mentions:
+       /* Read only bit on rk3576*/
+       [VOP2_WIN_AXI_BUS_ID] = REG_FIELD(RK3568_CLUSTER_CTRL, 13, 13),

And the bit is being written here. If it is indeed writable, then I would drop 
that comment.

> >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);
> >

Detlev.
Re:Re: [PATCH v5 05/18] drm/rockchip: vop2: Set AXI id for rk3588
Posted by Andy Yan 1 year ago 
Hi Detlev,

At 2024-12-11 23:45:01, "Detlev Casanova" <detlev.casanova@collabora.com> wrote:
>On Wednesday, 11 December 2024 01:34:34 EST Andy Yan wrote:
>> Hi Detlev,
>> 
>> At 2024-12-11 02:40:14, "Detlev Casanova" <detlev.casanova@collabora.com> 
>wrote:
>> >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 think this is because you tested before with the patch I gave Derek 
>> without axi id assigned for rk3576.
>> I assigned axi id for rk3576 in this version 18/18（The new ID assignment can
>> adapt to more application scenarios.）, can you test it with the whole V5
>> version again?
>
>Yes, I will do that. But patch 18 mentions:
>+       /* Read only bit on rk3576*/
>+       [VOP2_WIN_AXI_BUS_ID] = REG_FIELD(RK3568_CLUSTER_CTRL, 13, 13),
>
>And the bit is being written here. If it is indeed writable, then I would drop 
>that comment.
The AXI_BUS_ID for two Cluster windows on RK3576 are fixed by hardware, that means
whether we write it or not, it won't change anything.
But the AXI_BUS_ID for Esmart windows on rk3576 and  AXI_YRGB/UV_R_ID for all
cluster/esmart windows on rk3576 are writeable.
I think we directly write it here as the current code can make things easy.



>
>> >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);
>> >
>
>Detlev.
>
>
>
>_______________________________________________
>Linux-rockchip mailing list
>Linux-rockchip@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH v5 05/18] drm/rockchip: vop2: Set AXI id for rk3588
Posted by Detlev Casanova 1 year ago 
On Thursday, 12 December 2024 02:52:34 EST Andy Yan wrote:
> Hi Detlev,
> 
> At 2024-12-11 23:45:01, "Detlev Casanova" <detlev.casanova@collabora.com> 
wrote:
> >On Wednesday, 11 December 2024 01:34:34 EST Andy Yan wrote:
> >> Hi Detlev,
> >> 
> >> At 2024-12-11 02:40:14, "Detlev Casanova" <detlev.casanova@collabora.com>
> >
> >wrote:
> >> >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 think this is because you tested before with the patch I gave Derek
> >> without axi id assigned for rk3576.
> >> I assigned axi id for rk3576 in this version 18/18（The new ID assignment
> >> can adapt to more application scenarios.）, can you test it with the
> >> whole V5 version again?
> >
> >Yes, I will do that. But patch 18 mentions:
> >+       /* Read only bit on rk3576*/
> >+       [VOP2_WIN_AXI_BUS_ID] = REG_FIELD(RK3568_CLUSTER_CTRL, 13, 13),
> >
> >And the bit is being written here. If it is indeed writable, then I would
> >drop that comment.
> 
> The AXI_BUS_ID for two Cluster windows on RK3576 are fixed by hardware, that
> means whether we write it or not, it won't change anything.
> But the AXI_BUS_ID for Esmart windows on rk3576 and  AXI_YRGB/UV_R_ID for
> all cluster/esmart windows on rk3576 are writeable.
> I think we directly write it here as the current code can make things easy.

I see, saying it is read only may be a bit confusing then, but it makes sense. 
You could be more specific adding that writing has no effect.

Anyway, I tested this and it works as expected:

Tested-by: Detlev Casanova <detlev.casanova@collabora.com>

Regards,
Detlev.

> >> >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);
> >
> >Detlev.
> >
> >
> >
> >_______________________________________________
> >Linux-rockchip mailing list
> >Linux-rockchip@lists.infradead.org
> >http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re:Re: [PATCH v5 05/18] drm/rockchip: vop2: Set AXI id for rk3588
Posted by Andy Yan 1 year ago 
Hi Detlev,

At 2024-12-14 02:34:03, "Detlev Casanova" <detlev.casanova@collabora.com> wrote:
>On Thursday, 12 December 2024 02:52:34 EST Andy Yan wrote:
>> Hi Detlev,
>> 
>> At 2024-12-11 23:45:01, "Detlev Casanova" <detlev.casanova@collabora.com> 
>wrote:
>> >On Wednesday, 11 December 2024 01:34:34 EST Andy Yan wrote:
>> >> Hi Detlev,
>> >> 
>> >> At 2024-12-11 02:40:14, "Detlev Casanova" <detlev.casanova@collabora.com>
>> >
>> >wrote:
>> >> >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 think this is because you tested before with the patch I gave Derek
>> >> without axi id assigned for rk3576.
>> >> I assigned axi id for rk3576 in this version 18/18（The new ID assignment
>> >> can adapt to more application scenarios.）, can you test it with the
>> >> whole V5 version again?
>> >
>> >Yes, I will do that. But patch 18 mentions:
>> >+       /* Read only bit on rk3576*/
>> >+       [VOP2_WIN_AXI_BUS_ID] = REG_FIELD(RK3568_CLUSTER_CTRL, 13, 13),
>> >
>> >And the bit is being written here. If it is indeed writable, then I would
>> >drop that comment.
>> 
>> The AXI_BUS_ID for two Cluster windows on RK3576 are fixed by hardware, that
>> means whether we write it or not, it won't change anything.
>> But the AXI_BUS_ID for Esmart windows on rk3576 and  AXI_YRGB/UV_R_ID for
>> all cluster/esmart windows on rk3576 are writeable.
>> I think we directly write it here as the current code can make things easy.
>
>I see, saying it is read only may be a bit confusing then, but it makes sense. 
>You could be more specific adding that writing has no effect.

Okay,  I will add more specific explanation in v6

>
>Anyway, I tested this and it works as expected:
>
>Tested-by: Detlev Casanova <detlev.casanova@collabora.com>
>
>Regards,
>Detlev.
>
>> >> >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);
>> >
>> >Detlev.
>> >
>> >
>> >
>> >_______________________________________________
>> >Linux-rockchip mailing list
>> >Linux-rockchip@lists.infradead.org
>> >http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.