diff mbox series

[8/8] drm/bridge: dw-hdmi-i2s: add .get_eld support

Message ID 20190805134102.24173-9-jbrunet@baylibre.com
State Superseded
Headers show
Series drm/bridge: dw-hdmi: improve i2s support | expand

Commit Message

Jerome Brunet Aug. 5, 2019, 1:41 p.m. UTC
Provide the eld to the generic hdmi-codec driver.
This will let the driver enforce the maximum channel number and set the
channel allocation depending on the hdmi sink.

Cc: Jonas Karlman <jonas@kwiboo.se>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h     |  1 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 10 ++++++++++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c           |  1 +
 3 files changed, 12 insertions(+)

-- 
2.21.0

Comments

Jonas Karlman Aug. 7, 2019, 2:57 p.m. UTC | #1
On 2019-08-05 15:41, Jerome Brunet wrote:
> Provide the eld to the generic hdmi-codec driver.

> This will let the driver enforce the maximum channel number and set the

> channel allocation depending on the hdmi sink.

>

> Cc: Jonas Karlman <jonas@kwiboo.se>

> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

> ---

>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h     |  1 +

>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 10 ++++++++++

>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c           |  1 +

>  3 files changed, 12 insertions(+)

>

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h

> index 63b5756f463b..cb07dc0da5a7 100644

> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h

> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h

> @@ -14,6 +14,7 @@ struct dw_hdmi_audio_data {

>  

>  struct dw_hdmi_i2s_audio_data {

>  	struct dw_hdmi *hdmi;

> +	u8 *eld;

>  

>  	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);

>  	u8 (*read)(struct dw_hdmi *hdmi, int offset);

> 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 b8ece9c1ba2c..14d499b344c0 100644

> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c

> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c

> @@ -121,6 +121,15 @@ static void dw_hdmi_i2s_audio_shutdown(struct device *dev, void *data)

>  	dw_hdmi_audio_disable(hdmi);

>  }

>  

> +static int dw_hdmi_i2s_get_eld(struct device *dev, void *data, uint8_t *buf,

> +			       size_t len)

> +{

> +	struct dw_hdmi_i2s_audio_data *audio = data;

> +

> +	memcpy(buf, audio->eld, min(sizeof(audio->eld), len));


Above sizeof does not work as intended, sizeof(audio->eld) is probably the size of a pointer and not MAX_ELD_BYTES (128),
resulting in only part of the ELD gets copied to buf.


Thanks for reworking dw-hdmi multi channel lpcm support!, this works on Rockchip for most parts.
Without the [1] patch (reset cts+n after clock is enabled) audio sometime stay silent when switching between e.g. 2ch 44.1khz and 6ch 48khz tracks.
It is not fully clear to me why reset cts+n helps, if that have negative affects on other platforms or if there is another way to solve loosing audio.

I also have a small issue with the channel allocation being selected by hdmi-codec, my soundbar reports LPCM 6.1ch instead of LPCM 7.1ch when I play a 7.1ch speaker test clip.
I have a hdmi-codec patch to fix selection of channel allocation in queue.


For patch 1-7:

Reviewed-by: Jonas Karlman <jonas@kwiboo.se>



[1] https://github.com/Kwiboo/linux-rockchip/commit/c0839e874f843ad173ddde958303d6878394ef92

Regards,
Jonas

> +	return 0;

> +}

> +

>  static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component,

>  				  struct device_node *endpoint)

