On 1/27/26 4:45 PM, Andy Shevchenko wrote: > On Tue, Jan 27, 2026 at 10:45:36AM +0200, 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. > > ... > >> + /* >> + * Background color is programmed with 10 bits of precision. >> + * Since performance is more important than accuracy here, >> + * do *not* make use of the DRM_ARGB64_GET*_BPC() helpers. >> + */ >> + 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); > > There is FIELD_MODIFY() for a few cycles already.
I've been aware, but found it more appropriate when modifying an existing value (e.g. previously read from a register) rather than when computing one from scratch. > But here it probably makes no much difference. Thinking again about this, it is actually handy when keeping the first FIELD_PREP (for initialization), and using it just for the subsequent operations. Hence considered this approach in v6. > ... > >> + seq_printf(s, "\tbackground color (10bpc): r=0x%x g=0x%x b=0x%x\n", >> + DRM_ARGB64_GETR(cstate->background_color) >> 6, >> + DRM_ARGB64_GETG(cstate->background_color) >> 6, >> + DRM_ARGB64_GETB(cstate->background_color) >> 6); > > Probably you want to have the alternative to the DRM_ARGB64_GETx() macros > which > incorporates a right-shift. But it all in regard to DRM style and preferences. I tried to keep the API as simple as possible, but you're right, it's better to have those macros rather than open-coding the bit-shift all over. Also handled in v6. Thanks again, Cristian
