Re: [PATCH 1/4] drm/display: hdmi: provide central data authority for ACR params

[Maxime Ripard]

On Sun, Mar 09, 2025 at 10:13:56AM +0200, Dmitry Baryshkov wrote:
> From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> 
> HDMI standard defines recommended N and CTS values for Audio Clock
> Regeneration. Currently each driver implements those, frequently in
> somewhat unique way. Provide a generic helper for getting those values
> to be used by the HDMI drivers.
> 
> The helper is added to drm_hdmi_helper.c rather than drm_hdmi_audio.c
> since HDMI drivers can be using this helper function even without
> switching to DRM HDMI Audio helpers.
> 
> Note: currently this only handles the values per HDMI 1.4b Section 7.2
> and HDMI 2.0 Section 9.2.1. Later the table can be expanded to
> accommodate for Deep Color TMDS char rates per HDMI 1.4 Appendix D
> and/or HDMI 2.0 / 2.1 Appendix C).
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/display/drm_hdmi_helper.c | 164 ++++++++++++++++++++++++++++++
>  include/drm/display/drm_hdmi_helper.h     |   6 ++
>  2 files changed, 170 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_helper.c b/drivers/gpu/drm/display/drm_hdmi_helper.c
> index 74dd4d01dd9bb2c9e69ec1c60b0056bd69417e8a..89d25571bfd21c56c6835821d2272a12c816a76e 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c
> @@ -256,3 +256,167 @@ drm_hdmi_compute_mode_clock(const struct drm_display_mode *mode,
>  	return DIV_ROUND_CLOSEST_ULL(clock * bpc, 8);
>  }
>  EXPORT_SYMBOL(drm_hdmi_compute_mode_clock);
> +
> +struct drm_hdmi_acr_n_cts_entry {
> +	unsigned int n;
> +	unsigned int cts;
> +};
> +
> +struct drm_hdmi_acr_data {
> +	unsigned long tmds_clock_khz;
> +	struct drm_hdmi_acr_n_cts_entry n_cts_32k,
> +					n_cts_44k1,
> +					n_cts_48k;
> +};
> +
> +static const struct drm_hdmi_acr_data hdmi_acr_n_cts[] = {
> +	{
> +		/* "Other" entry */
> +		.n_cts_32k =  { .n = 4096, },
> +		.n_cts_44k1 = { .n = 6272, },
> +		.n_cts_48k =  { .n = 6144, },
> +	}, {
> +		.tmds_clock_khz = 25175,
> +		.n_cts_32k =  { .n = 4576,  .cts = 28125, },
> +		.n_cts_44k1 = { .n = 7007,  .cts = 31250, },
> +		.n_cts_48k =  { .n = 6864,  .cts = 28125, },
> +	}, {
> +		.tmds_clock_khz = 25200,
> +		.n_cts_32k =  { .n = 4096,  .cts = 25200, },
> +		.n_cts_44k1 = { .n = 6272,  .cts = 28000, },
> +		.n_cts_48k =  { .n = 6144,  .cts = 25200, },
> +	}, {
> +		.tmds_clock_khz = 27000,
> +		.n_cts_32k =  { .n = 4096,  .cts = 27000, },
> +		.n_cts_44k1 = { .n = 6272,  .cts = 30000, },
> +		.n_cts_48k =  { .n = 6144,  .cts = 27000, },
> +	}, {
> +		.tmds_clock_khz = 27027,
> +		.n_cts_32k =  { .n = 4096,  .cts = 27027, },
> +		.n_cts_44k1 = { .n = 6272,  .cts = 30030, },
> +		.n_cts_48k =  { .n = 6144,  .cts = 27027, },
> +	}, {
> +		.tmds_clock_khz = 54000,
> +		.n_cts_32k =  { .n = 4096,  .cts = 54000, },
> +		.n_cts_44k1 = { .n = 6272,  .cts = 60000, },
> +		.n_cts_48k =  { .n = 6144,  .cts = 54000, },
> +	}, {
> +		.tmds_clock_khz = 54054,
> +		.n_cts_32k =  { .n = 4096,  .cts = 54054, },
> +		.n_cts_44k1 = { .n = 6272,  .cts = 60060, },
> +		.n_cts_48k =  { .n = 6144,  .cts = 54054, },
> +	}, {
> +		.tmds_clock_khz = 74176,
> +		.n_cts_32k =  { .n = 11648, .cts = 210937, }, /* and 210938 */
> +		.n_cts_44k1 = { .n = 17836, .cts = 234375, },
> +		.n_cts_48k =  { .n = 11648, .cts = 140625, },
> +	}, {
> +		.tmds_clock_khz = 74250,
> +		.n_cts_32k =  { .n = 4096,  .cts = 74250, },
> +		.n_cts_44k1 = { .n = 6272,  .cts = 82500, },
> +		.n_cts_48k =  { .n = 6144,  .cts = 74250, },
> +	}, {
> +		.tmds_clock_khz = 148352,
> +		.n_cts_32k =  { .n = 11648, .cts = 421875, },
> +		.n_cts_44k1 = { .n = 8918,  .cts = 234375, },
> +		.n_cts_48k =  { .n = 5824,  .cts = 140625, },
> +	}, {
> +		.tmds_clock_khz = 148500,
> +		.n_cts_32k =  { .n = 4096,  .cts = 148500, },
> +		.n_cts_44k1 = { .n = 6272,  .cts = 165000, },
> +		.n_cts_48k =  { .n = 6144,  .cts = 148500, },
> +	}, {
> +		.tmds_clock_khz = 296703,
> +		.n_cts_32k =  { .n = 5824,  .cts = 421875, },
> +		.n_cts_44k1 = { .n = 4459,  .cts = 234375, },
> +		.n_cts_48k =  { .n = 5824,  .cts = 281250, },
> +	}, {
> +		.tmds_clock_khz = 297000,
> +		.n_cts_32k =  { .n = 3072,  .cts = 222750, },
> +		.n_cts_44k1 = { .n = 4704,  .cts = 247500, },
> +		.n_cts_48k =  { .n = 5120,  .cts = 247500, },
> +	}, {
> +		.tmds_clock_khz = 593407,
> +		.n_cts_32k =  { .n = 5824,  .cts = 843750, },
> +		.n_cts_44k1 = { .n = 8918,  .cts = 937500, },
> +		.n_cts_48k =  { .n = 5824,  .cts = 562500, },
> +	}, {
> +		.tmds_clock_khz = 594000,
> +		.n_cts_32k =  { .n = 3072,  .cts = 445500, },
> +		.n_cts_44k1 = { .n = 9408,  .cts = 990000, },
> +		.n_cts_48k =  { .n = 6144,  .cts = 594000, },
> +	},
> +};
> +
> +static int drm_hdmi_acr_find_tmds_entry(unsigned long tmds_clock_khz)
> +{
> +	int i;
> +
> +	/* skip the "other" entry */
> +	for (i = 1; i < ARRAY_SIZE(hdmi_acr_n_cts); i++) {
> +		if (hdmi_acr_n_cts[i].tmds_clock_khz == tmds_clock_khz)
> +			return i;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * drm_hdmi_acr_get_n_cts() - get N and CTS values for Audio Clock Regeneration
> + *
> + * @tmds_char_rate: TMDS clock (char rate) as used by the HDMI connector
> + * @sample_rate: audio sample rate
> + * @out_n: a pointer to write the N value
> + * @out_cts: a pointer to write the CTS value
> + *
> + * Get the N and CTS values (either by calculating them or by returning data
> + * from the tables. This follows the HDMI 1.4b Section 7.2 "Audio Sample Clock
> + * Capture and Regeneration".
> + */

I think we need to make it clear that it's for L-PCM only (I think?),
either through a format parameter or through the documentation.

> +void
> +drm_hdmi_acr_get_n_cts(unsigned long long tmds_char_rate,
> +		       unsigned int sample_rate,
> +		       unsigned int *out_n,
> +		       unsigned int *out_cts)

And we should probably take the connector (or EDID) to make sure the
monitor can support the format and sample rates.

> +{
> +	/* be a bit more tolerant, especially for the 1.001 entries */
> +	unsigned long tmds_clock_khz = DIV_ROUND_CLOSEST_ULL(tmds_char_rate, 1000);
> +	const struct drm_hdmi_acr_n_cts_entry *entry;
> +	unsigned int n, cts, mult;
> +	int tmds_idx;
> +
> +	tmds_idx = drm_hdmi_acr_find_tmds_entry(tmds_clock_khz);
> +
> +	/*
> +	 * Don't change the order, 192 kHz is divisible by 48k and 32k, but it
> +	 * should use 48k entry.
> +	 */
> +	if (sample_rate % 48000 == 0) {
> +		entry = &hdmi_acr_n_cts[tmds_idx].n_cts_48k;
> +		mult = sample_rate / 48000;
> +	} else if (sample_rate % 44100 == 0) {
> +		entry = &hdmi_acr_n_cts[tmds_idx].n_cts_44k1;
> +		mult = sample_rate / 44100;
> +	} else if (sample_rate % 32000 == 0) {
> +		entry = &hdmi_acr_n_cts[tmds_idx].n_cts_32k;
> +		mult = sample_rate / 32000;
> +	} else {
> +		entry = NULL;
> +	}
> +
> +	if (entry) {
> +		n = entry->n * mult;
> +		cts = entry->cts;
> +	} else {
> +		/* Recommended optimal value, HDMI 1.4b, Section 7.2.1 */
> +		n = 128 * sample_rate / 1000;
> +		cts = 0;
> +	}
> +
> +	if (!cts)
> +		cts = DIV_ROUND_CLOSEST_ULL(tmds_char_rate * n,
> +					    128 * sample_rate);
> +
> +	*out_n = n;
> +	*out_cts = cts;
> +}

EXPORT_SYMBOL?

Also, I'd really like to have some unit tests for this. Not for all the
combinations, obviously, but testing that, say, 44.1kHz with a 148.5 MHz
char rate works as expected, and then all the failure conditions
depending on the monitor capabilities.

Maxime

Attachment: signature.asc
Description: PGP signature