>  {

> @@ -144,6 +153,7 @@ static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component,

>  static struct hdmi_codec_ops dw_hdmi_i2s_ops = {

>  	.hw_params	= dw_hdmi_i2s_hw_params,

>  	.audio_shutdown	= dw_hdmi_i2s_audio_shutdown,

> +	.get_eld	= dw_hdmi_i2s_get_eld,

>  	.get_dai_id	= dw_hdmi_i2s_get_dai_id,

>  };

>  

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c

> index bed4bb017afd..8df69c9dbfad 100644

> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c

> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c

> @@ -2797,6 +2797,7 @@ __dw_hdmi_probe(struct platform_device *pdev,

>  		struct dw_hdmi_i2s_audio_data audio;

>  

>  		audio.hdmi	= hdmi;

> +		audio.eld	= hdmi->connector.eld;

>  		audio.write	= hdmi_writeb;

>  		audio.read	= hdmi_readb;

>  		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
Jerome Brunet Aug. 7, 2019, 4:10 p.m. UTC | #2
On Wed 07 Aug 2019 at 14:57, Jonas Karlman <jonas@kwiboo.se> wrote:

> On 2019-08-05 15:41, Jerome Brunet wrote:

>> Provide the eld to the generic hdmi-codec driver.

>> This will let the driver enforce the maximum channel number and set the

>> channel allocation depending on the hdmi sink.

>>

>> Cc: Jonas Karlman <jonas@kwiboo.se>

>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

>> ---

>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h     |  1 +

>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 10 ++++++++++

>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c           |  1 +

>>  3 files changed, 12 insertions(+)

>>

>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h

>> index 63b5756f463b..cb07dc0da5a7 100644

>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h

>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h

>> @@ -14,6 +14,7 @@ struct dw_hdmi_audio_data {

>>  

>>  struct dw_hdmi_i2s_audio_data {

>>  	struct dw_hdmi *hdmi;

>> +	u8 *eld;

>>  

>>  	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);

>>  	u8 (*read)(struct dw_hdmi *hdmi, int offset);

>> 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 b8ece9c1ba2c..14d499b344c0 100644

>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c

>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c

>> @@ -121,6 +121,15 @@ static void dw_hdmi_i2s_audio_shutdown(struct device *dev, void *data)

>>  	dw_hdmi_audio_disable(hdmi);

>>  }

>>  

>> +static int dw_hdmi_i2s_get_eld(struct device *dev, void *data, uint8_t *buf,

>> +			       size_t len)

>> +{

>> +	struct dw_hdmi_i2s_audio_data *audio = data;

>> +

>> +	memcpy(buf, audio->eld, min(sizeof(audio->eld), len));

>

> Above sizeof does not work as intended, sizeof(audio->eld) is probably the size of a pointer and not MAX_ELD_BYTES (128),

> resulting in only part of the ELD gets copied to buf.


Silly ... thanks for pointing this out. I'll respin

>

>

> Thanks for reworking dw-hdmi multi channel lpcm support!, this works on Rockchip for most parts.

> Without the [1] patch (reset cts+n after clock is enabled) audio sometime stay silent when switching between e.g. 2ch 44.1khz and 6ch 48khz tracks.

> It is not fully clear to me why reset cts+n helps, if that have

> negative affects on other platforms or if there is another way to

> solve loosing audio.


I did not see that issue my self but I guess could propose this change ?

>

> I also have a small issue with the channel allocation being selected by hdmi-codec, my soundbar reports LPCM 6.1ch instead of LPCM 7.1ch when I play a 7.1ch speaker test clip.

> I have a hdmi-codec patch to fix selection of channel allocation in

> queue.


Yes, I know there is still a few problems around that and stale eld.
But those problem are not really coming from the i2s interface of the
dw-hdmi itself and should be dealt with separatly.

This is just the beginning ;)

>

>

> For patch 1-7:

>

> Reviewed-by: Jonas Karlman <jonas@kwiboo.se>

>

>

> [1] https://github.com/Kwiboo/linux-rockchip/commit/c0839e874f843ad173ddde958303d6878394ef92

>

> Regards,

> Jonas

>

>> +	return 0;

>> +}

>> +

>>  static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component,

>>  				  struct device_node *endpoint)

>>  {

>> @@ -144,6 +153,7 @@ static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component,

>>  static struct hdmi_codec_ops dw_hdmi_i2s_ops = {

>>  	.hw_params	= dw_hdmi_i2s_hw_params,

>>  	.audio_shutdown	= dw_hdmi_i2s_audio_shutdown,

>> +	.get_eld	= dw_hdmi_i2s_get_eld,

>>  	.get_dai_id	= dw_hdmi_i2s_get_dai_id,

>>  };

>>  

>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c

>> index bed4bb017afd..8df69c9dbfad 100644

>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c

>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c

>> @@ -2797,6 +2797,7 @@ __dw_hdmi_probe(struct platform_device *pdev,

>>  		struct dw_hdmi_i2s_audio_data audio;

>>  

>>  		audio.hdmi	= hdmi;

>> +		audio.eld	= hdmi->connector.eld;

>>  		audio.write	= hdmi_writeb;

>>  		audio.read	= hdmi_readb;

>>  		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
index 63b5756f463b..cb07dc0da5a7 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
@@ -14,6 +14,7 @@  struct dw_hdmi_audio_data {
 
 struct dw_hdmi_i2s_audio_data {
 	struct dw_hdmi *hdmi;
+	u8 *eld;
 
 	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
 	u8 (*read)(struct dw_hdmi *hdmi, int offset);
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 b8ece9c1ba2c..14d499b344c0 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
@@ -121,6 +121,15 @@  static void dw_hdmi_i2s_audio_shutdown(struct device *dev, void *data)
 	dw_hdmi_audio_disable(hdmi);
 }
 
+static int dw_hdmi_i2s_get_eld(struct device *dev, void *data, uint8_t *buf,
+			       size_t len)
+{
+	struct dw_hdmi_i2s_audio_data *audio = data;
+
+	memcpy(buf, audio->eld, min(sizeof(audio->eld), len));
+	return 0;
+}
+
 static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component,
 				  struct device_node *endpoint)
 {
@@ -144,6 +153,7 @@  static int dw_hdmi_i2s_get_dai_id(struct snd_soc_component *component,
 static struct hdmi_codec_ops dw_hdmi_i2s_ops = {
 	.hw_params	= dw_hdmi_i2s_hw_params,
 	.audio_shutdown	= dw_hdmi_i2s_audio_shutdown,
+	.get_eld	= dw_hdmi_i2s_get_eld,
 	.get_dai_id	= dw_hdmi_i2s_get_dai_id,
 };
 
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bed4bb017afd..8df69c9dbfad 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2797,6 +2797,7 @@  __dw_hdmi_probe(struct platform_device *pdev,
 		struct dw_hdmi_i2s_audio_data audio;
 
 		audio.hdmi	= hdmi;
+		audio.eld	= hdmi->connector.eld;
 		audio.write	= hdmi_writeb;
 		audio.read	= hdmi_readb;
 		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;