From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/tidss: oldi: Fix OLDI signal polarities Date: Thu, 23 Apr 2026 09:45:59 +1000 Message-ID: In-Reply-To: <20260420-beagley-ai-display-v1-11-f628543dfd14@ideasonboard.com> References: <20260420-beagley-ai-display-v1-0-f628543dfd14@ideasonboard.com> <20260420-beagley-ai-display-v1-11-f628543dfd14@ideasonboard.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review Good fix. The old code unconditionally set `OLDI_DEPOL` (active low DE) whi= le simultaneously declaring `DRM_BUS_FLAG_DE_HIGH` in unused bridge timings= =E2=80=94 clearly inconsistent. The new approach: - Removes the dead `drm_bridge_timings` - Adds `tidss_oldi_atomic_check()` that sets `DRM_BUS_FLAG_PIXDATA_SAMPLE_P= OSEDGE | DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE` - Makes `OLDI_DEPOL` conditional on `DRM_BUS_FLAG_DE_LOW` This is correct per the TRM description ("clocked on rising edge"). The bus= flags are now properly propagated via the bridge state rather than through= an unused timings struct. **Minor nit:** The `tidss_oldi_atomic_check` clears negedge flags and sets = posedge flags, but doesn't touch `DRM_BUS_FLAG_DE_LOW`/`DRM_BUS_FLAG_DE_HIG= H`. Since the default bridge state presumably has neither set, `tidss_oldi_= config()` will never set `OLDI_DEPOL`. This should be fine for standard OLD= I panels (DE active high is the norm), but worth documenting whether OLDI p= anels with active-low DE are expected to work. --- Generated by Claude Code Patch Reviewer