Re: [PATCH 1/3] drm/bridge: analogix_dp: deassert DP_PHY_PD on power-on for exynos

[Inki Dae]

Hi,

2026년 10월 27일 (화) 오후 9:17, Alexandre Marquet via B4 Relay
<devnull+tb.a-marquet.fr@xxxxxxxxxx>님이 작성:
>
> From: Alexandre Marquet <tb@xxxxxxxxxxxx>
>
> On exynos platforms at least, calling
> analogix_dp_set_analog_power_down() to power on (with enable=false) CH0
> to CH3 or the AUX bus currently produces no effect, unless the function
> is first called to power on everything (by setting block to POWER_ALL or
> ANALOG_TOTAL). The reason for this is that DP_PHY_PD being asserted
> (which is its default state) renders every other register bit
> ineffective.
>
> This is particularly an issue for AUX bus initialization, that tends to
> happen before anything else (e.g., to request EDID), and thus, before
> anything has any chance to request a power on of the whole analog
> section of the chip.
>
> This commit allows analogix_dp_set_analog_power_down() to power on any
> block, by deasserting DP_PHY_PD if needed.
>
> Signed-off-by: Alexandre Marquet <tb@xxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index c1344a3f013a..9f634d4e4793 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -256,6 +256,19 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>         if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type))
>                 phy_pd_addr = ANALOGIX_DP_PD;
>
> +       reg = readl(dp->reg_base + phy_pd_addr);
> +
> +       if (dp->plat_data && (dp->plat_data->dev_type == EXYNOS_DP)) {
> +               /* On exynos, register AUX_PD and CH[0-3]_PD have no effect if
> +                * DP_PHY_PD is asserted. So, when powering any of them up, we
> +                * need to deassert DP_PHY_PD first.
> +                */
> +               if (!enable && (reg & DP_PHY_PD)) {
> +                       reg &= ~DP_PHY_PD;
> +                       reg |= AUX_PD | CH0_PD | CH1_PD | CH2_PD | CH3_PD;
> +               }

When DP_PHY_PD is set, this path clears it but leaves the bandgap
power-down bits unchanged. If the register retains 0xff from
POWER_ALL(true), an AUX_BLOCK, false call produces 0xcf; both
DP_INC_BG and DP_EXP_BG remain set. Could you check whether AUX can
operate in that state?
I suggest limiting the new handling to AUX_BLOCK and CH[0-3]_BLOCK.
When the bandgap bits are set, keep the other blocks powered down,
release DP_INC_BG, wait as the existing POWER_ALL(false) path does,
and then release the remaining required bits and the requested block.
This also avoids changing ANALOG_TOTAL, false, which currently only
clears DP_PHY_PD.

Thanks,
Inki Dae

> +       }
> +
>         switch (block) {
>         case AUX_BLOCK:
>                 if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type))
> @@ -263,7 +276,6 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>                 else
>                         mask = AUX_PD;
>
> -               reg = readl(dp->reg_base + phy_pd_addr);
>                 if (enable)
>                         reg |= mask;
>                 else
> @@ -272,7 +284,6 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>                 break;
>         case CH0_BLOCK:
>                 mask = CH0_PD;
> -               reg = readl(dp->reg_base + phy_pd_addr);
>
>                 if (enable)
>                         reg |= mask;
> @@ -282,7 +293,6 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>                 break;
>         case CH1_BLOCK:
>                 mask = CH1_PD;
> -               reg = readl(dp->reg_base + phy_pd_addr);
>
>                 if (enable)
>                         reg |= mask;
> @@ -292,7 +302,6 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>                 break;
>         case CH2_BLOCK:
>                 mask = CH2_PD;
> -               reg = readl(dp->reg_base + phy_pd_addr);
>
>                 if (enable)
>                         reg |= mask;
> @@ -302,7 +311,6 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>                 break;
>         case CH3_BLOCK:
>                 mask = CH3_PD;
> -               reg = readl(dp->reg_base + phy_pd_addr);
>
>                 if (enable)
>                         reg |= mask;
> @@ -321,7 +329,6 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>                 else
>                         mask = DP_PHY_PD;
>
> -               reg = readl(dp->reg_base + phy_pd_addr);
>                 if (enable)
>                         reg |= mask;
>                 else
>
> --
> 2.43.0
>
>
>