[PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Detlev Casanova]

To support HDMI audio on the rk3588 based devices, the generic HDMI
Codec framework is used in the dw-hdmi-qp DRM bridge driver.

The implementation is mainly based on the downstream driver, ported to the
generic HDMI Codec framework [1] recently merged in the master branch.
The parameters computation has been kept as is and the data stored in the
dw_hdmi_qp struct as been cleaned up.

The table for the N values has been edited to reflect N recommended values
as well as CTS recommended values.

The downstream kernel also implements a machine driver for HDMI audio but
it is doing exactly what the simple-audio-card driver does, so use that
instead in the RK3588 SoC device tree.

This adds HDMI audio support for both HDMI TX ports.

*** Dependencies ***

Based on Linus' master branch, but also needs Cristian's dts patches for HDMI1
support [2], which depends on Heiko's patchset for
phy-rockchip-samsung-hdptx [3]. Patches will apply without [3], but HDMI will
not work (at all).

[1]: https://lore.kernel.org/all/20241224-drm-bridge-hdmi-connector-v10-0-dc89577cd438@linaro.org
[2]: https://lore.kernel.org/linux-rockchip/20241211-rk3588-hdmi1-v2-0-02cdca22ff68@collabora.com
[3]: https://lore.kernel.org/lkml/20241206103401.1780416-3-heiko@sntech.de/

Changes since v6:
 - Fix arguments alignement (checkpatch --strict warnings)
 - Add hdmi1 audio support too
 - Move hdmi0_sound node under hdmi0_out_con

Changes since v5:
 - Simplify audio math computation for N
 - Move hdmi0-sound node up with other address-less nodes

Changes since v4:
 - Moved hdmi0_audio node the rk3588-base.dtsi
 - Enable hdmi0_audio in rk3588-rock-5b.dts

Changes since v3:
 - Renamed function to start with dw_hdmi_qp

Changes since v2:
 - Also clear the audio infoframe
 - Write AUDI_CONTENTS0 to its default value in case it gets overwritten.
 - Store tmds_char_rate in the dw_hdmi_qp struct in atomic_enable
 - Clear tmds_char_rate in atomic_disable and only write registers when
   tmds_char_rate is not 0.
 - Do not use connector_state duplicates

Changes since v1:
 - Remove useless audio_mutex (was used downstream for multiple drivers access
   to audio functions)
 - Let hdmi_codec build and setup audio infoframes
 - Only access audio registers when connector is connected
 - Rebased on master branch

Detlev Casanova (2):
  arm64: dts: rockchip: Add HDMI audio outputs for rk3588 SoC
  arm64: dts: rockchip: Enable HDMI audio outputs for Rock 5B

Sugar Zhang (1):
  drm/bridge: synopsys: Add audio support for dw-hdmi-qp

 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  17 +
 .../arm64/boot/dts/rockchip/rk3588-extra.dtsi |  17 +
 .../boot/dts/rockchip/rk3588-rock-5b.dts      |  16 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c  | 489 ++++++++++++++++++
 4 files changed, 539 insertions(+)

-- 
2.48.1

[PATCH v7 1/3] drm/bridge: synopsys: Add audio support for dw-hdmi-qp

[Detlev Casanova]

From: Sugar Zhang <sugar.zhang@rock-chips.com>

Register the dw-hdmi-qp bridge driver as an HDMI audio codec.

The register values computation functions (for n) are based on the
downstream driver, as well as the register writing functions.

The driver uses the generic HDMI Codec framework in order to implement
the HDMI audio support.

Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Tested-by: Quentin Schulz <quentin.schulz@cherry.de>
Reviewed-by: Robert Foss <rfoss@kernel.org>
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 489 +++++++++++++++++++
 1 file changed, 489 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index b281cabfe992e..7bbe39546163d 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -36,6 +36,88 @@
 
 #define SCRAMB_POLL_DELAY_MS	3000
 
