Re: [PATCH 2/2] drm/panel: add Ilitek ILI7836A panel driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Unbalanced panel operations due to missing .enable callback
- [High] Unsafe backlight DSI transmissions without panel power state validation
- [High] Data race on `dsi->mode_flags` between backlight updates and panel state changes
- [Medium] Early return in backlight update callback leaks DSI mode state
- [Medium] The get_modes callback ignores descriptor physical attributes and multiple modes
--

--- Patch [2]: [PATCH 2/2] drm/panel: add Ilitek ILI7836A panel driver ---
commit 1ef963af27698ad0874d830a6b469bf1d540bc0b
Author: Aaron Kling <webgeek1234@gmail.com>
Subject: drm/panel: add Ilitek ILI7836A panel driver

This commit introduces a new DRM panel driver for the Ilitek ILI7836A DDIC,
initially supporting the Retroid Pocket Nova panel.

> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili7836a.c b/drivers/gpu/drm/panel/panel-ilitek-ili7836a.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili7836a.c
[ ... ]
> +static int ili7836a_get_modes(struct drm_panel *panel,
> +					struct drm_connector *connector)
> +{
> +	struct ili7836a_panel *ctx = to_ili7836a_panel(panel);
> +
> +	return drm_connector_helper_get_modes_fixed(connector, ctx->desc->modes);
> +}

[Severity: Medium]
Does this ignore the physical dimensions and color depth defined in nova_desc?

The descriptor explicitly defines width_mm, height_mm, and bpc, but
drm_connector_helper_get_modes_fixed() only extracts a single fixed mode without
populating the connector's physical size or color format information.
Additionally, the num_modes property in the descriptor appears to be ignored.

> +static enum drm_panel_orientation ili7836a_get_orientation(struct drm_panel *panel)
> +{
> +	struct ili7836a_panel *ctx = to_ili7836a_panel(panel);
> +
> +	return ctx->orientation;
> +}
> +
> +static const struct drm_panel_funcs ili7836a_panel_funcs = {
> +	.prepare = ili7836a_prepare,
> +	.unprepare = ili7836a_unprepare,
> +	.disable = ili7836a_disable,
> +	.get_modes = ili7836a_get_modes,
> +	.get_orientation = ili7836a_get_orientation,
> +};

[Severity: High]
Will this leave the panel permanently black after certain display power
management transitions?

The driver implements the .disable callback which sends DCS sleep commands,
but it omits the corresponding .enable callback. Since drm_panel_enable()
will be a no-op, the panel may remain in sleep mode without a full power
cycle.

> +
> +static int ili7836a_bl_update_status(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	u16 brightness = backlight_get_brightness(bl);
> +	int ret;
> +
> +	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);

[Severity: High]
Can this cause DSI host transmission timeouts if called while the panel is
powered down?

The backlight device is accessible via sysfs independently of the panel's DRM
state. Writing to the brightness attribute while the panel regulators are
disabled would execute DCS writes on an unpowered DSI bus.

[Severity: High]
Is there a data race on dsi->mode_flags here?

Backlight updates can execute concurrently via sysfs while the DRM subsystem
invokes panel enable/disable operations. Since the mode_flags bitmask is
modified using non-atomic bitwise operations, a read-modify-write race
could occur between this function and the DRM ioctl paths:

ili7836a_on() {
    ...
    ctx->dsi->mode_flags |= MIPI_DSI_MODE_LPM;
}

ili7836a_disable() {
    ...
    ctx->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
}

> +	if (ret < 0)
> +		return ret;

[Severity: Medium]
If mipi_dsi_dcs_set_display_brightness_large() fails, does this early return
permanently clear the MIPI_DSI_MODE_LPM bit?

The error path exits before restoring the mode_flags, which might disrupt
subsequent DSI communication.

> +
> +	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260808-ili7836a-v1-0-07a7becc5636@gmail.com?part=2