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: writeback: Document behaviour for framebuffer size mismatch Date: Thu, 04 Jun 2026 13:41:03 +1000 Message-ID: In-Reply-To: <20260601195732.72538-1-derek.foreman@collabora.com> References: <20260601195732.72538-1-derek.foreman@collabora.com> <20260601195732.72538-1-derek.foreman@collabora.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 **Correctness:** The placement of the new text is appropriate =E2=80=94 it'= s inserted into the `WRITEBACK_FB_ID` property description between the "wil= l always read as zero and is not preserved across commits" sentence and the= "Userspace must set this property" sentence. Looking at the existing code = at line 60-62: ``` * and is not preserved across commits. * Userspace must set this property to an output buffer every time it ``` The new text: ``` * If the width and height of the framebuffer do not match those of the * attached CRTC, the driver may either fail or scale (not crop) the * content to fit the framebuffer. ``` This reads well in context and is positioned logically. **Suggestions:** - Consider making the failure mode more explicit: "the driver may either fa= il the atomic commit or scale..." =E2=80=94 this clarifies that failure sho= uld happen at check time, which is important for userspace doing `TEST_ONLY= ` commits to probe capabilities. - It may be worth adding a note about aspect ratio behavior being driver-de= fined, e.g., "the driver may either fail the atomic commit or scale (not cr= op) the content to fit the framebuffer; the scaling method is driver-define= d." - The commit message mentions "Cropping would make less sense, as the regio= n to crop is underspecified without some other properties to define an orig= in." This is good reasoning. It might be worth capturing the "not crop" rat= ionale briefly in the doc itself (or at least confirming the commit message= is sufficient for future archaeology). **Nit:** The subject says "Document behaviour" (British English) =E2=80=94 = this is fine but note the kernel tends toward American English ("behavior")= in most documentation, though it's not strictly enforced. **Overall:** This is a clean, focused documentation patch. The content is c= orrect and fills a real gap in the UAPI specification. It could benefit fro= m slightly more precise wording around failure mode (at atomic check time) = and scaling semantics, but as-is it's a reasonable improvement. **Verdict:** Looks good with optional wording improvements. Reviewed-by wor= thy. --- Generated by Claude Code Patch Reviewer