diff mbox series

drm/bridge/synopsys: dw-hdmi: Handle audio for more clock rates

Message ID 20190617235558.64571-1-dianders@chromium.org (mailing list archive)
State New, archived
Headers show
Series drm/bridge/synopsys: dw-hdmi: Handle audio for more clock rates | expand

Commit Message

Douglas Anderson June 17, 2019, 11:55 p.m. UTC
Let's add some better support for HDMI audio to dw_hdmi.
Specifically:

1. For 44.1 kHz audio the old code made the assumption that an N of
6272 was right most of the time.  That wasn't true and the new table
should give better 44.1 kHz audio for many more rates.

2. The new table has values from the HDMI spec for 297 MHz and 594
MHz.

3. There is now code to try to come up with a more idea N/CTS for
clock rates that aren't in the table.  This code is a bit slow because
it iterates over every possible value of N and picks the best one, but
it should make a good fallback.

4. The new code allows for platforms that know they make a clock rate
slightly differently to pick different N/CTS values.  For instance on
rk3288 we can make 25,176,471 Hz instead of 25,174,825.1748... Hz
(25.2 MHz / 1.001).  A future patch to the rk3288 platform code could
enable support for this clock rate and specify the N/CTS that would be
ideal.

NOTE: the oddest part of this patch comes about because computing the
ideal N/CTS means knowing the _exact_ clock rate, not a rounded
version of it.  The drm framework makes this harder by rounding rates
to kHz, but even if it didn't there might be cases where the ideal
rate could only be calculated if we knew the real (non-integral) rate.
This means that in cases where we know (or believe) that the true rate
is something other than the rate we are told by drm.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 208 +++++++++++++++++-----
 include/drm/bridge/dw_hdmi.h              |   8 +
 2 files changed, 175 insertions(+), 41 deletions(-)

Comments

Jernej Škrabec June 18, 2019, 5:23 p.m. UTC | #1
Hi!

Dne torek, 18. junij 2019 ob 01:55:58 CEST je Douglas Anderson napisal(a):
> Let's add some better support for HDMI audio to dw_hdmi.
> Specifically:
> 
> 1. For 44.1 kHz audio the old code made the assumption that an N of
> 6272 was right most of the time.  That wasn't true and the new table
> should give better 44.1 kHz audio for many more rates.
> 
> 2. The new table has values from the HDMI spec for 297 MHz and 594
> MHz.
> 
> 3. There is now code to try to come up with a more idea N/CTS for
> clock rates that aren't in the table.  This code is a bit slow because
> it iterates over every possible value of N and picks the best one, but
> it should make a good fallback.
> 
> 4. The new code allows for platforms that know they make a clock rate
> slightly differently to pick different N/CTS values.  For instance on
> rk3288 we can make 25,176,471 Hz instead of 25,174,825.1748... Hz
> (25.2 MHz / 1.001).  A future patch to the rk3288 platform code could
> enable support for this clock rate and specify the N/CTS that would be
> ideal.
> 
> NOTE: the oddest part of this patch comes about because computing the
> ideal N/CTS means knowing the _exact_ clock rate, not a rounded
> version of it.  The drm framework makes this harder by rounding rates
> to kHz, but even if it didn't there might be cases where the ideal
> rate could only be calculated if we knew the real (non-integral) rate.
> This means that in cases where we know (or believe) that the true rate
> is something other than the rate we are told by drm.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Which bus is used for audio transfer on your device? If it is I2S, which is 
commonly used, then please be aware of this patch:
https://lists.freedesktop.org/archives/dri-devel/2019-June/221539.html

It avoids exact N/CTS calculation by enabling auto detection. It is well 
tested on multiple SoCs from Allwinner, Amlogic and Rockchip.

