This patch series supports HDMI jack reporting on RK3288, which uses
DRM dw-hdmi driver and hdmi-codec codec driver.
The previous discussion about reporting jack status using hdmi-notifier
and drm_audio_component is at
https://lore.kernel.org/patchwork/patch/1083027/
The new approach is to use a callback mechanism that is
specific to hdmi-codec.
The dependent change on hdmi-codec.c
https://patchwork.kernel.org/patch/11047447
has been picked up by Mark Brown in ASoC tree for-5.4 branch.
Changes from v6 to v7:
1. rockchip_max98090: Use phandle of HDMI from DTS to find
codec_dai. With this we don't need to set fixed id for the
created hdmi-audio-codec device.
Cheng-Yi Chiang (4):
drm: bridge: dw-hdmi: Report connector status using callback
ASoC: rockchip-max98090: Add description for rockchip,hdmi-codec
ASoC: rockchip_max98090: Add dai_link for HDMI
ASoC: rockchip_max98090: Add HDMI jack support
.../bindings/sound/rockchip-max98090.txt | 2 +
.../boot/dts/rk3288-veyron-analog-audio.dtsi | 1 +
.../drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 11 ++
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 41 ++++-
include/drm/bridge/dw_hdmi.h | 4 +
sound/soc/rockchip/Kconfig | 3 +-
sound/soc/rockchip/rockchip_max98090.c | 149 ++++++++++++++----
7 files changed, 182 insertions(+), 29 deletions(-)
--
2.23.0.237.gc6a4ce50a0-goog
Allow codec driver register callback function for plug event.
The callback registration flow:
dw-hdmi <--- hw-hdmi-i2s-audio <--- hdmi-codec
dw-hdmi-i2s-audio implements hook_plugged_cb op
so codec driver can register the callback.
dw-hdmi exports a function dw_hdmi_set_plugged_cb so platform device
can register the callback.
When connector plug/unplug event happens, report this event using the
callback.
Make sure that audio and drm are using the single source of truth for
connector status.
Signed-off-by: Cheng-Yi Chiang <[email protected]>
---
.../drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 11 +++++
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 41 ++++++++++++++++++-
include/drm/bridge/dw_hdmi.h | 4 ++
3 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
index 20f4f92dd866..d7e65c869415 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
@@ -160,12 +160,23 @@ static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component,
return -EINVAL;
}
+static int dw_hdmi_i2s_hook_plugged_cb(struct device *dev, void *data,
+ hdmi_codec_plugged_cb fn,
+ struct device *codec_dev)
+{
+ struct dw_hdmi_i2s_audio_data *audio = data;
+ struct dw_hdmi *hdmi = audio->hdmi;
+
+ return dw_hdmi_set_plugged_cb(hdmi, fn, codec_dev);
+}
+
static struct hdmi_codec_ops dw_hdmi_i2s_ops = {
.hw_params = dw_hdmi_i2s_hw_params,
.audio_startup = dw_hdmi_i2s_audio_startup,
.audio_shutdown = dw_hdmi_i2s_audio_shutdown,
.get_eld = dw_hdmi_i2s_get_eld,
.get_dai_id = dw_hdmi_i2s_get_dai_id,
+ .hook_plugged_cb = dw_hdmi_i2s_hook_plugged_cb,
};
static int snd_dw_hdmi_probe(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index aa7efd4da1c8..7ffe8ed675ff 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -193,6 +193,10 @@ struct dw_hdmi {
struct mutex cec_notifier_mutex;
struct cec_notifier *cec_notifier;
+
+ hdmi_codec_plugged_cb plugged_cb;
+ struct device *codec_dev;
+ enum drm_connector_status last_connector_result;
};
#define HDMI_IH_PHY_STAT0_RX_SENSE \
@@ -217,6 +221,28 @@ static inline u8 hdmi_readb(struct dw_hdmi *hdmi, int offset)
return val;
}
+static void handle_plugged_change(struct dw_hdmi *hdmi, bool plugged)
+{
+ if (hdmi->plugged_cb && hdmi->codec_dev)
+ hdmi->plugged_cb(hdmi->codec_dev, plugged);
+}
+
+int dw_hdmi_set_plugged_cb(struct dw_hdmi *hdmi, hdmi_codec_plugged_cb fn,
+ struct device *codec_dev)
+{
+ bool plugged;
+
+ mutex_lock(&hdmi->mutex);
+ hdmi->plugged_cb = fn;
+ hdmi->codec_dev = codec_dev;
+ plugged = hdmi->last_connector_result == connector_status_connected;
+ handle_plugged_change(hdmi, plugged);
+ mutex_unlock(&hdmi->mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_set_plugged_cb);
+
static void hdmi_modb(struct dw_hdmi *hdmi, u8 data, u8 mask, unsigned reg)
{
regmap_update_bits(hdmi->regm, reg << hdmi->reg_shift, mask, data);
@@ -2183,6 +2209,7 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
{
struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
connector);
+ enum drm_connector_status result;
mutex_lock(&hdmi->mutex);
hdmi->force = DRM_FORCE_UNSPECIFIED;
@@ -2190,7 +2217,18 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
dw_hdmi_update_phy_mask(hdmi);
mutex_unlock(&hdmi->mutex);
- return hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
+ result = hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
+
+ mutex_lock(&hdmi->mutex);
+ if (result != hdmi->last_connector_result) {
+ dev_dbg(hdmi->dev, "read_hpd result: %d", result);
+ handle_plugged_change(hdmi,
+ result == connector_status_connected);
+ hdmi->last_connector_result = result;
+ }
+ mutex_unlock(&hdmi->mutex);
+
+ return result;
}
static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
@@ -2641,6 +2679,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
hdmi->rxsense = true;
hdmi->phy_mask = (u8)~(HDMI_PHY_HPD | HDMI_PHY_RX_SENSE);
hdmi->mc_clkdis = 0x7f;
+ hdmi->last_connector_result = connector_status_disconnected;
mutex_init(&hdmi->mutex);
mutex_init(&hdmi->audio_mutex);
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 4b3e863c4f8a..45a05e97e78a 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -6,6 +6,8 @@
#ifndef __DW_HDMI__
#define __DW_HDMI__
+#include <sound/hdmi-codec.h>
+
struct drm_connector;
struct drm_display_mode;
struct drm_encoder;
@@ -154,6 +156,8 @@ void dw_hdmi_resume(struct dw_hdmi *hdmi);
void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
+int dw_hdmi_set_plugged_cb(struct dw_hdmi *hdmi, hdmi_codec_plugged_cb fn,
+ struct device *codec_dev);
void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
void dw_hdmi_set_channel_count(struct dw_hdmi *hdmi, unsigned int cnt);
void dw_hdmi_set_channel_status(struct dw_hdmi *hdmi, u8 *channel_status);
--
2.23.0.237.gc6a4ce50a0-goog
Add support for HDMI codec in rockchip-max98090.
Let user specify HDMI device node in DTS so machine driver can find
hdmi-codec device node for codec DAI.
Signed-off-by: Cheng-Yi Chiang <[email protected]>
---
Documentation/devicetree/bindings/sound/rockchip-max98090.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/rockchip-max98090.txt b/Documentation/devicetree/bindings/sound/rockchip-max98090.txt
index a805aa99ad75..97fc838458f7 100644
--- a/Documentation/devicetree/bindings/sound/rockchip-max98090.txt
+++ b/Documentation/devicetree/bindings/sound/rockchip-max98090.txt
@@ -7,6 +7,7 @@ Required properties:
connected to the CODEC
- rockchip,audio-codec: The phandle of the MAX98090 audio codec
- rockchip,headset-codec: The phandle of Ext chip for jack detection
+- rockchip,hdmi-codec: The phandle of HDMI device for HDMI codec.
Example:
@@ -16,4 +17,5 @@ sound {
rockchip,i2s-controller = <&i2s>;
rockchip,audio-codec = <&max98090>;
rockchip,headset-codec = <&headsetcodec>;
+ rockchip,hdmi-codec = <&hdmi>;
};
--
2.23.0.237.gc6a4ce50a0-goog
Use two dai_links. One for HDMI and one for max98090.
With this setup, audio can play to speaker and HDMI selectively.
Signed-off-by: Cheng-Yi Chiang <[email protected]>
---
.../boot/dts/rk3288-veyron-analog-audio.dtsi | 1 +
sound/soc/rockchip/rockchip_max98090.c | 129 ++++++++++++++----
2 files changed, 103 insertions(+), 27 deletions(-)
diff --git a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
index 445270aa136e..51208d161d65 100644
--- a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
@@ -17,6 +17,7 @@
rockchip,hp-det-gpios = <&gpio6 RK_PA5 GPIO_ACTIVE_HIGH>;
rockchip,mic-det-gpios = <&gpio6 RK_PB3 GPIO_ACTIVE_LOW>;
rockchip,headset-codec = <&headsetcodec>;
+ rockchip,hdmi-codec = <&hdmi>;
};
};
diff --git a/sound/soc/rockchip/rockchip_max98090.c b/sound/soc/rockchip/rockchip_max98090.c
index c5fc24675a33..6c217492bb30 100644
--- a/sound/soc/rockchip/rockchip_max98090.c
+++ b/sound/soc/rockchip/rockchip_max98090.c
@@ -11,6 +11,7 @@
#include <linux/gpio.h>
#include <linux/of_gpio.h>
#include <sound/core.h>
+#include <sound/hdmi-codec.h>
#include <sound/jack.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
@@ -41,6 +42,7 @@ static const struct snd_soc_dapm_widget rk_dapm_widgets[] = {
SND_SOC_DAPM_MIC("Headset Mic", NULL),
SND_SOC_DAPM_MIC("Int Mic", NULL),
SND_SOC_DAPM_SPK("Speaker", NULL),
+ SND_SOC_DAPM_LINE("HDMI", NULL),
};
static const struct snd_soc_dapm_route rk_audio_map[] = {
@@ -52,6 +54,7 @@ static const struct snd_soc_dapm_route rk_audio_map[] = {
{"Headphone", NULL, "HPR"},
{"Speaker", NULL, "SPKL"},
{"Speaker", NULL, "SPKR"},
+ {"HDMI", NULL, "TX"},
};
static const struct snd_kcontrol_new rk_mc_controls[] = {
@@ -59,6 +62,7 @@ static const struct snd_kcontrol_new rk_mc_controls[] = {
SOC_DAPM_PIN_SWITCH("Headset Mic"),
SOC_DAPM_PIN_SWITCH("Int Mic"),
SOC_DAPM_PIN_SWITCH("Speaker"),
+ SOC_DAPM_PIN_SWITCH("HDMI"),
};
static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
@@ -92,38 +96,63 @@ static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
SND_SOC_CLOCK_OUT);
- if (ret < 0) {
- dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+ if (ret) {
+ dev_err(cpu_dai->dev, "Can't set cpu dai clock %d\n", ret);
return ret;
}
+ /* HDMI codec dai does not need to set sysclk. */
+ if (!strcmp(rtd->dai_link->name, "HDMI"))
+ return 0;
+
ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
SND_SOC_CLOCK_IN);
- if (ret < 0) {
- dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+ if (ret) {
+ dev_err(codec_dai->dev, "Can't set codec dai clock %d\n", ret);
return ret;
}
- return ret;
+ return 0;
}
static const struct snd_soc_ops rk_aif1_ops = {
.hw_params = rk_aif1_hw_params,
};
-SND_SOC_DAILINK_DEFS(hifi,
- DAILINK_COMP_ARRAY(COMP_EMPTY()),
- DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
- DAILINK_COMP_ARRAY(COMP_EMPTY()));
-
-static struct snd_soc_dai_link rk_dailink = {
- .name = "max98090",
- .stream_name = "Audio",
- .ops = &rk_aif1_ops,
- /* set max98090 as slave */
- .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
- SND_SOC_DAIFMT_CBS_CFS,
- SND_SOC_DAILINK_REG(hifi),
+SND_SOC_DAILINK_DEFS(analog,
+ DAILINK_COMP_ARRAY(COMP_EMPTY()),
+ DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
+ DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(hdmi,
+ DAILINK_COMP_ARRAY(COMP_EMPTY()),
+ DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "i2s-hifi")),
+ DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+enum {
+ DAILINK_MAX98090,
+ DAILINK_HDMI,
+};
+
+/* max98090 and HDMI codec dai_link */
+static struct snd_soc_dai_link rk_dailinks[] = {
+ [DAILINK_MAX98090] = {
+ .name = "max98090",
+ .stream_name = "Analog",
+ .ops = &rk_aif1_ops,
+ /* set max98090 as slave */
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBS_CFS,
+ SND_SOC_DAILINK_REG(analog),
+ },
+ [DAILINK_HDMI] = {
+ .name = "HDMI",
+ .stream_name = "HDMI",
+ .ops = &rk_aif1_ops,
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBS_CFS,
+ SND_SOC_DAILINK_REG(hdmi),
+ }
};
static int rk_98090_headset_init(struct snd_soc_component *component);
@@ -136,8 +165,8 @@ static struct snd_soc_aux_dev rk_98090_headset_dev = {
static struct snd_soc_card snd_soc_card_rk = {
.name = "ROCKCHIP-I2S",
.owner = THIS_MODULE,
- .dai_link = &rk_dailink,
- .num_links = 1,
+ .dai_link = rk_dailinks,
+ .num_links = ARRAY_SIZE(rk_dailinks),
.aux_dev = &rk_98090_headset_dev,
.num_aux_devs = 1,
.dapm_widgets = rk_dapm_widgets,
@@ -173,27 +202,73 @@ static int snd_rk_mc_probe(struct platform_device *pdev)
int ret = 0;
struct snd_soc_card *card = &snd_soc_card_rk;
struct device_node *np = pdev->dev.of_node;
+ struct device_node *np_analog;
+ struct device_node *np_cpu;
+ struct device_node *np_hdmi_codec;
+ struct of_phandle_args args;
/* register the soc card */
card->dev = &pdev->dev;
- rk_dailink.codecs->of_node = of_parse_phandle(np,
- "rockchip,audio-codec", 0);
- if (!rk_dailink.codecs->of_node) {
+ np_analog = of_parse_phandle(np, "rockchip,audio-codec", 0);
+ if (!np_analog) {
dev_err(&pdev->dev,
"Property 'rockchip,audio-codec' missing or invalid\n");
return -EINVAL;
}
+ rk_dailinks[DAILINK_MAX98090].codecs->of_node = np_analog;
+
+ ret = of_parse_phandle_with_fixed_args(np, "rockchip,audio-codec",
+ 0, 0, &args);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Unable to parse property 'rockchip,audio-codec'\n");
+ return ret;
+ }
+
+ ret = snd_soc_get_dai_name(
+ &args, &rk_dailinks[DAILINK_MAX98090].codecs->dai_name);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to get codec dai_name\n");
+ return ret;
+ }
+
+ np_cpu = of_parse_phandle(np, "rockchip,i2s-controller", 0);
- rk_dailink.cpus->of_node = of_parse_phandle(np,
- "rockchip,i2s-controller", 0);
- if (!rk_dailink.cpus->of_node) {
+ if (!np_cpu) {
dev_err(&pdev->dev,
"Property 'rockchip,i2s-controller' missing or invalid\n");
return -EINVAL;
}
- rk_dailink.platforms->of_node = rk_dailink.cpus->of_node;
+ np_hdmi_codec = of_parse_phandle(np, "rockchip,hdmi-codec", 0);
+ if (!np_hdmi_codec) {
+ dev_err(&pdev->dev,
+ "Property 'rockchip,hdmi-codec' missing or invalid\n");
+ return -EINVAL;
+ }
+
+ rk_dailinks[DAILINK_HDMI].codecs->of_node = np_hdmi_codec;
+
+ ret = of_parse_phandle_with_fixed_args(np, "rockchip,hdmi-codec",
+ 0, 0, &args);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Unable to parse property 'rockchip,hdmi-codec'\n");
+ return ret;
+ }
+
+ ret = snd_soc_get_dai_name(
+ &args, &rk_dailinks[DAILINK_HDMI].codecs->dai_name);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to get hdmi codec dai_name\n");
+ return ret;
+ }
+
+ rk_dailinks[DAILINK_MAX98090].cpus->of_node = np_cpu;
+ rk_dailinks[DAILINK_MAX98090].platforms->of_node = np_cpu;
+ rk_dailinks[DAILINK_HDMI].cpus->of_node = np_cpu;
+ rk_dailinks[DAILINK_HDMI].platforms->of_node = np_cpu;
rk_98090_headset_dev.codec_of_node = of_parse_phandle(np,
"rockchip,headset-codec", 0);
--
2.23.0.237.gc6a4ce50a0-goog
In machine driver, create a jack and let hdmi-codec report jack status.
Signed-off-by: Cheng-Yi Chiang <[email protected]>
---
sound/soc/rockchip/Kconfig | 3 ++-
sound/soc/rockchip/rockchip_max98090.c | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index b43657e6e655..d610b553ea3b 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -40,9 +40,10 @@ config SND_SOC_ROCKCHIP_MAX98090
select SND_SOC_ROCKCHIP_I2S
select SND_SOC_MAX98090
select SND_SOC_TS3A227E
+ select SND_SOC_HDMI_CODEC
help
Say Y or M here if you want to add support for SoC audio on Rockchip
- boards using the MAX98090 codec, such as Veyron.
+ boards using the MAX98090 codec and HDMI codec, such as Veyron.
config SND_SOC_ROCKCHIP_RT5645
tristate "ASoC support for Rockchip boards using a RT5645/RT5650 codec"
diff --git a/sound/soc/rockchip/rockchip_max98090.c b/sound/soc/rockchip/rockchip_max98090.c
index 6c217492bb30..11cf252e8391 100644
--- a/sound/soc/rockchip/rockchip_max98090.c
+++ b/sound/soc/rockchip/rockchip_max98090.c
@@ -134,6 +134,25 @@ enum {
DAILINK_HDMI,
};
+static struct snd_soc_jack rk_hdmi_jack;
+
+static int rk_hdmi_init(struct snd_soc_pcm_runtime *runtime)
+{
+ struct snd_soc_card *card = runtime->card;
+ struct snd_soc_component *component = runtime->codec_dai->component;
+ int ret;
+
+ /* enable jack detection */
+ ret = snd_soc_card_jack_new(card, "HDMI Jack", SND_JACK_LINEOUT,
+ &rk_hdmi_jack, NULL, 0);
+ if (ret) {
+ dev_err(card->dev, "Can't new HDMI Jack %d\n", ret);
+ return ret;
+ }
+
+ return hdmi_codec_set_jack_detect(component, &rk_hdmi_jack);
+}
+
/* max98090 and HDMI codec dai_link */
static struct snd_soc_dai_link rk_dailinks[] = {
[DAILINK_MAX98090] = {
@@ -151,6 +170,7 @@ static struct snd_soc_dai_link rk_dailinks[] = {
.ops = &rk_aif1_ops,
.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
SND_SOC_DAIFMT_CBS_CFS,
+ .init = rk_hdmi_init,
SND_SOC_DAILINK_REG(hdmi),
}
};
--
2.23.0.237.gc6a4ce50a0-goog
Hi!
Dne četrtek, 19. september 2019 ob 15:54:49 CEST je Cheng-Yi Chiang
napisal(a):
> Use two dai_links. One for HDMI and one for max98090.
> With this setup, audio can play to speaker and HDMI selectively.
>
> Signed-off-by: Cheng-Yi Chiang <[email protected]>
> ---
> .../boot/dts/rk3288-veyron-analog-audio.dtsi | 1 +
> sound/soc/rockchip/rockchip_max98090.c | 129 ++++++++++++++----
> 2 files changed, 103 insertions(+), 27 deletions(-)
>
> diff --git a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi index
> 445270aa136e..51208d161d65 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> +++ b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> @@ -17,6 +17,7 @@
> rockchip,hp-det-gpios = <&gpio6 RK_PA5
GPIO_ACTIVE_HIGH>;
> rockchip,mic-det-gpios = <&gpio6 RK_PB3
GPIO_ACTIVE_LOW>;
> rockchip,headset-codec = <&headsetcodec>;
> + rockchip,hdmi-codec = <&hdmi>;
> };
> };
>
> diff --git a/sound/soc/rockchip/rockchip_max98090.c
> b/sound/soc/rockchip/rockchip_max98090.c index c5fc24675a33..6c217492bb30
> 100644
> --- a/sound/soc/rockchip/rockchip_max98090.c
> +++ b/sound/soc/rockchip/rockchip_max98090.c
> @@ -11,6 +11,7 @@
> #include <linux/gpio.h>
> #include <linux/of_gpio.h>
> #include <sound/core.h>
> +#include <sound/hdmi-codec.h>
> #include <sound/jack.h>
> #include <sound/pcm.h>
> #include <sound/pcm_params.h>
> @@ -41,6 +42,7 @@ static const struct snd_soc_dapm_widget rk_dapm_widgets[]
> = { SND_SOC_DAPM_MIC("Headset Mic", NULL),
> SND_SOC_DAPM_MIC("Int Mic", NULL),
> SND_SOC_DAPM_SPK("Speaker", NULL),
> + SND_SOC_DAPM_LINE("HDMI", NULL),
> };
>
> static const struct snd_soc_dapm_route rk_audio_map[] = {
> @@ -52,6 +54,7 @@ static const struct snd_soc_dapm_route rk_audio_map[] = {
> {"Headphone", NULL, "HPR"},
> {"Speaker", NULL, "SPKL"},
> {"Speaker", NULL, "SPKR"},
> + {"HDMI", NULL, "TX"},
> };
>
> static const struct snd_kcontrol_new rk_mc_controls[] = {
> @@ -59,6 +62,7 @@ static const struct snd_kcontrol_new rk_mc_controls[] = {
> SOC_DAPM_PIN_SWITCH("Headset Mic"),
> SOC_DAPM_PIN_SWITCH("Int Mic"),
> SOC_DAPM_PIN_SWITCH("Speaker"),
> + SOC_DAPM_PIN_SWITCH("HDMI"),
> };
>
> static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
> @@ -92,38 +96,63 @@ static int rk_aif1_hw_params(struct snd_pcm_substream
> *substream,
>
> ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
> SND_SOC_CLOCK_OUT);
> - if (ret < 0) {
> - dev_err(codec_dai->dev, "Can't set codec clock %d\n",
ret);
> + if (ret) {
> + dev_err(cpu_dai->dev, "Can't set cpu dai clock %d\n",
ret);
> return ret;
> }
>
> + /* HDMI codec dai does not need to set sysclk. */
> + if (!strcmp(rtd->dai_link->name, "HDMI"))
> + return 0;
> +
> ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
> SND_SOC_CLOCK_IN);
> - if (ret < 0) {
> - dev_err(codec_dai->dev, "Can't set codec clock %d\n",
ret);
> + if (ret) {
> + dev_err(codec_dai->dev, "Can't set codec dai clock
%d\n", ret);
> return ret;
> }
>
> - return ret;
> + return 0;
> }
>
> static const struct snd_soc_ops rk_aif1_ops = {
> .hw_params = rk_aif1_hw_params,
> };
>
> -SND_SOC_DAILINK_DEFS(hifi,
> - DAILINK_COMP_ARRAY(COMP_EMPTY()),
> - DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
> - DAILINK_COMP_ARRAY(COMP_EMPTY()));
> -
> -static struct snd_soc_dai_link rk_dailink = {
> - .name = "max98090",
> - .stream_name = "Audio",
> - .ops = &rk_aif1_ops,
> - /* set max98090 as slave */
> - .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> - SND_SOC_DAIFMT_CBS_CFS,
> - SND_SOC_DAILINK_REG(hifi),
> +SND_SOC_DAILINK_DEFS(analog,
> + DAILINK_COMP_ARRAY(COMP_EMPTY()),
> + DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
> + DAILINK_COMP_ARRAY(COMP_EMPTY()));
> +
> +SND_SOC_DAILINK_DEFS(hdmi,
> + DAILINK_COMP_ARRAY(COMP_EMPTY()),
> + DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "i2s-hifi")),
> + DAILINK_COMP_ARRAY(COMP_EMPTY()));
> +
> +enum {
> + DAILINK_MAX98090,
> + DAILINK_HDMI,
> +};
> +
> +/* max98090 and HDMI codec dai_link */
> +static struct snd_soc_dai_link rk_dailinks[] = {
> + [DAILINK_MAX98090] = {
> + .name = "max98090",
> + .stream_name = "Analog",
> + .ops = &rk_aif1_ops,
> + /* set max98090 as slave */
> + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> + SND_SOC_DAIFMT_CBS_CFS,
> + SND_SOC_DAILINK_REG(analog),
> + },
> + [DAILINK_HDMI] = {
> + .name = "HDMI",
> + .stream_name = "HDMI",
> + .ops = &rk_aif1_ops,
> + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> + SND_SOC_DAIFMT_CBS_CFS,
> + SND_SOC_DAILINK_REG(hdmi),
> + }
> };
>
> static int rk_98090_headset_init(struct snd_soc_component *component);
> @@ -136,8 +165,8 @@ static struct snd_soc_aux_dev rk_98090_headset_dev = {
> static struct snd_soc_card snd_soc_card_rk = {
> .name = "ROCKCHIP-I2S",
> .owner = THIS_MODULE,
> - .dai_link = &rk_dailink,
> - .num_links = 1,
> + .dai_link = rk_dailinks,
> + .num_links = ARRAY_SIZE(rk_dailinks),
> .aux_dev = &rk_98090_headset_dev,
> .num_aux_devs = 1,
> .dapm_widgets = rk_dapm_widgets,
> @@ -173,27 +202,73 @@ static int snd_rk_mc_probe(struct platform_device
> *pdev) int ret = 0;
> struct snd_soc_card *card = &snd_soc_card_rk;
> struct device_node *np = pdev->dev.of_node;
> + struct device_node *np_analog;
> + struct device_node *np_cpu;
> + struct device_node *np_hdmi_codec;
> + struct of_phandle_args args;
>
> /* register the soc card */
> card->dev = &pdev->dev;
>
> - rk_dailink.codecs->of_node = of_parse_phandle(np,
> - "rockchip,audio-codec", 0);
> - if (!rk_dailink.codecs->of_node) {
> + np_analog = of_parse_phandle(np, "rockchip,audio-codec", 0);
> + if (!np_analog) {
> dev_err(&pdev->dev,
> "Property 'rockchip,audio-codec' missing or
invalid\n");
> return -EINVAL;
> }
> + rk_dailinks[DAILINK_MAX98090].codecs->of_node = np_analog;
> +
> + ret = of_parse_phandle_with_fixed_args(np, "rockchip,audio-codec",
> + 0, 0, &args);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "Unable to parse property 'rockchip,audio-
codec'\n");
> + return ret;
> + }
> +
> + ret = snd_soc_get_dai_name(
> + &args, &rk_dailinks[DAILINK_MAX98090].codecs-
>dai_name);
> + if (ret) {
> + dev_err(&pdev->dev, "Unable to get codec dai_name\n");
> + return ret;
> + }
> +
> + np_cpu = of_parse_phandle(np, "rockchip,i2s-controller", 0);
>
> - rk_dailink.cpus->of_node = of_parse_phandle(np,
> - "rockchip,i2s-controller", 0);
> - if (!rk_dailink.cpus->of_node) {
> + if (!np_cpu) {
> dev_err(&pdev->dev,
> "Property 'rockchip,i2s-controller' missing
or invalid\n");
> return -EINVAL;
> }
>
> - rk_dailink.platforms->of_node = rk_dailink.cpus->of_node;
> + np_hdmi_codec = of_parse_phandle(np, "rockchip,hdmi-codec", 0);
> + if (!np_hdmi_codec) {
> + dev_err(&pdev->dev,
> + "Property 'rockchip,hdmi-codec' missing or
invalid\n");
> + return -EINVAL;
> + }
Property "rockchip,hdmi-codec" is added in this series, right? You can't make
it mandatory, because kernel must be backward compatible with old device tree
files and they don't have this property.
Think about use case when user happily used this driver and after kernel
update, it suddenly stops working. You can't assume that board DTB file will be
updated along with kernel update.
Just make it optional and don't expose jack functionality if it's not present.
Best regards,
Jernej
> +
> + rk_dailinks[DAILINK_HDMI].codecs->of_node = np_hdmi_codec;
> +
> + ret = of_parse_phandle_with_fixed_args(np, "rockchip,hdmi-codec",
> + 0, 0, &args);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "Unable to parse property 'rockchip,hdmi-
codec'\n");
> + return ret;
> + }
> +
> + ret = snd_soc_get_dai_name(
> + &args, &rk_dailinks[DAILINK_HDMI].codecs-
>dai_name);
> + if (ret) {
> + dev_err(&pdev->dev, "Unable to get hdmi codec
dai_name\n");
> + return ret;
> + }
> +
> + rk_dailinks[DAILINK_MAX98090].cpus->of_node = np_cpu;
> + rk_dailinks[DAILINK_MAX98090].platforms->of_node = np_cpu;
> + rk_dailinks[DAILINK_HDMI].cpus->of_node = np_cpu;
> + rk_dailinks[DAILINK_HDMI].platforms->of_node = np_cpu;
>
> rk_98090_headset_dev.codec_of_node = of_parse_phandle(np,
> "rockchip,headset-codec", 0);
On Thu, Sep 19, 2019 at 11:08 PM Jernej Škrabec <[email protected]> wrote:
>
> Hi!
>
> Dne četrtek, 19. september 2019 ob 15:54:49 CEST je Cheng-Yi Chiang
> napisal(a):
> > Use two dai_links. One for HDMI and one for max98090.
> > With this setup, audio can play to speaker and HDMI selectively.
> >
> > Signed-off-by: Cheng-Yi Chiang <[email protected]>
> > ---
> > .../boot/dts/rk3288-veyron-analog-audio.dtsi | 1 +
> > sound/soc/rockchip/rockchip_max98090.c | 129 ++++++++++++++----
> > 2 files changed, 103 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> > b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi index
> > 445270aa136e..51208d161d65 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> > +++ b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi
> > @@ -17,6 +17,7 @@
> > rockchip,hp-det-gpios = <&gpio6 RK_PA5
> GPIO_ACTIVE_HIGH>;
> > rockchip,mic-det-gpios = <&gpio6 RK_PB3
> GPIO_ACTIVE_LOW>;
> > rockchip,headset-codec = <&headsetcodec>;
> > + rockchip,hdmi-codec = <&hdmi>;
> > };
> > };
> >
> > diff --git a/sound/soc/rockchip/rockchip_max98090.c
> > b/sound/soc/rockchip/rockchip_max98090.c index c5fc24675a33..6c217492bb30
> > 100644
> > --- a/sound/soc/rockchip/rockchip_max98090.c
> > +++ b/sound/soc/rockchip/rockchip_max98090.c
> > @@ -11,6 +11,7 @@
> > #include <linux/gpio.h>
> > #include <linux/of_gpio.h>
> > #include <sound/core.h>
> > +#include <sound/hdmi-codec.h>
> > #include <sound/jack.h>
> > #include <sound/pcm.h>
> > #include <sound/pcm_params.h>
> > @@ -41,6 +42,7 @@ static const struct snd_soc_dapm_widget rk_dapm_widgets[]
> > = { SND_SOC_DAPM_MIC("Headset Mic", NULL),
> > SND_SOC_DAPM_MIC("Int Mic", NULL),
> > SND_SOC_DAPM_SPK("Speaker", NULL),
> > + SND_SOC_DAPM_LINE("HDMI", NULL),
> > };
> >
> > static const struct snd_soc_dapm_route rk_audio_map[] = {
> > @@ -52,6 +54,7 @@ static const struct snd_soc_dapm_route rk_audio_map[] = {
> > {"Headphone", NULL, "HPR"},
> > {"Speaker", NULL, "SPKL"},
> > {"Speaker", NULL, "SPKR"},
> > + {"HDMI", NULL, "TX"},
> > };
> >
> > static const struct snd_kcontrol_new rk_mc_controls[] = {
> > @@ -59,6 +62,7 @@ static const struct snd_kcontrol_new rk_mc_controls[] = {
> > SOC_DAPM_PIN_SWITCH("Headset Mic"),
> > SOC_DAPM_PIN_SWITCH("Int Mic"),
> > SOC_DAPM_PIN_SWITCH("Speaker"),
> > + SOC_DAPM_PIN_SWITCH("HDMI"),
> > };
> >
> > static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
> > @@ -92,38 +96,63 @@ static int rk_aif1_hw_params(struct snd_pcm_substream
> > *substream,
> >
> > ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
> > SND_SOC_CLOCK_OUT);
> > - if (ret < 0) {
> > - dev_err(codec_dai->dev, "Can't set codec clock %d\n",
> ret);
> > + if (ret) {
> > + dev_err(cpu_dai->dev, "Can't set cpu dai clock %d\n",
> ret);
> > return ret;
> > }
> >
> > + /* HDMI codec dai does not need to set sysclk. */
> > + if (!strcmp(rtd->dai_link->name, "HDMI"))
> > + return 0;
> > +
> > ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
> > SND_SOC_CLOCK_IN);
> > - if (ret < 0) {
> > - dev_err(codec_dai->dev, "Can't set codec clock %d\n",
> ret);
> > + if (ret) {
> > + dev_err(codec_dai->dev, "Can't set codec dai clock
> %d\n", ret);
> > return ret;
> > }
> >
> > - return ret;
> > + return 0;
> > }
> >
> > static const struct snd_soc_ops rk_aif1_ops = {
> > .hw_params = rk_aif1_hw_params,
> > };
> >
> > -SND_SOC_DAILINK_DEFS(hifi,
> > - DAILINK_COMP_ARRAY(COMP_EMPTY()),
> > - DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
> > - DAILINK_COMP_ARRAY(COMP_EMPTY()));
> > -
> > -static struct snd_soc_dai_link rk_dailink = {
> > - .name = "max98090",
> > - .stream_name = "Audio",
> > - .ops = &rk_aif1_ops,
> > - /* set max98090 as slave */
> > - .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> > - SND_SOC_DAIFMT_CBS_CFS,
> > - SND_SOC_DAILINK_REG(hifi),
> > +SND_SOC_DAILINK_DEFS(analog,
> > + DAILINK_COMP_ARRAY(COMP_EMPTY()),
> > + DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")),
> > + DAILINK_COMP_ARRAY(COMP_EMPTY()));
> > +
> > +SND_SOC_DAILINK_DEFS(hdmi,
> > + DAILINK_COMP_ARRAY(COMP_EMPTY()),
> > + DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "i2s-hifi")),
> > + DAILINK_COMP_ARRAY(COMP_EMPTY()));
> > +
> > +enum {
> > + DAILINK_MAX98090,
> > + DAILINK_HDMI,
> > +};
> > +
> > +/* max98090 and HDMI codec dai_link */
> > +static struct snd_soc_dai_link rk_dailinks[] = {
> > + [DAILINK_MAX98090] = {
> > + .name = "max98090",
> > + .stream_name = "Analog",
> > + .ops = &rk_aif1_ops,
> > + /* set max98090 as slave */
> > + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> > + SND_SOC_DAIFMT_CBS_CFS,
> > + SND_SOC_DAILINK_REG(analog),
> > + },
> > + [DAILINK_HDMI] = {
> > + .name = "HDMI",
> > + .stream_name = "HDMI",
> > + .ops = &rk_aif1_ops,
> > + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> > + SND_SOC_DAIFMT_CBS_CFS,
> > + SND_SOC_DAILINK_REG(hdmi),
> > + }
> > };
> >
> > static int rk_98090_headset_init(struct snd_soc_component *component);
> > @@ -136,8 +165,8 @@ static struct snd_soc_aux_dev rk_98090_headset_dev = {
> > static struct snd_soc_card snd_soc_card_rk = {
> > .name = "ROCKCHIP-I2S",
> > .owner = THIS_MODULE,
> > - .dai_link = &rk_dailink,
> > - .num_links = 1,
> > + .dai_link = rk_dailinks,
> > + .num_links = ARRAY_SIZE(rk_dailinks),
> > .aux_dev = &rk_98090_headset_dev,
> > .num_aux_devs = 1,
> > .dapm_widgets = rk_dapm_widgets,
> > @@ -173,27 +202,73 @@ static int snd_rk_mc_probe(struct platform_device
> > *pdev) int ret = 0;
> > struct snd_soc_card *card = &snd_soc_card_rk;
> > struct device_node *np = pdev->dev.of_node;
> > + struct device_node *np_analog;
> > + struct device_node *np_cpu;
> > + struct device_node *np_hdmi_codec;
> > + struct of_phandle_args args;
> >
> > /* register the soc card */
> > card->dev = &pdev->dev;
> >
> > - rk_dailink.codecs->of_node = of_parse_phandle(np,
> > - "rockchip,audio-codec", 0);
> > - if (!rk_dailink.codecs->of_node) {
> > + np_analog = of_parse_phandle(np, "rockchip,audio-codec", 0);
> > + if (!np_analog) {
> > dev_err(&pdev->dev,
> > "Property 'rockchip,audio-codec' missing or
> invalid\n");
> > return -EINVAL;
> > }
> > + rk_dailinks[DAILINK_MAX98090].codecs->of_node = np_analog;
> > +
> > + ret = of_parse_phandle_with_fixed_args(np, "rockchip,audio-codec",
> > + 0, 0, &args);
> > + if (ret) {
> > + dev_err(&pdev->dev,
> > + "Unable to parse property 'rockchip,audio-
> codec'\n");
> > + return ret;
> > + }
> > +
> > + ret = snd_soc_get_dai_name(
> > + &args, &rk_dailinks[DAILINK_MAX98090].codecs-
> >dai_name);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Unable to get codec dai_name\n");
> > + return ret;
> > + }
> > +
> > + np_cpu = of_parse_phandle(np, "rockchip,i2s-controller", 0);
> >
> > - rk_dailink.cpus->of_node = of_parse_phandle(np,
> > - "rockchip,i2s-controller", 0);
> > - if (!rk_dailink.cpus->of_node) {
> > + if (!np_cpu) {
> > dev_err(&pdev->dev,
> > "Property 'rockchip,i2s-controller' missing
> or invalid\n");
> > return -EINVAL;
> > }
> >
> > - rk_dailink.platforms->of_node = rk_dailink.cpus->of_node;
> > + np_hdmi_codec = of_parse_phandle(np, "rockchip,hdmi-codec", 0);
> > + if (!np_hdmi_codec) {
> > + dev_err(&pdev->dev,
> > + "Property 'rockchip,hdmi-codec' missing or
> invalid\n");
> > + return -EINVAL;
> > + }
>
> Property "rockchip,hdmi-codec" is added in this series, right? You can't make
> it mandatory, because kernel must be backward compatible with old device tree
> files and they don't have this property.
>
> Think about use case when user happily used this driver and after kernel
> update, it suddenly stops working. You can't assume that board DTB file will be
> updated along with kernel update.
>
> Just make it optional and don't expose jack functionality if it's not present.
Hi Jernej,
Thanks for the reply.
I see. Yes I can make it optional.
But it will become a little bit messy for two types of usage to share
one machine driver.
I think I will create two instances of structs for
dapm widgets,
dapm routes,
kcontrols,
dai_links
for "max98090 only" and "max98090+hdmi"
and set those fields in snd_soc_card depending on depending on the property.
These two usages can still share most of the function calls.
Hope this looks clean.
If you have a cleaner way of sharing machine driver please let me know.
I'll post an update probably next week.
Thanks a lot!
>
> Best regards,
> Jernej
>
> > +
> > + rk_dailinks[DAILINK_HDMI].codecs->of_node = np_hdmi_codec;
> > +
> > + ret = of_parse_phandle_with_fixed_args(np, "rockchip,hdmi-codec",
> > + 0, 0, &args);
> > + if (ret) {
> > + dev_err(&pdev->dev,
> > + "Unable to parse property 'rockchip,hdmi-
> codec'\n");
> > + return ret;
> > + }
> > +
> > + ret = snd_soc_get_dai_name(
> > + &args, &rk_dailinks[DAILINK_HDMI].codecs-
> >dai_name);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Unable to get hdmi codec
> dai_name\n");
> > + return ret;
> > + }
> > +
> > + rk_dailinks[DAILINK_MAX98090].cpus->of_node = np_cpu;
> > + rk_dailinks[DAILINK_MAX98090].platforms->of_node = np_cpu;
> > + rk_dailinks[DAILINK_HDMI].cpus->of_node = np_cpu;
> > + rk_dailinks[DAILINK_HDMI].platforms->of_node = np_cpu;
> >
> > rk_98090_headset_dev.codec_of_node = of_parse_phandle(np,
> > "rockchip,headset-codec", 0);
>
>
>
>