Re: [v4 2/2] drm/i915/audio: Prune ELD SADs based on HDMI audio bandwidth

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 8/13/2026 6:04 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The bandwidth check incorrectly uses the maximum pixel clock instead of the minimum pixel clock to calculate worst-case audio packet accumulation per line.
> --
>
> --- Patch [2]: [v4 2/2] drm/i915/audio: Prune ELD SADs based on HDMI audio bandwidth ---
> commit fa98484a48f1957f560711682a22233b5a6fa1f5
> Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>
> drm/i915/audio: Prune ELD SADs based on HDMI audio bandwidth
>
> This commit adds a bandwidth check to determine whether a given audio
> sample rate and channel count can be carried within the hblank period
> for HDMI TMDS mode. It uses this check to prune unsupported sample rates
> from each SAD in the ELD.
>
>> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
>> index eae76e9611055..9eb6867fa3f3a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> [ ... ]
>
>> @@ -697,6 +698,158 @@ static void ibx_audio_codec_enable(struct intel_encoder *encoder,
> [ ... ]
>
>> +static bool hdmi_audio_rate_supported(const struct intel_crtc_state *crtc_state,
>> +				      int available_tmds,
>> +				      int audio_rate, int channels)
>> +{
>> +	const struct drm_display_mode *mode = &crtc_state->hw.adjusted_mode;
>> +	int pixel_clk_max_hz;
>> +	int audio_pkt_factor;
>> +	u64 audio_pkt_rate_x4_x1000;
>> +	int audio_packets_line;
>> +	int hblank_overhead;
>> +	int required_tmds;
>> +
>> +	/*
>> +	 * Part 2: Calculate TMDS clock cycles required for Audio Bandwidth
>> +	 *
>> +	 * Step 1: pixelclk_max = nominal_pixel_rate * (1 + 0.5%)
>> +	 * crtc_clock (kHz) * 1000 * 1.005 = crtc_clock * 1005 (Hz)
>> +	 */
>> +	pixel_clk_max_hz = mode->crtc_clock * 1005;
> [ ... ]
>
>> +	/*
>> +	 * Steps 2+5-6: Audio packets per line.
>> +	 *   AudioPackets_Line = CEIL[R_AP * htotal / f_pixelclk_max]
>> +	 *
>> +	 * With audio_pkt_rate_x4_x1000 = R_AP * 4 * 1000:
>> +	 *   = CEIL[audio_pkt_rate_x4_x1000 * htotal / (4 * 1000 * pixel_clk_max_hz)]
>> +	 */
>> +	audio_packets_line = DIV64_U64_ROUND_UP(audio_pkt_rate_x4_x1000 * mode->htotal,
>> +						(u64)4 * 1000 * pixel_clk_max_hz);
> [Severity: Medium]
> To calculate the worst-case (maximum possible) required audio packets per line,
> should this logic use the minimum possible pixel clock instead of the maximum?
>
> A slower pixel clock means a video line takes longer to transmit in real time,
> causing more audio packets to accumulate in the FIFO during that line. These
> accumulated packets then require more TMDS bandwidth to transmit during the
> corresponding blanking period.
>
> By multiplying the nominal clock by 1005 (adding 0.5% tolerance) instead of
> 995 (subtracting 0.5% tolerance) and using it as a divisor here, the code
> calculates the minimum required packets instead of the maximum. This optimistic
> calculation could incorrectly allow audio configurations that will overflow
> the HDMI audio FIFO and cause audio dropouts when the pixel clock runs at
> nominal or slower speeds.
>
> Although this matches the exact naming in the 'f_pixelclk_max' comment
> (suggesting a likely typo in the underlying BSpec/HDMI hardware specification),
> is it mathematically flawed for a worst-case capability bounds check?


Mathematically the reasoning is correct, but this is exactly how it's 
defined in Bspec and the HDMI spec.

Perhaps the ceiling function and the audio clock tolerance can take care 
of the boundary case where the audio packets per line with (clock + 
0.5%) falls to 18 and the actual clock comes out to 19, though I am not 
entirely sure.

Since the patch matches the specs, I think we are covered.

Regards,

Ankit
>