Best regards,
Jernej
Neil Armstrong June 19, 2019, 3:22 p.m. UTC | #2
On 18/06/2019 19:23, Jernej Škrabec wrote:
> Hi!
> 
> Dne torek, 18. junij 2019 ob 01:55:58 CEST je Douglas Anderson napisal(a):
>> Let's add some better support for HDMI audio to dw_hdmi.
>> Specifically:
>>
>> 1. For 44.1 kHz audio the old code made the assumption that an N of
>> 6272 was right most of the time.  That wasn't true and the new table
>> should give better 44.1 kHz audio for many more rates.
>>
>> 2. The new table has values from the HDMI spec for 297 MHz and 594
>> MHz.
>>
>> 3. There is now code to try to come up with a more idea N/CTS for
>> clock rates that aren't in the table.  This code is a bit slow because
>> it iterates over every possible value of N and picks the best one, but
>> it should make a good fallback.
>>
>> 4. The new code allows for platforms that know they make a clock rate
>> slightly differently to pick different N/CTS values.  For instance on
>> rk3288 we can make 25,176,471 Hz instead of 25,174,825.1748... Hz
>> (25.2 MHz / 1.001).  A future patch to the rk3288 platform code could
>> enable support for this clock rate and specify the N/CTS that would be
>> ideal.
>>
>> NOTE: the oddest part of this patch comes about because computing the
>> ideal N/CTS means knowing the _exact_ clock rate, not a rounded
>> version of it.  The drm framework makes this harder by rounding rates
>> to kHz, but even if it didn't there might be cases where the ideal
>> rate could only be calculated if we knew the real (non-integral) rate.
>> This means that in cases where we know (or believe) that the true rate
>> is something other than the rate we are told by drm.
>>
>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> 
> Which bus is used for audio transfer on your device? If it is I2S, which is 
> commonly used, then please be aware of this patch:
> https://lists.freedesktop.org/archives/dri-devel/2019-June/221539.html
> 
> It avoids exact N/CTS calculation by enabling auto detection. It is well 
> tested on multiple SoCs from Allwinner, Amlogic and Rockchip.
> 
> Best regards,
> Jernej
> 
> 
Hi Douglas,

Thanks for your work !

If you could rebase on top of https://lists.freedesktop.org/archives/dri-devel/2019-June/221539.html
so we can make use of your extended N table with automatic CTS HW calculation, it would be great !

Finally could you add the plat_data tmds table as a separate patch to simplify review ?

Thanks,
Neil
Douglas Anderson June 19, 2019, 9:11 p.m. UTC | #3
Hi,

On Wed, Jun 19, 2019 at 8:23 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 18/06/2019 19:23, Jernej Škrabec wrote:
> > Hi!
> >
> > Dne torek, 18. junij 2019 ob 01:55:58 CEST je Douglas Anderson napisal(a):
> >> Let's add some better support for HDMI audio to dw_hdmi.
> >> Specifically:
> >>
> >> 1. For 44.1 kHz audio the old code made the assumption that an N of
> >> 6272 was right most of the time.  That wasn't true and the new table
> >> should give better 44.1 kHz audio for many more rates.
> >>
> >> 2. The new table has values from the HDMI spec for 297 MHz and 594
> >> MHz.
> >>
> >> 3. There is now code to try to come up with a more idea N/CTS for
> >> clock rates that aren't in the table.  This code is a bit slow because
> >> it iterates over every possible value of N and picks the best one, but
> >> it should make a good fallback.
> >>
> >> 4. The new code allows for platforms that know they make a clock rate
> >> slightly differently to pick different N/CTS values.  For instance on
> >> rk3288 we can make 25,176,471 Hz instead of 25,174,825.1748... Hz
> >> (25.2 MHz / 1.001).  A future patch to the rk3288 platform code could
> >> enable support for this clock rate and specify the N/CTS that would be
> >> ideal.
> >>
> >> NOTE: the oddest part of this patch comes about because computing the
> >> ideal N/CTS means knowing the _exact_ clock rate, not a rounded
> >> version of it.  The drm framework makes this harder by rounding rates
> >> to kHz, but even if it didn't there might be cases where the ideal
> >> rate could only be calculated if we knew the real (non-integral) rate.
> >> This means that in cases where we know (or believe) that the true rate
> >> is something other than the rate we are told by drm.
> >>
> >> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >
> > Which bus is used for audio transfer on your device? If it is I2S, which is
> > commonly used, then please be aware of this patch:
> > https://lists.freedesktop.org/archives/dri-devel/2019-June/221539.html
> >
> > It avoids exact N/CTS calculation by enabling auto detection. It is well
> > tested on multiple SoCs from Allwinner, Amlogic and Rockchip.
> >
> > Best regards,
> > Jernej
> >
> >
> Hi Douglas,
>
> Thanks for your work !
>
> If you could rebase on top of https://lists.freedesktop.org/archives/dri-devel/2019-June/221539.html
> so we can make use of your extended N table with automatic CTS HW calculation, it would be great !

Thanks to you and Jernej for pointing me at this.  It seems likely
that patch by itself would solve problems we found and I'll pick that
into my tree.

