Re: [PATCH v5 07/11] drm/tegra: Make panel parsing by tegra_output_probe() optional

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/tegra: skipped bridge lookup in tegra_output_probe() breaks HDMI
--

commit 7887570371c7e1244731285458cba8facf234cb7
Author: Svyatoslav Ryhel <clamor95@gmail.com>

drm/tegra: Make panel parsing by tegra_output_probe() optional

This commit adds a boolean get_panel parameter to tegra_output_probe() so
callers can skip panel and bridge lookup in the common output probe path.
It passes false from tegra_dsi_probe() and tegra_hdmi_probe(), and true
from tegra_dc_rgb_probe() and tegra_sor_probe().

> diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
> index 0d2c0ab0a63e9..c0845bb45bb15 100644
> --- a/drivers/gpu/drm/tegra/hdmi.c
> +++ b/drivers/gpu/drm/tegra/hdmi.c
> @@ -1855,7 +1855,7 @@ static int tegra_hdmi_probe(struct platform_device *pdev)
>  
>  	hdmi->output.dev = &pdev->dev;
>  
> -	err = tegra_output_probe(&hdmi->output);
> +	err = tegra_output_probe(&hdmi->output, false);
>  	if (err < 0)
>  		return err;
>  
> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
> index d0ba3bd0aab8f..ff2e28a1e00b7 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -94,7 +94,7 @@ static irqreturn_t hpd_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -int tegra_output_probe(struct tegra_output *output)
> +int tegra_output_probe(struct tegra_output *output, bool get_panel)
>  {
>  	struct device_node *ddc, *panel;
>  	const void *edid;
> @@ -104,31 +104,33 @@ int tegra_output_probe(struct tegra_output *output)
>  	if (!output->of_node)
>  		output->of_node = output->dev->of_node;
>  
> -	err = drm_of_find_panel_or_bridge(output->of_node, -1, -1,
> -					  &output->panel, &output->bridge);
> -	if (err && err != -ENODEV)
> -		return err;
> -
> -	panel = of_parse_phandle(output->of_node, "nvidia,panel", 0);
> -	if (panel) {
> -		/*
> -		 * Don't mix nvidia,panel phandle with the graph in a
> -		 * device-tree.
> -		 */
> -		WARN_ON(output->panel || output->bridge);
> -
> -		if (output->panel) {
> -			drm_panel_put(output->panel);
> -			output->panel = NULL;
> -		}
> -
> -		output->panel = of_drm_find_panel(panel);
> -		of_node_put(panel);
> -
> -		if (IS_ERR(output->panel)) {
> -			err = PTR_ERR(output->panel);
> -			output->panel = NULL;
> +	if (get_panel) {
> +		err = drm_of_find_panel_or_bridge(output->of_node, -1, -1,
> +						  &output->panel, &output->bridge);
> +		if (err && err != -ENODEV)
>  			return err;

[Severity: High]
Does guarding drm_of_find_panel_or_bridge() behind get_panel in
tegra_output_probe() while passing false from tegra_hdmi_probe() break
HDMI outputs that use an OF graph bridge or connector?

In tegra_output_probe(), drm_of_find_panel_or_bridge() populates both
output->panel and output->bridge. Because tegra_output_probe() is the only
place in the driver that resolves output->bridge from the device tree OF
graph, calling tegra_output_probe(&hdmi->output, false) from
tegra_hdmi_probe() leaves hdmi->output.bridge permanently NULL and skips
probe deferral when a downstream bridge has not probed yet.

When tegra_hdmi_init() later runs:

drivers/gpu/drm/tegra/hdmi.c:tegra_hdmi_init() {
	...
	if (hdmi->output.bridge) {
		err = drm_bridge_attach(&hdmi->output.encoder, hdmi->output.bridge,
					NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
		...
		connector = drm_bridge_connector_init(drm, &hdmi->output.encoder);
		...
	} else {
		drm_connector_init_with_ddc(drm, &hdmi->output.connector,
					    &tegra_hdmi_connector_funcs,
					    DRM_MODE_CONNECTOR_HDMIA,
					    hdmi->output.ddc);
		...
	}
}

the if (hdmi->output.bridge) branch becomes unreachable. On Tegra20,
Tegra30, and Tegra114 boards that describe HDMI via an OF graph port
connected to an hdmi-connector or external bridge (where DDC and HPD
belong to the remote node rather than legacy properties on the HDMI node),
falling back to the non-bridge connector path leaves output->ddc and
output->hpd_gpio NULL, causing tegra_output_connector_detect() to
permanently report connector_status_disconnected.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029152909.234584-1-clamor95@gmail.com?part=7