* [PATCH] drm/panel: ilitek-ili9882t: Fine-tune HFP for tianma, tl121bvms07-00
@ 2026-02-09 9:07 Langyan Ye
2026-02-11 7:10 ` Claude review: " Claude Code Review Bot
2026-02-11 7:10 ` Claude Code Review Bot
0 siblings, 2 replies; 3+ messages in thread
From: Langyan Ye @ 2026-02-09 9:07 UTC (permalink / raw)
To: neil.armstrong, maarten.lankhorst, mripard, tzimmermann, airlied,
simona
Cc: dri-devel, linux-kernel, Langyan Ye
The measured refresh rate is around 120.4Hz, which does not meet the
requirements of the stylus and causes it to malfunction.
After the adjustment, the refresh rate is measured at 120Hz, which meets
the requirements for proper stylus operation.
Signed-off-by: Langyan Ye <yelangyan@huaqin.corp-partner.google.com>
---
drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c
index 370424ddfc80..6f9d4a4198a9 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c
@@ -689,9 +689,9 @@ static const struct drm_display_mode starry_ili9882t_default_mode = {
static const struct drm_display_mode tianma_il79900a_default_mode = {
.clock = 543850,
.hdisplay = 1600,
- .hsync_start = 1600 + 20,
- .hsync_end = 1600 + 20 + 2,
- .htotal = 1600 + 20 + 2 + 20,
+ .hsync_start = 1600 + 18,
+ .hsync_end = 1600 + 18 + 2,
+ .htotal = 1600 + 18 + 2 + 20,
.vdisplay = 2560,
.vsync_start = 2560 + 62,
.vsync_end = 2560 + 62 + 2,
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Claude review: drm/panel: ilitek-ili9882t: Fine-tune HFP for tianma, tl121bvms07-00
2026-02-09 9:07 [PATCH] drm/panel: ilitek-ili9882t: Fine-tune HFP for tianma, tl121bvms07-00 Langyan Ye
@ 2026-02-11 7:10 ` Claude Code Review Bot
2026-02-11 7:10 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-02-11 7:10 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/panel: ilitek-ili9882t: Fine-tune HFP for tianma,
tl121bvms07-00
Author: Langyan Ye <yelangyan@huaqin.corp-partner.google.com>
Patches: 1
Reviewed: 2026-02-11T17:10:16.964206
---
This is a single-patch series that addresses a display panel timing issue for the Tianma TL121BVMS07-00 panel in the ilitek-ili9882t panel driver. The patch modifies the horizontal front porch (HFP) timing to correct a refresh rate discrepancy that was causing stylus malfunction.
**Summary Assessment:**
- **Purpose**: Clear and well-defined - fixes refresh rate from 120.4Hz to 120Hz for stylus compatibility
- **Scope**: Minimal and focused - only touches the affected timing parameters
- **Correctness**: The math appears sound, but there are some review concerns
- **Testing**: Implied hardware testing (measured refresh rate), but not explicitly documented
**Key Concerns:**
1. Lack of detailed justification for the specific HFP value chosen
2. No explanation of why this timing change is safe/correct per panel datasheet
3. Missing information about testing methodology and devices tested
4. Potential impact on other display characteristics not addressed
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
* Claude review: drm/panel: ilitek-ili9882t: Fine-tune HFP for tianma, tl121bvms07-00
2026-02-09 9:07 [PATCH] drm/panel: ilitek-ili9882t: Fine-tune HFP for tianma, tl121bvms07-00 Langyan Ye
2026-02-11 7:10 ` Claude review: " Claude Code Review Bot
@ 2026-02-11 7:10 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-02-11 7:10 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Commit Message Analysis:**
The commit message is brief but functional. It explains the problem (120.4Hz causing stylus malfunction) and the result (120Hz after fix). However, it lacks several important details:
- **Missing**: How was the refresh rate measured?
- **Missing**: Why is 120Hz the target vs 120.4Hz?
- **Missing**: Reference to panel datasheet or vendor specification
- **Missing**: Information about testing scope (which devices, which kernels)
**Technical Review:**
```c
- .hsync_start = 1600 + 20,
- .hsync_end = 1600 + 20 + 2,
- .htotal = 1600 + 20 + 2 + 20,
+ .hsync_start = 1600 + 18,
+ .hsync_end = 1600 + 18 + 2,
+ .htotal = 1600 + 18 + 2 + 20,
```
**Timing Calculation Verification:**
Old timing:
- HFP (front porch): 20
- HSYNC: 2
- HBP (back porch): 20
- htotal = 1600 + 20 + 2 + 20 = 1642
New timing:
- HFP (front porch): 18
- HSYNC: 2
- HBP (back porch): 20
- htotal = 1600 + 18 + 2 + 20 = 1640
Refresh rate calculation:
- Old: 543,850,000 / (1642 × 2624) = 126.23 Hz ❌ (doesn't match stated 120.4Hz)
- New: 543,850,000 / (1640 × 2624) = 126.35 Hz ❌ (doesn't match stated 120Hz)
**CRITICAL ISSUE**: The math doesn't align with the commit message. Either:
1. The clock value is incorrect in the driver
2. The measurements in the commit message are incorrect
3. There's a pixel clock multiplier/divider not shown in this code
**Questions for Author:**
1. **Timing Source**: What is the authoritative source for these timings? Panel datasheet? Vendor recommendation? If so, please reference it.
2. **Measurement Method**: How was the refresh rate measured? Using what tool? The calculated refresh rate based on the pixel clock and totals doesn't match the stated values.
3. **Stylus Requirement**: What is the exact refresh rate tolerance required by the stylus? Is it specified in documentation?
4. **Safe Operating Range**: Does the panel datasheet specify minimum/maximum HFP values? Is 18 pixels within spec?
5. **Display Quality**: Has this been tested for:
- Visual artifacts at the reduced HFP?
- Proper blanking intervals?
- Panel stability over extended operation?
- Different temperature conditions?
6. **Regression Risk**: Was the original HFP=20 value chosen for a reason? Are there other panels using the same timings that might be affected?
**Code Quality:**
The code change itself is mechanically correct:
- Consistently updates all three related fields
- Maintains the mathematical relationship between hsync_start, hsync_end, and htotal
- Follows the existing code pattern
**Recommendation:**
**NEEDS CLARIFICATION** - The patch addresses a real issue, but the justification needs strengthening:
1. Explain the discrepancy between calculated and measured refresh rates
2. Provide datasheet reference or vendor confirmation for the new timing
3. Document the testing performed (devices, duration, scenarios)
4. Clarify if this timing is within the panel's specified operating range
The code change is minimal and low-risk, but without proper justification for the timing values, it's difficult to confirm this is the correct fix rather than just a fix that happens to work on the tested hardware.
**Minor nit**: The subject line could be more specific: "Reduce HFP from 20 to 18 pixels to correct 120Hz refresh rate" would better describe the actual change.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-02-11 7:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-09 9:07 [PATCH] drm/panel: ilitek-ili9882t: Fine-tune HFP for tianma, tl121bvms07-00 Langyan Ye
2026-02-11 7:10 ` Claude review: " Claude Code Review Bot
2026-02-11 7:10 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox