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.delre...@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.delre...@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


Reply via email to