linux-kernel - Re: [PATCH v3 4/4] drm/rockchip: vop2: Support setting custom background color

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0ed71b8d-5a2e-4213-bd2f-7ba317d1e4cb@rock-chips.com>
Date: Fri, 28 Nov 2025 16:46:46 +0800
From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
To: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Sandy Huang <hjc@...k-chips.com>, Heiko Stübner
 <heiko@...ech.de>, Andy Yan <andy.yan@...k-chips.com>,
 Louis Chauvet <louis.chauvet@...tlin.com>,
 Haneen Mohammed <hamohammed.sa@...il.com>,
 Melissa Wen <melissa.srw@...il.com>
Cc: Robert Mader <robert.mader@...labora.com>, kernel@...labora.com,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v3 4/4] drm/rockchip: vop2: Support setting custom
 background color

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@...labora.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