2020-03-26 10:52:13

by Alex Riesen

[permalink] [raw]
Subject: [PATCH v4 0/9] media: adv748x: add support for HDMI audio

This adds minimal support for accessing the HDMI audio provided through the
I2S port available on ADV7481 and ADV7482 decoder devices by ADI.
The port carries audio signal from the decoded HDMI stream.

Currently, the driver only supports I2S in TDM, 8 channels a 24bit at 48kHz.
Furthermore, only left-justified, 8 slots, 32bit/slot TDM, at 256fs has been
ever tried.

An ADV7482 on the Renesas Salvator-X ES1.1 (R8A77950 SoC) was used during
development of this code.

Changes since v3:
- use clk_hw instead of clk
Suggested-by: Stephen Boyd <[email protected]>

- formatting improvements and use const where possible

- removed implementation of log_status and EDID setting ioctls,
those will be submitted as separate patches.
Suggested-by: Hans Verkuil <[email protected]>

Changes since v2:
- prepare/enable the clock when it is used, as it seems nothing else does
this otherwise

- give the clock a unique name to ensure it can be registered if there are
multiple adv748x devices in the system

- remove optionality note from clock cell description to ensure the device
description matches the real device (the line is always present, even
if not used)

Changes since v1:
- Add ssi4_ctrl pin group to the sound pins. The pins are responsible for
SCK4 (sample clock) WS4 and (word boundary input), and are required for
SSI audio input over I2S.
Reported-by: Geert Uytterhoeven <[email protected]>

- Removed the audio clock C from the list of clocks of adv748x,
it is exactly the other way around.
Reported-by: Geert Uytterhoeven <[email protected]>

- Add an instance of (currently) fixed rate I2S master clock (MCLK),
connected to the audio_clk_c line of the R-Car SoC.
Explicitly declare the device a clock producer and add it to the
list of clocks used by the audio system of the Salvator-X board.
Suggested-by: Geert Uytterhoeven <[email protected]>

- The implementation of DAI driver has been moved in a separate file
and modified to activate audio decoding and I2S streaming using
snd_soc_dai_... interfaces. This allows the driver to be used with
just ALSA interfaces.

- The ioctls for selecting audio output and muting have been removed,
as not applicable.
Suggested-by: Hans Verkuil <[email protected]>
I have left implementation of the QUERYCAP in, as it seems to be required
by v4l-ctl to support loading of EDID for this node. And setting the EDID
is one feature I desperately need: there are devices which plainly refuse
to talk to the sink if it does not provide EDID they like.

- A device tree configuration without audio port will disable the audio code
altogether, supporting integrations where the port is not connected.

- The patches have been re-arranged, starting with the generic changes and
changes not related to audio directly. Those will be probably sent as a
separate series later.

- The whole series has been rebased on top of v5.6-rc6

Alex Riesen (9):
media: adv748x: fix end-of-line terminators in diagnostic statements
media: adv748x: include everything adv748x.h needs into the file
media: adv748x: reduce amount of code for bitwise modifications of
device registers
media: adv748x: add definitions for audio output related registers
media: adv748x: add support for HDMI audio
media: adv748x: prepare/enable mclk when the audio is used
media: adv748x: only activate DAI if it is described in device tree
dt-bindings: adv748x: add information about serial audio interface
(I2S/TDM)
arm64: dts: renesas: salvator: add a connection from adv748x codec
(HDMI input) to the R-Car SoC

.../devicetree/bindings/media/i2c/adv748x.txt | 16 +-
.../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +-
.../boot/dts/renesas/salvator-common.dtsi | 47 ++-
drivers/media/i2c/adv748x/Makefile | 3 +-
drivers/media/i2c/adv748x/adv748x-afe.c | 6 +-
drivers/media/i2c/adv748x/adv748x-core.c | 45 +--
drivers/media/i2c/adv748x/adv748x-csi2.c | 8 +-
drivers/media/i2c/adv748x/adv748x-dai.c | 278 ++++++++++++++++++
drivers/media/i2c/adv748x/adv748x-hdmi.c | 6 +-
drivers/media/i2c/adv748x/adv748x.h | 65 +++-
10 files changed, 435 insertions(+), 42 deletions(-)
create mode 100644 drivers/media/i2c/adv748x/adv748x-dai.c

--
2.25.1.25.g9ecbe7eb18


2020-03-26 10:55:00

by Alex Riesen

[permalink] [raw]
Subject: [PATCH v4 6/9] media: adv748x: prepare/enable mclk when the audio is used

As there is nothing else (the consumers are supposed to do that) which
enables the clock, do it in the driver.

Signed-off-by: Alexander Riesen <[email protected]>
--

v3: added
---
drivers/media/i2c/adv748x/adv748x-dai.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c b/drivers/media/i2c/adv748x/adv748x-dai.c
index c9191f8f1ca8..185f78023e91 100644
--- a/drivers/media/i2c/adv748x/adv748x-dai.c
+++ b/drivers/media/i2c/adv748x/adv748x-dai.c
@@ -117,11 +117,22 @@ static int adv748x_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)

