Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Maxime Ripard" <mripard@kernel.org>
Cc: robh@kernel.org, bpf@vger.kernel.org,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	conor+dt@kernel.org
Subject: Re: [PATCH 5/6] drm/panel: dsi-bpf: Add Raspberry Pi 5-inch panel BPF program
Date: Mon, 28 Sep 2026 16:33:00 +0000	[thread overview]
Message-ID: <20260928163300.DF8B71F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260928-drm-mipi-dsi-panel-ebpf-v1-5-5244926aace4@kernel.org>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] drm/panel: resource leak and unbalanced regulator disable on error path
- [Medium] drm/panel: DCS write errors are ignored during initialization sequence
- [Medium] drm/panel: commit message and file content mismatch (5-inch vs 7-inch panel)
--

commit 6f19328aa9181fa01e0009c188e0a4fe6e32ccb1
Author: Maxime Ripard <mripard@kernel.org>

drm/panel: dsi-bpf: Add Raspberry Pi 5-inch panel BPF program
    
This commit translates the initialization sequence for the Raspberry Pi
5-inch MIPI-DSI panel into a BPF program.

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

[Severity: Medium]
Is there a mismatch between the commit message and the contents of this
patch? The commit message states it adds the "5-inch panel" and translates
the "raspberrypi,dsi-5inch" initialization sequence. However, the patch
actually adds a file named Raspberrypi__dsi-7inch.bpf.c containing the
7-inch initialization sequence, and uses the "raspberrypi,dsi-7inch"
compatible string.

> diff --git a/drivers/gpu/drm/panel/bpf/progs/Raspberrypi__dsi-7inch.bpf.c b/drivers/gpu/drm/panel/bpf/progs/Raspberrypi__dsi-7inch.bpf.c
> new file mode 100644
> index 0000000000000..2055828de2f40
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/bpf/progs/Raspberrypi__dsi-7inch.bpf.c
> @@ -0,0 +1,271 @@
[ ... ]
> +#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 ignore DCS write errors during the initialization sequence?
By discarding the return value of panel_bpf_mipi_dsi_dcs_write() and
panel_bpf_mipi_dsi_dcs_write_byte(), a failure (such as a DSI bus timeout)
will not abort the program. It will blindly continue sending over 150
commands to a potentially broken panel, sequentially timing out on each
command, which causes a massive initialization delay and falsely reports
a successful panel 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: Medium]
Does this error path leak the previously enabled IOVCC regulator?
If panel_bpf_mipi_dsi_regulator_enable_and_wait() for VCC fails here, or if
subsequent operations like exit_sleep_mode() fail, panel_prepare() returns
an error without disabling the already-enabled IOVCC regulator or reverting
the GPIO state.