Probably my patch is no longer quite as useful atop yours, but I'll
still post a v2 since (in theory) folks that aren't using I2S might
find it useful.  I guess it's also possible (?) that picking an N
where CTS would be able to be integral has some type of advantage,
even with auto CTS?


> Finally could you add the plat_data tmds table as a separate patch to simplify review ?

Sure.  I'm probably not going to be able to post the patch to actually
use it, so I guess we could just not bother applying the 2nd patch
unless someone ever needs it.

-Doug
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index c6490949d9db..edede21e85e4 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -60,6 +60,75 @@  enum hdmi_datamap {
 	YCbCr422_12B = 0x12,
 };
 
+/*
+ * Unless otherwise noted, entries in this table are 100% optimization.
+ * Values can be obtained from hdmi_compute_n() but that function is
+ * slow so we pre-compute values we expect to see.
+ *
+ * All 32k and 48k values are expected to be the same (due to the way
+ * the math works) for any rate that's an exact kHz.
+ *
+ * If a particular platform knows that it makes a rate slightly
+ * differently then it should add a platform-specific match.
+ */
+static const struct dw_hdmi_audio_tmds_n common_tmds_n_table[] = {
+	/* Doesn't match computations, assumes real clock = 25.2 MHz / 1.001 */
+	{ .tmds = 25175000, .n_32k = 4576, .n_44k1 = 7007, .n_48k = 6864, },
+
+	{ .tmds = 25200000, .n_32k = 4096, .n_44k1 = 5656, .n_48k = 6144, },
+	{ .tmds = 27000000, .n_32k = 4096, .n_44k1 = 5488, .n_48k = 6144, },
+	{ .tmds = 27027000, .n_32k = 4096, .n_44k1 = 6272, .n_48k = 6144, },
+	{ .tmds = 28320000, .n_32k = 4096, .n_44k1 = 5586, .n_48k = 6144, },
+	{ .tmds = 30240000, .n_32k = 4096, .n_44k1 = 5642, .n_48k = 6144, },
+	{ .tmds = 31500000, .n_32k = 4096, .n_44k1 = 5600, .n_48k = 6144, },
+	{ .tmds = 32000000, .n_32k = 4096, .n_44k1 = 5733, .n_48k = 6144, },
+	{ .tmds = 33750000, .n_32k = 4096, .n_44k1 = 6272, .n_48k = 6144, },
+	{ .tmds = 36000000, .n_32k = 4096, .n_44k1 = 5684, .n_48k = 6144, },
+	{ .tmds = 40000000, .n_32k = 4096, .n_44k1 = 5733, .n_48k = 6144, },
+	{ .tmds = 49500000, .n_32k = 4096, .n_44k1 = 5488, .n_48k = 6144, },
+	{ .tmds = 50000000, .n_32k = 4096, .n_44k1 = 5292, .n_48k = 6144, },
+	{ .tmds = 54000000, .n_32k = 4096, .n_44k1 = 5684, .n_48k = 6144, },
+	{ .tmds = 65000000, .n_32k = 4096, .n_44k1 = 7056, .n_48k = 6144, },
+	{ .tmds = 68250000, .n_32k = 4096, .n_44k1 = 5376, .n_48k = 6144, },
+	{ .tmds = 71000000, .n_32k = 4096, .n_44k1 = 7056, .n_48k = 6144, },
+	{ .tmds = 72000000, .n_32k = 4096, .n_44k1 = 5635, .n_48k = 6144, },
+	{ .tmds = 73250000, .n_32k = 4096, .n_44k1 = 14112, .n_48k = 6144, },
+
+	/* Doesn't match computations, assumes real clock = 74.25 MHz / 1.001 */
+	{ .tmds = 74176000, .n_32k = 11648, .n_44k1 = 17836, .n_48k = 11648, },
+
+	{ .tmds = 74250000, .n_32k = 4096, .n_44k1 = 6272, .n_48k = 6144, },
+	{ .tmds = 75000000, .n_32k = 4096, .n_44k1 = 5880, .n_48k = 6144, },
+	{ .tmds = 78750000, .n_32k = 4096, .n_44k1 = 5600, .n_48k = 6144, },
+	{ .tmds = 78800000, .n_32k = 4096, .n_44k1 = 5292, .n_48k = 6144, },
+	{ .tmds = 79500000, .n_32k = 4096, .n_44k1 = 4704, .n_48k = 6144, },
+	{ .tmds = 83500000, .n_32k = 4096, .n_44k1 = 7056, .n_48k = 6144, },
+	{ .tmds = 85500000, .n_32k = 4096, .n_44k1 = 5488, .n_48k = 6144, },
+	{ .tmds = 88750000, .n_32k = 4096, .n_44k1 = 14112, .n_48k = 6144, },
+	{ .tmds = 97750000, .n_32k = 4096, .n_44k1 = 14112, .n_48k = 6144, },
+	{ .tmds = 101000000, .n_32k = 4096, .n_44k1 = 7056, .n_48k = 6144, },
+	{ .tmds = 106500000, .n_32k = 4096, .n_44k1 = 4704, .n_48k = 6144, },
+	{ .tmds = 108000000, .n_32k = 4096, .n_44k1 = 5684, .n_48k = 6144, },
+	{ .tmds = 115500000, .n_32k = 4096, .n_44k1 = 5712, .n_48k = 6144, },
+	{ .tmds = 119000000, .n_32k = 4096, .n_44k1 = 5544, .n_48k = 6144, },
+	{ .tmds = 135000000, .n_32k = 4096, .n_44k1 = 5488, .n_48k = 6144, },
+	{ .tmds = 146250000, .n_32k = 4096, .n_44k1 = 6272, .n_48k = 6144, },
+
+	/* Doesn't match computations, assumes real clock = 148.5 MHz / 1.001 */
+	{ .tmds = 148352000, .n_32k = 11648, .n_44k1 = 8918, .n_48k = 5824, },
+
+	{ .tmds = 148500000, .n_32k = 4096, .n_44k1 = 5488, .n_48k = 6144, },
+	{ .tmds = 154000000, .n_32k = 4096, .n_44k1 = 5544, .n_48k = 6144, },
+	{ .tmds = 162000000, .n_32k = 4096, .n_44k1 = 5684, .n_48k = 6144, },
+
+	/* For 297 MHz+ HDMI spec has some other rule for setting N */
+	{ .tmds = 297000000, .n_32k = 3073, .n_44k1 = 4704, .n_48k = 5120, },
+	{ .tmds = 594000000, .n_32k = 3073, .n_44k1 = 9408, .n_48k = 10240, },
+
+	/* End of table */
+	{ .tmds = 0,         .n_32k = 0,    .n_44k1 = 0,    .n_48k = 0, },
+};
+
 static const u16 csc_coeff_default[3][4] = {
 	{ 0x2000, 0x0000, 0x0000, 0x0000 },
 	{ 0x0000, 0x2000, 0x0000, 0x0000 },
@@ -518,60 +587,117 @@  static void hdmi_set_cts_n(struct dw_hdmi *hdmi, unsigned int cts,
 	hdmi_writeb(hdmi, n & 0xff, HDMI_AUD_N1);
 }
 
-static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
+static int hdmi_match_tmds_n_table(struct dw_hdmi *hdmi, unsigned int freq,
+				   unsigned long pixel_clk)
 {
-	unsigned int n = (128 * freq) / 1000;
-	unsigned int mult = 1;
+	const struct dw_hdmi_plat_data *plat_data = hdmi->plat_data;
+	const struct dw_hdmi_audio_tmds_n *tmds_n = NULL;
+	int mult = 1;
+	int i;
 
 	while (freq > 48000) {
 		mult *= 2;
 		freq /= 2;
 	}
 
+	if (plat_data->tmds_n_table) {
+		for (i = 0; plat_data->tmds_n_table[i].tmds != 0; i++) {
+			if (pixel_clk == plat_data->tmds_n_table[i].tmds) {
+				tmds_n = &plat_data->tmds_n_table[i];
+				break;
+			}
+		}
+	}
+
+	if (tmds_n == NULL) {
+		for (i = 0; common_tmds_n_table[i].tmds != 0; i++) {
+			if (pixel_clk == common_tmds_n_table[i].tmds) {
+				tmds_n = &common_tmds_n_table[i];
+				break;
+			}
+		}
+	}
+
+	if (tmds_n == NULL)
+		return -ENOENT;
+
 	switch (freq) {
 	case 32000:
-		if (pixel_clk == 25175000)
-			n = 4576;
-		else if (pixel_clk == 27027000)
-			n = 4096;
-		else if (pixel_clk == 74176000 || pixel_clk == 148352000)
-			n = 11648;
-		else
-			n = 4096;
-		n *= mult;
-		break;
-
+		return tmds_n->n_32k * mult;
 	case 44100:
-		if (pixel_clk == 25175000)
-			n = 7007;
-		else if (pixel_clk == 74176000)
-			n = 17836;
-		else if (pixel_clk == 148352000)
-			n = 8918;
-		else
-			n = 6272;
-		n *= mult;
-		break;
-
+		return tmds_n->n_44k1 * mult;
 	case 48000:
-		if (pixel_clk == 25175000)
-			n = 6864;
-		else if (pixel_clk == 27027000)
-			n = 6144;
-		else if (pixel_clk == 74176000)
-			n = 11648;
-		else if (pixel_clk == 148352000)
-			n = 5824;
-		else
-			n = 6144;
-		n *= mult;
-		break;
-
+		return tmds_n->n_48k * mult;
 	default:
-		break;
+		return -ENOENT;
 	}
+}
+
+static u64 hdmi_audio_math_diff(unsigned int freq, unsigned int n,
+				unsigned int pixel_clk)
+{
+	u64 final, diff;
+	u64 cts;
+
+	final = (u64)pixel_clk * n;
+
+	cts = final;
+	do_div(cts, 128 * freq);
+
+	diff = final - (u64)cts * (128 * freq);
+
+	return diff;
+}
+
+static unsigned int hdmi_compute_n(struct dw_hdmi *hdmi, unsigned int freq,
+				   unsigned long pixel_clk)
+{
+	unsigned int min_n = DIV_ROUND_UP((128 * freq), 1500);
+	unsigned int max_n = (128 * freq) / 300;
+	unsigned int ideal_n = (128 * freq) / 1000;
+	unsigned int best_n_distance = ideal_n;
+	unsigned int best_n = 0;
+	u64 best_diff = U64_MAX;
+	int n;
+
+	/* If the ideal N could satisfy the audio math, then just take it */
+	if (hdmi_audio_math_diff(freq, ideal_n, pixel_clk) == 0)
+		return ideal_n;
+
+	for (n = min_n; n <= max_n; n++) {
+		u64 diff = hdmi_audio_math_diff(freq, n, pixel_clk);
+
+		if (diff < best_diff || (diff == best_diff &&
+		    abs(n - ideal_n) < best_n_distance)) {
+			best_n = n;
+			best_diff = diff;
+			best_n_distance = abs(best_n - ideal_n);
+		}
+
+		/*
+		 * The best N already satisfy the audio math, and also be
+		 * the closest value to ideal N, so just cut the loop.
+		 */
+		if ((best_diff == 0) && (abs(n - ideal_n) > best_n_distance))
+			break;
+	}
+
+	return best_n;
+}
+
+static unsigned int hdmi_find_n(struct dw_hdmi *hdmi, unsigned int freq,
+				unsigned long pixel_clk)
+{
+	int n;
+
+	n = hdmi_match_tmds_n_table(hdmi, freq, pixel_clk);
+	if (n > 0)
+		return n;
+
+	dev_warn(hdmi->dev, "Rate %lu missing; compute N dynamically\n",
+		 pixel_clk);
 
-	return n;
+	return hdmi_compute_n(hdmi, freq, pixel_clk);
 }
 
 static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
@@ -581,7 +707,7 @@  static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
 	unsigned int n, cts;
 	u64 tmp;
 
-	n = hdmi_compute_n(sample_rate, pixel_clk);
+	n = hdmi_find_n(hdmi, sample_rate, pixel_clk);
 
 	/*
 	 * Compute the CTS value from the N value.  Note that CTS and N
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index c402364aec0d..5ee6b0a127aa 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -90,6 +90,13 @@  enum dw_hdmi_phy_type {
 	DW_HDMI_PHY_VENDOR_PHY = 0xfe,
 };
 
+struct dw_hdmi_audio_tmds_n {
+	unsigned long tmds;
+	unsigned int n_32k;
+	unsigned int n_44k1;
+	unsigned int n_48k;
+};
+
 struct dw_hdmi_mpll_config {
 	unsigned long mpixelclock;
 	struct {
@@ -137,6 +144,7 @@  struct dw_hdmi_plat_data {
 	const struct dw_hdmi_mpll_config *mpll_cfg;
 	const struct dw_hdmi_curr_ctrl *cur_ctr;
 	const struct dw_hdmi_phy_config *phy_config;
+	const struct dw_hdmi_audio_tmds_n *tmds_n_table;
 	int (*configure_phy)(struct dw_hdmi *hdmi,
 			     const struct dw_hdmi_plat_data *pdata,
 			     unsigned long mpixelclock);