On 11/28/25 11:44 AM, Cristian Ciocaltea wrote: > 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 <[email protected]> >>> --- >>> 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.
Done in v4: https://lore.kernel.org/all/[email protected]/
