Re: [PATCH v4 10/11] drm: zynqmp_dp: switch to of_drm_get_bridge_by_endpoint()

Laurent Pinchart
2026-05-04T15:24:21Z

Hi Luca,

Thank you for the patch.

On Mon, May 04, 2026 at 12:45:13PM +0200, Luca Ceresoli wrote:
> This driver calls drm_of_find_panel_or_bridge() with a NULL pointer in the
> @panel parameter, thus using a reduced feature set of that function.
> Replace this call with the simpler of_drm_get_bridge_by_endpoint().
>
> Since of_drm_get_bridge_by_endpoint() increases the refcount of the
> returned bridge, ensure it is put on removal. To achieve this, instead of
> adding an explicit drm_bridge_put(), migrate to the bridge::next_bridge
> pointer which is automatically put when the bridge is eventually freed.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>
> Changes in v4:
> - fix missing assignment
>
> Changes in v3:
> - fix ERR_PTR deref when -ENODEV is returned
> ---
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 379180fb3004..fd30491a6697 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -353,7 +353,6 @@ struct zynqmp_dp_train_set_priv {
> * @lock: Mutex protecting this struct and register access (but not AUX)
> * @irq: irq
> * @bridge: DRM bridge for the DP encoder
> - * @next_bridge: The downstream bridge
> * @test: Configuration for test mode
> * @config: IP core configuration from DTS
> * @aux: aux channel
> @@ -385,7 +384,6 @@ struct zynqmp_dp {
> struct completion aux_done;
> struct mutex lock;
>
> - struct drm_bridge *next_bridge;
> struct device *dev;
> struct zynqmp_dpsub *dpsub;
> void __iomem *iomem;
> @@ -1494,8 +1492,8 @@ static int zynqmp_dp_bridge_attach(struct drm_bridge *bridge,
> return ret;
> }
>
> - if (dp->next_bridge) {
> - ret = drm_bridge_attach(encoder, dp->next_bridge,
> + if (dp->bridge.next_bridge) {
> + ret = drm_bridge_attach(encoder, dp->bridge.next_bridge,
> bridge, flags);
> if (ret < 0)
> goto error;
> @@ -2461,10 +2459,15 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
> * Acquire the next bridge in the chain. Ignore errors caused by port@5
> * not being connected for backward-compatibility with older DTs.
> */
> - ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 5, 0, NULL,
> - &dp->next_bridge);
> - if (ret < 0 && ret != -ENODEV)
> - goto err_reset;
> + dp->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dp->dev->of_node, 5, 0);
> + if (IS_ERR(dp->bridge.next_bridge)) {
> + if (PTR_ERR(dp->bridge.next_bridge) == -ENODEV) {
> + dp->bridge.next_bridge = NULL;
> + } else {
> + ret = PTR_ERR(dp->bridge.next_bridge);
> + goto err_reset;
> + }

I would find

if (PTR_ERR(dp->bridge.next_bridge) != -ENODEV) {
ret = PTR_ERR(dp->bridge.next_bridge);
goto err_reset;
}

dp->bridge.next_bridge = NULL;

easier to read. Up to you.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + }
>
> /* Initialize the hardware. */
> dp->config.misc0 &= ~ZYNQMP_DP_MAIN_STREAM_MISC0_SYNC_LOCK;

--
Regards,

Laurent Pinchart