[PATCH v3 4/4] drm/rockchip: vop2: Support setting custom background color

Cristian Ciocaltea posted 4 patches 2 weeks ago
[PATCH v3 4/4] drm/rockchip: vop2: Support setting custom background color
Posted by Cristian Ciocaltea 2 weeks ago
The Rockchip VOP2 display controller allows configuring the background
color of each video output port.

Since a previous patch introduced the BACKGROUND_COLOR CRTC property,
which defaults to solid black, make use of it when programming the
hardware.

Note the maximum precision allowed by the display controller is 10bpc,
while the alpha component is not supported, hence ignored.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 13 ++++++++++++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  4 ++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 498df0ce4680..87110beba366 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1554,6 +1554,7 @@ static void vop2_post_config(struct drm_crtc *crtc)
 	struct vop2_video_port *vp = to_vop2_video_port(crtc);
 	struct vop2 *vop2 = vp->vop2;
 	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
+	u64 bgcolor = crtc->state->background_color;
 	u16 vtotal = mode->crtc_vtotal;
 	u16 hdisplay = mode->crtc_hdisplay;
 	u16 hact_st = mode->crtc_htotal - mode->crtc_hsync_start;
@@ -1599,7 +1600,11 @@ static void vop2_post_config(struct drm_crtc *crtc)
 		vop2_vp_write(vp, RK3568_VP_POST_DSP_VACT_INFO_F1, val);
 	}
 
-	vop2_vp_write(vp, RK3568_VP_DSP_BG, 0);
+	/* Background color is programmed with 10 bits of precision */
+	val = FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_RED, DRM_ARGB64_GETR_BPC(bgcolor, 10));
+	val |= FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_GREEN, DRM_ARGB64_GETG_BPC(bgcolor, 10));
+	val |= FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_BLUE, DRM_ARGB64_GETB_BPC(bgcolor, 10));
+	vop2_vp_write(vp, RK3568_VP_DSP_BG, val);
 }
 
 static int us_to_vertical_line(struct drm_display_mode *mode, int us)
@@ -1984,6 +1989,10 @@ static int vop2_crtc_state_dump(struct drm_crtc *crtc, struct seq_file *s)
 		   drm_get_bus_format_name(vcstate->bus_format));
 	seq_printf(s, "\toutput_mode[%x]", vcstate->output_mode);
 	seq_printf(s, " color_space[%d]\n", vcstate->color_space);
+	seq_printf(s, "\tbackground color (10bpc): r=0x%x g=0x%x b=0x%x\n",
+		   DRM_ARGB64_GETR_BPC(cstate->background_color, 10),
+		   DRM_ARGB64_GETG_BPC(cstate->background_color, 10),
+		   DRM_ARGB64_GETB_BPC(cstate->background_color, 10));
 	seq_printf(s, "    Display mode: %dx%d%s%d\n",
 		   mode->hdisplay, mode->vdisplay, interlaced ? "i" : "p",
 		   drm_mode_vrefresh(mode));
@@ -2473,6 +2482,8 @@ static int vop2_create_crtcs(struct vop2 *vop2)
 			return dev_err_probe(drm->dev, ret,
 					     "crtc init for video_port%d failed\n", i);
 
