Quoting Laurent Pinchart (2022-09-28 01:58:10) > The BIT() macro is meant to represent a single bit. Don't use it for > values of register fields that span multiple bits. > > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > --- > Changes since v1: > > - Use hex for field values > --- > drivers/gpu/drm/mxsfb/lcdif_regs.h | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_regs.h > b/drivers/gpu/drm/mxsfb/lcdif_regs.h > index 013f2cace2a0..0d5d9bedd94a 100644 > --- a/drivers/gpu/drm/mxsfb/lcdif_regs.h > +++ b/drivers/gpu/drm/mxsfb/lcdif_regs.h > @@ -138,9 +138,9 @@ > > #define DISP_PARA_DISP_ON BIT(31) > #define DISP_PARA_SWAP_EN BIT(30) > -#define DISP_PARA_LINE_PATTERN_UYVY_H (GENMASK(29, 28) | BIT(26))
I mean, I like BIT (and sometimes GENMASK) but ... What was going on there! Defintely better this way. Reviewed-by: Kieran Bingham <kieran.bing...@ideasonboard.com> > -#define DISP_PARA_LINE_PATTERN_RGB565 GENMASK(28, 26) > -#define DISP_PARA_LINE_PATTERN_RGB888 0 > +#define DISP_PARA_LINE_PATTERN_UYVY_H (0xd << 26) > +#define DISP_PARA_LINE_PATTERN_RGB565 (0x7 << 26) > +#define DISP_PARA_LINE_PATTERN_RGB888 (0x0 << 26) > #define DISP_PARA_LINE_PATTERN_MASK GENMASK(29, 26) > #define DISP_PARA_DISP_MODE_MASK GENMASK(25, 24) > #define DISP_PARA_BGND_R_MASK GENMASK(23, 16) > @@ -202,18 +202,18 @@ > > #define CTRLDESCL0_5_EN BIT(31) > #define CTRLDESCL0_5_SHADOW_LOAD_EN BIT(30) > -#define CTRLDESCL0_5_BPP_16_RGB565 BIT(26) > -#define CTRLDESCL0_5_BPP_16_ARGB1555 (BIT(26) | BIT(24)) > -#define CTRLDESCL0_5_BPP_16_ARGB4444 (BIT(26) | BIT(25)) > -#define CTRLDESCL0_5_BPP_YCbCr422 (BIT(26) | BIT(25) | BIT(24)) > -#define CTRLDESCL0_5_BPP_24_RGB888 BIT(27) > -#define CTRLDESCL0_5_BPP_32_ARGB8888 (BIT(27) | BIT(24)) > -#define CTRLDESCL0_5_BPP_32_ABGR8888 (BIT(27) | BIT(25)) > +#define CTRLDESCL0_5_BPP_16_RGB565 (0x4 << 24) > +#define CTRLDESCL0_5_BPP_16_ARGB1555 (0x5 << 24) > +#define CTRLDESCL0_5_BPP_16_ARGB4444 (0x6 << 24) > +#define CTRLDESCL0_5_BPP_YCbCr422 (0x7 << 24) > +#define CTRLDESCL0_5_BPP_24_RGB888 (0x8 << 24) > +#define CTRLDESCL0_5_BPP_32_ARGB8888 (0x9 << 24) > +#define CTRLDESCL0_5_BPP_32_ABGR8888 (0xa << 24) > #define CTRLDESCL0_5_BPP_MASK GENMASK(27, 24) > -#define CTRLDESCL0_5_YUV_FORMAT_Y2VY1U 0 > -#define CTRLDESCL0_5_YUV_FORMAT_Y2UY1V BIT(14) > -#define CTRLDESCL0_5_YUV_FORMAT_VY2UY1 BIT(15) > -#define CTRLDESCL0_5_YUV_FORMAT_UY2VY1 (BIT(15) | BIT(14)) > +#define CTRLDESCL0_5_YUV_FORMAT_Y2VY1U (0x0 << 14) > +#define CTRLDESCL0_5_YUV_FORMAT_Y2UY1V (0x1 << 14) > +#define CTRLDESCL0_5_YUV_FORMAT_VY2UY1 (0x2 << 14) > +#define CTRLDESCL0_5_YUV_FORMAT_UY2VY1 (0x3 << 14) > #define CTRLDESCL0_5_YUV_FORMAT_MASK GENMASK(15, 14) > > #define CSC0_CTRL_CSC_MODE_RGB2YCbCr GENMASK(2, 1) > -- > Regards, > > Laurent Pinchart >
