[PATCH v4 06/12] drm/mediatek: mtk_dp: Rework register offsets for proper PHY usage
AngeloGioacchino Del Regno
angelogioacchino.delregno at collabora.com
Wed Sep 9 04:35:05 PDT 2026
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 at collabora.com>
>> ---
>> 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)
>>
>
More information about the Linux-mediatek
mailing list