Re: [PATCH 4/6] drm/panel: dsi-bpf: Add Raspberry Pi 7-inch panel BPF program

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] drm/panel: panel_prepare aborts prematurely due to positive return check
- [High] drm/panel: IOVCC regulator leaked in panel_prepare error path
- [Medium] drm/panel: commit message and implementation mismatch
- [Medium] drm/panel: DCS write errors silently ignored by PAGE/CMD macros
--

commit 2f8c8459c9b015745342c7329f8ef342082a4f1a
Author: Maxime Ripard <mripard@kernel.org>
drm/panel: dsi-bpf: Add Raspberry Pi 7-inch panel BPF program

This commit introduces a BPF program for initializing a Raspberry Pi MIPI-DSI
panel. The patch translates a C-based DCS sequence into BPF macros.

> drm/panel: dsi-bpf: Add Raspberry Pi 7-inch panel BPF program
>
> Translate the raspberrypi,dsi-7inch initialization sequence from
> drivers/gpu/drm/panel/panel-ilitek-ili9881c.c into a BPF program.

[Severity: Medium]
The commit message explicitly claims to add the "Raspberry Pi 7-inch panel
BPF program" and translates the 7-inch sequence. However, as seen in the
code below, it implements the 5-inch variant. Should the commit message be
updated to match the implemented code?

> diff --git a/drivers/gpu/drm/panel/bpf/progs/Raspberrypi__dsi-5inch.bpf.c b/drivers/gpu/drm/panel/bpf/progs/Raspberrypi__dsi-5inch.bpf.c
> new file mode 100644
> index 0000000000000..1f9ff7f2fd3ce
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/bpf/progs/Raspberrypi__dsi-5inch.bpf.c
> @@ -0,0 +1,266 @@

[ ... ]

> +#define PAGE(p) do {						\
> +	const __u8 _d[] = { 0x98, 0x81, (p) };			\
> +	panel_bpf_mipi_dsi_dcs_write(pctx, 0xff, _d, sizeof(_d));\
> +} while (0)
> +
> +#define CMD(c, d) panel_bpf_mipi_dsi_dcs_write_byte(pctx, (c), (d))

[Severity: Medium]
Do these macros need to check the return value of panel_bpf_mipi_dsi_dcs_write()
and panel_bpf_mipi_dsi_dcs_write_byte()? 

By dropping the error codes, if a DSI transmission fails during the command
sequence, it seems the BPF program will continue blindly instead of aborting
the initialization.

[ ... ]

> +SEC(PANEL_BPF_MIPI_DSI_PREPARE)
> +int BPF_PROG(panel_prepare, struct panel_bpf_mipi_dsi_ctx *pctx)
> +{
> +	int ret;
> +
> +	ret = panel_bpf_mipi_dsi_regulator_enable_and_wait(pctx, PANEL_BPF_MIPI_DSI_SUPPLY_IOVCC, 5);
> +	if (ret)
> +		return ret;
> +
> +	ret = panel_bpf_mipi_dsi_regulator_enable_and_wait(pctx, PANEL_BPF_MIPI_DSI_SUPPLY_VCC, 5);
> +	if (ret)
> +		return ret;

[Severity: High]
If enabling the VCC regulator fails here, does this code leak the previously
enabled IOVCC regulator's enable count? It looks like we return immediately
without calling panel_bpf_mipi_dsi_regulator_disable() to disable IOVCC.

[ ... ]

> +	/* set_tear_on with VBLANK mode (0x00) */
> +	CMD(0x35, 0x00);
> +
> +	ret = panel_bpf_mipi_dsi_exit_sleep_mode(pctx);
> +	if (ret)
> +		return ret;

[Severity: Critical]
Does panel_bpf_mipi_dsi_exit_sleep_mode() return a positive number of bytes on
success? 

Since it calls panel_bpf_mipi_dsi_dcs_write_and_wait(), which forwards the
underlying mipi_dsi_dcs_write() result, a successful write would make ret > 0.
This check would then evaluate to true on a successful transmission, causing
panel_prepare() to abort prematurely before enabling the display.

[ ... ]

> +PANEL_BPF_MIPI_DSI_OPS(raspberrypi_dsi_5inch) = {
> +	.panel_id		= "/soc/dsi@7e700000/panel@0",
> +	.compatible		= "raspberrypi,dsi-5inch",

[Severity: Medium]
This isn't a code regression, but this compatible string and struct name bind
the program strictly to the 5-inch panel, which contradicts the stated purpose
in the commit message.

> +	.format			= MIPI_DSI_FMT_RGB888,
> +	.lanes			= 2,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928-drm-mipi-dsi-panel-ebpf-v1-0-5244926aace4@kernel.org?part=4