[ ... ]
> +SEC(PANEL_BPF_MIPI_DSI_UNPREPARE)
> +int BPF_PROG(panel_unprepare, struct panel_bpf_mipi_dsi_ctx *pctx)
> +{
> +	panel_bpf_mipi_dsi_set_display_off(pctx);
> +	panel_bpf_mipi_dsi_enter_sleep_mode(pctx);
> +
> +	panel_bpf_mipi_dsi_regulator_disable(pctx, PANEL_BPF_MIPI_DSI_SUPPLY_VCC);
> +	panel_bpf_mipi_dsi_regulator_disable(pctx, PANEL_BPF_MIPI_DSI_SUPPLY_IOVCC);

[Severity: Medium]
Does this unconditionally disable regulators, even on error paths?
When the DRM core tears down the pipeline following an error in
panel_prepare(), it unconditionally calls panel_unprepare(). Attempting
to disable a regulator that failed to enable (e.g., VCC) will trigger
a kernel warning for an unbalanced disable.

> +	panel_bpf_mipi_dsi_gpio_enable(pctx, PANEL_BPF_MIPI_DSI_GPIO_RESET);
> +
> +	return 0;
> +}
> +
> +PANEL_BPF_MIPI_DSI_OPS(raspberrypi_dsi_7inch) = {
> +	.panel_id		= "/soc/dsi@7e700000/panel@0",
> +	.compatible		= "raspberrypi,dsi-7inch",

[Severity: Medium]
Does this code explicitly configure the 7-inch panel? As mentioned in the
commit message comment, this directly contradicts the claim of adding the
5-inch panel.

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

  reply	other threads:[~2026-09-28 16:33 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-28 16:22 [PATCH 0/6] drm/bridge: Add a BPF-based MIPI-DSI panel driver Maxime Ripard
2026-09-28 16:22 ` [PATCH 1/6] dt-bindings: display: Add panel-mipi-dsi-bpf generic panel binding Maxime Ripard
2026-09-28 16:35   ` sashiko-bot
2026-09-28 19:16   ` Neil Armstrong
2026-09-28 20:40   ` Rob Herring
2026-09-29  8:39     ` Maxime Ripard
2026-09-29 22:28       ` Rob Herring
2026-09-30  8:04         ` Maxime Ripard
2026-09-30 19:07           ` Rob Herring
2026-10-01  7:22             ` Maxime Ripard
2026-10-01 16:03               ` Rob Herring
2026-09-30 13:32       ` Neil Armstrong
2026-10-01  7:09         ` Maxime Ripard
2026-10-02  7:28           ` Neil Armstrong
2026-09-28 20:43   ` Rob Herring (Arm)
2026-09-29 23:44   ` bot+bpf-ci
2026-09-28 16:22 ` [PATCH 2/6] drm/panel: Add generic MIPI-DSI panel driver with BPF init sequences Maxime Ripard
2026-09-28 16:34   ` sashiko-bot
2026-09-28 16:22 ` [PATCH 3/6] drm/panel: dsi-bpf: Add BPF program build infrastructure and helper header Maxime Ripard
2026-09-28 16:31   ` sashiko-bot
2026-09-29 23:44   ` bot+bpf-ci
2026-09-28 16:22 ` [PATCH 4/6] drm/panel: dsi-bpf: Add Raspberry Pi 7-inch panel BPF program Maxime Ripard
2026-09-28 16:34   ` sashiko-bot
2026-09-28 16:22 ` [PATCH 5/6] drm/panel: dsi-bpf: Add Raspberry Pi 5-inch " Maxime Ripard
2026-09-28 16:33   ` sashiko-bot [this message]
2026-09-28 16:22 ` [PATCH DO NOT MERGE 6/6] arm64: dts: broadcom: Add Raspberry Pi ILI9881C DSI panel overlays Maxime Ripard
2026-09-28 16:31   ` sashiko-bot
2026-09-28 16:37 ` [PATCH 0/6] drm/bridge: Add a BPF-based MIPI-DSI panel driver Laurent Pinchart
2026-09-28 17:31   ` Benjamin Tissoires
2026-09-28 18:12     ` Laurent Pinchart
2026-09-29  6:54   ` Maxime Ripard
2026-09-28 16:39 ` Neil Armstrong
2026-09-28 17:24   ` Benjamin Tissoires
2026-09-28 19:20     ` Neil Armstrong
2026-09-28 19:48       ` Benjamin Tissoires
2026-09-28 20:36         ` Neil Armstrong
2026-09-29  7:41           ` Maxime Ripard
2026-09-29  7:55             ` Javier Martinez Canillas
2026-09-30 13:37               ` Neil Armstrong
2026-10-01  6:50                 ` Maxime Ripard
2026-10-02  7:30                   ` Neil Armstrong
2026-10-02  7:37                     ` Javier Martinez Canillas
2026-10-02  7:53                       ` Neil Armstrong
2026-10-02  9:04                         ` Javier Martinez Canillas
2026-09-30 13:36             ` Neil Armstrong
2026-09-30 20:59               ` Kumar Kartikeya Dwivedi
2026-10-01  6:38               ` Maxime Ripard
2026-10-01 17:03               ` Maxime Ripard
2026-09-29  7:27       ` Maxime Ripard
2026-09-30 13:49         ` Neil Armstrong
2026-10-01  7:01           ` Maxime Ripard
2026-10-02  7:39             ` Neil Armstrong
2026-09-29  7:13   ` Maxime Ripard
2026-09-29  7:46     ` Benjamin Tissoires
2026-09-30 13:46     ` Neil Armstrong
2026-10-01  6:48       ` Maxime Ripard
2026-10-02  8:11         ` Neil Armstrong
2026-09-29  9:03 ` Jani Nikula
2026-09-29  9:32   ` Benjamin Tissoires
2026-09-29 10:16     ` Jani Nikula
2026-09-29 12:28       ` Maxime Ripard
2026-09-30 14:01   ` Neil Armstrong
2026-09-30 19:18     ` Jani Nikula
2026-10-01  7:05     ` Maxime Ripard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260928163300.DF8B71F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=mripard@kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox