All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Arpit Saini" <arpit.saini@oss.qualcomm.com>
Cc: dri-devel@lists.freedesktop.org, robh@kernel.org,
	conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v3 2/2] drm/panel: add Ilitek ILI7807S panel driver
Date: Wed, 24 Jun 2026 09:04:49 +0000	[thread overview]
Message-ID: <20260624090450.423F21F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260624-ili7807s-v3-2-ddf37052a289@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect and non-atomic modification of `dsi->mode_flags` permanently enables Low Power Mode and introduces a data race.
- [High] Backlight `update_status` sends DSI commands without verifying that the panel is powered on, risking DSI host timeouts.
--

commit 2f966f15c21adb82ce5699cdffad22cac6628b98
Author: Arpit Saini <arpit.saini@oss.qualcomm.com>

drm/panel: add Ilitek ILI7807S panel driver
    
Add 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
> index 0000000000000..8ddfab2693ec8
> --- /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: High]
Does this code send DSI commands without verifying if the panel is powered on?

If the panel regulators are disabled and reset is asserted via
ili7807s_unprepare(), attempting to communicate with it here during a sysfs
backlight update could result in DSI host bus timeouts or hardware lockups.

Should there be a check against a power state tracking flag before
sending the DCS command?

> +	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 unconditionally setting MIPI_DSI_MODE_LPM here permanently alter the
panel's base operating state?

Since the mode_flags are modified without synchronization with DRM modeset
paths (like ili7807s_on() and ili7807s_off()), could this create a data
race?

Also, the logic clears the flag before the transaction and unconditionally
sets it afterwards, leaving Low Power Mode permanently enabled for
subsequent DSI transmissions.

> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-ili7807s-v3-0-ddf37052a289@oss.qualcomm.com?part=2

      reply	other threads:[~2026-06-24  9:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-24  8:53 [PATCH v3 0/2] drm/panel: add support for Ilitek ILI7807S DSI panels Arpit Saini
2026-06-24  8:53 ` [PATCH v3 1/2] dt-bindings: display: panel: add Ilitek ILI7807S panel controller Arpit Saini
2026-06-24  8:53 ` [PATCH v3 2/2] drm/panel: add Ilitek ILI7807S panel driver Arpit Saini
2026-06-24  9:04   ` sashiko-bot [this message]

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=20260624090450.423F21F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=arpit.saini@oss.qualcomm.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.