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

On Wed, 12 Aug 2026, Chaitanya Kumar Borah <[email protected]> wrote:
> Add bandwidth check to determine whether a given audio sample rate and
> channel count can be carried within the hblank period for HDMI TMDS mode.
>
> Use this check to prune unsupported sample rates from each SAD in the
> ELD during intel_audio_compute_config(). SADs with no remaining
> supported rates are removed entirely.
>
> Sample rates are pruned rather than channel counts, since compressed
> formats (e.g. AC-3) are associated with specific channel counts.
>
> v2:
>  - Use DIV64_U64_ROUND_UP() instead of DIV_ROUND_UP_ULL() to avoid
>    do_div() truncating the 64-bit divisor to 32-bit, which caused
>    audio_packets_line to be wildly inflated and all SADs to be pruned.
>  - Guard intel_audio_hdmi_eld_compute_config() against HDMI FRL modes.
>    (Remove it when BW calculations for FRL are added.)
>
> v3:
>  - Rebase
>
> BSpec: 68944
> Cc: Kai Vehmanen <[email protected]>
> Cc: Ankit Nautiyal <[email protected]>
> Cc: Ville Syrjälä <[email protected]>
> Cc: Vinod Govindapillai <[email protected]>
> Cc: Mitul Golani <[email protected]>
> Assisted-by: GitHub-Copilot:claude-opus-4.6
> Signed-off-by: Chaitanya Kumar Borah <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c | 149 +++++++++++++++++++++
>  1 file changed, 149 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index eae76e961105..d59acb390f8a 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -39,6 +39,7 @@
>  #include "intel_display_types.h"
>  #include "intel_display_wa.h"
>  #include "intel_dp.h"
> +#include "intel_hdmi.h"
>  #include "intel_lpe_audio.h"
>  
>  /**
> @@ -697,6 +698,151 @@ static void ibx_audio_codec_enable(struct intel_encoder *encoder,
>  	mutex_unlock(&display->audio.mutex);
>  }
>  
> +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 3-4: Audio Packet Rate.
> +	 *   R_AP = (audio_rate * AP + 2 * acrrate_max) * (1 + 1000 / 1e6)
> +	 *        = (audio_rate * AP + 2*1500) * 1.001
> +	 *
> +	 * AP = 0.25 (2ch) or 1.0 (3-8ch); acrrate_max = 1500 Hz (max ACR
> +	 * packet transmission rate per HDMI spec)
> +	 *
> +	 * Scale by 4*1000 to stay integer:
> +	 *  x4: eliminates AP=0.25 -> audio_pkt_factor=1(2ch) or 4(3-8ch),
> +	 *      scaled acrrate_max: 2 * 1500 * 4 = 12000
> +	 *  x1000: eliminates 1.001 -> *1000*1.001 = *1001
> +	 *
> +	 * R_AP * 4 * 1000 = (audio_rate * audio_pkt_factor + 12000) * 1001
> +	 */
> +	audio_pkt_factor = (channels <= 2) ? 1 : 4;
> +	audio_pkt_rate_x4_x1000 = (u64)(audio_rate * audio_pkt_factor + 12000) * 1001;
> +
> +	/*
> +	 * 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);
> +
> +	/*
> +	 * Steps 7-9: Hblank overhead.
> +	 * Standard:  2*dip_guardband + 2*control_period + video_guardband
> +	 *          = 2*2 + 2*12 + 2 = 30
> +	 * HDCP 1.x:  rekey_period + dip_guardband + control_period + video_guardband
> +	 *          = 58 + 2 + 12 + 2 = 74
> +	 *
> +	 * Always use HDCP 1.x worst case (74) since HDCP can be toggled
> +	 * via fastset without compute_config.
> +	 */
> +	hblank_overhead = 74;
> +
> +	/*
> +	 * Step 10: Required TMDS cycles for Audio.
> +	 *  32 TMDS clock cycles per audio packet.
> +	 *   Hblank_audio_min = 32 * AudioPackets_Line + Hblank_overhead
> +	 */
> +	required_tmds = 32 * audio_packets_line + hblank_overhead;
> +
> +	/*
> +	 * Part 3: audio supported if Hblank_audio_min <= TB_blank and
> +	 * audio packets per line <= Maximum allowed packets per line (18)
> +	 */
> +
> +	return required_tmds <= available_tmds && audio_packets_line <= 18;
> +}
> +
> +static void intel_audio_hdmi_eld_compute_config(struct intel_crtc_state *crtc_state)
> +{
> +	static const int sad_freqs[] = {
> +		32000, 44100, 48000, 88200, 96000, 176400, 192000
> +	};
> +	const struct drm_display_mode *mode = &crtc_state->hw.adjusted_mode;
> +	int hblank = mode->htotal - mode->hdisplay;
> +	int bpc = crtc_state->pipe_bpp / 3;
> +	int ycbcr_420_divider = (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) ? 2 : 1;
> +	int available_tmds;
> +	u8 *eld = crtc_state->eld;
> +	int mnl = drm_eld_mnl(eld);
> +	int sad_count = drm_eld_sad_count(eld);
> +	int i = 0;
> +
> +	/* Only applies to HDMI TMDS, not FRL */
> +	if (intel_hdmi_is_frl(crtc_state->port_clock))
> +		return;
> +	/*
> +	 * Part 1: Calculate available TMDS clock cycles (TB_blank).
> +	 *
> +	 * TB_blank = CEILING[hblank * K_CD / K_420]
> +	 *
> +	 * K_CD = 1 for YCbCr4:2:2, bpc / 8 otherwise.
> +	 * K_420 = 2 for YCbCr4:2:0, 1 otherwise.
> +	 * Rearranged: CEILING[hblank * bpc / (8 * K_420)]
> +	 *
> +	 * TODO: As and when support for YCbCr4:2:2 is added, set bpc = 8
> +	 * to achieve K_CD = 1
> +	 */
> +	available_tmds = DIV_ROUND_UP(hblank * bpc, 8 * ycbcr_420_divider);
> +
> +	while (i < sad_count) {
> +		int sad_offset = DRM_ELD_CEA_SAD(mnl, i);
> +		int channels = (eld[sad_offset] & 0x7) + 1;
> +		u8 freq_mask = eld[sad_offset + 1];
> +		u8 new_freq_mask = 0;
> +		int bit;
> +
> +		for (bit = 0; bit < 7; bit++) {
> +			if (!(freq_mask & BIT(bit)))
> +				continue;
> +			if (hdmi_audio_rate_supported(crtc_state, available_tmds,
> +						      sad_freqs[bit], channels))
> +				new_freq_mask |= BIT(bit);
> +		}
> +
> +		eld[sad_offset + 1] = new_freq_mask;
> +
> +		if (!new_freq_mask) {
> +			/* Remove this SAD by compacting the rest */
> +			memmove(&eld[DRM_ELD_CEA_SAD(mnl, i)],
> +				&eld[DRM_ELD_CEA_SAD(mnl, i + 1)],
> +				(sad_count - i - 1) * 3);

What if there is no SAD i + 1?

> +			memset(&eld[DRM_ELD_CEA_SAD(mnl, sad_count - 1)], 0, 3);
> +			sad_count--;
> +			continue;
> +		}
> +		i++;

I believe it would all be more readable and robust if you used a for
loop to go through all the sads, without modifing the end conditions
while iterating, and used a separate destination index for when you have
to move sads over. And only moved the sads one at a time as you iterate,
not all the time.

Sometimes decrementing the sad_count and sometimes incrementing i makes
this difficult to reason.

> +	}
> +
> +	/* Update SAD count in ELD header */
> +	eld[DRM_ELD_SAD_COUNT_CONN_TYPE] &= ~DRM_ELD_SAD_COUNT_MASK;
> +	eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= sad_count << DRM_ELD_SAD_COUNT_SHIFT;
> +
> +	/* Recalculate baseline ELD length (in dwords) */
> +	eld[DRM_ELD_BASELINE_ELD_LEN] =
> +		DIV_ROUND_UP(drm_eld_calc_baseline_block_size(eld), 4);
> +}
> +
>  static
>  bool intel_audio_needs_cpu_transcoder_id(const struct intel_crtc_state *crtc_state)
>  {
> @@ -725,6 +871,9 @@ bool intel_audio_compute_config(struct intel_encoder *encoder,
>  	BUILD_BUG_ON(sizeof(crtc_state->eld) != sizeof(connector->eld));
>  	memcpy(crtc_state->eld, connector->eld, sizeof(crtc_state->eld));
>  
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> +		intel_audio_hdmi_eld_compute_config(crtc_state);
> +
>  	crtc_state->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
>  	mutex_unlock(&connector->eld_mutex);

-- 
Jani Nikula, Intel