Re: [PATCH v4 06/12] drm/mediatek: mtk_dp: Rework register offsets for proper PHY usage

From: AngeloGioacchino Del Regno

Date: Wed Sep 09 2026 - 07:45:33 EST


On 8/28/26 05:32, CK Hu (胡俊光) wrote:
On Thu, 2026-07-09 at 13:31 +0200, AngeloGioacchino Del Regno wrote:
Rework all of the register offsets to subtract the DP PHY register
range from the MediaTek DisplayPort IP register range which starts
from TOP_OFFSET instead.

This is done in preparation for adding support for registering the
PHY from devicetree, and also for properly handling PHY registers
in the PHY driver (instead of half and half...), which is also one
prerogative to add support for new IP versions in the future.

Note that the current regmap_config was renamed to make sure that
it being legacy is mentioned, as a new config will be required in
future changes.

Of course, with the TOP offset being related to hardware registers
this will never change for the currently supported SoCs, hence for
simplifying code, the MTK_DP_TOP_OFFSET_LEGACY is added statically
to the legacy regmap configuration while, on the other hand, it is
added dynamically to the mtk_dp structure, as the latter is used
to add an offset to every register write in the mtk_dp driver.

The regmap_config provided reg_base (which does exactly the same)
cannot be used here, because the PHY driver is using the very same
regmap pointer, and the offset shall not be taken into account in
writes performed by the PHY driver (only ones performed by mtk_dp!).

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
---
drivers/gpu/drm/mediatek/mtk_dp.c | 101 +++++-----
drivers/gpu/drm/mediatek/mtk_dp_reg.h | 255 ++++++++++++--------------
2 files changed, 179 insertions(+), 177 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index b50264ca1f31..b2e5fa0914bc 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -35,6 +35,27 @@
#include "mtk_dp_reg.h"
+/* PHY Registers - for legacy only */
+#define DP_PHY_GLB_BIAS_GEN_00 0x0
+# define RG_XTP_GLB_BIAS_INTR_CTRL GENMASK(20, 16)

Align the register definition indent to other mtk dp register definition.


The plan is to make cosmetic changes later to align all register definitions
in the entire mediatek-drm to follow that, because this indent style is a bit
more readable, especially when definitions get a bit more complex.

#define REGISTER
# define REG_BITMASK0
# define BITMASK_RELATIVE_VALUE
# define REG_BITMASK1

+#define DP_PHY_GLB_DPAUX_TX 0x8
+# define RG_CKM_PT0_CKTX_IMPSEL GENMASK(23, 20)
+#define MTK_DP_0034 0x34
+# define DA_CKM_CKTX0_EN_FORCE_EN BIT(10)
+#define DP_PHY_LANE_TX_0 0x104
+#define DP_PHY_LANE_TX_1 0x204
+#define DP_PHY_LANE_TX_2 0x304
+#define DP_PHY_LANE_TX_3 0x404
+# define RG_XTP_LNx_TX_IMPSEL_PMOS GENMASK(15, 12)
+# define RG_XTP_LNx_TX_IMPSEL_NMOS GENMASK(19, 16)
+#define DP_PHY_AUX_RX_CTL 0x1040
+# define RG_DPAUX_RX_VALID_DEGLITCH_EN BIT(2)
+# define RG_XTP_GLB_CKDET_EN BIT(1)
+# define RG_DPAUX_RX_EN BIT(0)
+

[snip]

