Re: [PATCH 2/5] drm: panel: jd9365da: Add prepare_prev_first flag to drm_panel

[sashiko-bot@kernel.org]

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

New issues:
- [Low] Redundant duplicate assignment of `prepare_prev_first` flag.

Pre-existing issues:
- [High] Missing cleanup in the error paths of `jadard_prepare()` causes a permanent leak of the regulator enable counts.
- [High] Premature publication of the panel in `jadard_dsi_probe()` allows a race condition leading to a NULL pointer dereference.
--

--- Patch [2]: [PATCH 2/5] drm: panel: jd9365da: Add prepare_prev_first flag to drm_panel ---
commit 1a793dbc7476444fc7003da84dbce8a3bff9ad98
Author: Marc-Olivier Champagne <marc-olivier.champagne@savoirfairelinux.com>

drm: panel: jd9365da: Add prepare_prev_first flag to drm_panel

The DSI host must be enabled for the panel to be initialized in
prepare(). Set the prepare_prev_first flag to guarantee this.
This fixes the panel operation on Renesas RZ/G2LC SoC DSI host.

> diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> index 1ef0c937863a2..a79dc8f8d5af9 100644
> --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> @@ -2993,6 +2993,8 @@ static int jadard_dsi_probe(struct mipi_dsi_device *dsi)
>  		return dev_err_probe(&dsi->dev, PTR_ERR(jadard->vccio),
>  				"failed to get vccio regulator\n");
>  
> +	jadard->panel.prepare_prev_first = true;
> +

[Severity: Low]
Does this introduce a redundant assignment?

Looking further down in jadard_dsi_probe(), the exact same flag appears to be
set again just before panel registration:

jadard_dsi_probe() {
	...
	jadard->panel.prepare_prev_first = true;

	drm_panel_add(&jadard->panel);
	...
}

Can we remove one of these duplicate assignments?

[Severity: High]
This is a pre-existing issue, but does jadard_prepare() leak regulator
reference counts on error paths?

If regulator_enable(jadard->vccio) succeeds, but a subsequent step fails:

jadard_prepare() {
	...
	ret = regulator_enable(jadard->vccio);
	...
	ret = regulator_enable(jadard->vdd);
	if (ret)
		return ret;

and later with mipi_dsi_dcs_nop():

	ret = mipi_dsi_dcs_nop(jadard->dsi);
	if (ret)
		return ret;
}

The function returns without disabling the previously enabled regulators.
Because the panel prepare fails, the DRM core won't mark it as prepared, and
jadard_unprepare() will never be called to clean up.
    
Can this prevent the system from entering low-power states or re-initializing
the display properly?

[Severity: High]
This isn't a bug introduced by this patch, but is there a race condition with
premature panel publication in jadard_dsi_probe()?

The panel is added to the global list before its required internal pointers
are initialized:

jadard_dsi_probe() {
	...
	drm_panel_add(&jadard->panel);

	mipi_dsi_set_drvdata(dsi, jadard);
	jadard->dsi = dsi;
	jadard->desc = desc;
}

If a display controller finds the panel concurrently (e.g., via probe deferral
resolution) and calls drm_panel_prepare(), jadard_prepare() could execute
before jadard_dsi_probe() finishes:

jadard_prepare() {
	...
	if (jadard->desc->vcioo_to_lp11_delay_ms)
	...
}

Can this dereference a NULL jadard->desc pointer? Should drm_panel_add() be
moved to the very end of the probe function?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708213500.1867092-1-marc-olivier.champagne@savoirfairelinux.com?part=2