On 29/10/2025 11:57, Geert Uytterhoeven wrote: > Hi Tomi, > > On Tue, 28 Oct 2025 at 18:15, Tomi Valkeinen > <[email protected]> wrote: >> On 05/10/2025 06:02, Marek Vasut wrote: >>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h >>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h >>> @@ -246,14 +246,14 @@ >>> >>> #define VCLKSET 0x100c >>> #define VCLKSET_CKEN (1 << 16) >>> -#define VCLKSET_COLOR_RGB (0 << 8) >>> -#define VCLKSET_COLOR_YCC (1 << 8) >>> +#define VCLKSET_COLOR_YCC (1 << 8) /* 0:RGB 1:YCbCr */ >>> #define VCLKSET_DIV_V3U(x) (((x) & 0x3) << 4) >>> #define VCLKSET_DIV_V4H(x) (((x) & 0x7) << 4) >>> -#define VCLKSET_BPP_16 (0 << 2) >>> -#define VCLKSET_BPP_18 (1 << 2) >>> -#define VCLKSET_BPP_18L (2 << 2) >>> -#define VCLKSET_BPP_24 (3 << 2) >>> +#define VCLKSET_BPP_MASK (3 << 2) >>> +#define VCLKSET_BPP_16 FIELD_PREP(VCLKSET_BPP_MASK, >>> 0) >>> +#define VCLKSET_BPP_18 FIELD_PREP(VCLKSET_BPP_MASK, >>> 1) >>> +#define VCLKSET_BPP_18L FIELD_PREP(VCLKSET_BPP_MASK, >>> 2) >>> +#define VCLKSET_BPP_24 FIELD_PREP(VCLKSET_BPP_MASK, >>> 3) >>> #define VCLKSET_LANE(x) (((x) & 0x3) << 0) >> It probably doesn't matter, but just wanted to mention: here FIELD_PREP >> is used with, e.g., (3 << 2). GENMASK returns an unsigned value, whereas >> (3 << 2) is signed. > > Huh? > > Either you use the unshifted value "(define for) 3" with FIELD_PREP(), > or you use the shifted value "(define for) (3 << 2)" without FIELD_PREP()? Sure. My point was, VCLKSET_BPP_MASK is a signed value, but GENMASK() would produce an unsigned value. Normally FIELD_PREP() is used with GENMASK, i.e. with unsigned mask, but here FIELD_PREP is used with a signed mask. Does it matter? I don't know, most likely not.
Tomi
