| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250310-funny-malamute-of-promotion-bb759e@houat>
Date: Mon, 10 Mar 2025 15:46:33 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Dmitry Baryshkov <lumag@...nel.org>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Rob Clark <robdclark@...il.com>,
Abhinav Kumar <quic_abhinavk@...cinc.com>, Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>, Dave Stevenson <dave.stevenson@...pberrypi.com>,
Maíra Canal <mcanal@...lia.com>, Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>,
Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong <neil.armstrong@...aro.org>,
Robert Foss <rfoss@...nel.org>, Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
freedreno@...ts.freedesktop.org
Subject: Re: [PATCH 1/4] drm/display: hdmi: provide central data authority
for ACR params
On Sun, Mar 09, 2025 at 10:13:56AM +0200, Dmitry Baryshkov wrote:
> From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>
> 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@...aro.org>
> ---
> 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
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists