Change magic numerical masks with usage of the GENMASK() macro
to improve readability.
This commit brings no functional changes.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 46 ++++++++++++++++--------------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index a2fdfc8ddb15..23d2c5be8dbb 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -58,18 +58,18 @@
#define DSI_TXRX_CTRL 0x18
#define VC_NUM BIT(1)
-#define LANE_NUM (0xf << 2)
+#define LANE_NUM GENMASK(5, 2)
#define DIS_EOT BIT(6)
#define NULL_EN BIT(7)
#define TE_FREERUN BIT(8)
#define EXT_TE_EN BIT(9)
#define EXT_TE_EDGE BIT(10)
-#define MAX_RTN_SIZE (0xf << 12)
+#define MAX_RTN_SIZE GENMASK(15, 12)
#define HSTX_CKLP_EN BIT(16)
#define DSI_PSCTRL 0x1c
-#define DSI_PS_WC 0x3fff
-#define DSI_PS_SEL (3 << 16)
+#define DSI_PS_WC GENMASK(14, 0)
+#define DSI_PS_SEL GENMASK(19, 16)
#define PACKED_PS_16BIT_RGB565 (0 << 16)
#define LOOSELY_PS_18BIT_RGB666 (1 << 16)
#define PACKED_PS_18BIT_RGB666 (2 << 16)
@@ -109,26 +109,27 @@
#define LD0_WAKEUP_EN BIT(2)
#define DSI_PHY_TIMECON0 0x110
-#define LPX (0xff << 0)
-#define HS_PREP (0xff << 8)
-#define HS_ZERO (0xff << 16)
-#define HS_TRAIL (0xff << 24)
+#define LPX GENMASK(7, 0)
+#define HS_PREP GENMASK(15, 8)
+#define HS_ZERO GENMASK(23, 16)
+#define HS_TRAIL GENMASK(31, 24)
#define DSI_PHY_TIMECON1 0x114
-#define TA_GO (0xff << 0)
-#define TA_SURE (0xff << 8)
-#define TA_GET (0xff << 16)
-#define DA_HS_EXIT (0xff << 24)
+#define TA_GO GENMASK(7, 0)
+#define TA_SURE GENMASK(15, 8)
+#define TA_GET GENMASK(23, 16)
+#define DA_HS_EXIT GENMASK(31, 24)
#define DSI_PHY_TIMECON2 0x118
-#define CONT_DET (0xff << 0)
-#define CLK_ZERO (0xff << 16)
-#define CLK_TRAIL (0xff << 24)
+#define CONT_DET GENMASK(7, 0)
+#define DA_HS_SYNC GENMASK(15, 8)
+#define CLK_ZERO GENMASK(23, 16)
+#define CLK_TRAIL GENMASK(31, 24)
#define DSI_PHY_TIMECON3 0x11c
-#define CLK_HS_PREP (0xff << 0)
-#define CLK_HS_POST (0xff << 8)
-#define CLK_HS_EXIT (0xff << 16)
+#define CLK_HS_PREP GENMASK(7, 0)
+#define CLK_HS_POST GENMASK(15, 8)
+#define CLK_HS_EXIT GENMASK(23, 16)
#define DSI_VM_CMD_CON 0x130
#define VM_CMD_EN BIT(0)
@@ -138,13 +139,14 @@
#define FORCE_COMMIT BIT(0)
#define BYPASS_SHADOW BIT(1)
-#define CONFIG (0xff << 0)
+/* CMDQ related bits */
+#define CONFIG GENMASK(7, 0)
#define SHORT_PACKET 0
#define LONG_PACKET 2
#define BTA BIT(2)
-#define DATA_ID (0xff << 8)
-#define DATA_0 (0xff << 16)
-#define DATA_1 (0xff << 24)
+#define DATA_ID GENMASK(15, 8)
+#define DATA_0 GENMASK(23, 16)
+#define DATA_1 GENMASK(31, 24)
#define NS_TO_CYCLE(n, c) ((n) / (c) + (((n) % (c)) ? 1 : 0))
--
2.43.0
Hi Angelo, On Wed, Dec 20, 2023 at 9:57 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Change magic numerical masks with usage of the GENMASK() macro > to improve readability. > > This commit brings no functional changes. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 46 ++++++++++++++++-------------- > 1 file changed, 24 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > index a2fdfc8ddb15..23d2c5be8dbb 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -58,18 +58,18 @@ > > #define DSI_TXRX_CTRL 0x18 > #define VC_NUM BIT(1) > -#define LANE_NUM (0xf << 2) > +#define LANE_NUM GENMASK(5, 2) > #define DIS_EOT BIT(6) > #define NULL_EN BIT(7) > #define TE_FREERUN BIT(8) > #define EXT_TE_EN BIT(9) > #define EXT_TE_EDGE BIT(10) > -#define MAX_RTN_SIZE (0xf << 12) > +#define MAX_RTN_SIZE GENMASK(15, 12) > #define HSTX_CKLP_EN BIT(16) > > #define DSI_PSCTRL 0x1c > -#define DSI_PS_WC 0x3fff > -#define DSI_PS_SEL (3 << 16) > +#define DSI_PS_WC GENMASK(14, 0) > +#define DSI_PS_SEL GENMASK(19, 16) GENMASK(17, 16) > > #define PACKED_PS_16BIT_RGB565 (0 << 16) > #define LOOSELY_PS_18BIT_RGB666 (1 << 16) > #define PACKED_PS_18BIT_RGB666 (2 << 16) > @@ -109,26 +109,27 @@ > #define LD0_WAKEUP_EN BIT(2) > > #define DSI_PHY_TIMECON0 0x110 > -#define LPX (0xff << 0) > -#define HS_PREP (0xff << 8) > -#define HS_ZERO (0xff << 16) > -#define HS_TRAIL (0xff << 24) > +#define LPX GENMASK(7, 0) > +#define HS_PREP GENMASK(15, 8) > +#define HS_ZERO GENMASK(23, 16) > +#define HS_TRAIL GENMASK(31, 24) > > #define DSI_PHY_TIMECON1 0x114 > -#define TA_GO (0xff << 0) > -#define TA_SURE (0xff << 8) > -#define TA_GET (0xff << 16) > -#define DA_HS_EXIT (0xff << 24) > +#define TA_GO GENMASK(7, 0) > +#define TA_SURE GENMASK(15, 8) > +#define TA_GET GENMASK(23, 16) > +#define DA_HS_EXIT GENMASK(31, 24) > > #define DSI_PHY_TIMECON2 0x118 > -#define CONT_DET (0xff << 0) > -#define CLK_ZERO (0xff << 16) > -#define CLK_TRAIL (0xff << 24) > +#define CONT_DET GENMASK(7, 0) > +#define DA_HS_SYNC GENMASK(15, 8) This is new, so please introduce it in a separate patch if intended. The rest looks good to me. Regards, Fei > > +#define CLK_ZERO GENMASK(23, 16) > +#define CLK_TRAIL GENMASK(31, 24) > > #define DSI_PHY_TIMECON3 0x11c > -#define CLK_HS_PREP (0xff << 0) > -#define CLK_HS_POST (0xff << 8) > -#define CLK_HS_EXIT (0xff << 16) > +#define CLK_HS_PREP GENMASK(7, 0) > +#define CLK_HS_POST GENMASK(15, 8) > +#define CLK_HS_EXIT GENMASK(23, 16) > > #define DSI_VM_CMD_CON 0x130 > #define VM_CMD_EN BIT(0) > @@ -138,13 +139,14 @@ > #define FORCE_COMMIT BIT(0) > #define BYPASS_SHADOW BIT(1) > > -#define CONFIG (0xff << 0) > +/* CMDQ related bits */ > +#define CONFIG GENMASK(7, 0) > #define SHORT_PACKET 0 > #define LONG_PACKET 2 > #define BTA BIT(2) > -#define DATA_ID (0xff << 8) > -#define DATA_0 (0xff << 16) > -#define DATA_1 (0xff << 24) > +#define DATA_ID GENMASK(15, 8) > +#define DATA_0 GENMASK(23, 16) > +#define DATA_1 GENMASK(31, 24) > > #define NS_TO_CYCLE(n, c) ((n) / (c) + (((n) % (c)) ? 1 : 0)) > > -- > 2.43.0 > >
Il 26/12/23 11:46, Fei Shao ha scritto: > Hi Angelo, > > On Wed, Dec 20, 2023 at 9:57 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> Change magic numerical masks with usage of the GENMASK() macro >> to improve readability. >> >> This commit brings no functional changes. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- >> drivers/gpu/drm/mediatek/mtk_dsi.c | 46 ++++++++++++++++-------------- >> 1 file changed, 24 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c >> index a2fdfc8ddb15..23d2c5be8dbb 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c >> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c >> @@ -58,18 +58,18 @@ >> >> #define DSI_TXRX_CTRL 0x18 >> #define VC_NUM BIT(1) >> -#define LANE_NUM (0xf << 2) >> +#define LANE_NUM GENMASK(5, 2) >> #define DIS_EOT BIT(6) >> #define NULL_EN BIT(7) >> #define TE_FREERUN BIT(8) >> #define EXT_TE_EN BIT(9) >> #define EXT_TE_EDGE BIT(10) >> -#define MAX_RTN_SIZE (0xf << 12) >> +#define MAX_RTN_SIZE GENMASK(15, 12) >> #define HSTX_CKLP_EN BIT(16) >> >> #define DSI_PSCTRL 0x1c >> -#define DSI_PS_WC 0x3fff >> -#define DSI_PS_SEL (3 << 16) >> +#define DSI_PS_WC GENMASK(14, 0) >> +#define DSI_PS_SEL GENMASK(19, 16) > > GENMASK(17, 16) That was intended, and depends on the SoC - I should effectively advertise that in the commit description and I will. As per the datasheets: MT8192 - DSI_PS_SEL[18:16] MT8195 - DSI_PS_SEL[19:16] ...on SoCs that don't have those additional bits, the higher ones are reserved (unused), so it is safe to have this as 19, 16. >> >> #define PACKED_PS_16BIT_RGB565 (0 << 16) >> #define LOOSELY_PS_18BIT_RGB666 (1 << 16) >> #define PACKED_PS_18BIT_RGB666 (2 << 16) >> @@ -109,26 +109,27 @@ >> #define LD0_WAKEUP_EN BIT(2) >> >> #define DSI_PHY_TIMECON0 0x110 >> -#define LPX (0xff << 0) >> -#define HS_PREP (0xff << 8) >> -#define HS_ZERO (0xff << 16) >> -#define HS_TRAIL (0xff << 24) >> +#define LPX GENMASK(7, 0) >> +#define HS_PREP GENMASK(15, 8) >> +#define HS_ZERO GENMASK(23, 16) >> +#define HS_TRAIL GENMASK(31, 24) >> >> #define DSI_PHY_TIMECON1 0x114 >> -#define TA_GO (0xff << 0) >> -#define TA_SURE (0xff << 8) >> -#define TA_GET (0xff << 16) >> -#define DA_HS_EXIT (0xff << 24) >> +#define TA_GO GENMASK(7, 0) >> +#define TA_SURE GENMASK(15, 8) >> +#define TA_GET GENMASK(23, 16) >> +#define DA_HS_EXIT GENMASK(31, 24) >> >> #define DSI_PHY_TIMECON2 0x118 >> -#define CONT_DET (0xff << 0) >> -#define CLK_ZERO (0xff << 16) >> -#define CLK_TRAIL (0xff << 24) >> +#define CONT_DET GENMASK(7, 0) >> +#define DA_HS_SYNC GENMASK(15, 8) > > This is new, so please introduce it in a separate patch if intended. > I wouldn't really be comfortable doing so: this would mean that I'd have to first write a constant and then fix that in a later patch... P.S.: Sorry for working again on this one whole month after .... :-) Regards, Angelo > The rest looks good to me. > > Regards, > Fei > > >> >> +#define CLK_ZERO GENMASK(23, 16) >> +#define CLK_TRAIL GENMASK(31, 24) >> >> #define DSI_PHY_TIMECON3 0x11c >> -#define CLK_HS_PREP (0xff << 0) >> -#define CLK_HS_POST (0xff << 8) >> -#define CLK_HS_EXIT (0xff << 16) >> +#define CLK_HS_PREP GENMASK(7, 0) >> +#define CLK_HS_POST GENMASK(15, 8) >> +#define CLK_HS_EXIT GENMASK(23, 16) >> >> #define DSI_VM_CMD_CON 0x130 >> #define VM_CMD_EN BIT(0) >> @@ -138,13 +139,14 @@ >> #define FORCE_COMMIT BIT(0) >> #define BYPASS_SHADOW BIT(1) >> >> -#define CONFIG (0xff << 0) >> +/* CMDQ related bits */ >> +#define CONFIG GENMASK(7, 0) >> #define SHORT_PACKET 0 >> #define LONG_PACKET 2 >> #define BTA BIT(2) >> -#define DATA_ID (0xff << 8) >> -#define DATA_0 (0xff << 16) >> -#define DATA_1 (0xff << 24) >> +#define DATA_ID GENMASK(15, 8) >> +#define DATA_0 GENMASK(23, 16) >> +#define DATA_1 GENMASK(31, 24) >> >> #define NS_TO_CYCLE(n, c) ((n) / (c) + (((n) % (c)) ? 1 : 0)) >> >> -- >> 2.43.0 >> >>
© 2016 - 2024 Red Hat, Inc.