+		drm_crtc_attach_background_color_property(&vp->crtc);
+
 		drm_crtc_helper_add(&vp->crtc, &vop2_crtc_helper_funcs);
 		if (vop2->lut_regs) {
 			const struct vop2_video_port_data *vp_data = &vop2_data->vp[vp->id];
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
index 9124191899ba..37722652844a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
@@ -658,6 +658,10 @@ enum dst_factor_mode {
 #define RK3588_VP_CLK_CTRL__DCLK_OUT_DIV		GENMASK(3, 2)
 #define RK3588_VP_CLK_CTRL__DCLK_CORE_DIV		GENMASK(1, 0)
 
+#define RK3568_VP_DSP_BG__DSP_BG_RED			GENMASK(29, 20)
+#define RK3568_VP_DSP_BG__DSP_BG_GREEN			GENMASK(19, 10)
+#define RK3568_VP_DSP_BG__DSP_BG_BLUE			GENMASK(9, 0)
+
 #define RK3568_VP_POST_SCL_CTRL__VSCALEDOWN		BIT(1)
 #define RK3568_VP_POST_SCL_CTRL__HSCALEDOWN		BIT(0)
 

-- 
2.51.2
Re: [PATCH v3 4/4] drm/rockchip: vop2: Support setting custom background color
Posted by Chaoyi Chen 3 days, 18 hours ago
Hello Cristian,

On 11/18/2025 7:52 AM, Cristian Ciocaltea wrote:
> The Rockchip VOP2 display controller allows configuring the background
> color of each video output port.
> 
> Since a previous patch introduced the BACKGROUND_COLOR CRTC property,
> which defaults to solid black, make use of it when programming the
> hardware.
> 
> Note the maximum precision allowed by the display controller is 10bpc,
> while the alpha component is not supported, hence ignored.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 13 ++++++++++++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  4 ++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 498df0ce4680..87110beba366 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -1554,6 +1554,7 @@ static void vop2_post_config(struct drm_crtc *crtc)
>  	struct vop2_video_port *vp = to_vop2_video_port(crtc);
>  	struct vop2 *vop2 = vp->vop2;
>  	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
> +	u64 bgcolor = crtc->state->background_color;
>  	u16 vtotal = mode->crtc_vtotal;
>  	u16 hdisplay = mode->crtc_hdisplay;
>  	u16 hact_st = mode->crtc_htotal - mode->crtc_hsync_start;
> @@ -1599,7 +1600,11 @@ static void vop2_post_config(struct drm_crtc *crtc)
>  		vop2_vp_write(vp, RK3568_VP_POST_DSP_VACT_INFO_F1, val);
>  	}
>  
> -	vop2_vp_write(vp, RK3568_VP_DSP_BG, 0);
> +	/* Background color is programmed with 10 bits of precision */
> +	val = FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_RED, DRM_ARGB64_GETR_BPC(bgcolor, 10));
> +	val |= FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_GREEN, DRM_ARGB64_GETG_BPC(bgcolor, 10));
> +	val |= FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_BLUE, DRM_ARGB64_GETB_BPC(bgcolor, 10));

Division is expensive. If we convert a 16 bpc value to 10 bpc using
direct bit-shifts, that is "DRM_ARGB64_GETX(bgcolor) >> 6" will
keep the relative error within 1 compared to DIV_ROUND_CLOSEST().

Should we be concerned about the precision problem here?

