Re: [PATCH 05/11] drm/mediatek: mtk_dp: Rework register offsets for proper PHY usage

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Writes to PHY registers are incorrectly offset by `legacy_regoff` (0x2000), corrupting DP IP registers and breaking PHY configuration.
- [Medium] Removed bit shifts from PIXEL_ENCODE_FORMAT enums cause YCbCr color formats to silently fall back to RGB.
--

commit ef9afa51895ca44a0e35947eba008bc8a9868897
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

drm/mediatek: mtk_dp: Rework register offsets for proper PHY usage

This commit subtracts the DP PHY register range from the MediaTek DisplayPort
IP register range. It prepares the driver for PHY registration from devicetree
by moving the top offset into the driver structure and out of the legacy regmap
configuration.

> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index f551c4543f4ee..97bce8966a1f3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
[ ... ]
> @@ -424,7 +448,7 @@ static u32 mtk_dp_read(struct mtk_dp *mtk_dp, u32 offset)
>  
>  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);
[ ... ]
> @@ -1401,7 +1425,7 @@ 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,
> +	mtk_dp_write(mtk_dp, DP_PHY_AUX_RX_CTL,
>  		     RG_DPAUX_RX_VALID_DEGLITCH_EN | RG_XTP_GLB_CKDET_EN |
>  		     RG_DPAUX_RX_EN);

[Severity: High]
Does this code inadvertently misdirect PHY register accesses and corrupt
encoder state?

When legacy devicetrees are used, mtk_dp->legacy_regoff is set to 0x2000.
Because the accessor functions like mtk_dp_write() and mtk_dp_update_bits()
now unconditionally add mtk_dp->legacy_regoff, calls using legacy PHY
registers will be offset incorrectly.

For example, when mtk_dp_power_enable() calls mtk_dp_write() with
DP_PHY_AUX_RX_CTL (which is 0x1040), the write will actually go to 0x3040.
This leaves the PHY unconfigured and silently overwrites the DP encoder
register MTK_DP_ENC0_P0_3040 instead.

[ ... ]
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp_reg.h b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> index 8ad7a9cc259eb..616ea6440b08d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
[ ... ]
> @@ -130,34 +106,36 @@
>  #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
> +#  define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_YONLY		3
> +#  define PIXEL_ENCODE_FORMAT_DP_ENC0_P0_RAW		4

[Severity: Medium]
Will this change cause YCbCr color formats to silently fall back to RGB?

The bit shifts were removed from these format definitions, but the usage
site in mtk_dp_set_color_format() passes these values directly to
mtk_dp_update_bits() alongside the PIXEL_ENCODE_FORMAT_DP_ENC0_P0_MASK,
which is GENMASK(14, 12).

Because regmap_update_bits() internally applies the mask to the value, an
unshifted value like PIXEL_ENCODE_FORMAT_DP_ENC0_P0_YCBCR422 (1) combined
with the mask (0x7000) evaluates to 0. This clears the color format bits
entirely and forces the hardware to output in RGB mode.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701122024.19557-1-angelogioacchino.delregno@collabora.com?part=5