+/*
+ * Unless otherwise noted, entries in this table are 100% optimization.
+ * Values can be obtained from dw_hdmi_qp_compute_n() but that function is
+ * slow so we pre-compute values we expect to see.
+ *
+ * The values for TMDS 25175, 25200, 27000, 54000, 74250 and 148500 kHz are
+ * the recommended N values specified in the Audio chapter of the HDMI
+ * specification.
+ */
+static const struct dw_hdmi_audio_tmds_n {
+	unsigned long tmds;
+	unsigned int n_32k;
+	unsigned int n_44k1;
+	unsigned int n_48k;
+} common_tmds_n_table[] = {
+	{ .tmds = 25175000,  .n_32k = 4576,  .n_44k1 = 7007,  .n_48k = 6864, },
+	{ .tmds = 25200000,  .n_32k = 4096,  .n_44k1 = 6272,  .n_48k = 6144, },
+	{ .tmds = 27000000,  .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 = 6272,  .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 = 11648, .n_44k1 = 14112, .n_48k = 6144, },
+	{ .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 = 11648, .n_44k1 = 6272,  .n_48k = 6144, },
+	{ .tmds = 148500000, .n_32k = 4096,  .n_44k1 = 6272,  .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 have 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,    },
+};
+
+/*
+ * These are the CTS values as recommended in the Audio chapter of the HDMI
+ * specification.
+ */
+static const struct dw_hdmi_audio_tmds_cts {
+	unsigned long tmds;
+	unsigned int cts_32k;
+	unsigned int cts_44k1;
+	unsigned int cts_48k;
+} common_tmds_cts_table[] = {
+	{ .tmds = 25175000,  .cts_32k = 28125,  .cts_44k1 = 31250,  .cts_48k = 28125,  },
+	{ .tmds = 25200000,  .cts_32k = 25200,  .cts_44k1 = 28000,  .cts_48k = 25200,  },
+	{ .tmds = 27000000,  .cts_32k = 27000,  .cts_44k1 = 30000,  .cts_48k = 27000,  },
+	{ .tmds = 54000000,  .cts_32k = 54000,  .cts_44k1 = 60000,  .cts_48k = 54000,  },
+	{ .tmds = 74250000,  .cts_32k = 74250,  .cts_44k1 = 82500,  .cts_48k = 74250,  },
+	{ .tmds = 148500000, .cts_32k = 148500, .cts_44k1 = 165000, .cts_48k = 148500, },
+
+	/* End of table */
+	{ .tmds = 0,         .cts_32k = 0,      .cts_44k1 = 0,      .cts_48k = 0,      },
+};
+
 struct dw_hdmi_qp_i2c {
 	struct i2c_adapter	adap;
 
@@ -60,6 +142,8 @@ struct dw_hdmi_qp {
 	} phy;
 
 	struct regmap *regm;
+
+	unsigned long tmds_char_rate;
 };
 
 static void dw_hdmi_qp_write(struct dw_hdmi_qp *hdmi, unsigned int val,
@@ -83,6 +167,346 @@ static void dw_hdmi_qp_mod(struct dw_hdmi_qp *hdmi, unsigned int data,
 	regmap_update_bits(hdmi->regm, reg, mask, data);
 }
 
+static struct dw_hdmi_qp *dw_hdmi_qp_from_bridge(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct dw_hdmi_qp, bridge);
+}
+
+static void dw_hdmi_qp_set_cts_n(struct dw_hdmi_qp *hdmi, unsigned int cts,
+				 unsigned int n)
+{
+	/* Set N */
+	dw_hdmi_qp_mod(hdmi, n, AUDPKT_ACR_N_VALUE, AUDPKT_ACR_CONTROL0);
+
+	/* Set CTS */
+	if (cts)
+		dw_hdmi_qp_mod(hdmi, AUDPKT_ACR_CTS_OVR_EN, AUDPKT_ACR_CTS_OVR_EN_MSK,
+			       AUDPKT_ACR_CONTROL1);
+	else
+		dw_hdmi_qp_mod(hdmi, 0, AUDPKT_ACR_CTS_OVR_EN_MSK,
+			       AUDPKT_ACR_CONTROL1);
+
+	dw_hdmi_qp_mod(hdmi, AUDPKT_ACR_CTS_OVR_VAL(cts), AUDPKT_ACR_CTS_OVR_VAL_MSK,
+		       AUDPKT_ACR_CONTROL1);
+}
+
+static int dw_hdmi_qp_match_tmds_n_table(struct dw_hdmi_qp *hdmi,
+					 unsigned long pixel_clk,
+					 unsigned long freq)
+{
+	const struct dw_hdmi_audio_tmds_n *tmds_n = NULL;
+	int i;
+
+	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)
+		return -ENOENT;
+
+	switch (freq) {
+	case 32000:
+		return tmds_n->n_32k;
+	case 44100:
+	case 88200:
+	case 176400:
+		return (freq / 44100) * tmds_n->n_44k1;
+	case 48000:
+	case 96000:
+	case 192000:
+		return (freq / 48000) * tmds_n->n_48k;
+	default:
+		return -ENOENT;
+	}
+}
+
+static u32 dw_hdmi_qp_audio_math_diff(unsigned int freq, unsigned int n,
+				      unsigned int pixel_clk)
+{
+	u64 cts = mul_u32_u32(pixel_clk, n);
+
+	return do_div(cts, 128 * freq);
+}
+
+static unsigned int dw_hdmi_qp_compute_n(struct dw_hdmi_qp *hdmi,
+					 unsigned long pixel_clk,
+					 unsigned long freq)
+{
+	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 (dw_hdmi_qp_audio_math_diff(freq, ideal_n, pixel_clk) == 0)
+		return ideal_n;
+
+	for (n = min_n; n <= max_n; n++) {
+		u64 diff = dw_hdmi_qp_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 dw_hdmi_qp_find_n(struct dw_hdmi_qp *hdmi, unsigned long pixel_clk,
+				      unsigned long sample_rate)
+{
+	int n = dw_hdmi_qp_match_tmds_n_table(hdmi, pixel_clk, sample_rate);
+
+	if (n > 0)
+		return n;
+
+	dev_warn(hdmi->dev, "Rate %lu missing; compute N dynamically\n",
+		 pixel_clk);
+
+	return dw_hdmi_qp_compute_n(hdmi, pixel_clk, sample_rate);
+}
+
+static unsigned int dw_hdmi_qp_find_cts(struct dw_hdmi_qp *hdmi, unsigned long pixel_clk,
+					unsigned long sample_rate)
+{
+	const struct dw_hdmi_audio_tmds_cts *tmds_cts = NULL;
+	int i;
+
+	for (i = 0; common_tmds_cts_table[i].tmds != 0; i++) {
+		if (pixel_clk == common_tmds_cts_table[i].tmds) {
+			tmds_cts = &common_tmds_cts_table[i];
+			break;
+		}
+	}
+
+	if (!tmds_cts)
+		return 0;
+
+	switch (sample_rate) {
+	case 32000:
+		return tmds_cts->cts_32k;
+	case 44100:
+	case 88200:
+	case 176400:
+		return tmds_cts->cts_44k1;
+	case 48000:
+	case 96000:
+	case 192000:
+		return tmds_cts->cts_48k;
+	default:
+		return -ENOENT;
+	}
+}
+
+static void dw_hdmi_qp_set_audio_interface(struct dw_hdmi_qp *hdmi,
+					   struct hdmi_codec_daifmt *fmt,
+					   struct hdmi_codec_params *hparms)
+{
+	u32 conf0 = 0;
+
+	/* Reset the audio data path of the AVP */
+	dw_hdmi_qp_write(hdmi, AVP_DATAPATH_PACKET_AUDIO_SWINIT_P, GLOBAL_SWRESET_REQUEST);
+
+	/* Disable AUDS, ACR, AUDI */
+	dw_hdmi_qp_mod(hdmi, 0,
+		       PKTSCHED_ACR_TX_EN | PKTSCHED_AUDS_TX_EN | PKTSCHED_AUDI_TX_EN,
+		       PKTSCHED_PKT_EN);
+
+	/* Clear the audio FIFO */
+	dw_hdmi_qp_write(hdmi, AUDIO_FIFO_CLR_P, AUDIO_INTERFACE_CONTROL0);
+
+	/* Select I2S interface as the audio source */
+	dw_hdmi_qp_mod(hdmi, AUD_IF_I2S, AUD_IF_SEL_MSK, AUDIO_INTERFACE_CONFIG0);
+
+	/* Enable the active i2s lanes */
+	switch (hparms->channels) {
+	case 7 ... 8:
+		conf0 |= I2S_LINES_EN(3);
+		fallthrough;
+	case 5 ... 6:
+		conf0 |= I2S_LINES_EN(2);
+		fallthrough;
+	case 3 ... 4:
+		conf0 |= I2S_LINES_EN(1);
+		fallthrough;
+	default:
+		conf0 |= I2S_LINES_EN(0);
+		break;
+	}
+
+	dw_hdmi_qp_mod(hdmi, conf0, I2S_LINES_EN_MSK, AUDIO_INTERFACE_CONFIG0);
+
+	/*
+	 * Enable bpcuv generated internally for L-PCM, or received
+	 * from stream for NLPCM/HBR.
+	 */
+	switch (fmt->bit_fmt) {
+	case SNDRV_PCM_FORMAT_IEC958_SUBFRAME_LE:
+		conf0 = (hparms->channels == 8) ? AUD_HBR : AUD_ASP;
+		conf0 |= I2S_BPCUV_RCV_EN;
+		break;
+	default:
+		conf0 = AUD_ASP | I2S_BPCUV_RCV_DIS;
+		break;
+	}
+
+	dw_hdmi_qp_mod(hdmi, conf0, I2S_BPCUV_RCV_MSK | AUD_FORMAT_MSK,
+		       AUDIO_INTERFACE_CONFIG0);
+
+	/* Enable audio FIFO auto clear when overflow */
+	dw_hdmi_qp_mod(hdmi, AUD_FIFO_INIT_ON_OVF_EN, AUD_FIFO_INIT_ON_OVF_MSK,
+		       AUDIO_INTERFACE_CONFIG0);
+}
+
+/*
+ * When transmitting IEC60958 linear PCM audio, these registers allow to
+ * configure the channel status information of all the channel status
+ * bits in the IEC60958 frame. For the moment this configuration is only
+ * used when the I2S audio interface, General Purpose Audio (GPA),
+ * or AHB audio DMA (AHBAUDDMA) interface is active
+ * (for S/PDIF interface this information comes from the stream).
+ */
+static void dw_hdmi_qp_set_channel_status(struct dw_hdmi_qp *hdmi,
+					  u8 *channel_status, bool ref2stream)
+{
+	/*
+	 * AUDPKT_CHSTATUS_OVR0: { RSV, RSV, CS1, CS0 }
+	 * AUDPKT_CHSTATUS_OVR1: { CS6, CS5, CS4, CS3 }
+	 *
+	 *      |  7  |  6  |  5  |  4  |  3  |  2  |  1  |  0  |
+	 * CS0: |   Mode    |        d        |  c  |  b  |  a  |
+	 * CS1: |               Category Code                   |
+	 * CS2: |    Channel Number     |     Source Number     |
+	 * CS3: |    Clock Accuracy     |     Sample Freq       |
+	 * CS4: |    Ori Sample Freq    |     Word Length       |
+	 * CS5: |                                   |   CGMS-A  |
+	 * CS6~CS23: Reserved
+	 *
+	 * a: use of channel status block
+	 * b: linear PCM identification: 0 for lpcm, 1 for nlpcm
+	 * c: copyright information
+	 * d: additional format information
+	 */
+
+	if (ref2stream)
+		channel_status[0] |= IEC958_AES0_NONAUDIO;
+
+	if ((dw_hdmi_qp_read(hdmi, AUDIO_INTERFACE_CONFIG0) & GENMASK(25, 24)) == AUD_HBR) {
+		/* fixup cs for HBR */
+		channel_status[3] = (channel_status[3] & 0xf0) | IEC958_AES3_CON_FS_768000;
+		channel_status[4] = (channel_status[4] & 0x0f) | IEC958_AES4_CON_ORIGFS_NOTID;
+	}
+
+	dw_hdmi_qp_write(hdmi, channel_status[0] | (channel_status[1] << 8),
+			 AUDPKT_CHSTATUS_OVR0);
+
+	regmap_bulk_write(hdmi->regm, AUDPKT_CHSTATUS_OVR1, &channel_status[3], 1);
+
+	if (ref2stream)
+		dw_hdmi_qp_mod(hdmi, 0,
+			       AUDPKT_PBIT_FORCE_EN_MASK | AUDPKT_CHSTATUS_OVR_EN_MASK,
+			       AUDPKT_CONTROL0);
+	else
+		dw_hdmi_qp_mod(hdmi, AUDPKT_PBIT_FORCE_EN | AUDPKT_CHSTATUS_OVR_EN,
+			       AUDPKT_PBIT_FORCE_EN_MASK | AUDPKT_CHSTATUS_OVR_EN_MASK,
+			       AUDPKT_CONTROL0);
+}
+
+static void dw_hdmi_qp_set_sample_rate(struct dw_hdmi_qp *hdmi, unsigned long long tmds_char_rate,
+				       unsigned int sample_rate)
+{
+	unsigned int n, cts;
+
+	n = dw_hdmi_qp_find_n(hdmi, tmds_char_rate, sample_rate);
+	cts = dw_hdmi_qp_find_cts(hdmi, tmds_char_rate, sample_rate);
+
+	dw_hdmi_qp_set_cts_n(hdmi, cts, n);
+}
+
+static int dw_hdmi_qp_audio_enable(struct drm_connector *connector,
+				   struct drm_bridge *bridge)
+{
+	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
+
+	if (hdmi->tmds_char_rate)
+		dw_hdmi_qp_mod(hdmi, 0, AVP_DATAPATH_PACKET_AUDIO_SWDISABLE, GLOBAL_SWDISABLE);
+
+	return 0;
+}
+
+static int dw_hdmi_qp_audio_prepare(struct drm_connector *connector,
+				    struct drm_bridge *bridge,
+				    struct hdmi_codec_daifmt *fmt,
+				    struct hdmi_codec_params *hparms)
+{
+	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
+	bool ref2stream = false;
+
+	if (!hdmi->tmds_char_rate)
+		return -ENODEV;
+
+	if (fmt->bit_clk_provider | fmt->frame_clk_provider) {
+		dev_err(hdmi->dev, "unsupported clock settings\n");
+		return -EINVAL;
+	}
+
+	if (fmt->bit_fmt == SNDRV_PCM_FORMAT_IEC958_SUBFRAME_LE)
+		ref2stream = true;
+
+	dw_hdmi_qp_set_audio_interface(hdmi, fmt, hparms);
+	dw_hdmi_qp_set_sample_rate(hdmi, hdmi->tmds_char_rate, hparms->sample_rate);
+	dw_hdmi_qp_set_channel_status(hdmi, hparms->iec.status, ref2stream);
+	drm_atomic_helper_connector_hdmi_update_audio_infoframe(connector, &hparms->cea);
+
+	return 0;
+}
+
+static void dw_hdmi_qp_audio_disable_regs(struct dw_hdmi_qp *hdmi)
+{
+	/*
+	 * Keep ACR, AUDI, AUDS packet always on to make SINK device
+	 * active for better compatibility and user experience.
+	 *
+	 * This also fix POP sound on some SINK devices which wakeup
+	 * from suspend to active.
+	 */
+	dw_hdmi_qp_mod(hdmi, I2S_BPCUV_RCV_DIS, I2S_BPCUV_RCV_MSK,
+		       AUDIO_INTERFACE_CONFIG0);
+	dw_hdmi_qp_mod(hdmi, AUDPKT_PBIT_FORCE_EN | AUDPKT_CHSTATUS_OVR_EN,
+		       AUDPKT_PBIT_FORCE_EN_MASK | AUDPKT_CHSTATUS_OVR_EN_MASK,
+		       AUDPKT_CONTROL0);
+
+	dw_hdmi_qp_mod(hdmi, AVP_DATAPATH_PACKET_AUDIO_SWDISABLE,
+		       AVP_DATAPATH_PACKET_AUDIO_SWDISABLE, GLOBAL_SWDISABLE);
+}
+
+static void dw_hdmi_qp_audio_disable(struct drm_connector *connector,
+				     struct drm_bridge *bridge)
+{
+	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
+
+	drm_atomic_helper_connector_hdmi_clear_audio_infoframe(connector);
+
+	if (hdmi->tmds_char_rate)
+		dw_hdmi_qp_audio_disable_regs(hdmi);
+}
+
 static int dw_hdmi_qp_i2c_read(struct dw_hdmi_qp *hdmi,
 			       unsigned char *buf, unsigned int length)
 {
@@ -361,6 +785,51 @@ static int dw_hdmi_qp_config_drm_infoframe(struct dw_hdmi_qp *hdmi,
 	return 0;
 }
 
+/*
+ * Static values documented in the TRM
+ * Different values are only used for debug purposes
+ */
+#define DW_HDMI_QP_AUDIO_INFOFRAME_HB1	0x1
+#define DW_HDMI_QP_AUDIO_INFOFRAME_HB2	0xa
+
+static int dw_hdmi_qp_config_audio_infoframe(struct dw_hdmi_qp *hdmi,
+					     const u8 *buffer, size_t len)
+{
+	/*
+	 * AUDI_CONTENTS0: { RSV, HB2, HB1, RSV }
+	 * AUDI_CONTENTS1: { PB3, PB2, PB1, PB0 }
+	 * AUDI_CONTENTS2: { PB7, PB6, PB5, PB4 }
+	 *
+	 * PB0: CheckSum
+	 * PB1: | CT3    | CT2  | CT1  | CT0  | F13  | CC2 | CC1 | CC0 |
+	 * PB2: | F27    | F26  | F25  | SF2  | SF1  | SF0 | SS1 | SS0 |
+	 * PB3: | F37    | F36  | F35  | F34  | F33  | F32 | F31 | F30 |
+	 * PB4: | CA7    | CA6  | CA5  | CA4  | CA3  | CA2 | CA1 | CA0 |
+	 * PB5: | DM_INH | LSV3 | LSV2 | LSV1 | LSV0 | F52 | F51 | F50 |
+	 * PB6~PB10: Reserved
+	 *
+	 * AUDI_CONTENTS0 default value defined by HDMI specification,
+	 * and shall only be changed for debug purposes.
+	 */
+	u32 header_bytes = (DW_HDMI_QP_AUDIO_INFOFRAME_HB1 << 8) |
+			  (DW_HDMI_QP_AUDIO_INFOFRAME_HB2 << 16);
+
+	regmap_bulk_write(hdmi->regm, PKT_AUDI_CONTENTS0, &header_bytes, 1);
+	regmap_bulk_write(hdmi->regm, PKT_AUDI_CONTENTS1, &buffer[3], 1);
+	regmap_bulk_write(hdmi->regm, PKT_AUDI_CONTENTS2, &buffer[4], 1);
+
+	/* Enable ACR, AUDI, AMD */
+	dw_hdmi_qp_mod(hdmi,
+		       PKTSCHED_ACR_TX_EN | PKTSCHED_AUDI_TX_EN | PKTSCHED_AMD_TX_EN,
+		       PKTSCHED_ACR_TX_EN | PKTSCHED_AUDI_TX_EN | PKTSCHED_AMD_TX_EN,
+		       PKTSCHED_PKT_EN);
+
+	/* Enable AUDS */
+	dw_hdmi_qp_mod(hdmi, PKTSCHED_AUDS_TX_EN, PKTSCHED_AUDS_TX_EN, PKTSCHED_PKT_EN);
+
+	return 0;
+}
+
 static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
 					    struct drm_bridge_state *old_state)
 {
@@ -382,6 +851,7 @@ static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
 		dev_dbg(hdmi->dev, "%s mode=HDMI rate=%llu\n",
 			__func__, conn_state->hdmi.tmds_char_rate);
 		op_mode = 0;
+		hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
 	} else {
 		dev_dbg(hdmi->dev, "%s mode=DVI\n", __func__);
 		op_mode = OPMODE_DVI;
@@ -400,6 +870,8 @@ static void dw_hdmi_qp_bridge_atomic_disable(struct drm_bridge *bridge,
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 
+	hdmi->tmds_char_rate = 0;
+
 	hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
 }
 
@@ -455,6 +927,13 @@ static int dw_hdmi_qp_bridge_clear_infoframe(struct drm_bridge *bridge,
 		dw_hdmi_qp_mod(hdmi, 0, PKTSCHED_DRMI_TX_EN, PKTSCHED_PKT_EN);
 		break;
 
+	case HDMI_INFOFRAME_TYPE_AUDIO:
+		dw_hdmi_qp_mod(hdmi, 0,
+			       PKTSCHED_ACR_TX_EN |
+			       PKTSCHED_AUDS_TX_EN |
+			       PKTSCHED_AUDI_TX_EN,
+			       PKTSCHED_PKT_EN);
+		break;
 	default:
 		dev_dbg(hdmi->dev, "Unsupported infoframe type %x\n", type);
 	}
@@ -477,6 +956,9 @@ static int dw_hdmi_qp_bridge_write_infoframe(struct drm_bridge *bridge,
 	case HDMI_INFOFRAME_TYPE_DRM:
 		return dw_hdmi_qp_config_drm_infoframe(hdmi, buffer, len);
 
+	case HDMI_INFOFRAME_TYPE_AUDIO:
+		return dw_hdmi_qp_config_audio_infoframe(hdmi, buffer, len);
+
 	default:
 		dev_dbg(hdmi->dev, "Unsupported infoframe type %x\n", type);
 		return 0;
@@ -494,6 +976,9 @@ static const struct drm_bridge_funcs dw_hdmi_qp_bridge_funcs = {
 	.hdmi_tmds_char_rate_valid = dw_hdmi_qp_bridge_tmds_char_rate_valid,
 	.hdmi_clear_infoframe = dw_hdmi_qp_bridge_clear_infoframe,
 	.hdmi_write_infoframe = dw_hdmi_qp_bridge_write_infoframe,
+	.hdmi_audio_startup = dw_hdmi_qp_audio_enable,
+	.hdmi_audio_shutdown = dw_hdmi_qp_audio_disable,
+	.hdmi_audio_prepare = dw_hdmi_qp_audio_prepare,
 };
 
 static irqreturn_t dw_hdmi_qp_main_hardirq(int irq, void *dev_id)
@@ -603,6 +1088,10 @@ struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,
 	if (IS_ERR(hdmi->bridge.ddc))
 		return ERR_CAST(hdmi->bridge.ddc);
 
+	hdmi->bridge.hdmi_audio_max_i2s_playback_channels = 8;
+	hdmi->bridge.hdmi_audio_dev = dev;
+	hdmi->bridge.hdmi_audio_dai_port = 1;
+
 	ret = devm_drm_bridge_add(dev, &hdmi->bridge);
 	if (ret)
 		return ERR_PTR(ret);
-- 
2.48.1

[PATCH v7 2/3] arm64: dts: rockchip: Add HDMI audio outputs for rk3588 SoC

[Detlev Casanova]

For hdmi0_sound, use the simple-audio-card driver with the hdmi0 QP node
as CODEC and the i2s5 device as CPU.

Similarly for hdmi1_sound, the CODEC is the hdmi1 node and the CPU is
i2s6, but only added in the rk3588-extra.dtsi device tree as the second
TX HDMI port is not available on base versions of the SoC.

The simple-audio-card,mclk-fs value is set to 128 as it is done in
the downstream driver.

The #sound-dai-cells value is set to 0 in the hdmi0 and hdmi1 nodes so
that they can be used as audio codec nodes.

Tested-by: Quentin Schulz <quentin.schulz@cherry.de> # RK3588 Tiger Haikou
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi  | 17 +++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi | 17 +++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 8cfa30837ce72..f9f888dedd8f0 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -382,6 +382,22 @@ scmi_reset: protocol@16 {
 		};
 	};
 
+	hdmi0_sound: hdmi0-sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,mclk-fs = <128>;
+		simple-audio-card,name = "hdmi0";
+		status = "disabled";
+
+		simple-audio-card,codec {
+			sound-dai = <&hdmi0>;
+		};
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s5_8ch>;
+		};
+	};
+
 	pmu-a55 {
 		compatible = "arm,cortex-a55-pmu";
 		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH &ppi_partition0>;
@@ -1394,6 +1410,7 @@ hdmi0: hdmi@fde80000 {
 		reset-names = "ref", "hdp";
 		rockchip,grf = <&sys_grf>;
 		rockchip,vo-grf = <&vo1_grf>;
+		#sound-dai-cells = <0>;
 		status = "disabled";
 
 		ports {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
index 567f8f74c7b3c..cf04847afd040 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
@@ -7,6 +7,22 @@
 #include "rk3588-extra-pinctrl.dtsi"
 
 / {
+	hdmi1_sound: hdmi1-sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,mclk-fs = <128>;
+		simple-audio-card,name = "hdmi1";
+		status = "disabled";
+
+		simple-audio-card,codec {
+			sound-dai = <&hdmi1>;
+		};
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s6_8ch>;
+		};
+	};
+
 	usb_host1_xhci: usb@fc400000 {
 		compatible = "rockchip,rk3588-dwc3", "snps,dwc3";
 		reg = <0x0 0xfc400000 0x0 0x400000>;
@@ -165,6 +181,7 @@ hdmi1: hdmi@fdea0000 {
 		reset-names = "ref", "hdp";
 		rockchip,grf = <&sys_grf>;
 		rockchip,vo-grf = <&vo1_grf>;
+		#sound-dai-cells = <0>;
 		status = "disabled";
 
 		ports {
-- 
2.48.1

[PATCH v7 3/3] arm64: dts: rockchip: Enable HDMI audio outputs for Rock 5B

[Detlev Casanova]

HDMI audio is available on the Rock 5B HDMI TX ports.
Enable it for both ports.

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
index 27128c6a05a2f..8f42cd0427701 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
@@ -231,6 +231,10 @@ hdmi0_out_con: endpoint {
 	};
 };
 
+&hdmi0_sound {
+	status = "okay";
+};
+
 &hdmi1 {
 	pinctrl-0 = <&hdmim0_tx1_cec &hdmim0_tx1_hpd
 		     &hdmim1_tx1_scl &hdmim1_tx1_sda>;
@@ -249,6 +253,10 @@ hdmi1_out_con: endpoint {
 	};
 };
 
+&hdmi1_sound {
+	status = "okay";
+};
+
 &hdptxphy_hdmi0 {
 	status = "okay";
 };
@@ -351,6 +359,14 @@ i2s0_8ch_p0_0: endpoint {
 	};
 };
 
+&i2s5_8ch {
+	status = "okay";
+};
+
+&i2s6_8ch {
+	status = "okay";
+};
+
 &package_thermal {
 	polling-delay = <1000>;
 
-- 
2.48.1

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Piotr Oniszczuk]

> Wiadomość napisana przez Detlev Casanova <detlev.casanova@collabora.com> w dniu 17 lut 2025, o godz. 22:47:
> 
> To support HDMI audio on the rk3588 based devices, the generic HDMI
> Codec framework is used in the dw-hdmi-qp DRM bridge driver.
> 
> The implementation is mainly based on the downstream driver, ported to the
> generic HDMI Codec framework [1] recently merged in the master branch.
> The parameters computation has been kept as is and the data stored in the
> dw_hdmi_qp struct as been cleaned up.
> 
> The table for the N values has been edited to reflect N recommended values
> as well as CTS recommended values.
> 
> The downstream kernel also implements a machine driver for HDMI audio but
> it is doing exactly what the simple-audio-card driver does, so use that
> instead in the RK3588 SoC device tree.
> 
> This adds HDMI audio support for both HDMI TX ports.
> 
> *** Dependencies ***
> 
> Based on Linus' master branch, but also needs Cristian's dts patches for HDMI1
> support [2], which depends on Heiko's patchset for
> phy-rockchip-samsung-hdptx [3]. Patches will apply without [3], but HDMI will
> not work (at all).
> 
> [1]: https://lore.kernel.org/all/20241224-drm-bridge-hdmi-connector-v10-0-dc89577cd438@linaro.org
> [2]: https://lore.kernel.org/linux-rockchip/20241211-rk3588-hdmi1-v2-0-02cdca22ff68@collabora.com
> [3]: https://lore.kernel.org/lkml/20241206103401.1780416-3-heiko@sntech.de/
> 
> Changes since v6:
> - Fix arguments alignement (checkpatch --strict warnings)
> - Add hdmi1 audio support too
> - Move hdmi0_sound node under hdmi0_out_con
> 
> Changes since v5:
> - Simplify audio math computation for N
> - Move hdmi0-sound node up with other address-less nodes
> 
> Changes since v4:
> - Moved hdmi0_audio node the rk3588-base.dtsi
> - Enable hdmi0_audio in rk3588-rock-5b.dts
> 
> Changes since v3:
> - Renamed function to start with dw_hdmi_qp
> 
> Changes since v2:
> - Also clear the audio infoframe
> - Write AUDI_CONTENTS0 to its default value in case it gets overwritten.
> - Store tmds_char_rate in the dw_hdmi_qp struct in atomic_enable
> - Clear tmds_char_rate in atomic_disable and only write registers when
>   tmds_char_rate is not 0.
> - Do not use connector_state duplicates
> 
> Changes since v1:
> - Remove useless audio_mutex (was used downstream for multiple drivers access
>   to audio functions)
> - Let hdmi_codec build and setup audio infoframes
> - Only access audio registers when connector is connected
> - Rebased on master branch
> 
> Detlev Casanova (2):
>  arm64: dts: rockchip: Add HDMI audio outputs for rk3588 SoC
>  arm64: dts: rockchip: Enable HDMI audio outputs for Rock 5B
> 
> Sugar Zhang (1):
>  drm/bridge: synopsys: Add audio support for dw-hdmi-qp
> 
> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  17 +
> .../arm64/boot/dts/rockchip/rk3588-extra.dtsi |  17 +
> .../boot/dts/rockchip/rk3588-rock-5b.dts      |  16 +
> drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c  | 489 ++++++++++++++++++
> 4 files changed, 539 insertions(+)
> 
> -- 
> 2.48.1
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


Detelv,

Just curious of your opinion:

I’m on 6.14-rc3 and using https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-bridge + this v7 series.

On mine rock5b all works nicely but - at boot time - i’m getting some oops in kernel like this: https://gist.github.com/warpme/e1668acbef7817e5d2a88a6840328722

I’m not sure where issue is but it looks to me like kind of interference between analog audio and hdmi audio as commenting analog audio in dts ( https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts?h=v6.14-rc3#n24 ) stops kernel from oops’ing….

 rock5b dts i’m using is like this: https://gist.github.com/warpme/a8a32afd4a05d4b7f25d2808984b05ac

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Detlev Casanova]

Hi Piotr,

On Thursday, 20 February 2025 06:16:20 EST Piotr Oniszczuk wrote:
> > Wiadomość napisana przez Detlev Casanova <detlev.casanova@collabora.com> w
> > dniu 17 lut 2025, o godz. 22:47:
> > 
> > To support HDMI audio on the rk3588 based devices, the generic HDMI
> > Codec framework is used in the dw-hdmi-qp DRM bridge driver.
> > 
> > The implementation is mainly based on the downstream driver, ported to the
> > generic HDMI Codec framework [1] recently merged in the master branch.
> > The parameters computation has been kept as is and the data stored in the
> > dw_hdmi_qp struct as been cleaned up.
> > 
> > The table for the N values has been edited to reflect N recommended values
> > as well as CTS recommended values.
> > 
> > The downstream kernel also implements a machine driver for HDMI audio but
> > it is doing exactly what the simple-audio-card driver does, so use that
> > instead in the RK3588 SoC device tree.
> > 
> > This adds HDMI audio support for both HDMI TX ports.
> > 
> > *** Dependencies ***
> > 
> > Based on Linus' master branch, but also needs Cristian's dts patches for
> > HDMI1 support [2], which depends on Heiko's patchset for
> > phy-rockchip-samsung-hdptx [3]. Patches will apply without [3], but HDMI
> > will not work (at all).
> > 
> > [1]:
> > https://lore.kernel.org/all/20241224-drm-bridge-hdmi-connector-v10-0-dc89
> > 577cd438@linaro.org [2]:
> > https://lore.kernel.org/linux-rockchip/20241211-rk3588-hdmi1-v2-0-02cdca2
> > 2ff68@collabora.com [3]:
> > https://lore.kernel.org/lkml/20241206103401.1780416-3-heiko@sntech.de/
> > 
> > Changes since v6:
> > - Fix arguments alignement (checkpatch --strict warnings)
> > - Add hdmi1 audio support too
> > - Move hdmi0_sound node under hdmi0_out_con
> > 
> > Changes since v5:
> > - Simplify audio math computation for N
> > - Move hdmi0-sound node up with other address-less nodes
> > 
> > Changes since v4:
> > - Moved hdmi0_audio node the rk3588-base.dtsi
> > - Enable hdmi0_audio in rk3588-rock-5b.dts
> > 
> > Changes since v3:
> > - Renamed function to start with dw_hdmi_qp
> > 
> > Changes since v2:
> > - Also clear the audio infoframe
> > - Write AUDI_CONTENTS0 to its default value in case it gets overwritten.
> > - Store tmds_char_rate in the dw_hdmi_qp struct in atomic_enable
> > - Clear tmds_char_rate in atomic_disable and only write registers when
> > 
> >   tmds_char_rate is not 0.
> > 
> > - Do not use connector_state duplicates
> > 
> > Changes since v1:
> > - Remove useless audio_mutex (was used downstream for multiple drivers
> > access> 
> >   to audio functions)
> > 
> > - Let hdmi_codec build and setup audio infoframes
> > - Only access audio registers when connector is connected
> > - Rebased on master branch
> > 
> > Detlev Casanova (2):
> >  arm64: dts: rockchip: Add HDMI audio outputs for rk3588 SoC
> >  arm64: dts: rockchip: Enable HDMI audio outputs for Rock 5B
> > 
> > Sugar Zhang (1):
> >  drm/bridge: synopsys: Add audio support for dw-hdmi-qp
> > 
> > arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  17 +
> > .../arm64/boot/dts/rockchip/rk3588-extra.dtsi |  17 +
> > .../boot/dts/rockchip/rk3588-rock-5b.dts      |  16 +
> > drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c  | 489 ++++++++++++++++++
> > 4 files changed, 539 insertions(+)
> 
> Detelv,
> 
> Just curious of your opinion:
> 
> I’m on 6.14-rc3 and using
> https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-brid
> ge + this v7 series.

Do you have the branch available somewhere ?

> On mine rock5b all works nicely but - at boot time - i’m getting some oops
> in kernel like this:
> https://gist.github.com/warpme/e1668acbef7817e5d2a88a6840328722

I did notice that at one point but it disappeard after a rebase on the the 
latest master so I didn't look further into that.
Could it be related to 2518a0e1b878042f9afa45ae063e544a16efc1a3 "ASoC: simple-
card: use __free(device_node) for device node" ?

I'm not exactly sure how __free(device_node) works though, but reverting that 
commit could fix the issue.

> I’m not sure where issue is but it looks to me like kind of interference
> between analog audio and hdmi audio as commenting analog audio in dts (
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/
> arm64/boot/dts/rockchip/rk3588-rock-5b.dts?h=v6.14-rc3#n24 ) stops kernel
> from oops’ing….

I cannot reproduce anymore, so if you have a branch/config that you use, I 
could try, looking into that.
I don't see any relevant commits that would change this behavior on master 
since v6.14-rc3, so I'm not sure what is going on.

>  rock5b dts i’m using is like this:
> https://gist.github.com/warpme/a8a32afd4a05d4b7f25d2808984b05ac

Regards,
Detlev.

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Piotr Oniszczuk]

> Wiadomość napisana przez Detlev Casanova <detlev.casanova@collabora.com> w dniu 20 lut 2025, o godz. 18:03:
> 
> Hi Piotr,
> 
> On Thursday, 20 February 2025 06:16:20 EST Piotr Oniszczuk wrote:
>> 
>> 
>> Detelv,
>> 
>> Just curious of your opinion:
>> 
>> I’m on 6.14-rc3 and using
>> https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-brid
>> ge + this v7 series.
> 
> Do you have the branch available somewhere ?

My tests are on my MiniMyth2 distro. 
My build system is vanilla upstream+patches style.  
Kernel is mainline 6.14-rc3 kernel with applied series of patches:

PATCHFILES += 1001-math.h-add-DIV_ROUND_UP_NO_OVERFLOW.patch
PATCHFILES += 1002-clk-divider-Fix-divisor-masking-on-64-bit-platforms.patch
PATCHFILES += 1003-clk-composite-replace-open-coded-abs_diff.patch
# hdmi video support
PATCHFILES += 1010-FROM-ML-phy-phy-rockchip-samsung-hdptx-Don-t-use-dt-.patch
PATCHFILES += 1011-FROM-UPSTREAM-drm-rockchip-Don-t-change-hdmi-referen.patch
PATCHFILES += 1012-FROM-UPSTREAM-drm-rockchip-vop2-Drop-unnecessary-if_.patch
PATCHFILES += 1013-FROM-UPSTREAM-drm-rockchip-vop2-Improve-display-mode.patch
PATCHFILES += 1014-WIP-drm-rockchip-vop2-Improve-display-modes-handling.patch
PATCHFILES += 1015-drm-bridge-dw-hdmi-Sync-comments-with-actual-bus-for.patch
PATCHFILES += 1016-drm-bridge-connector-Sync-supported_formats-with-com.patch
PATCHFILES += 1017-drm-connector-hdmi-Evaluate-limited-range-after-comp.patch
PATCHFILES += 1018-drm-connector-hdmi-Add-support-for-YUV420-format-ver.patch
PATCHFILES += 1019-drm-connector-hdmi-Improve-debug-message-for-support.patch
PATCHFILES += 1020-drm-connector-hdmi-Use-YUV420-output-format-as-an-RG.patch
PATCHFILES += 1021-phy-Add-HDMI-configuration-options.patch
PATCHFILES += 1022-phy-hdmi-Add-color-depth-configuration.patch
PATCHFILES += 1023-phy-rockchip-samsung-hdptx-Fix-clock-ratio-setup.patch
PATCHFILES += 1024-phy-rockchip-samsung-hdptx-Drop-unused-lcpll_config.patch
PATCHFILES += 1025-phy-rockchip-samsung-hdptx-Setup-TMDS-char-rate-via-.patch
PATCHFILES += 1026-phy-rockchip-samsung-hdptx-Add-high-color-depth-mana.patch
PATCHFILES += 1027-phy-rockchip-samsung-hdptx-Cleanup-internal-rate-han.patch
PATCHFILES += 1028-phy-rockchip-samsung-hdptx-Avoid-Hz-hHz-unit-convers.patch
PATCHFILES += 1029-TEST-phy-rockchip-samsung-hdptx-Add-verbose-logging.patch
PATCHFILES += 1030-WIP-drm-bridge-Add-detect_ctx-hook.patch
PATCHFILES += 1031-WIP-drm-bridge-connector-Switch-from-detect-to-detec.patch
PATCHFILES += 1032-WIP-drm-bridge-dw-hdmi-qp-Add-high-TMDS-clock-ratio-.patch
PATCHFILES += 1033-WIP-drm-rockchip-vop2-Add-high-color-depth-support.patch
PATCHFILES += 1034-WIP-drm-rockchip-vop2-Add-YUV420-support.patch
PATCHFILES += 1035-WIP-drm-rockchip-dw_hdmi_qp-Make-use-of-phy_configur.patch
PATCHFILES += 1036-WIP-drm-rockchip-dw_hdmi_qp-Add-10bpc-and-YUV420-out.patch
PATCHFILES += 1037-WIP-drm-bridge-dw-hdmi-qp-Enable-10bpc-and-YUV420.patch
# hdmi audio support
PATCHFILES += 1040-drm-bridge-synopsys-add-audio-support-for-dw-hdmi-qp-v7.patch
# cec support
PATCHFILES += 1045-drm-bridge-synopsys-add-cec-support.patch
# var additions
PATCHFILES += 1060-net-ethernet-add-yt6801-gige-pcie-controller.patch
PATCHFILES += 1061-net-ethernet-yt6801-gige-pcie-silence-debug-msgs.patch
PATCHFILES += 1062-WIP-iommu-rockchip-add-flush_iotlb_all-ops.patch
PATCHFILES += 1063-media-rockchip-add-rkvdec2-driver.patch
PATCHFILES += 1064-media-rkvdec2-add-iommu-support-v3.patch
PATCHFILES += 1065-wip-add-hevc-support.patch
PATCHFILES += 1066-wip-hevc-add-ref-frames-support.patch
# dtsi additions
PATCHFILES += 1070-arm64-dtsi-rk3588s-add-vop2-clock-resets.patch
PATCHFILES += 1071-arm64-dtsi-rockchip-3588s-add-hdmi-bridge.patch
PATCHFILES += 1072-arm64-dtsi-rockchip-3588-hdmi-add-audio-support.patch
PATCHFILES += 1074-arm64-dtsi-rockchip-add-rkvdec2-video-vecoder-on-rk3588.patch
PATCHFILES += 1077-arm64-dtsi-rkvdec2-add-iommu-support-v3.patch
PATCHFILES += 1078-arm64-dtsi-rockchip-rk356x-add-rkvdec2-video-decoder-nodes.patch
# dts patches
PATCHFILES += 1080-arm64-dts-rockchip-rk3588s-rock5a-dts-improvements.patch
PATCHFILES += 1081-arm64-dts-rockchip-rk3588-rock5b-dts-improvements.patch
PATCHFILES += 1082-arm64-dts-rockchip-rk3588s-rock5c-dts-improvements.patch
PATCHFILES += 1083-arm64-dts-rockchip-rk3588-rock5itx-dts-improvements.patch
PATCHFILES += 1084-arm64-dts-rockchip-rk3588s-opi5-dts-improvements.patch
PATCHFILES += 1085-arm64-dts-rockchip-rk3588-opi5plus-dts-improvements.patch
PATCHFILES += 1086-arm64-dts-rockchip-rk3588s-add-opi5pro-dts.patch
PATCHFILES += 1087-arm64-dts-rockchip-rk3588s-add-nanopi-m6-dts.patch
PATCHFILES += 1088-arm64-dts-rockchip-rk3588s-nanopc-r6s-dts-improvements.patch
PATCHFILES += 1089-arm64-dts-rockchip-rk3588-nanopc-t6-dtsi-improvements.patch
PATCHFILES += 1090-arm64-dts-rockchip-rk3588-add-rock5t-dt.patch

 patches are from: https://github.com/warpme/minimyth2/tree/master/script/kernel/linux-6.14/files

Kernel config is: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.14/files/linux-6.14-arm64-armv8.config

Above patching is effectively: 

- https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-bridge
- Yours hdmi audio v7
. added cec support
- added rkvdec2 (including hevc)
- some dts enablements for above


> 
>> On mine rock5b all works nicely but - at boot time - i’m getting some oops
>> in kernel like this:
>> https://gist.github.com/warpme/e1668acbef7817e5d2a88a6840328722
> 
> I did notice that at one point but it disappeard after a rebase on the the 
> latest master so I didn't look further into that.

Indeed - i.e. i don’t have these oops on rk3588 based orange5plus.
Also 6.12 kernel is clean.
But i have them reproducible on rock5b (and also e.g. on rock5t)
  
> Could it be related to 2518a0e1b878042f9afa45ae063e544a16efc1a3 "ASoC: simple-
> card: use __free(device_node) for device node" ?
> 

I tried with 2518a0e1b878042f9afa45ae063e544a16efc1a3 revered and this NOT helps.
With reverted above commit, dmesg is: https://gist.github.com/warpme/dbfe813583e4660a02b74315f193e768

  

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Piotr Oniszczuk]

Small data point: on rock5b switching in dts analog audio: from audio-graph-card to simple-audio-card (so dts is: https://gist.github.com/warpme/349b27e49bc6f617ef1041047e75adab ) makes kernel oops go away with analog audio still working…

so maybe issue is in audio-graph-card code (or its dts fragments)?

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Detlev Casanova]

On Friday, 21 February 2025 06:42:31 EST Piotr Oniszczuk wrote:
> Small data point: on rock5b switching in dts analog audio: from
> audio-graph-card to simple-audio-card (so dts is:
> https://gist.github.com/warpme/349b27e49bc6f617ef1041047e75adab ) makes
> kernel oops go away with analog audio still working…
> 
> so maybe issue is in audio-graph-card code (or its dts fragments)?

I can reproduce with your config (I guess that removing the driver for analog 
audio hides the issue)

I'm really feel like simple_util_clean_reference(card) in simple_probe() 
errors path should not be called anymore, since
https://lore.kernel.org/all/87zfk4o5l2.wl-kuninori.morimoto.gx@renesas.com/

I'm adding Kuninori Morimoto in the to list of this thread for extra input 
(See thread at https://lore.kernel.org/all/B8EF5196-55FB-44EC-B93C-E327C791225B@gmail.com/) as they made that change.
Especially those commits:
      ASoC: audio-graph-card2: use __free(device_node) for device node
      ASoC: audio-graph-card: use __free(device_node) for device node
      ASoC: simple-card: use __free(device_node) for device node

Detlev.

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Kuninori Morimoto]

Hi Detlev

Thank you for Cc:ing to me.
Sorry for my late response. Monday was holiday in Japan.

> I'm really feel like simple_util_clean_reference(card) in simple_probe() 
> errors path should not be called anymore, since
(snip)
> I'm adding Kuninori Morimoto in the to list of this thread for extra input
(snip)
> Especially those commits:
>       ASoC: audio-graph-card2: use __free(device_node) for device node
>       ASoC: audio-graph-card: use __free(device_node) for device node
>       ASoC: simple-card: use __free(device_node) for device node

I got same report from our test team. I'm now tring to solve it.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Kuninori Morimoto]

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]


Hi Detlev, again

> > Especially those commits:
> >       ASoC: audio-graph-card2: use __free(device_node) for device node
> >       ASoC: audio-graph-card: use __free(device_node) for device node
> >       ASoC: simple-card: use __free(device_node) for device node
> 
> I got same report from our test team. I'm now tring to solve it.

Unfortunately, I can't reproduce the issue on my environment,
but I guess I found the root cause. Does attached patch can solve
your issue ?
I will officially post the patch to ML, but want to confirm it before it.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

[-- Attachment #2: 0001-ASoC-simple-card-utils-Don-t-use-__free-device_node-.patch --]
[-- Type: text/plain, Size: 1941 bytes --]

From 3995478ae84240b24964a458ecbefe9fb6dc2272 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Tue, 25 Feb 2025 09:54:47 +0900
Subject: [PATCH] ASoC: simple-card-utils: Don't use __free(device_node) at
 graph_util_parse_dai()

commit 419d1918105e ("ASoC: simple-card-utils: use __free(device_node) for
device node") uses __free(device_node) for dlc->of_node, but we need to
keep it while driver is in use.

Don't use __free(device_node) in graph_util_parse_dai().

Fixes: 419d1918105e ("ASoC: simple-card-utils: use __free(device_node) for device node")
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/simple-card-utils.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 51e0e434514d..79fdd57a470c 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -1102,6 +1102,7 @@ static int graph_get_dai_id(struct device_node *ep)
 int graph_util_parse_dai(struct simple_util_priv *priv, struct device_node *ep,
 			 struct snd_soc_dai_link_component *dlc, int *is_single_link)
 {
+	struct device_node *node;
 	struct device *dev = simple_priv_to_dev(priv);
 	struct of_phandle_args args = {};
 	struct snd_soc_dai *dai;
@@ -1110,7 +1111,7 @@ int graph_util_parse_dai(struct simple_util_priv *priv, struct device_node *ep,
 	if (!ep)
 		return 0;
 
-	struct device_node *node __free(device_node) = of_graph_get_port_parent(ep);
+	node = of_graph_get_port_parent(ep);
 
 	/*
 	 * Try to find from DAI node
@@ -1153,8 +1154,10 @@ int graph_util_parse_dai(struct simple_util_priv *priv, struct device_node *ep,
 	 *    if he unbinded CPU or Codec.
 	 */
 	ret = snd_soc_get_dlc(&args, dlc);
-	if (ret < 0)
+	if (ret < 0) {
+		of_node_put(node);
 		goto end;
+	}
 
 parse_dai_end:
 	if (is_single_link)
-- 
2.43.0

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Detlev Casanova]

Hi Morimoto-san,

On Tuesday, 25 February 2025 01:16:50 EST Kuninori Morimoto wrote:
> Hi Detlev, again
> 
> > > Especially those commits:
> > >       ASoC: audio-graph-card2: use __free(device_node) for device node
> > >       ASoC: audio-graph-card: use __free(device_node) for device node
> > >       ASoC: simple-card: use __free(device_node) for device node
> > 
> > I got same report from our test team. I'm now tring to solve it.
> 
> Unfortunately, I can't reproduce the issue on my environment,
> but I guess I found the root cause. Does attached patch can solve
> your issue ?

From what I see, the error is not present anymore on linux 6.14-rc4. I tried 
reverting your patch "ASoC: simple-card-utils.c: add missing dlc->of_node" 
(dabbd325b25edb5cdd99c94391817202dd54b651) and the error reappears.

On 6.14-rc3, any of your patches (dabbd325b25e, or the one you attached here) 
will fix the issue and on 6.14-rc4, there is already a patch that fixes the 
issue.

Also, since dabbd325b25e, the node indeed should be kept while the driver is 
used. So even though the issue reported here is fixed by another patch, both 
are likely needed.

That being said, I'm not sure I completely understand why that extra line fixes 
the issue. Is the __free() attribute smart enough to know that the pointer has 
been copied and not free it at the end of scope ?

> I will officially post the patch to ML, but want to confirm it before it.
> 
> Thank you for your help !!

Thank you for looking into this too :)

Detlev.

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Kuninori Morimoto]

Hi Detlev

Thank you for the testing.

> I got same report from our test team. I'm now tring to solve it.

It seems our test team and your team are encounter same issues by different
way, and it seems it happen if some kind of deffer issue happen (?).

> That being said, I'm not sure I completely understand why that extra line fixes 
> the issue. Is the __free() attribute smart enough to know that the pointer has 
> been copied and not free it at the end of scope ?

__free(device_node) attribute will automatically calls of_node_put() when it
lost the scope. By below patch, graph_util_parse_dai() will use
__free(device_node) and thus dlc->of_node counter was not increased.

	419d1918105e5d9926ab02f1f834bb416dc76f65
	("ASoC: simple-card-utils: use __free(device_node) for device node")

In such situation, if some kind of deffer issue happened, xxx-card probe()
will failed and calls simple_util_clean_reference(). It calls of_node_put()
for each dlc->of_node, but above one was already called of_node_put() via
__free(device_node). So you encounter the error.

I'm not 100% sure why dabbd325b2 commit removed the issue, but I think
it is just luck, I think.

Thank you for your test !!
I will post the patch with below tag if our test team could confirm it.

	Tested-by: Detlev Casanova <detlev.casanova@collabora.com>

Best regards
---
Kuninori Morimoto

Re: (subset) [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Heiko Stuebner]

On Mon, 17 Feb 2025 16:47:39 -0500, Detlev Casanova wrote:
> To support HDMI audio on the rk3588 based devices, the generic HDMI
> Codec framework is used in the dw-hdmi-qp DRM bridge driver.
> 
> The implementation is mainly based on the downstream driver, ported to the
> generic HDMI Codec framework [1] recently merged in the master branch.
> The parameters computation has been kept as is and the data stored in the
> dw_hdmi_qp struct as been cleaned up.
> 
> [...]

Applied, thanks!

[1/3] drm/bridge: synopsys: Add audio support for dw-hdmi-qp
      commit: fd0141d1a8a2a26675ee88df75615c05a55044de

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Re: (subset) [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Heiko Stuebner]

On Mon, 17 Feb 2025 16:47:39 -0500, Detlev Casanova wrote:
> To support HDMI audio on the rk3588 based devices, the generic HDMI
> Codec framework is used in the dw-hdmi-qp DRM bridge driver.
> 
> The implementation is mainly based on the downstream driver, ported to the
> generic HDMI Codec framework [1] recently merged in the master branch.
> The parameters computation has been kept as is and the data stored in the
> dw_hdmi_qp struct as been cleaned up.
> 
> [...]

Applied, thanks!

[2/3] arm64: dts: rockchip: Add HDMI audio outputs for rk3588 SoC
      commit: b8c6c136971c0e9750eec89f367529b2854d3a3c
[3/3] arm64: dts: rockchip: Enable HDMI audio outputs for Rock 5B
      commit: 97aa62ed1e970bf8aa9f57e87c946a95fa3d5bef

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Piotr Oniszczuk]

> Wiadomość napisana przez Detlev Casanova <detlev.casanova@collabora.com> w dniu 25 lut 2025, o godz. 15:58:
> 
> From what I see, the error is not present anymore on linux 6.14-rc4. I tried 
> reverting your patch "ASoC: simple-card-utils.c: add missing dlc->of_node" 
> (dabbd325b25edb5cdd99c94391817202dd54b651) and the error reappears.

Guys,

Just FYI:

On 6.14-rc4 without 0001-ASoC-simple-card-utils-Don-t-use-__free-device_node-.patch - i still have oops like this: https://gist.github.com/warpme/ed75c05d3b68f995d429dbd9097005ba
They are happening not every boot - but still happening.

However applying 0001-ASoC-simple-card-utils-Don-t-use-__free-device_node-.patch (with some adaptations as it not applies cleanly on 6.140rc4) - dmesg becomes clean (10 boots; all ok)

Re: [PATCH v7 0/3] Add HDMI audio on the rk3588 SoC

[Heiko Stübner]

Am Samstag, 1. März 2025, 10:11:54 MEZ schrieb Piotr Oniszczuk:
> 
> > Wiadomość napisana przez Detlev Casanova <detlev.casanova@collabora.com> w dniu 25 lut 2025, o godz. 15:58:
> > 
> > From what I see, the error is not present anymore on linux 6.14-rc4. I tried 
> > reverting your patch "ASoC: simple-card-utils.c: add missing dlc->of_node" 
> > (dabbd325b25edb5cdd99c94391817202dd54b651) and the error reappears.
> 
> Guys,
> 
> Just FYI:
> 
> On 6.14-rc4 without 0001-ASoC-simple-card-utils-Don-t-use-__free-device_node-.patch - i still have oops like this: https://gist.github.com/warpme/ed75c05d3b68f995d429dbd9097005ba
> They are happening not every boot - but still happening.
> 
> However applying 0001-ASoC-simple-card-utils-Don-t-use-__free-device_node-.patch (with some adaptations as it not applies cleanly on 6.140rc4) - dmesg becomes clean (10 boots; all ok)

that patch was submitted yesterday [0], so hopefully will make its
way into 6.14-rc next week or so.



[0] https://lore.kernel.org/all/87eczisyhh.wl-kuninori.morimoto.gx@renesas.com/T/#me866307a928c2d592a2ba883867f028c5c8b9b40