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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Inverted manipulation of MIPI_DSI_MODE_LPM in backlight update_status callback permanently corrupts DSI mode flags.
- [High] Concurrent read-modify-write operations on dsi->mode_flags can cause state corruption.
- [Medium] Backlight update_status callback sends DCS commands without checking if the panel is powered on.
- [Medium] Incorrect devm registration order causes the DSI device to be detached while the DRM panel and backlight sysfs are still active.
- [Medium] Missing NULL check for the return value of of_device_get_match_data() leads to a potential NULL pointer dereference.
--

commit 97a4deef0ca55e71f3420d40260130de4242cae3
Author: Arpit Saini <arpit.saini@oss.qualcomm.com>

drm/panel: add Ilitek ILI7807S panel driver

This commit adds a DRM panel driver for the DLC DLC0697 1080x1920@60Hz
MIPI DSI panel based on the Ilitek ILI7807S display controller.

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

[ ... ]

> +static int ili7807s_bl_update_status(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	u16 brightness = backlight_get_brightness(bl);
> +	int ret;
> +

[Severity: Medium]
Is it safe to send DCS commands here without checking if the panel is
currently powered on? If userspace writes to the sysfs brightness control
while the regulators are disabled and the panel is off, will this attempt
to send commands and cause DSI bus timeouts?

> +	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
> +
> +	dsi->mode_flags |= MIPI_DSI_MODE_LPM;

[Severity: High]
Does this sequence invert the intended mode flags? The default panel mode
flags do not include MIPI_DSI_MODE_LPM. Clearing it beforehand and setting it
afterward will permanently force the DSI host into Low Power Mode after the
first backlight update, which might break subsequent video transmissions.

[Severity: High]
Can concurrent calls to ili7807s_on(), ili7807s_off(), and this backlight
update callback cause a data race? These functions all modify dsi->mode_flags
using non-atomic bitwise operations, which might clobber each other's changes
if a backlight update is triggered from userspace simultaneously with a DRM
DPMS event.

[ ... ]

> +static int ili7807s_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	const struct panel_desc *desc;
> +	struct ili7807s *ctx;
> +	int ret;
> +
> +	ctx = devm_drm_panel_alloc(dev, struct ili7807s, panel,
> +				   &ili7807s_panel_funcs,
> +				   DRM_MODE_CONNECTOR_DSI);
> +	if (IS_ERR(ctx))
> +		return PTR_ERR(ctx);
> +
> +	desc = of_device_get_match_data(dev);
> +	ctx->desc = desc;

[Severity: Medium]
Can of_device_get_match_data() return NULL if the driver is bound manually
via sysfs without an associated device tree node? If so, will dereferencing
desc->lanes later in this function cause a NULL pointer dereference?

[ ... ]

> +	ret = devm_drm_panel_add(dev, &ctx->panel);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to add panel\n");
> +
> +	ret = devm_mipi_dsi_attach(dev, dsi);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to attach dsi\n");
> +
> +	return 0;
> +}

[Severity: Medium]
Does this devres registration order cause a teardown issue? Since devres
cleans up in reverse order of allocation, devm_mipi_dsi_detach() will run
before the DRM panel and backlight interfaces are removed. Could this leave
the sysfs interfaces exposed to userspace while the underlying DSI device is
already detached?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-ili7807s-v2-0-b3f0c109b102@oss.qualcomm.com?part=2