[PATCH v2 2/4] drm: lcdif: Don't use BIT() for multi-bit register fields

[Kieran Bingham]

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.pinchart at 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.bingham at 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
>