Re: [PATCH v3 1/4] drm: panel: Add LG LH609QH1 Panel with SW49410 controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing `drm_panel_remove()` in the `mipi_dsi_attach()` error path causes a global list corruption and Use-After-Free.
- [High] DSI commands are sent while the panel is held in reset in `sw49410_panel_unprepare()`.
- [Medium] Misplaced `MIPI_DSI_MODE_LPM` flag manipulation around a software memory packing function.
- [Medium] The panel is published to the system before its DSC configuration is fully initialized.
--

commit 1a6b069af275d9965692f4acaa02ebd1a8517da9
Author: Amir Dahan <system64fumo@tuta.io>
Subject: drm: panel: Add LG LH609QH1 Panel with SW49410 controller

This commit adds a new panel driver for the LG SW49410 controller used
in the LG G7 ThinQ. It configures the panel, sets up the DSI commands,
and initializes the DSC parameters.

> diff --git a/drivers/gpu/drm/panel/panel-lg-sw49410.c b/drivers/gpu/drm/panel/panel-lg-sw49410.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-lg-sw49410.c

[ ... ]

> +static int sw49410_panel_program(struct sw49410_panel *ctx)
> +{

[ ... ]

> +	ctx->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	drm_dsc_pps_payload_pack(&pps, &ctx->dsc);
> +
> +	ctx->dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	mipi_dsi_picture_parameter_set_multi(&dsi_ctx, &pps);

[Severity: Medium]
Does this code incorrectly clear and restore the MIPI_DSI_MODE_LPM flag?

The drm_dsc_pps_payload_pack() function merely formats a local structure
in memory and performs no hardware DSI transactions, so manipulating DSI
hardware transmission flags around it seems unnecessary.

By restoring the flag immediately after packing, does it cause the
subsequent mipi_dsi_picture_parameter_set_multi() call to execute in Low
Power Mode rather than the intended High Speed mode?

[ ... ]

> +static int sw49410_panel_unprepare(struct drm_panel *panel)
> +{
> +	struct sw49410_panel *ctx = to_sw49410_panel(panel);
> +
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
> +
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);

[Severity: High]
Is it intentional to assert the reset GPIO before sending the sleep
commands?

Because the display controller is held in hardware reset, it cannot receive
or process the display off and sleep commands, which could cause the DSI
host's transmissions to fail or time out.

Looking further down in this function, the reset GPIO is asserted a second
time, which suggests this first call might be misplaced.

[ ... ]

> +static int sw49410_panel_probe(struct mipi_dsi_device *dsi)
> +{

[ ... ]

> +	drm_panel_add(&ctx->panel);
> +
> +	/* This panel only supports DSC; unconditionally enable it */
> +	dsi->dsc = &ctx->dsc;
> +
> +	ctx->dsc.dsc_version_major = 1;
> +	ctx->dsc.dsc_version_minor = 1;

[Severity: Medium]
Could this cause a race condition where a concurrently probing display
driver reads uninitialized DSC settings?

Since drm_panel_add() makes the panel visible globally, another driver
could find it before the DSC parameters like slice_width and
bits_per_component are fully populated.

Should drm_panel_add() be called as the final step of initialization?

[ ... ]

> +	ctx->dsc.slice_count = 1440 / ctx->dsc.slice_width;
> +	ctx->dsc.bits_per_component = 8;
> +	ctx->dsc.bits_per_pixel = 8 << 4; /* 4 fractional bits */
> +	ctx->dsc.block_pred_enable = true;
> +
> +	return mipi_dsi_attach(dsi);
> +}

[Severity: High]
Does returning the error from mipi_dsi_attach() without calling
drm_panel_remove() leave a dangling pointer in the global panel list?

If mipi_dsi_attach() fails, the probe function returns the error directly.
Because the panel context is allocated via devm_drm_panel_alloc(), it will
be automatically freed by the driver core.

However, since drm_panel_add() was already called, this freed memory would
remain linked in the global DRM panel list, leading to a use-after-free
the next time the list is traversed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614-judyln-panel-v3-0-07f4134441bd@postmarketos.org?part=1