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/amd/display: support up to 256 samples per region in degamma/blend LUT Date: Thu, 07 May 2026 13:11:57 +1000 Message-ID: In-Reply-To: <20260506191606.15022-5-mwen@igalia.com> References: <20260506191606.15022-1-mwen@igalia.com> <20260506191606.15022-5-mwen@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Purpose**: Extends the degamma helper to support more than 16 samples per region by introducing 4-bit sub-sample interpolation between adjacent `tf_pts` entries. **The `interp_tf_pts` helper** (lines 309-321): Clean linear interpolation. When `t = i & 0xf` is 0 (which is always the case for `seg_distr[k] <= 4`), `value = in + 0 = in`, reducing to a direct lookup. The formula `in + (in_plus_one - in) * t / 16` is mathematically correct. **Bounds safety for higher seg_distr values**: For `seg_distr[k] = 8` (max supported): - `increment = (16 << 4) / (1 << 8) = 256 / 256 = 1` - Per-region: 256 iterations, accessing tf_pts indices from `start_index` to `start_index + 16`, with fractional sub-positions 0/16 through 15/16. - The `(i >> 4) + 1 >= TRANSFER_FUNC_POINTS` guard correctly checks the +1 for interpolation access. **Potential concern with `i` type**: `i` is declared as `int32_t`. After the shift, `start_index << 4` has a maximum value of `400 * 16 = 6400`, well within `int32_t` range. No overflow risk. **Style nit**: The blank line between the `#define NUM_DEGAMMA_REGIONS` removal and the `interp_tf_pts` comment creates slightly inconsistent spacing, but this is trivial. **No functional issues found.** --- Generated by Claude Code Patch Reviewer