static int adv748x_dai_startup(struct snd_pcm_substream *sub, struct snd_soc_dai *dai)
{
+ int ret;
struct adv748x_state *state = state_of(dai);

if (sub->stream != SNDRV_PCM_STREAM_CAPTURE)
return -EINVAL;
- return set_audio_pads_state(state, 1);
+ ret = set_audio_pads_state(state, 1);
+ if (ret)
+ goto fail;
+ ret = clk_prepare_enable(mclk_of(state));
+ if (ret)
+ goto fail_pwdn;
+ return 0;
+fail_pwdn:
+ set_audio_pads_state(state, 0);
+fail:
+ return ret;
}

static int adv748x_dai_hw_params(struct snd_pcm_substream *sub,
@@ -174,6 +185,7 @@ static void adv748x_dai_shutdown(struct snd_pcm_substream *sub, struct snd_soc_d
{
struct adv748x_state *state = state_of(dai);

+ clk_disable_unprepare(mclk_of(state));
set_audio_pads_state(state, 0);
}

--
2.25.1.25.g9ecbe7eb18


2020-03-26 10:57:25

by Alex Riesen

[permalink] [raw]
Subject: [PATCH v4 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

As all known variants of the Salvator board have the HDMI decoder
chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482
endpoint and the connection definitions are placed in the common board
file.

For the same reason, the CLK_C clock line and I2C configuration (similar
to the ak4613, on the same interface) are added into the common file.

Signed-off-by: Alexander Riesen <[email protected]>

--

v2: Also add ssi4_ctrl pin group in the sound pins. The pins are
responsible for SCK4 (sample clock) WS4 and (word boundary input),
and are required for SSI audio input over I2S.

The adv748x shall provide its own implementation of the output clock
(MCLK), connected to the audio_clk_c line of the R-Car SoC.

If the frequency of the ADV748x MCLK were fixed, the clock
implementation were not necessary, but it does not seem so: the MCLK
depends on the value in a speed multiplier register and the input sample
rate (48kHz).

Remove audio clock C from the clocks of adv7482.

The clocks property of the video-receiver node lists the input
clocks of the device, which is quite the opposite from the
original intention: the adv7482 on Salvator X boards is a
provide of the MCLK clock for I2S audio output.

Remove old definition of &sound_card.dais and reduce size of changes
in the Salvator-X specific device tree source.

Declare video-receiver a clock producer, as the adv748x driver
implements the master clock used I2S audio output.

Suggested-by: Geert Uytterhoeven <[email protected]>

v2: The driver provides only MCLK clock, not the SCLK and LRCLK,
which are part of the I2S protocol.

Suggested-by: Laurent Pinchart <[email protected]>
---
.../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +-
.../boot/dts/renesas/salvator-common.dtsi | 47 +++++++++++++++++--
2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
index 2438825c9b22..e16c146808b6 100644
--- a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
@@ -146,7 +146,8 @@ &sata {
&sound_card {
dais = <&rsnd_port0 /* ak4613 */
&rsnd_port1 /* HDMI0 */
- &rsnd_port2>; /* HDMI1 */
+ &rsnd_port2 /* HDMI1 */
+ &rsnd_port3>; /* adv7482 hdmi-in */
};

&usb2_phy2 {
diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 98bbcafc8c0d..ead7f8d7a929 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -460,7 +460,7 @@ pca9654: gpio@20 {
#gpio-cells = <2>;
};

- video-receiver@70 {
+ adv7482_hdmi_in: video-receiver@70 {
compatible = "adi,adv7482";
reg = <0x70 0x71 0x72 0x73 0x74 0x75
0x60 0x61 0x62 0x63 0x64 0x65>;
@@ -469,6 +469,7 @@ video-receiver@70 {

#address-cells = <1>;
#size-cells = <0>;
+ #clock-cells = <0>; /* the MCLK for I2S output */

interrupt-parent = <&gpio6>;
interrupt-names = "intrq1", "intrq2";
@@ -510,6 +511,15 @@ adv7482_txb: endpoint {
remote-endpoint = <&csi20_in>;
};
};
+
+ port@c {
+ reg = <12>;
+
+ adv7482_i2s: endpoint {
+ remote-endpoint = <&rsnd_endpoint3>;
+ system-clock-direction-out;
+ };
+ };
};

csa_vdd: adc@7c {
@@ -684,7 +694,8 @@ sdhi3_pins_uhs: sd3_uhs {
};

sound_pins: sound {
- groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a";
+ groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a",
+ "ssi4_data", "ssi4_ctrl";
function = "ssi";
};

@@ -733,8 +744,8 @@ &rcar_sound {
pinctrl-0 = <&sound_pins &sound_clk_pins>;
pinctrl-names = "default";

- /* Single DAI */
- #sound-dai-cells = <0>;
+ /* multi DAI */
+ #sound-dai-cells = <1>;

/* audio_clkout0/1/2/3 */
#clock-cells = <1>;
@@ -758,8 +769,19 @@ &rcar_sound {
<&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
<&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
<&audio_clk_a>, <&cs2000>,
- <&audio_clk_c>,
+ <&adv7482_hdmi_in>,
<&cpg CPG_CORE CPG_AUDIO_CLK_I>;
+ clock-names = "ssi-all",
+ "ssi.9", "ssi.8", "ssi.7", "ssi.6",
+ "ssi.5", "ssi.4", "ssi.3", "ssi.2",
+ "ssi.1", "ssi.0",
+ "src.9", "src.8", "src.7", "src.6",
+ "src.5", "src.4", "src.3", "src.2",
+ "src.1", "src.0",
+ "mix.1", "mix.0",
+ "ctu.1", "ctu.0",
+ "dvc.0", "dvc.1",
+ "clk_a", "clk_b", "clk_c", "clk_i";

ports {
#address-cells = <1>;
@@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint {
capture = <&ssi1 &src1 &dvc1>;
};
};
+ rsnd_port3: port@3 {
+ reg = <3>;
+ rsnd_endpoint3: endpoint {
+ remote-endpoint = <&adv7482_i2s>;
+
+ dai-tdm-slot-num = <8>;
+ dai-tdm-slot-width = <32>;
+ dai-format = "left_j";
+ mclk-fs = <256>;
+ bitclock-master = <&adv7482_i2s>;
+ frame-master = <&adv7482_i2s>;
+
+ capture = <&ssi4>;
+ };
+ };
};
};

--
2.25.1.25.g9ecbe7eb18

2020-03-26 11:00:56

by Alex Riesen

[permalink] [raw]
Subject: [PATCH v4 7/9] media: adv748x: only activate DAI if it is described in device tree

To avoid setting it up even if the hardware is not actually connected
to anything physically.

Besides, the bindings explicitly notes that port definitions are
"optional if they are not connected to anything at the hardware level".

Signed-off-by: Alexander Riesen <[email protected]>
---
drivers/media/i2c/adv748x/adv748x-dai.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c b/drivers/media/i2c/adv748x/adv748x-dai.c
index 185f78023e91..f9cc47fa9ad1 100644
--- a/drivers/media/i2c/adv748x/adv748x-dai.c
+++ b/drivers/media/i2c/adv748x/adv748x-dai.c
@@ -216,6 +216,11 @@ int adv748x_dai_init(struct adv748x_dai *dai)
int ret;
struct adv748x_state *state = adv748x_dai_to_state(dai);

+ if (!state->endpoints[ADV748X_PORT_I2S]) {
+ adv_info(state, "no I2S port, DAI disabled\n");
+ ret = 0;
+ goto fail;
+ }
dai->mclk_name = kasprintf(GFP_KERNEL, "%s.%s-i2s-mclk",
state->dev->driver->name,
dev_name(state->dev));
--
2.25.1.25.g9ecbe7eb18


2020-03-26 11:04:44

by Alex Riesen

[permalink] [raw]
Subject: [PATCH v4 8/9] dt-bindings: adv748x: add information about serial audio interface (I2S/TDM)

As the driver has some support for the audio interface of the device,
the bindings file should mention it.

Signed-off-by: Alexander Riesen <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>

--

v3: remove optionality off MCLK clock cell to ensure the description
matches the hardware no matter if the line is connected.
Suggested-by: Geert Uytterhoeven <[email protected]>
---
.../devicetree/bindings/media/i2c/adv748x.txt | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/adv748x.txt b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
index 4f91686e54a6..50a753189b81 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv748x.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
@@ -2,7 +2,9 @@

The ADV7481 and ADV7482 are multi format video decoders with an integrated
HDMI receiver. They can output CSI-2 on two independent outputs TXA and TXB
-from three input sources HDMI, analog and TTL.
+from three input sources HDMI, analog and TTL. There is also support for an
+I2S-compatible interface connected to the audio processor of the HDMI decoder.
+The interface has TDM capability (8 slots, 32 bits, left or right justified).

Required Properties:

@@ -16,6 +18,8 @@ Required Properties:
slave device on the I2C bus. The main address is mandatory, others are
optional and remain at default values if not specified.

+ - #clock-cells: must be <0>
+
Optional Properties:

- interrupt-names: Should specify the interrupts as "intrq1", "intrq2" and/or
@@ -47,6 +51,7 @@ are numbered as follows.
TTL sink 9
TXA source 10
TXB source 11
+ I2S source 12

The digital output port nodes, when present, shall contain at least one
endpoint. Each of those endpoints shall contain the data-lanes property as
@@ -72,6 +77,7 @@ Example:

#address-cells = <1>;
#size-cells = <0>;
+ #clock-cells = <0>;

interrupt-parent = <&gpio6>;
interrupt-names = "intrq1", "intrq2";
@@ -113,4 +119,12 @@ Example:
remote-endpoint = <&csi20_in>;
};
};
+
+ port@c {
+ reg = <12>;
+
+ adv7482_i2s: endpoint {
+ remote-endpoint = <&i2s_in>;
+ };
+ };
};
--
2.25.1.25.g9ecbe7eb18


2020-03-26 11:07:47

by Alex Riesen

[permalink] [raw]
Subject: [PATCH v4 5/9] media: adv748x: add support for HDMI audio

This adds an implemention of SoC DAI driver which provides access to the
I2S port of the device.

Signed-off-by: Alexander Riesen <[email protected]>
--

v3: fix clock registration in case of multiple adv748x devices
Suggested-by: Geert Uytterhoeven <[email protected]>

v4: use clk_hw instead of clk
Suggested-by: Stephen Boyd <[email protected]>

v4: use const for capture snd_soc_pcm_stream instance
Suggested-by: Stephen Boyd <[email protected]>
---
drivers/media/i2c/adv748x/Makefile | 3 +-
drivers/media/i2c/adv748x/adv748x-core.c | 9 +-
drivers/media/i2c/adv748x/adv748x-dai.c | 261 +++++++++++++++++++++++
drivers/media/i2c/adv748x/adv748x.h | 17 +-
4 files changed, 287 insertions(+), 3 deletions(-)
create mode 100644 drivers/media/i2c/adv748x/adv748x-dai.c

diff --git a/drivers/media/i2c/adv748x/Makefile b/drivers/media/i2c/adv748x/Makefile
index 93844f14cb10..6e7a302ef199 100644
--- a/drivers/media/i2c/adv748x/Makefile
+++ b/drivers/media/i2c/adv748x/Makefile
@@ -3,6 +3,7 @@ adv748x-objs := \
adv748x-afe.o \
adv748x-core.o \
adv748x-csi2.o \
- adv748x-hdmi.o
+ adv748x-hdmi.o \
+ adv748x-dai.o

obj-$(CONFIG_VIDEO_ADV748X) += adv748x.o
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index 8580e6624276..3513ca138e53 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -765,8 +765,14 @@ static int adv748x_probe(struct i2c_client *client)
goto err_cleanup_txa;
}

+ ret = adv748x_dai_init(&state->dai);
+ if (ret) {
+ adv_err(state, "Failed to probe DAI\n");
+ goto err_cleanup_txb;
+ }
return 0;
-
+err_cleanup_txb:
+ adv748x_csi2_cleanup(&state->txb);
err_cleanup_txa:
adv748x_csi2_cleanup(&state->txa);
err_cleanup_afe:
@@ -787,6 +793,7 @@ static int adv748x_remove(struct i2c_client *client)
{
struct adv748x_state *state = i2c_get_clientdata(client);

+ adv748x_dai_cleanup(&state->dai);
adv748x_afe_cleanup(&state->afe);
adv748x_hdmi_cleanup(&state->hdmi);

diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c b/drivers/media/i2c/adv748x/adv748x-dai.c
new file mode 100644
index 000000000000..c9191f8f1ca8
--- /dev/null
+++ b/drivers/media/i2c/adv748x/adv748x-dai.c
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Analog Devices ADV748X HDMI receiver with AFE
+ * The implementation of DAI.
+ */
+
+#include "adv748x.h"
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <sound/pcm_params.h>
+
+#define state_of(soc_dai) \
+ adv748x_dai_to_state(container_of((soc_dai)->driver, \
+ struct adv748x_dai, drv))
+#define mclk_of(state) ((state)->dai.mclk_hw->clk)
+
+static const char ADV748X_DAI_NAME[] = "adv748x-i2s";
+
+static int set_audio_pads_state(struct adv748x_state *state, int on)
+{
+ return io_clrset(state, ADV748X_IO_PAD_CONTROLS,
+ ADV748X_IO_PAD_CONTROLS_TRI_AUD |
+ ADV748X_IO_PAD_CONTROLS_PDN_AUD,
+ on ? 0 : 0xff);
+}
+
+static int set_dpll_mclk_fs(struct adv748x_state *state, int fs)
+{
+ return dpll_clrset(state, ADV748X_DPLL_MCLK_FS,
+ ADV748X_DPLL_MCLK_FS_N_MASK, (fs / 128) - 1);
+}
+
+static int set_i2s_format(struct adv748x_state *state, uint outmode,
+ uint bitwidth)
+{
+ return hdmi_clrset(state, ADV748X_HDMI_I2S,
+ ADV748X_HDMI_I2SBITWIDTH_MASK |
+ ADV748X_HDMI_I2SOUTMODE_MASK,
+ (outmode << ADV748X_HDMI_I2SOUTMODE_SHIFT) |
+ bitwidth);
+}
+
+static int set_i2s_tdm_mode(struct adv748x_state *state, int is_tdm)
+{
+ int ret;
+
+ ret = hdmi_clrset(state, ADV748X_HDMI_AUDIO_MUTE_SPEED,
+ ADV748X_MAN_AUDIO_DL_BYPASS |
+ ADV748X_AUDIO_DELAY_LINE_BYPASS,
+ is_tdm ? 0xff : 0);
+ if (ret < 0)
+ return ret;
+ ret = hdmi_clrset(state, ADV748X_HDMI_REG_6D,
+ ADV748X_I2S_TDM_MODE_ENABLE,
+ is_tdm ? 0xff : 0);
+ return ret;
+}
+
+static int set_audio_mute(struct adv748x_state *state, int enable)
+{
+ return hdmi_clrset(state, ADV748X_HDMI_MUTE_CTRL,
+ ADV748X_HDMI_MUTE_CTRL_MUTE_AUDIO,
+ enable ? 0xff : 0);
+}
+
+static int adv748x_dai_set_sysclk(struct snd_soc_dai *dai,
+ int clk_id, unsigned int freq, int dir)
+{
+ struct adv748x_state *state = state_of(dai);
+
+ /* currently supporting only one fixed rate clock */
+ if (clk_id != 0 || freq != clk_get_rate(mclk_of(state))) {
+ dev_err(dai->dev, "invalid clock (%d) or frequency (%u, dir %d)\n",
+ clk_id, freq, dir);
+ return -EINVAL;
+ }
+ state->dai.freq = freq;
+ return 0;
+}
+
+static int adv748x_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+ struct adv748x_state *state = state_of(dai);
+ int ret = 0;
+
+ if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM) {
+ dev_err(dai->dev, "only I2S master clock mode supported\n");
+ ret = -EINVAL;
+ goto done;
+ }
+ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAI_FORMAT_I2S:
+ state->dai.tdm = 0;
+ state->dai.fmt = ADV748X_I2SOUTMODE_I2S;
+ break;
+ case SND_SOC_DAI_FORMAT_RIGHT_J:
+ state->dai.tdm = 1;
+ state->dai.fmt = ADV748X_I2SOUTMODE_RIGHT_J;
+ break;
+ case SND_SOC_DAI_FORMAT_LEFT_J:
+ state->dai.tdm = 1;
+ state->dai.fmt = ADV748X_I2SOUTMODE_LEFT_J;
+ break;
+ default:
+ dev_err(dai->dev, "only i2s, left_j and right_j supported\n");
+ ret = -EINVAL;
+ goto done;
+ }
+ if ((fmt & SND_SOC_DAIFMT_INV_MASK) != SND_SOC_DAIFMT_NB_NF) {
+ dev_err(dai->dev, "only normal bit clock + frame supported\n");
+ ret = -EINVAL;
+ }
+done:
+ return ret;
+}
+
+static int adv748x_dai_startup(struct snd_pcm_substream *sub, struct snd_soc_dai *dai)
+{
+ struct adv748x_state *state = state_of(dai);
+
+ if (sub->stream != SNDRV_PCM_STREAM_CAPTURE)
+ return -EINVAL;
+ return set_audio_pads_state(state, 1);
+}
+
+static int adv748x_dai_hw_params(struct snd_pcm_substream *sub,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ int ret;
+ struct adv748x_state *state = state_of(dai);
+ uint fs = state->dai.freq / params_rate(params);
+
+ dev_dbg(dai->dev, "dai %s substream %s rate=%u (fs=%u), channels=%u sample width=%u(%u)\n",
+ dai->name, sub->name,
+ params_rate(params), fs,
+ params_channels(params),
+ params_width(params),
+ params_physical_width(params));
+ switch (fs) {
+ case 128:
+ case 256:
+ case 384:
+ case 512:
+ case 640:
+ case 768:
+ break;
+ default:
+ ret = -EINVAL;
+ dev_err(dai->dev, "invalid clock frequency (%u) or rate (%u)\n",
+ state->dai.freq, params_rate(params));
+ goto done;
+ }
+ ret = set_dpll_mclk_fs(state, fs);
+ if (ret)
+ goto done;
+ ret = set_i2s_tdm_mode(state, state->dai.tdm);
+ if (ret)
+ goto done;
+ ret = set_i2s_format(state, state->dai.fmt, params_width(params));
+done:
+ return ret;
+}
+
+static int adv748x_dai_mute_stream(struct snd_soc_dai *dai, int mute, int dir)
+{
+ struct adv748x_state *state = state_of(dai);
+
+ return set_audio_mute(state, mute);
+}
+
+static void adv748x_dai_shutdown(struct snd_pcm_substream *sub, struct snd_soc_dai *dai)
+{
+ struct adv748x_state *state = state_of(dai);
+
+ set_audio_pads_state(state, 0);
+}
+
+static const struct snd_soc_dai_ops adv748x_dai_ops = {
+ .set_sysclk = adv748x_dai_set_sysclk,
+ .set_fmt = adv748x_dai_set_fmt,
+ .startup = adv748x_dai_startup,
+ .hw_params = adv748x_dai_hw_params,
+ .mute_stream = adv748x_dai_mute_stream,
+ .shutdown = adv748x_dai_shutdown,
+};
+
+static int adv748x_of_xlate_dai_name(struct snd_soc_component *component,
+ struct of_phandle_args *args,
+ const char **dai_name)
+{
+ if (dai_name)
+ *dai_name = ADV748X_DAI_NAME;
+ return 0;
+}
+
+static const struct snd_soc_component_driver adv748x_codec = {
+ .of_xlate_dai_name = adv748x_of_xlate_dai_name,
+};
+
+int adv748x_dai_init(struct adv748x_dai *dai)
+{
+ int ret;
+ struct adv748x_state *state = adv748x_dai_to_state(dai);
+
+ dai->mclk_name = kasprintf(GFP_KERNEL, "%s.%s-i2s-mclk",
+ state->dev->driver->name,
+ dev_name(state->dev));
+ if (!dai->mclk_name) {
+ ret = -ENOMEM;
+ adv_err(state, "No memory for MCLK\n");
+ goto fail;
+ }
+ dai->mclk_hw = clk_hw_register_fixed_rate(state->dev, dai->mclk_name,
+ NULL, 0, 12288000);
+ if (IS_ERR(dai->mclk_hw)) {
+ ret = PTR_ERR(dai->mclk_hw);
+ adv_err(state, "Failed to register MCLK (%d)\n", ret);
+ goto fail;
+ }
+ ret = of_clk_add_hw_provider(state->dev->of_node, of_clk_hw_simple_get,
+ dai->mclk_hw->clk);
+ if (ret < 0) {
+ adv_err(state, "Failed to add MCLK provider (%d)\n", ret);
+ goto unreg_mclk;
+ }
+ dai->drv.name = ADV748X_DAI_NAME;
+ dai->drv.ops = &adv748x_dai_ops;
+ dai->drv.capture = (const struct snd_soc_pcm_stream){
+ .stream_name = "Capture",
+ .channels_min = 8,
+ .channels_max = 8,
+ .rates = SNDRV_PCM_RATE_48000,
+ .formats = SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_U24_LE,
+ };
+
+ ret = devm_snd_soc_register_component(state->dev, &adv748x_codec,
+ &dai->drv, 1);
+ if (ret < 0) {
+ adv_err(state, "Failed to register the codec (%d)\n", ret);
+ goto cleanup_mclk;
+ }
+ return 0;
+
+cleanup_mclk:
+ of_clk_del_provider(state->dev->of_node);
+unreg_mclk:
+ clk_hw_unregister_fixed_rate(dai->mclk_hw);
+fail:
+ return ret;
+}
+
+void adv748x_dai_cleanup(struct adv748x_dai *dai)
+{
+ struct adv748x_state *state = adv748x_dai_to_state(dai);
+
+ of_clk_del_provider(state->dev->of_node);
+ clk_hw_unregister_fixed_rate(dai->mclk_hw);
+ kfree(dai->mclk_name);
+}
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index 1a1ea70086c6..454f97ff7b54 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -19,6 +19,7 @@
*/

#include <linux/i2c.h>
+#include <sound/soc.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>

@@ -63,7 +64,8 @@ enum adv748x_ports {
ADV748X_PORT_TTL = 9,
ADV748X_PORT_TXA = 10,
ADV748X_PORT_TXB = 11,
- ADV748X_PORT_MAX = 12,
+ ADV748X_PORT_I2S = 12,
+ ADV748X_PORT_MAX = 13,
};

enum adv748x_csi2_pads {
@@ -166,6 +168,13 @@ struct adv748x_afe {
container_of(ctrl->handler, struct adv748x_afe, ctrl_hdl)
#define adv748x_sd_to_afe(sd) container_of(sd, struct adv748x_afe, sd)

+struct adv748x_dai {
+ struct snd_soc_dai_driver drv;
+ struct clk_hw *mclk_hw;
+ char *mclk_name;
+ unsigned int freq, fmt, tdm;
+};
+
/**
* struct adv748x_state - State of ADV748X
* @dev: (OF) device
@@ -182,6 +191,7 @@ struct adv748x_afe {
* @afe: state of AFE receiver context
* @txa: state of TXA transmitter context
* @txb: state of TXB transmitter context
+ * @mclk: MCLK clock of the I2S port
*/
struct adv748x_state {
struct device *dev;
@@ -197,10 +207,12 @@ struct adv748x_state {
struct adv748x_afe afe;
struct adv748x_csi2 txa;
struct adv748x_csi2 txb;
+ struct adv748x_dai dai;
};

#define adv748x_hdmi_to_state(h) container_of(h, struct adv748x_state, hdmi)
#define adv748x_afe_to_state(a) container_of(a, struct adv748x_state, afe)
+#define adv748x_dai_to_state(p) container_of(p, struct adv748x_state, dai)

#define adv_err(a, fmt, arg...) dev_err(a->dev, fmt, ##arg)
#define adv_info(a, fmt, arg...) dev_info(a->dev, fmt, ##arg)
@@ -484,4 +496,7 @@ int adv748x_csi2_set_pixelrate(struct v4l2_subdev *sd, s64 rate);
int adv748x_hdmi_init(struct adv748x_hdmi *hdmi);
void adv748x_hdmi_cleanup(struct adv748x_hdmi *hdmi);

+int adv748x_dai_init(struct adv748x_dai *);
+void adv748x_dai_cleanup(struct adv748x_dai *);
+
#endif /* _ADV748X_H_ */
--
2.25.1.25.g9ecbe7eb18


2020-03-30 08:34:46

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v4 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

Hi Alex,

On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen
<[email protected]> wrote:
> As all known variants of the Salvator board have the HDMI decoder
> chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482
> endpoint and the connection definitions are placed in the common board
> file.
>
> For the same reason, the CLK_C clock line and I2C configuration (similar
> to the ak4613, on the same interface) are added into the common file.
>
> Signed-off-by: Alexander Riesen <[email protected]>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -460,7 +460,7 @@ pca9654: gpio@20 {
> #gpio-cells = <2>;
> };
>
> - video-receiver@70 {
> + adv7482_hdmi_in: video-receiver@70 {
> compatible = "adi,adv7482";
> reg = <0x70 0x71 0x72 0x73 0x74 0x75
> 0x60 0x61 0x62 0x63 0x64 0x65>;
> @@ -469,6 +469,7 @@ video-receiver@70 {
>
> #address-cells = <1>;
> #size-cells = <0>;
> + #clock-cells = <0>; /* the MCLK for I2S output */
>
> interrupt-parent = <&gpio6>;
> interrupt-names = "intrq1", "intrq2";
> @@ -510,6 +511,15 @@ adv7482_txb: endpoint {
> remote-endpoint = <&csi20_in>;
> };
> };
> +
> + port@c {
> + reg = <12>;
> +
> + adv7482_i2s: endpoint {
> + remote-endpoint = <&rsnd_endpoint3>;
> + system-clock-direction-out;
> + };
> + };

As the adv748x driver just ignores "invalid" endpoints...

> @@ -733,8 +744,8 @@ &rcar_sound {
> pinctrl-0 = <&sound_pins &sound_clk_pins>;
> pinctrl-names = "default";
>
> - /* Single DAI */
> - #sound-dai-cells = <0>;
> + /* multi DAI */
> + #sound-dai-cells = <1>;
>
> /* audio_clkout0/1/2/3 */
> #clock-cells = <1>;
> @@ -758,8 +769,19 @@ &rcar_sound {
> <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
> <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
> <&audio_clk_a>, <&cs2000>,
> - <&audio_clk_c>,
> + <&adv7482_hdmi_in>,
> <&cpg CPG_CORE CPG_AUDIO_CLK_I>;

... and the rsnd driver ignores nonexistent-clocks, the DT change has no
hard dependency on the driver change, and won't introduce regressions
when included, right?

> @@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint {
> capture = <&ssi1 &src1 &dvc1>;
> };
> };
> + rsnd_port3: port@3 {
> + reg = <3>;
> + rsnd_endpoint3: endpoint {
> + remote-endpoint = <&adv7482_i2s>;
> +
> + dai-tdm-slot-num = <8>;
> + dai-tdm-slot-width = <32>;
> + dai-format = "left_j";
> + mclk-fs = <256>;
> + bitclock-master = <&adv7482_i2s>;
> + frame-master = <&adv7482_i2s>;
> +
> + capture = <&ssi4>;
> + };
> + };
> };
> };

However, as salvator-common.dtsi is shared by all Salvator-X(S) variants,
you'll have to add a dummy ssi4 node to r8a77961.dtsi first.

Gr{oetje,eeting}s,

Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2020-04-02 15:04:35

by Alex Riesen

[permalink] [raw]
Subject: Re: [PATCH v4 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

Hi Geert,

I'm sorry for late reply. Some unrelated happenings here in south Germany.

Geert Uytterhoeven, Mon, Mar 30, 2020 10:32:47 +0200:
> On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen <[email protected]> wrote:
> > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > @@ -510,6 +511,15 @@ adv7482_txb: endpoint {
> > remote-endpoint = <&csi20_in>;
> > };
> > };
> > +
> > + port@c {
> > + reg = <12>;
> > +
> > + adv7482_i2s: endpoint {
> > + remote-endpoint = <&rsnd_endpoint3>;
> > + system-clock-direction-out;
> > + };
> > + };
>
> As the adv748x driver just ignores "invalid" endpoints...
>
> > @@ -758,8 +769,19 @@ &rcar_sound {
> > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
> > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
> > <&audio_clk_a>, <&cs2000>,
> > - <&audio_clk_c>,
> > + <&adv7482_hdmi_in>,
> > <&cpg CPG_CORE CPG_AUDIO_CLK_I>;
>
> ... and the rsnd driver ignores nonexistent-clocks, the DT change has no
> hard dependency on the driver change, and won't introduce regressions
> when included, right?

Well, it maybe won't, but isn't it a little ... implicit?
And I'm no haste to include the changes, if you mean I can (or should) submit
the device tree patch separately.

> > @@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint {
> > capture = <&ssi1 &src1 &dvc1>;
> > };
> > };
> > + rsnd_port3: port@3 {
> > + reg = <3>;
> > + rsnd_endpoint3: endpoint {
> > + remote-endpoint = <&adv7482_i2s>;
> > +
> > + dai-tdm-slot-num = <8>;
> > + dai-tdm-slot-width = <32>;
> > + dai-format = "left_j";
> > + mclk-fs = <256>;
> > + bitclock-master = <&adv7482_i2s>;
> > + frame-master = <&adv7482_i2s>;
> > +
> > + capture = <&ssi4>;
> > + };
> > + };
> > };
> > };
>
> However, as salvator-common.dtsi is shared by all Salvator-X(S) variants,
> you'll have to add a dummy ssi4 node to r8a77961.dtsi first.

I see. There are even two dummy SSI nodes already. I would prefer to submit
the change together with other Salvator device tree changes. Is that alright?

Regards,
Alex

2020-04-02 15:27:37

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v4 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

Hi Alex,

On Thu, Apr 2, 2020 at 5:03 PM Alex Riesen <[email protected]> wrote:
> Geert Uytterhoeven, Mon, Mar 30, 2020 10:32:47 +0200:
> > On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen <[email protected]> wrote:
> > > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > > @@ -510,6 +511,15 @@ adv7482_txb: endpoint {
> > > remote-endpoint = <&csi20_in>;
> > > };
> > > };
> > > +
> > > + port@c {
> > > + reg = <12>;
> > > +
> > > + adv7482_i2s: endpoint {
> > > + remote-endpoint = <&rsnd_endpoint3>;
> > > + system-clock-direction-out;
> > > + };
> > > + };
> >
> > As the adv748x driver just ignores "invalid" endpoints...
> >
> > > @@ -758,8 +769,19 @@ &rcar_sound {
> > > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
> > > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
> > > <&audio_clk_a>, <&cs2000>,
> > > - <&audio_clk_c>,
> > > + <&adv7482_hdmi_in>,
> > > <&cpg CPG_CORE CPG_AUDIO_CLK_I>;
> >
> > ... and the rsnd driver ignores nonexistent-clocks, the DT change has no
> > hard dependency on the driver change, and won't introduce regressions
> > when included, right?
>
> Well, it maybe won't, but isn't it a little ... implicit?
> And I'm no haste to include the changes, if you mean I can (or should) submit
> the device tree patch separately.

OK, fine for me to postpone (that'll be for v5.9, I guess?).

> > > @@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint {
> > > capture = <&ssi1 &src1 &dvc1>;
> > > };
> > > };
> > > + rsnd_port3: port@3 {
> > > + reg = <3>;
> > > + rsnd_endpoint3: endpoint {
> > > + remote-endpoint = <&adv7482_i2s>;
> > > +
> > > + dai-tdm-slot-num = <8>;
> > > + dai-tdm-slot-width = <32>;
> > > + dai-format = "left_j";
> > > + mclk-fs = <256>;
> > > + bitclock-master = <&adv7482_i2s>;
> > > + frame-master = <&adv7482_i2s>;
> > > +
> > > + capture = <&ssi4>;
> > > + };
> > > + };
> > > };
> > > };
> >
> > However, as salvator-common.dtsi is shared by all Salvator-X(S) variants,
> > you'll have to add a dummy ssi4 node to r8a77961.dtsi first.
>
> I see. There are even two dummy SSI nodes already. I would prefer to submit
> the change together with other Salvator device tree changes. Is that alright?

Fine for me.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2020-04-02 16:45:35

by Alex Riesen

[permalink] [raw]
Subject: Re: [PATCH v4 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

Geert Uytterhoeven, Thu, Apr 02, 2020 17:26:15 +0200:
> On Thu, Apr 2, 2020 at 5:03 PM Alex Riesen <[email protected]> wrote:
> > Geert Uytterhoeven, Mon, Mar 30, 2020 10:32:47 +0200:
> > > On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen <[email protected]> wrote:
> > > > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > > > @@ -510,6 +511,15 @@ adv7482_txb: endpoint {
> > > > remote-endpoint = <&csi20_in>;
> > > > };
> > > > };
> > > > +
> > > > + port@c {
> > > > + reg = <12>;
> > > > +
> > > > + adv7482_i2s: endpoint {
> > > > + remote-endpoint = <&rsnd_endpoint3>;
> > > > + system-clock-direction-out;
> > > > + };
> > > > + };
> > >
> > > As the adv748x driver just ignores "invalid" endpoints...
> > >
> > > > @@ -758,8 +769,19 @@ &rcar_sound {
> > > > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
> > > > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
> > > > <&audio_clk_a>, <&cs2000>,
> > > > - <&audio_clk_c>,
> > > > + <&adv7482_hdmi_in>,
> > > > <&cpg CPG_CORE CPG_AUDIO_CLK_I>;
> > >
> > > ... and the rsnd driver ignores nonexistent-clocks, the DT change has no
> > > hard dependency on the driver change, and won't introduce regressions
> > > when included, right?
> >
> > Well, it maybe won't, but isn't it a little ... implicit?
> > And I'm no haste to include the changes, if you mean I can (or should) submit
> > the device tree patch separately.
>
> OK, fine for me to postpone (that'll be for v5.9, I guess?).
>

Looks scary :)
But yes, fine with me too.

v5 with ssi4 dummy in a moment.

Regards,
Alex