Re: [PATCH 03/11] drm: of: drm_of_find_panel_or_bridge: prioritize the bridge, not the panel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Changing the lookup priority from panel to bridge breaks drivers that use `panel != NULL` checks to apply panel-specific hardware constraints or initialization logic.
--

--- Patch [3]: [PATCH 03/11] drm: of: drm_of_find_panel_or_bridge: prioritize the bridge, not the panel ---
commit e0a1351b53c705f02789905fd967720e44e8a076
Author: Luca Ceresoli <[email protected]>

drm: of: drm_of_find_panel_or_bridge: prioritize the bridge, not the panel

This commit changes drm_of_find_panel_or_bridge() to prioritize returning
a drm_bridge instead of a drm_panel when both are present. This avoids
re-creating a panel_bridge if a panel already automatically creates one.

> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index 8ec352f3df93e..f92f02f9b202a 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -308,14 +308,7 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
>  	if (!remote)
>  		return -ENODEV;
>  
> -	*panel = of_drm_find_panel(remote);
> -	if (!IS_ERR(*panel))
> -		return 0;
> -
> -	*panel = NULL;
> -
>  	if (bridge) {
> -		/* No panel found yet, check for a bridge next. */
>  		*bridge = of_drm_find_bridge(remote);
>  		if (*bridge)
>  			return 0;

[Severity: High]
Will changing the lookup priority to return a bridge before a panel break
drivers that rely on the panel pointer being non-NULL to apply
hardware-specific logic?

For example, in sun4i_rgb_mode_valid(), if a panel wraps itself in a bridge,
the panel pointer will be NULL, causing valid modes to be rejected:

drivers/gpu/drm/sun4i/sun4i_rgb.c:sun4i_rgb_mode_valid() {
	if (rgb->panel) {
		DRM_DEBUG_DRIVER("RGB panel used, skipping clock rate checks");
		goto out;
	}
	...
}

Similarly in fsl_dcu_create_outputs(), it relies on panel to enforce
hdisplay alignment and custom connector initialization:

drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c:fsl_dcu_create_outputs() {
	if (panel) {
		ret = devm_add_action_or_reset(fsl_dev->dev,
					       fsl_dcu_panel_put_action, panel);
		if (ret)
			return ret;

		fsl_dev->connector.panel = panel;
		return fsl_dcu_attach_panel(fsl_dev, panel);
	}
	...
}

And tegra_sor_dp_enable() uses the panel pointer to identify eDPs and
misconfigures internal eDP panels as standard DP displays if panel is NULL:

drivers/gpu/drm/tegra/sor.c:tegra_sor_dp_enable() {
	if (output->panel)
		value |= SOR_CLK_CNTRL_DP_CLK_SEL_SINGLE_DPCLK;
	else
		value |= SOR_CLK_CNTRL_DP_CLK_SEL_DIFF_DPCLK;

	tegra_sor_writel(sor, value, SOR_CLK_CNTRL);
	...
}

Could this cause display initialization to fail on these platforms?

>  
>  		*bridge = NULL;
>  	}
>  
> +	/* No bridge found yet, check for a panel next. */
> +	*panel = of_drm_find_panel(remote);
> +	if (!IS_ERR(*panel))
> +		return 0;
> +
> +	*panel = NULL;
> +
>  	return -EPROBE_DEFER;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3