| 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: <xidaohhyugexyapghteaioladfs2ma3trett6lyenmz2ubnlud@hnxjccqvbvz2>
Date: Tue, 11 Mar 2025 18:24:57 +0200
From: Dmitry Baryshkov <lumag@...nel.org>
To: Maxime Ripard <mripard@...nel.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
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 Tue, Mar 11, 2025 at 08:59:45AM +0100, Maxime Ripard wrote:
> On Mon, Mar 10, 2025 at 10:14:52PM +0200, Dmitry Baryshkov wrote:
> > On Mon, Mar 10, 2025 at 03:46:33PM +0100, Maxime Ripard wrote:
> > > 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.
> >
> > Ack
> >
> > >
> > > > +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.
> >
> > Interesting perspective, I'll give it a thought. I was really just
> > trying to get rid of the duplication.
> >
> > I think that 'supported' parts should be implemented in the hdmi-codec
> > instead, parsing the ELD and updating hw constraints. WDYT?
>
> Basically, I want to make sure we cover section 7.3 of HDMI 1.4, ie,
> make sure we can't end up (or validate) in a situation that isn't
> allowed by the spec.
I think that's a question for a separate function. This one really
targets 7.2 rather than 7.3.
> If ALSA covers it already, then I guess it's fine, but we should
> document it and point to where it's dealt with.
I'm not sure if it covers that right now, but it should be handled on
ALSA side. For example, see sound/pci/hda/patch_hdmi.c, I think it is
handling those bits. We are providing ELD to hdmi-codec, it can
implement and propagate HW constraints.
--
With best wishes
Dmitry
Powered by blists - more mailing lists