From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/panel/boe-tv101wum-nl6: set MIPI_DSI_MODE_LPM after sending panel disable cmds
Date: Thu, 23 Apr 2026 08:22:30 +1000 [thread overview]
Message-ID: <review-patch1-20260421153147.4378-2-brady.norander@mainlining.org> (raw)
In-Reply-To: <20260421153147.4378-2-brady.norander@mainlining.org>
Patch Review
**Correctness: Good.** The change is straightforward and correct:
```c
boe->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; // line 1322: clear LPM (switch to HS)
mipi_dsi_dcs_set_display_off_multi(&ctx); // line 1324: send disable in HS
mipi_dsi_dcs_enter_sleep_mode_multi(&ctx); // line 1325: send sleep in HS
mipi_dsi_msleep(&ctx, 150); // line 1327: wait for panel
boe->dsi->mode_flags |= MIPI_DSI_MODE_LPM; // line 1329: restore LPM <-- NEW
```
The flag is restored with `|=` rather than `=`, which is correct — it only sets the `MIPI_DSI_MODE_LPM` bit without disturbing other flags. Since only `MIPI_DSI_MODE_LPM` was cleared earlier (line 1322), this perfectly reverses the earlier modification.
**Pattern consistency:** The clear-before/restore-after pattern is used in other panel drivers (e.g., `panel-asus-z00t-tm5p5-n35596.c` lines 174/180). Interestingly, some other panel drivers like `panel-ebbg-ft8719.c` and `panel-boe-td4320.c` do NOT restore LPM after their off sequence, so this is not universally consistent — but restoring it is clearly the safer approach and matches the initial state set at probe time (`dsi->mode_flags = desc->mode_flags` at line 1758, where all descriptors include `MIPI_DSI_MODE_LPM`).
**Placement:** Restoring LPM after the `mipi_dsi_msleep()` is the right choice — all DSI commands have been fully sent before the flag is changed back.
**Impact scope:** This driver covers multiple panel variants (BOE TV101WUM-NL6, TV105WUM-NW0, AUO KD101N80-45NA, etc.). The `boe_panel_disable()` function is shared across all of them, so this fix applies to all. The submitter tested on krane sku176; other devices should also benefit since the asymmetry was a latent bug.
**Nits:**
- **Missing `Fixes:` tag.** This would be very helpful for stable tree backport consideration. It should reference either the Mediatek HS mode commit that exposed this, or the original commit that introduced the `mode_flags &= ~MIPI_DSI_MODE_LPM` without a corresponding restore.
- **Commit subject line** is a bit long (76 chars after the prefix). Minor — well within tolerance.
---
Generated by Claude Code Patch Reviewer
prev parent reply other threads:[~2026-04-22 22:22 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-21 15:31 [PATCH] drm/panel/boe-tv101wum-nl6: set MIPI_DSI_MODE_LPM after sending panel disable cmds Brady Norander
2026-04-22 22:22 ` Claude review: " Claude Code Review Bot
2026-04-22 22:22 ` Claude Code Review 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=review-patch1-20260421153147.4378-2-brady.norander@mainlining.org \
--to=claude-review@example.com \
--cc=dri-devel-reviews@example.com \
/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