tda998x: add sound support
cover tda998x: add sound support
This patch adds sound over HDMI feature in TDA998x. The work was inspired in a Linaro repository:
https://git.linaro.org/kernel/linux-linaro-tracking.git/tree/4b131c9bf8af9bc7a25441251f60a8ca7fc69c07
Joao Pinto (3):
tda998x: adding sound support
tda998: adding sound support for Juno in the DT
tda998: add HPD delay to avoid disabling sound when EDID checksum fails.
arch/arm64/boot/dts/arm/juno-base.dtsi | 92 ++++++-- drivers/gpu/drm/i2c/Makefile | 2 +- drivers/gpu/drm/i2c/tda998x_codec.c | 248 ++++++++++++++++++++++ drivers/gpu/drm/i2c/tda998x_drv.c | 115 +++++++--- drivers/gpu/drm/i2c/tda998x_drv.h | 41 ++++ include/drm/i2c/tda998x.h | 1 + 6 files changed, 453 insertions(+), 46 deletions(-) create mode 100644 drivers/gpu/drm/i2c/tda998x_codec.c create mode 100644 drivers/gpu/drm/i2c/tda998x_drv.h
Signature · 1 line
1.8.1.5
PATCH 1/3 tda998x: adding sound support
This patch adds the sound over HDMI feature to the TDA998x driver.
--- drivers/gpu/drm/i2c/Makefile | 2 +- drivers/gpu/drm/i2c/tda998x_codec.c | 248 +++++++++++++++++++++++++ drivers/gpu/drm/i2c/tda998x_drv.c | 103 ++++++---- drivers/gpu/drm/i2c/tda998x_drv.h | 41 ++++ include/drm/i2c/tda998x.h | 1 + 5 files changed, 363 insertions(+), 32 deletions(-) create mode 100644 drivers/gpu/drm/i2c/tda998x_codec.c create mode 100644 drivers/gpu/drm/i2c/tda998x_drv.h
drivers/gpu/drm/i2c/Makefile +1 -1
@@ -8,5 +8,5 @@ obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o sil164-y := sil164_drv.o obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o -tda998x-y := tda998x_drv.o +tda998x-y := tda998x_drv.o tda998x_codec.o obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o
drivers/gpu/drm/i2c/tda998x_codec.c new file +248 -0
248 lines not on this page · raw message
drivers/gpu/drm/i2c/tda998x_drv.c +62 -31
166 lines not on this page · raw message
drivers/gpu/drm/i2c/tda998x_drv.h new file +36 -0
@@ -0,0 +1,36 @@ +/* tda998x private data */ + +struct tda998x_priv { +struct i2c_client *cec; +struct i2c_client *hdmi; +struct mutex mutex; +struct delayed_work dwork; +uint16_t rev; +uint8_t current_page; +int dpms; +bool is_hdmi_sink; +u8 vip_cntrl_0; +u8 vip_cntrl_1; +u8 vip_cntrl_2; +struct tda998x_encoder_params params; +wait_queue_head_t wq_edid; +int wq_edid_wait; + +struct work_struct detect_work; +struct timer_list edid_delay_timer; +wait_queue_head_t edid_delay_waitq; +bool edid_delay_active; +struct drm_encoder encoder; +struct drm_connector connector; +u8 audio_ports[2 ]; +int audio_sample_format; +int dai_id;/* DAI ID when streaming active */ +u8 *eld; +struct snd_pcm_hw_constraint_list rate_constraints; +}; + +int tda998x_codec_register (struct device *dev); +void tda998x_codec_unregister (struct device *dev); + +void tda998x_audio_start (struct tda998x_priv *priv,int full); +void tda998x_audio_stop (struct tda998x_priv *priv);
include/drm/i2c/tda998x.h +1 -0
@@ -20,6 +20,7 @@ struct tda998x_encoder_params {u8 audio_frame[6 ];enum { +AFMT_NO_AUDIO =0 ,AFMT_SPDIF ,AFMT_I2S } audio_format;
Signature · 1 line
1.8.1.5
PATCH 2/3 tda998x: adding sound support for Juno in the DT
In reply to Joao PintoThis patch adds the sound over HDMI feature to the Juno Device Tree.
--- arch/arm64/boot/dts/arm/juno-base.dtsi | 92 +++++++++++++++++++---- 1 file changed, 78 insertions(+), 14 deletions(-)
arch/arm64/boot/dts/arm/juno-base.dtsi +78 -14
@@ -105,17 +105,17 @@ clocks { compatible = "arm,scpi-clocks"; - scpi_dvfs: scpi-dvfs { + scpi_dvfs: scpi_clocks@0 { compatible = "arm,scpi-dvfs-clocks"; #clock-cells = <1>; clock-indices = <0>, <1>, <2>; - clock-output-names = "atlclk", "aplclk","gpuclk"; + clock-output-names = "atlclk", "aplclk","clk_mali"; }; - scpi_clk: scpi-clk { + scpi_clk: scpi_clocks@3 { compatible = "arm,scpi-variable-clocks"; #clock-cells = <1>; - clock-indices = <3>; - clock-output-names = "pxlclk"; + clock-indices = <3>, <4>, <5>; + clock-output-names = "pxlclk0", "pxlclk1", "i2sclk"; }; };
@@ -127,7 +127,7 @@ /include/ "juno-clocks.dtsi" - dma@7ff00000 { + dma0: dma@7ff00000 { compatible = "arm,pl330", "arm,primecell"; reg = <0x0 0x7ff00000 0 0x1000>; #dma-cells = <1>;
@@ -144,8 +144,10 @@ <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>; clocks = <&soc_faxiclk>; clock-names = "apb_pclk"; + dmas = <&dma0 1>, <&dma0 2>; + dma-names = "rx", "tx"; }; - +/* hdlcd@7ff50000 { compatible = "arm,hdlcd"; reg = <0 0x7ff50000 0 0x1000>;
@@ -154,21 +156,21 @@ clock-names = "pxlclk"; port { - hdlcd1_output: hdlcd1-endpoint { + hdlcd1_output: endpoint@0 { remote-endpoint = <&tda998x_1_input>; }; }; }; - +*/ hdlcd@7ff60000 { compatible = "arm,hdlcd"; reg = <0 0x7ff60000 0 0x1000>; interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&scpi_clk 3>; + clocks = <&scpi_clk 4>; clock-names = "pxlclk"; port { - hdlcd0_output: hdlcd0-endpoint { + hdlcd0_output: endpoint@0 { remote-endpoint = <&tda998x_0_input>; }; };
@@ -192,13 +194,20 @@ i2c-sda-hold-time-ns = <500>; clocks = <&soc_smc50mhz>; - hdmi-transmitter@70 { + hdmi_transmitter0: hdmi-transmitter@70 { compatible = "nxp,tda998x"; reg = <0x70>; + audio-ports = <0x03>, <0x04>; + audio-port-names = "i2s", "spdif"; + #sound-dai-cells = <1>; port { - tda998x_0_input: tda998x-0-endpoint { + tda998x_0_input: endpoint@0 { remote-endpoint = <&hdlcd0_output>; }; + + tda998x_0_output: endpoint@1 { + remote-endpoint = <&hdmi0_connector_output>; + }; }; };
@@ -206,9 +215,64 @@ compatible = "nxp,tda998x"; reg = <0x71>; port { - tda998x_1_input: tda998x-1-endpoint { +/* tda998x_1_input: endpoint@0 { remote-endpoint = <&hdlcd1_output>; }; +*/ + tda998x_1_output: endpoint@1 { + remote-endpoint = <&hdmi1_connector_output>; + }; + }; + }; + }; + + soc_i2s: i2s@7ff90000 { + compatible = "snps,designware-i2s"; + reg = <0x0 0x7ff90000 0x0 0x1000>; + clocks = <&scpi_clk 5>, <&soc_refclk100mhz>; + clock-names = "i2sclk", "apb_pclk"; + #sound-dai-cells = <0>; + dmas = <&dma0 5>; + dma-names = "tx"; + }; + + hdmi_audio: hdmi_audio@0 { + compatible = "linux,hdmi-audio"; + #sound-dai-cells = <0>; + status = "okay"; + }; + + sound { + compatible = "simple-audio-card"; + + simple-audio-card,format = "i2s"; + + simple-audio-card,cpu { + sound-dai = <&soc_i2s>; + }; + + simple-audio-card,codec { + sound-dai = <&hdmi_transmitter0 0>; + }; + + }; + + hdmi0: connector@0 { + compatible = "hdmi-connector"; + type = "a"; + port { + hdmi0_connector_output: endpoint { + remote-endpoint = <&tda998x_0_output>; + }; + }; + }; + + hdmi1: connector@1 { + compatible = "hdmi-connector"; + type = "a"; + port { + hdmi1_connector_output: endpoint { + remote-endpoint = <&tda998x_1_output>; }; }; };
Signature · 1 line
1.8.1.5
PATCH 3/3 tda998x: add HPD delay to avoid disabling sound when EDID checksum fails.
In reply to Joao PintoWhen using ffplay to reproduce video+sound it was noticed that sometimes the sound was disabled. The cause was an initial EDID checksum error that disabled the HDMI sound. By adding this tweak, it was noticed that the sound is not even when initial EDID checksum error ocurres.
--- drivers/gpu/drm/i2c/tda998x_drv.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/gpu/drm/i2c/tda998x_drv.c +12 -0
@@ -587,6 +587,17 @@ static void tda998x_detect_work(struct work_struct *work)drm_kms_helper_hotplug_event (dev); } +/* handle HDMI connect/disconnect */ +static void tda998x_hpd (struct work_struct *work) +{ +struct delayed_work *dwork =to_delayed_work (work); +struct tda998x_priv *priv = + container_of(dwork, struct tda998x_priv, dwork); + +if (&priv->encoder && priv->encoder.dev) +drm_kms_helper_hotplug_event (priv->encoder.dev); +} +/* * only 2 interrupts may occur: screen plug/unplug and EDID read */
@@ -1313,6 +1324,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)/* init read EDID waitqueue and HDP work */ init_waitqueue_head (&priv->wq_edid); +INIT_DELAYED_WORK (&priv->dwork, tda998x_hpd);/* clear pending interrupts */ reg_read (priv,REG_INT_FLAGS_0 );
Signature · 1 line
1.8.1.5
Re: [PATCH 3/3] tda998x: add HPD delay to avoid disabling sound when EDID checksum fails.
Quoted 33 lines · Joao Pinto
When using ffplay to reproduce video+sound it was noticed that sometimes the sound was disabled. The cause was an initial EDID checksum error that disabled the HDMI sound. By adding this tweak, it was noticed that the sound is not even when initial EDID checksum error ocurres.
Signed-off-by Joao Pinto <[email protected]>--- drivers/gpu/drm/i2c/tda998x_drv.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/gpu/drm/i2c/tda998x_drv.c +11
-0@@ -587,6 +587,17 @@ static void tda998x_detect_work(struct work_struct *work)drm_kms_helper_hotplug_event (dev); } +/* handle HDMI connect/disconnect */ +static void tda998x_hpd (struct work_struct *work) +{ +struct delayed_work *dwork =to_delayed_work (work); +struct tda998x_priv *priv = + container_of(dwork, struct tda998x_priv, dwork); + +if (&priv->encoder && priv->encoder.dev) +drm_kms_helper_hotplug_event (priv->encoder.dev); +} +/* * only 2 interrupts may occur: screen plug/unplug and EDID read */
drivers/gpu/drm/i2c/tda998x_drv.c static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) · line 1330
Quoted 7 lines
drivers/gpu/drm/i2c/tda998x_drv.c +1
-0@@ -1313,6 +1324,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)/* init read EDID waitqueue and HDP work */ init_waitqueue_head (&priv->wq_edid); +INIT_DELAYED_WORK (&priv->dwork, tda998x_hpd);/* clear pending interrupts */ reg_read (priv,REG_INT_FLAGS_0 );
Clearly, this patch is incomplete. There's nothing that schedules this work to be run.
In any case, this is reintroducing the code which I deleted when I fixed the (rather crappy) previous implemention of delaying the EDID read after a hotplug event. You should not need this patch.
Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
Re: [PATCH 3/3] tda998x: add HPD delay to avoid disabling sound when EDID checksum fails.
Hi Russell,
Quoted 3 lines · Russell King - ARM Linux
Quoted 2 lines · Joao Pinto
When using ffplay to reproduce video+sound it was noticed that sometimes the sound was disabled. The cause was an initial EDID checksum error that disabled
(...)
Quoted 11 lines
drivers/gpu/drm/i2c/tda998x_drv.c static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) · line 1330
Quoted 8 lines
drivers/gpu/drm/i2c/tda998x_drv.c +1
-0@@ -1313,6 +1324,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)/* init read EDID waitqueue and HDP work */ init_waitqueue_head (&priv->wq_edid); +INIT_DELAYED_WORK (&priv->dwork, tda998x_hpd);/* clear pending interrupts */ reg_read (priv,REG_INT_FLAGS_0 );Clearly, this patch is incomplete. There's nothing that schedules this work to be run.
You are right, forgot to include the schedule in the patch!
Quoted 5 lines
In any case, this is reintroducing the code which I deleted when I fixed the (rather crappy) previous implemention of delaying the EDID read after a hotplug event. You should not need this patch.
If a checksum validation fails the video reproduction is done muted if you use a simple app like ffplay. This does not happen if using mplayer.
Could you please check the sound support patch?
Thanks,
Joao
Re: [PATCH 3/3] tda998x: add HPD delay to avoid disabling sound when EDID checksum fails.
Quoted 24 lines · Joao Pinto
Hi Russell,
Quoted 3 lines · Russell King - ARM Linux
Quoted 2 lines · Joao Pinto
When using ffplay to reproduce video+sound it was noticed that sometimes the sound was disabled. The cause was an initial EDID checksum error that disabled
(...)
Quoted 11 lines
drivers/gpu/drm/i2c/tda998x_drv.c static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) · line 1330
Quoted 8 lines
drivers/gpu/drm/i2c/tda998x_drv.c +1
-0@@ -1313,6 +1324,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)/* init read EDID waitqueue and HDP work */ init_waitqueue_head (&priv->wq_edid); +INIT_DELAYED_WORK (&priv->dwork, tda998x_hpd);/* clear pending interrupts */ reg_read (priv,REG_INT_FLAGS_0 );Clearly, this patch is incomplete. There's nothing that schedules this work to be run.
You are right, forgot to include the schedule in the patch!
Quoted 1 line
Quoted 7 lines
Quoted 4 lines
In any case, this is reintroducing the code which I deleted when I fixed the (rather crappy) previous implemention of delaying the EDID read after a hotplug event. You should not need this patch.
If a checksum validation fails the video reproduction is done muted if you use a simple app like ffplay. This does not happen if using mplayer.
So?
We already delay the EDID read to work around reading a corrupted EDID. If you need an additional delay, then it seems that the existing delay is not long enough, and maybe we should extend it a little more.
What we should not do is to delay the HPD signalling. That is definitely the wrong solution.
In any case, I'm having a hard time understanding what the problem here is. You've unplugged the connected HDMI sink, so there's no longer a display device connected, and the display hardware is shut down. The EDID becomes invalid. You then plug in a HDMI sink, and now you have a display device connected. The EDID is read, and the display hardware is configured according to the EDID details. Video output is resumed, and it takes a second or so for the sink to lock on and display the resulting video.
At what point does the problem occur?
Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
Re: [PATCH 3/3] tda998x: add HPD delay to avoid disabling sound when EDID checksum fails.
Quoted 30 lines · Russell King - ARM Linux
Quoted 29 lines · Joao Pinto
Hi Russell,
Quoted 3 lines · Russell King - ARM Linux
Quoted 2 lines · Joao Pinto
When using ffplay to reproduce video+sound it was noticed that sometimes the sound was disabled. The cause was an initial EDID checksum error that disabled
(...)
Quoted 11 lines
drivers/gpu/drm/i2c/tda998x_drv.c static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) · line 1330
Quoted 8 lines
drivers/gpu/drm/i2c/tda998x_drv.c +1
-0@@ -1313,6 +1324,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)/* init read EDID waitqueue and HDP work */ init_waitqueue_head (&priv->wq_edid); +INIT_DELAYED_WORK (&priv->dwork, tda998x_hpd);/* clear pending interrupts */ reg_read (priv,REG_INT_FLAGS_0 );Clearly, this patch is incomplete. There's nothing that schedules this work to be run.
You are right, forgot to include the schedule in the patch!
Quoted 5 lines
In any case, this is reintroducing the code which I deleted when I fixed the (rather crappy) previous implemention of delaying the EDID read after a hotplug event. You should not need this patch.
Quoted 8 lines
Quoted 2 lines
If a checksum validation fails the video reproduction is done muted if you use a simple app like ffplay. This does not happen if using mplayer.
So?
We already delay the EDID read to work around reading a corrupted EDID. If you need an additional delay, then it seems that the existing delay is not long enough, and maybe we should extend it a little more.
I increased the delay to 200ms (x2) and the problem persisted.
Quoted 13 lines
What we should not do is to delay the HPD signalling. That is definitely the wrong solution.
In any case, I'm having a hard time understanding what the problem here is. You've unplugged the connected HDMI sink, so there's no longer a display device connected, and the display hardware is shut down. The EDID becomes invalid. You then plug in a HDMI sink, and now you have a display device connected. The EDID is read, and the display hardware is configured according to the EDID details. Video output is resumed, and it takes a second or so for the sink to lock on and display the resulting video.
Quoted 1 line
At what point does the problem occur?
The problem happens at the kernel boot. If when the driver is instantiated the EDID checksum is ok, ffplay plays well video+sound. If the checksum is not ok, it assumes that the output is DVI (I think that this is the reason why we don't have video output with sound), producing video without sound.
I have tested this and it is always the same.
If you use mplayer, the problem does not happen, since the app surely makes some tweak that leads the driver to enable the sound.
Joao