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: Add mechanism to detect DPI output Date: Thu, 23 Apr 2026 09:45:58 +1000 Message-ID: In-Reply-To: <20260420-beagley-ai-display-v1-8-f628543dfd14@ideasonboard.com> References: <20260420-beagley-ai-display-v1-0-f628543dfd14@ideasonboard.com> <20260420-beagley-ai-display-v1-8-f628543dfd14@ideasonboard.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review The reverse-detection approach (not-DSI and not-OLDI means DPI) is pragmatic. The implementation is clean. ```c static bool tidss_is_bridge_internal(struct tidss_device *tidss, struct drm_bridge *bridge) ``` The function checks `of_find_property()` for compatible, then iterates known internal bridge compatibles. The OLDI detection via parent/grandparent traversal is reasonable. **Minor issue:** The `dpi_output` value is stored redundantly. It's set once in `tidss_crtc_create()` on `tcrtc->dpi_output`, then passed through to `dispc_vp_setup()` on every atomic flush/enable, which writes it into `dispc->vp_data[hw_videoport].dpi_output`. Since `dpi_output` never changes for a given VP, this per-frame write is wasteful. Consider setting `dispc->vp_data[].dpi_output` once during modeset init instead of threading it through the setup call. This is a design nit, not a bug. --- Generated by Claude Code Patch Reviewer