> +	vop2_vp_write(vp, RK3568_VP_DSP_BG, val);
>  }
>  
>  static int us_to_vertical_line(struct drm_display_mode *mode, int us)
> @@ -1984,6 +1989,10 @@ static int vop2_crtc_state_dump(struct drm_crtc *crtc, struct seq_file *s)
>  		   drm_get_bus_format_name(vcstate->bus_format));
>  	seq_printf(s, "\toutput_mode[%x]", vcstate->output_mode);
>  	seq_printf(s, " color_space[%d]\n", vcstate->color_space);
> +	seq_printf(s, "\tbackground color (10bpc): r=0x%x g=0x%x b=0x%x\n",
> +		   DRM_ARGB64_GETR_BPC(cstate->background_color, 10),
> +		   DRM_ARGB64_GETG_BPC(cstate->background_color, 10),
> +		   DRM_ARGB64_GETB_BPC(cstate->background_color, 10));
>  	seq_printf(s, "    Display mode: %dx%d%s%d\n",
>  		   mode->hdisplay, mode->vdisplay, interlaced ? "i" : "p",
>  		   drm_mode_vrefresh(mode));
> @@ -2473,6 +2482,8 @@ static int vop2_create_crtcs(struct vop2 *vop2)
>  			return dev_err_probe(drm->dev, ret,
>  					     "crtc init for video_port%d failed\n", i);
>  
> +		drm_crtc_attach_background_color_property(&vp->crtc);
> +
>  		drm_crtc_helper_add(&vp->crtc, &vop2_crtc_helper_funcs);
>  		if (vop2->lut_regs) {
>  			const struct vop2_video_port_data *vp_data = &vop2_data->vp[vp->id];
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> index 9124191899ba..37722652844a 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> @@ -658,6 +658,10 @@ enum dst_factor_mode {
>  #define RK3588_VP_CLK_CTRL__DCLK_OUT_DIV		GENMASK(3, 2)
>  #define RK3588_VP_CLK_CTRL__DCLK_CORE_DIV		GENMASK(1, 0)
>  
> +#define RK3568_VP_DSP_BG__DSP_BG_RED			GENMASK(29, 20)
> +#define RK3568_VP_DSP_BG__DSP_BG_GREEN			GENMASK(19, 10)
> +#define RK3568_VP_DSP_BG__DSP_BG_BLUE			GENMASK(9, 0)
> +
>  #define RK3568_VP_POST_SCL_CTRL__VSCALEDOWN		BIT(1)
>  #define RK3568_VP_POST_SCL_CTRL__HSCALEDOWN		BIT(0)
>  

-- 
Best, 
Chaoyi
Re: [PATCH v3 4/4] drm/rockchip: vop2: Support setting custom background color
Posted by Cristian Ciocaltea 3 days, 17 hours ago
Hi Chaoyi,

On 11/28/25 10:46 AM, Chaoyi Chen wrote:
> Hello Cristian,
> 
> On 11/18/2025 7:52 AM, Cristian Ciocaltea wrote:
>> The Rockchip VOP2 display controller allows configuring the background
>> color of each video output port.
>>
>> Since a previous patch introduced the BACKGROUND_COLOR CRTC property,
>> which defaults to solid black, make use of it when programming the
>> hardware.
>>
>> Note the maximum precision allowed by the display controller is 10bpc,
>> while the alpha component is not supported, hence ignored.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 13 ++++++++++++-
>>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  4 ++++
>>  2 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> index 498df0ce4680..87110beba366 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> @@ -1554,6 +1554,7 @@ static void vop2_post_config(struct drm_crtc *crtc)
>>  	struct vop2_video_port *vp = to_vop2_video_port(crtc);
>>  	struct vop2 *vop2 = vp->vop2;
>>  	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
>> +	u64 bgcolor = crtc->state->background_color;
>>  	u16 vtotal = mode->crtc_vtotal;
>>  	u16 hdisplay = mode->crtc_hdisplay;
>>  	u16 hact_st = mode->crtc_htotal - mode->crtc_hsync_start;
>> @@ -1599,7 +1600,11 @@ static void vop2_post_config(struct drm_crtc *crtc)
>>  		vop2_vp_write(vp, RK3568_VP_POST_DSP_VACT_INFO_F1, val);
>>  	}
>>  
>> -	vop2_vp_write(vp, RK3568_VP_DSP_BG, 0);
>> +	/* Background color is programmed with 10 bits of precision */
>> +	val = FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_RED, DRM_ARGB64_GETR_BPC(bgcolor, 10));
>> +	val |= FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_GREEN, DRM_ARGB64_GETG_BPC(bgcolor, 10));
>> +	val |= FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_BLUE, DRM_ARGB64_GETB_BPC(bgcolor, 10));
> 
> Division is expensive. If we convert a 16 bpc value to 10 bpc using
> direct bit-shifts, that is "DRM_ARGB64_GETX(bgcolor) >> 6" will
> keep the relative error within 1 compared to DIV_ROUND_CLOSEST().
> 
> Should we be concerned about the precision problem here?

The precision was something I initially looked into for CRC verification, in the
context of the related IGT test.  But since I've added the VKMS support, I think
we should not worry about that anymore. 

Moreover, as already pointed out in [1], only RK3576 supports CRC generation at
display controller level, and that is not particularly useful because it doesn't
take the background color into account.  Therefore I had to capture the frame
CRCs at DisplayPort AUX channel level, by using the USB-C DP AltMode capable
port of my RK3588-based board.  However, that solution is not yet available
upstream, as it requires further work for cleanup and improving the overall
USB-C reliability. 

Hence I'll move on with your suggestion and switch to the simple bit-shifting
approach for the next revision.

Thanks,
Cristian

[1] https://lore.kernel.org/all/20251118-crtc-bgcolor-v2-1-dce4063f85a9@collabora.com/
Re: [PATCH v3 4/4] drm/rockchip: vop2: Support setting custom background color
Posted by Chaoyi Chen 2 days, 23 hours ago
Hello Cristian, 

On 11/28/2025 5:44 PM, Cristian Ciocaltea wrote:
> The precision was something I initially looked into for CRC verification, in the
> context of the related IGT test.  But since I've added the VKMS support, I think
> we should not worry about that anymore. 
> 
> Moreover, as already pointed out in [1], only RK3576 supports CRC generation at
> display controller level, and that is not particularly useful because it doesn't

I believe you can get the CRTC CRC on the RK3576, even when only the 
background is visible and all plane is disabled. Feel free to let me
know if you run into any issues :)

