Re: [PATCH v4 06/10] drm/rockchip: dw_dp: Implement out-of-band HPD handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The VO GRF regmap lookup fails to parse the DisplayPort DT node for new platforms, contradicting its documented design.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but missing cancel_work_sync() for hpd_work in dw_dp_unbind() causes a Use-After-Free.
--

--- Patch [6]: [PATCH v4 06/10] drm/rockchip: dw_dp: Implement out-of-band HPD handling ---
commit 78e1ce0dbd7ef8a004080f25b4449ba54b71de3a
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

drm/rockchip: dw_dp: Implement out-of-band HPD handling

This patch implements out-of-band hotplug handling to receive external
hotplug information from the USB-C state machine. It uses the vo-grf
registers to control HPD signal routing and configuration.

> diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> index 005938dc66c9f..0ecbad24dd4e8 100644
> --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
[ ... ]
> @@ -71,6 +116,34 @@ static const struct drm_encoder_helper_funcs dw_dp_encoder_helper_funcs = {
>  	.atomic_check		= dw_dp_encoder_atomic_check,
>  };
>  
> +static struct regmap *dp_dp_rockchip_get_vo_grf(struct rockchip_dw_dp *dp)
> +{
> +	struct device_node *np = dev_of_node(dp->dev);
> +	struct of_phandle_args args;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	ret = of_parse_phandle_with_args(np, "phys", "#phy-cells", 0, &args);
> +	if (ret)
> +		return ERR_PTR(-ENODEV);
> +
> +	/*
> +	 * Limit this workaround to RK3576 and RK3588, new platforms should
> +	 * add a VO GRF phandle in the DisplayPort DT node.
> +	 */
> +	if (!of_device_is_compatible(args.np, "rockchip,rk3576-usbdp-phy") &&
> +	    !of_device_is_compatible(args.np, "rockchip,rk3588-usbdp-phy")) {
> +		regmap = ERR_PTR(-ENODEV);
> +		goto out_put_node;
> +	}
> +
> +	regmap = syscon_regmap_lookup_by_phandle(args.np, "rockchip,vo-grf");

[Severity: Medium]
Will this fail to probe on new platforms? 

The comment states new platforms should add a VO GRF phandle in the
DisplayPort DT node, but the code only looks up "rockchip,vo-grf" on the
phy node (args.np) after explicitly rejecting any phy other than rk3576 and
rk3588. 

Should it try to parse the property from the DisplayPort node (np) instead?

> +
> +out_put_node:
> +	of_node_put(args.np);
> +	return regmap;
> +}
[ ... ]

While reviewing this patch, I also noticed a pre-existing issue in the
related dw_dp_unbind() teardown path:

drivers/gpu/drm/bridge/synopsys/dw-dp.c:dw_dp_unbind() {
	...
	disable_irq(dp->irq);
	phy_exit(dp->phy);
	drm_bridge_put(dp->next_bridge);
	drm_dp_aux_unregister(&dp->aux);
	drm_bridge_remove(&dp->bridge);
}

[Severity: High]
This isn't a bug introduced by this patch, but can this cause a
use-after-free if an HPD interrupt fires right before disable_irq() is
called? 

If dp->hpd_work is scheduled and the devm-allocated struct dw_dp is freed
during device removal, the background worker might execute dw_dp_hpd_work()
on freed memory. 

Does dw_dp_unbind() need to call cancel_work_sync(&dp->hpd_work)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-synopsys-dw-dp-improvements-v4-0-f0f4a4ede712@collabora.com?part=6