static int mtk_dp_write(struct mtk_dp *mtk_dp, u32 offset, u32 val)
{
- int ret = regmap_write(mtk_dp->regs, offset, val);
+ int ret = regmap_write(mtk_dp->regs, offset + mtk_dp->legacy_regoff, val);
if (ret)
dev_err(mtk_dp->dev,
@@ -436,7 +460,7 @@ static int mtk_dp_write(struct mtk_dp *mtk_dp, u32 offset, u32 val)
static int mtk_dp_update_bits(struct mtk_dp *mtk_dp, u32 offset,
u32 val, u32 mask)
{
- int ret = regmap_update_bits(mtk_dp->regs, offset, mask, val);
+ int ret = regmap_update_bits(mtk_dp->regs, offset + mtk_dp->legacy_regoff, mask, val);
if (ret)
dev_err(mtk_dp->dev,
@@ -566,7 +590,8 @@ static int mtk_dp_set_color_format(struct mtk_dp *mtk_dp,
DP_TEST_COLOR_FORMAT_MASK);
mtk_dp_update_bits(mtk_dp, MTK_DP_ENC0_P0_303C,
- val, PIXEL_ENCODE_FORMAT_DP_ENC0_P0_MASK);
+ FIELD_PREP(PIXEL_ENCODE_FORMAT_DP_ENC0_P0_MASK, val),

This is not related to register offset, so move to other patch.

+ PIXEL_ENCODE_FORMAT_DP_ENC0_P0_MASK);
return 0;
}

[snip]

static int mtk_dp_phy_configure(struct mtk_dp *mtk_dp,
@@ -1401,21 +1414,21 @@ static void mtk_dp_power_enable(struct mtk_dp *mtk_dp)
SW_RST_B_PHYD, SW_RST_B_PHYD);
mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
DP_PWR_STATE_BANDGAP_TPLL, DP_PWR_STATE_MASK);
- mtk_dp_write(mtk_dp, MTK_DP_1040,
+ regmap_write(mtk_dp->regs, DP_PHY_AUX_RX_CTL,
RG_DPAUX_RX_VALID_DEGLITCH_EN | RG_XTP_GLB_CKDET_EN |
RG_DPAUX_RX_EN);
- mtk_dp_update_bits(mtk_dp, MTK_DP_0034, 0, DA_CKM_CKTX0_EN_FORCE_EN);
+ regmap_clear_bits(mtk_dp->regs, MTK_DP_0034, DA_CKM_CKTX0_EN_FORCE_EN);

How do you know MTK_DP_1040 has a meaningful name DP_PHY_AUX_RX_CTL?

Datasheet states the name.

Why don't you know MTK_DP_0034 has a meaningful name?

...datasheet doesn't state the name :-)


}


[snip]


@@ -69,43 +29,59 @@
#define DP_TX2_PRE_EMPH_MASK GENMASK(19, 18)
#define DP_TX3_VOLT_SWING_MASK GENMASK(25, 24)
#define DP_TX3_PRE_EMPH_MASK GENMASK(27, 26)
-#define MTK_DP_TOP_RESET_AND_PROBE 0x2020
+#define MTK_DP_TOP_RESET_AND_PROBE 0x20
#define SW_RST_B_PHYD BIT(4)
-#define MTK_DP_TOP_IRQ_MASK 0x202c
+#define RG_SW_RST_MASK GENMASK(7, 0)
+#define RG_SW_RST 0xff
+#define RG_PROBE_LOW_SEL_MASK GENMASK(18, 16)
+#define RG_PROBE_LOW_SEL BIT(16)
+#define RG_PROBE_LOW_HIGH_SWAP_MASK BIT(23)
+#define RG_PROBE_LOW_HIGH_SWAP BIT(23)

These new definition is not related to this patch, so move to other patch.

+
+#define MTK_DP_TOP_IRQ_MASK 0x2c
+#define ENCODER_IRQ_MSK BIT(0)
+#define TRANS_IRQ_MSK BIT(1)

Ditto.

#define IRQ_MASK_AUX_TOP_IRQ BIT(2)
-#define MTK_DP_TOP_MEM_PD 0x2038
+
+#define MTK_DP_TOP_MEM_PD 0x38
#define MEM_ISO_EN BIT(0)
#define FUSE_SEL BIT(2)
-/* offset: ENC0_OFFSET (0x3000) */
-#define MTK_DP_ENC0_P0_3000 0x3000
+#define EDP_TX_TOP_CLKGEN_0 0x74
+#define EDP_TX_TOP_CLKGEN_REST_MASK 0xf
+#define EDP_TX_TOP_CLKGEN_REST_VALUE 0xf

Ditto.

+
+/* offset: ENC0_OFFSET (0x1000) */
+#define MTK_DP_ENC0_P0_3000 0x1000
#define LANE_NUM_DP_ENC0_P0_MASK GENMASK(1, 0)
#define VIDEO_MUTE_SW_DP_ENC0_P0 BIT(2)
#define VIDEO_MUTE_SEL_DP_ENC0_P0 BIT(3)
#define ENHANCED_FRAME_EN_DP_ENC0_P0 BIT(4)
-#define MTK_DP_ENC0_P0_3004 0x3004
+#define DP_I_MODE_ENABLE BIT(6)
+#define REG_BS_SYMBOL_CNT_RESET BIT(7)

Ditto.

Ok


+#define MTK_DP_ENC0_P0_3004 0x1004
#define VIDEO_M_CODE_SEL_DP_ENC0_P0_MASK BIT(8)
#define DP_TX_ENCODER_4P_RESET_SW_DP_ENC0_P0 BIT(9)
#define SDP_RESET_SW_DP_ENC0_P0 BIT(13)


[snip]

#define VIDEO_COLOR_DEPTH_DP_ENC0_P0_8BIT (3 << 8)
#define VIDEO_COLOR_DEPTH_DP_ENC0_P0_6BIT (4 << 8)
#define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_MASK GENMASK(14, 12)
-#define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_RGB (0 << 12)
-#define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_YCBCR422 (1 << 12)
-#define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_YCBCR420 (2 << 12)
+# define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_RGB 0
+# define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_YCBCR422 1
+# define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_YCBCR420 2

You change the value. Why?


That's done in order to use the FIELD_PREP macro.

Cheers,
Angelo

Regards,
CK

+# define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_YONLY 3
+# define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_RAW 4
#define VIDEO_MN_GEN_EN_DP_ENC0_P0 BIT(15)
-#define MTK_DP_ENC0_P0_3040 0x3040
+#define MTK_DP_ENC0_P0_3040 0x1040
#define SDP_DOWN_CNT_DP_ENC0_P0_VAL 0x20
#define SDP_DOWN_CNT_INIT_DP_ENC0_P0_MASK GENMASK(11, 0)