> take the background color into account.  Therefore I had to capture the frame
> CRCs at DisplayPort AUX channel level, by using the USB-C DP AltMode capable

Oh that sounds interesting! I'm not sure how complex it would be to
implement.

> port of my RK3588-based board.  However, that solution is not yet available
> upstream, as it requires further work for cleanup and improving the overall
> USB-C reliability. 
> 
> Hence I'll move on with your suggestion and switch to the simple bit-shifting
> approach for the next revision.

-- 
Best, 
Chaoyi
Re: [PATCH v3 4/4] drm/rockchip: vop2: Support setting custom background color
Posted by Cristian Ciocaltea 2 days, 17 hours ago
Hi Chaoyi,

On 11/29/25 5:49 AM, Chaoyi Chen wrote:
> Hello Cristian, 
> 
> On 11/28/2025 5:44 PM, Cristian Ciocaltea wrote:
>> The precision was something I initially looked into for CRC verification, in the
>> context of the related IGT test.  But since I've added the VKMS support, I think
>> we should not worry about that anymore. 
>>
>> Moreover, as already pointed out in [1], only RK3576 supports CRC generation at
>> display controller level, and that is not particularly useful because it doesn't
> 
> I believe you can get the CRTC CRC on the RK3576, even when only the 
> background is visible and all plane is disabled. Feel free to let me
> know if you run into any issues :)

Yes, CRTC CRC works on RK3576 for the planes, but not for the background, i.e.
the CRC is not updated when setting a different background color.  Unless
there's a way to change this default behavior, which I'm not aware of. 

My current understanding is that the background color is applied at a later
stage in the display pipeline, *after* blending the planes and computing CRC.

>> take the background color into account.  Therefore I had to capture the frame
>> CRCs at DisplayPort AUX channel level, by using the USB-C DP AltMode capable
> 
> Oh that sounds interesting! I'm not sure how complex it would be to
> implement.

That's already implemented, I plan to submit the patches soon.

> 
>> port of my RK3588-based board.  However, that solution is not yet available
>> upstream, as it requires further work for cleanup and improving the overall
>> USB-C reliability. 
>>
>> Hence I'll move on with your suggestion and switch to the simple bit-shifting
>> approach for the next revision.
>
Re: [PATCH v3 4/4] drm/rockchip: vop2: Support setting custom background color
Posted by Chaoyi Chen 1 day, 23 hours ago
On 11/29/2025 5:50 PM, Cristian Ciocaltea wrote:
> Hi Chaoyi,
> 
> On 11/29/25 5:49 AM, Chaoyi Chen wrote:
>> Hello Cristian, 
>>
>> On 11/28/2025 5:44 PM, Cristian Ciocaltea wrote:
>>> The precision was something I initially looked into for CRC verification, in the
>>> context of the related IGT test.  But since I've added the VKMS support, I think
>>> we should not worry about that anymore. 
>>>
>>> Moreover, as already pointed out in [1], only RK3576 supports CRC generation at
>>> display controller level, and that is not particularly useful because it doesn't
>>
>> I believe you can get the CRTC CRC on the RK3576, even when only the 
>> background is visible and all plane is disabled. Feel free to let me
>> know if you run into any issues :)
> 
> Yes, CRTC CRC works on RK3576 for the planes, but not for the background, i.e.
> the CRC is not updated when setting a different background color.  Unless
> there's a way to change this default behavior, which I'm not aware of. 
> 
> My current understanding is that the background color is applied at a later
> stage in the display pipeline, *after* blending the planes and computing CRC.
> 

Which CRTC are you using? If you're using VP0, you might run into this
problem. Please try VP1 instead.

>>> take the background color into account.  Therefore I had to capture the frame
>>> CRCs at DisplayPort AUX channel level, by using the USB-C DP AltMode capable
>>
>> Oh that sounds interesting! I'm not sure how complex it would be to
>> implement.
> 
> That's already implemented, I plan to submit the patches soon.
> 
>>
>>> port of my RK3588-based board.  However, that solution is not yet available
>>> upstream, as it requires further work for cleanup and improving the overall
>>> USB-C reliability. 
>>>
>>> Hence I'll move on with your suggestion and switch to the simple bit-shifting
>>> approach for the next revision.
>>
> 
> 

-- 
Best, 
Chaoyi