Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] drm/i915: unsigned underflow defeats max() zero-clamping in FRL payload logic - [High] drm/i915: unsigned underflow in margin calculation bypasses FRL capacity validation - [High] drm/i915: 32-bit integer overflow artificially truncates required audio packet count - [Medium] drm/i915/hdmi_frl: 32-bit integer overflows in DFM timing calculations --
commit 6d21cc13348026a7ea42c4821a0b77f14d185b3e Author: Vandita Kulkarni <[email protected]> drm/i915/hdmi_frl_dfm: Add non dsc frl capacity computation helpers This commit adds helper functions to the i915 DRM driver for computing non-DSC FRL link characteristics. It includes computations for DFM constraints, link overheads, audio hblank requirements, and capacity verifications. [Severity: High] In drivers/gpu/drm/i915/display/intel_hdmi_frl_dfm.c, within the get_num_char_rc_compressible() function, the calculation for cfrl_free is: cfrl_free = max(((hblank * kcdx100) / (100 * k420) - 32 * audio_packets_line - 7), U32_MIN); Since these variables are unsigned 32-bit integers, does this subtraction underflow when the required audio blanking space exceeds the available hardware hblank? If an underflow occurs, the value wraps to a huge positive number. Because the comparison is unsigned, the max() macro would simply return this huge positive value rather than clamping it to zero. This huge value then propagates into get_num_char_compression_savings(): u32 cfrl_savings = max(((7 * cfrl_free) / 8) - cfrl_margin, U32_MIN); Could this corrupt the downstream link utilization checks and cause the driver to incorrectly validate video modes that lack sufficient blanking intervals? [Severity: High] In the verify_utilization_possible() function, the link capacity margin is calculated as: margin = 1000 - (utilization + frl_dfm->params.overhead_max); if (margin > 0) return true; If an over-utilized video mode requires more than 100 percent capacity (where utilization + overhead_max > 1000), does the unsigned subtraction underflow to a large positive value? This would cause the subsequent margin greater than 0 check to evaluate to true, which might lead the driver to validate modes that actually exceed the maximum FRL hardware bandwidth. [Severity: High] In the compute_audio_hblank_min() function, the required audio packets are calculated using standard 32-bit multiplication: frl_dfm->params.num_audio_pkts_line = DIV_ROUND_UP(audio_pkt_rate * frl_dfm->params.line_time_ns, FRL_TIMING_NS_MULTIPLIER); For standard low-refresh display timings transmitting high sample-rate audio (for instance, 192kHz multi-channel where audio_pkt_rate reaches around 195,195, and a 1080p24 line time of around 37,037ns), the product exceeds 7.2 billion. Does this overflow the U32_MAX limit of 4.29 billion, causing silent integer truncation before the division? Should this use mul_u32_u32() and DIV_ROUND_UP_ULL() to avoid calculating artificially small audio packet requirements? [Severity: Medium] In the verify_frl_capacity_requirement() function, timing requirements are calculated with direct 32-bit multiplication: tactive_ref_ns = (line_time_ns * hactive) / (hblank + hactive); tblank_ref_ns = (line_time_ns * hblank) / (hblank + hactive); Although the commit message mentions using 64-bit math for DFM timing products to avoid u32 overflow, these lines still appear to use 32-bit arithmetic. If a user-space client requests a mode with a very low pixel clock (where line_time_ns is large) and a large horizontal resolution, can line_time_ns * hactive overflow a 32-bit integer? If this wraps, it might trick the capacity check into returning true and bypassing mode rejection for invalid display modes. Does this require a transition to 64-bit arithmetic to safely handle edge-case timing values? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=29
