Re: [PATCH v9 06/38] phy: rockchip: usbdp: Drop seamless DP takeover

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Permanently breaks native DP and eDP displays if the bootloader pre-enables the PHY.
- [High] Unconditionally disabling a pre-enabled DP PHY during probe without deterministic synchronization with the display controller (VOP) will cause a hardware lockup.
--

commit b62be7660327d19c538f4a7e04fe1f77ddb82d23
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Drop seamless DP takeover

This commit removes support for seamless DP takeover in the Rockchip USBDP PHY
driver. It attempts to work around active bootloader configurations by forcing
HPD low and waiting before shutting down the PHY.

Link: https://lore.kernel.org/linux-phy/20260612164107.C7DB21F000E9@smtp.kernel.org/

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 3fc8222fcaec2..d734553701134 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c

[ ... ]

> @@ -938,10 +939,14 @@ static int rk_udphy_get_initial_status(struct rk_udphy *udphy)
>  	rk_udphy_reset_deassert_all(udphy);
>  
>  	regmap_read(udphy->pma_regmap, CMN_LANE_MUX_AND_EN_OFFSET, &value);
> -	if (FIELD_GET(CMN_DP_LANE_MUX_ALL, value) && FIELD_GET(CMN_DP_LANE_EN_ALL, value))
> -		udphy->status = UDPHY_MODE_DP;
> -	else
> -		rk_udphy_disable(udphy);
> +	if (FIELD_GET(CMN_DP_LANE_MUX_ALL, value) && FIELD_GET(CMN_DP_LANE_EN_ALL, value)) {
> +		dev_info(udphy->dev, "Started with DP PHY pre-enabled; seamless takeover unsupported\n");
> +		rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, false);

[Severity: High]
Does this permanently break native DP and eDP displays if the bootloader
pre-enables the PHY?

By writing false to the hpd_trigger register, this forces a software override
of the physical HPD pin to a low state. Since this override is never reverted
anywhere in the driver, will the hardware DP controller permanently read HPD
as low, preventing native displays from link training?

> +		/* Wait for VOP side to shut up */
> +		msleep(100);
> +	}
> +
> +	rk_udphy_disable(udphy);

[Severity: High]
Can unconditionally disabling a pre-enabled DP PHY here cause a hardware lockup?

Since the PHY driver probes very early, the DRM driver has not yet bound,
meaning no interrupt handler is present to process the HPD drop, and the Video
Output Processor (VOP) continues streaming. After the 100ms delay,
rk_udphy_disable() abruptly turns off the PHY and its PLL.

Does halting the pixel clock while the VOP is actively transacting cause an
interconnect stall on Rockchip SoCs?

>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-rockchip-usbdp-cleanup-v9-0-e31efbb62d2e@collabora.com?part=6