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

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

Hi Andy,

On 2/13/26 9:07 AM, Andy Yan wrote:
> Hello Cristian，
> 
> At 2026-01-16 23:22:11, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote:
>> On 1/10/26 11:58 AM, Cristian Ciocaltea wrote:
>>> Hi Andy,
>>>
>>> On 1/10/26 6:00 AM, Andy Yan wrote:
>>>>
>>>>
>>>> Hello Cristian,
>>>> At 2025-12-20 05:47:01, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> 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(-)

[...]

>>>>> -	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(bgcolor) >> 6); +
>>>>> val |= FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_GREEN, DRM_ARGB64_GETG(bgcolor)
>>>>> >> 6);
>>>>
>>>>> +	val |= FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_BLUE,
>>>>> DRM_ARGB64_GETB(bgcolor) >> 6);
>>>>
>>>>
>>>> the bit31 of  RK3568_VP_DSP_BG  is bg_display_en， that means when we set a
>>>> background color, we should set this bg_display_en bit.
>>
>> I performed several tests on my ROCK 3A (RK3568), ROCK 4D (RK3576) and ROCK
>> 5B (RK3588) boards and noticed that by setting bg_display_en bit to 1 or 0
>> doesn't have any influence on RK3568 and RK3576, the background color is
>> always active and cannot be disabled.
>>
>> However, flipping the bit to 1 on RK3588 has the unexpected effect of
>> covering the whole screen with the configured color, even when there's an
>> active plane displayed on top. Switching back to 0 makes it work as expected.
>>
>> Therefore I think we should keep this patch as is, unless there's something
>> else we're missing here.
>>
>>>> The default value of this bit is 1, which explains why the patch currently
>>>> works properly even though it doesn't set bit31.
>>>
>>> For some reason, the RK3588 TRM indicates 0x0 for the reset value.  I assume
>>> that's a mistake, as RK3576 TRM shows 0x1.
>>
>> Considering the observation above, it kinda makes sense now for RK3588 to
>> default to 0.
> 
>    I further confirmed with our IC team: for RK3588, RK3528, and RK3562, if
>    the display_enbit is set, the background color will indeed cover all
>    layers. For other chips, this bit has no effect. So ACK

Thanks for clarifying!  

This patch is the only one in the series missing a review, hence could you
please provide your R-b (or A-b) tag on the last revision [1]? 

Regards,
Cristian

[1] https://lore.kernel.org/all/20260204-rk3588-bgcolor-v7-4-78d1d01c5ca1@collabora.com/