Re: [PATCH v2 2/2] drm/panel: Add driver for Raydium RM69220 DDIC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The reset GPIO is requested with `GPIOD_OUT_LOW`, which deasserts it and can backpower the unpowered panel.
- [High] The backlight callback modifies `dsi->mode_flags` without locking and permanently corrupts the state if the DSI transfer fails.
- [Low] The macro `MIPI_DSI_MODE_DSC_ALL_SLICES_IN_PKT` is undefined in the kernel, causing a build failure.
--

commit 3b5fdc2d207db00cc147a18d14281a4361ece19b
Author: Alexander Koskovich <akoskovich@pm.me>

drm/panel: Add driver for Raydium RM69220 DDIC

Add a panel driver for Raydium RM69220 DDIC based displays such as the
BOE BF068MWM-TD0 found in the Nothing Phone (3a).

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

[ ... ]

> +static int raydium_rm69220_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);
> +	if (ret < 0)
> +		return ret;
> +
> +	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	return 0;
> +}

[Severity: High]
Does this error path permanently corrupt the DSI mode flags?

If mipi_dsi_dcs_set_display_brightness_large() fails, the function returns
early without restoring MIPI_DSI_MODE_LPM. Userspace can trigger this easily
by writing to the backlight sysfs node while the display is powered off,
which will cause a transfer timeout. All future DSI initialization commands
will then incorrectly be sent in high-speed mode, preventing the panel from
turning back on until the system is rebooted.

[ ... ]

> +static int raydium_rm69220_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct raydium_rm69220 *ctx;
> +	int ret;

[ ... ]

> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
> +				     "Failed to get reset-gpios\n");

[Severity: High]
Is GPIOD_OUT_LOW the correct initialization here?

Because the reset pin is typically active-low, a logical 0 translates to
driving the physical pin HIGH. This happens at probe time before the
regulators are enabled, which can drive voltage into the panel's reset pin
while its power supplies are off. This forward-biases the panel's ESD
protection diodes, leaking current into the power plane and potentially
causing hardware damage. Should this be initialized to GPIOD_OUT_HIGH to
hold it safely in reset (0V) until power is applied?

> +
> +	ctx->dsi = dsi;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB101010;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_NO_EOT_PACKET |
> +			  MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM |
> +			  MIPI_DSI_MODE_DSC_ALL_SLICES_IN_PKT;

[Severity: Low]
Will this compile correctly?

The macro MIPI_DSI_MODE_DSC_ALL_SLICES_IN_PKT appears to be undefined in the
upstream kernel, possibly a leftover from a vendor tree. Compiling this
driver will likely result in an undeclared identifier error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-asteroids-panel-support-v2-0-44b96c6eca91@pm.me?part=2