All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/bridge: dw-hdmi: handle ELD when DRM_BRIDGE_ATTACH_NO_CONNECTOR
@ 2021-10-29 13:59 Neil Armstrong
  2021-11-09 13:35   ` Neil Armstrong
  2021-11-10 19:20   ` Jernej Škrabec
  0 siblings, 2 replies; 7+ messages in thread
From: Neil Armstrong @ 2021-10-29 13:59 UTC (permalink / raw)
  To: a.hajda, Laurent.pinchart, robert.foss, jonas, jernej.skrabec
  Cc: daniel, dri-devel, linux-kernel, Neil Armstrong, Martin Blumenstingl

The current ELD handling takes the internal connector ELD buffer and
shares it to the I2S and AHB sub-driver.

But with DRM_BRIDGE_ATTACH_NO_CONNECTOR, the connector is created
elsewhere (not not), and an eventual connector is known only
if the bridge chain up to a connector is enabled.

The current dw-hdmi code gets the current connector from
atomic_enable() so use the already stored connector pointer and
replace the buffer pointer with a callback returning the current
connector ELD buffer.

Since a connector is not always available, either pass an empty
ELD to the alsa HDMI driver or don't call snd_pcm_hw_constraint_eld()
in AHB driver.

Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 10 +++++++---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h     |  4 ++--
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c |  9 ++++++++-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c           | 12 ++++++++++--
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
index d0db1acf11d7..7d2ed0ed2fe2 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
@@ -320,13 +320,17 @@ static int dw_hdmi_open(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_dw_hdmi *dw = substream->private_data;
 	void __iomem *base = dw->data.base;
+	u8 *eld;
 	int ret;
 
 	runtime->hw = dw_hdmi_hw;
 
-	ret = snd_pcm_hw_constraint_eld(runtime, dw->data.eld);
-	if (ret < 0)
-		return ret;
+	eld = dw->data.get_eld(dw->data.hdmi);
+	if (eld) {
+		ret = snd_pcm_hw_constraint_eld(runtime, eld);
+		if (ret < 0)
+			return ret;
+	}
 
 	ret = snd_pcm_limit_hw_rates(runtime);
 	if (ret < 0)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
index cb07dc0da5a7..f72d27208ebe 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
@@ -9,15 +9,15 @@ struct dw_hdmi_audio_data {
 	void __iomem *base;
 	int irq;
 	struct dw_hdmi *hdmi;
-	u8 *eld;
+	u8 *(*get_eld)(struct dw_hdmi *hdmi);
 };
 
 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);
+	u8 *(*get_eld)(struct dw_hdmi *hdmi);
 };
 
 #endif
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 feb04f127b55..f50b47ac11a8 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
@@ -135,8 +135,15 @@ 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;
+	u8 *eld;
+
+	eld = audio->get_eld(audio->hdmi);
+	if (eld)
+		memcpy(buf, eld, min_t(size_t, MAX_ELD_BYTES, len));
+	else
+		/* Pass en empty ELD if connector not available */
+		memset(buf, 0, len);
 
-	memcpy(buf, audio->eld, min_t(size_t, MAX_ELD_BYTES, len));
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 62ae63565d3a..54d8fdad395f 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -757,6 +757,14 @@ static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable)
 	hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
 }
 
+static u8 *hdmi_audio_get_eld(struct dw_hdmi *hdmi)
+{
+	if (!hdmi->curr_conn)
+		return NULL;
+
+	return hdmi->curr_conn->eld;
+}
+
 static void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
 {
 	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
@@ -3432,7 +3440,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 		audio.base = hdmi->regs;
 		audio.irq = irq;
 		audio.hdmi = hdmi;
-		audio.eld = hdmi->connector.eld;
+		audio.get_eld = hdmi_audio_get_eld;
 		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
 		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
 
@@ -3445,7 +3453,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 		struct dw_hdmi_i2s_audio_data audio;
 
 		audio.hdmi	= hdmi;
-		audio.eld	= hdmi->connector.eld;
+		audio.get_eld	= hdmi_audio_get_eld;
 		audio.write	= hdmi_writeb;
 		audio.read	= hdmi_readb;
 		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/bridge: dw-hdmi: handle ELD when DRM_BRIDGE_ATTACH_NO_CONNECTOR
  2021-10-29 13:59 [PATCH] drm/bridge: dw-hdmi: handle ELD when DRM_BRIDGE_ATTACH_NO_CONNECTOR Neil Armstrong
@ 2021-11-09 13:35   ` Neil Armstrong
  2021-11-10 19:20   ` Jernej Škrabec
  1 sibling, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2021-11-09 13:35 UTC (permalink / raw)
  To: a.hajda, Laurent.pinchart, robert.foss, jonas, jernej.skrabec
  Cc: daniel, dri-devel, linux-kernel, Martin Blumenstingl

Hi drm/bridge maintainers,

On 29/10/2021 15:59, Neil Armstrong wrote:
> The current ELD handling takes the internal connector ELD buffer and
> shares it to the I2S and AHB sub-driver.
> 
> But with DRM_BRIDGE_ATTACH_NO_CONNECTOR, the connector is created
> elsewhere (not not), and an eventual connector is known only
> if the bridge chain up to a connector is enabled.
> 
> The current dw-hdmi code gets the current connector from
> atomic_enable() so use the already stored connector pointer and
> replace the buffer pointer with a callback returning the current
> connector ELD buffer.
> 
> Since a connector is not always available, either pass an empty
> ELD to the alsa HDMI driver or don't call snd_pcm_hw_constraint_eld()
> in AHB driver.
> 
> Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 10 +++++++---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h     |  4 ++--
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c |  9 ++++++++-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c           | 12 ++++++++++--
>  4 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> index d0db1acf11d7..7d2ed0ed2fe2 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> @@ -320,13 +320,17 @@ static int dw_hdmi_open(struct snd_pcm_substream *substream)
>  	struct snd_pcm_runtime *runtime = substream->runtime;
>  	struct snd_dw_hdmi *dw = substream->private_data;
>  	void __iomem *base = dw->data.base;
> +	u8 *eld;
>  	int ret;
>  
>  	runtime->hw = dw_hdmi_hw;
>  
> -	ret = snd_pcm_hw_constraint_eld(runtime, dw->data.eld);
> -	if (ret < 0)
> -		return ret;
> +	eld = dw->data.get_eld(dw->data.hdmi);
> +	if (eld) {
> +		ret = snd_pcm_hw_constraint_eld(runtime, eld);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	ret = snd_pcm_limit_hw_rates(runtime);
>  	if (ret < 0)
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> index cb07dc0da5a7..f72d27208ebe 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> @@ -9,15 +9,15 @@ struct dw_hdmi_audio_data {
>  	void __iomem *base;
>  	int irq;
>  	struct dw_hdmi *hdmi;
> -	u8 *eld;
> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>  };
>  
>  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);
> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>  };
>  
>  #endif
> 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 feb04f127b55..f50b47ac11a8 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> @@ -135,8 +135,15 @@ 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;
> +	u8 *eld;
> +
> +	eld = audio->get_eld(audio->hdmi);
> +	if (eld)
> +		memcpy(buf, eld, min_t(size_t, MAX_ELD_BYTES, len));
> +	else
> +		/* Pass en empty ELD if connector not available */
> +		memset(buf, 0, len);
>  
> -	memcpy(buf, audio->eld, min_t(size_t, MAX_ELD_BYTES, len));
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 62ae63565d3a..54d8fdad395f 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -757,6 +757,14 @@ static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable)
>  	hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
>  }
>  
> +static u8 *hdmi_audio_get_eld(struct dw_hdmi *hdmi)
> +{
> +	if (!hdmi->curr_conn)
> +		return NULL;
> +
> +	return hdmi->curr_conn->eld;
> +}
> +
>  static void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> @@ -3432,7 +3440,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>  		audio.base = hdmi->regs;
>  		audio.irq = irq;
>  		audio.hdmi = hdmi;
> -		audio.eld = hdmi->connector.eld;
> +		audio.get_eld = hdmi_audio_get_eld;
>  		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
>  		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
>  
> @@ -3445,7 +3453,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>  		struct dw_hdmi_i2s_audio_data audio;
>  
>  		audio.hdmi	= hdmi;
> -		audio.eld	= hdmi->connector.eld;
> +		audio.get_eld	= hdmi_audio_get_eld;
>  		audio.write	= hdmi_writeb;
>  		audio.read	= hdmi_readb;
>  		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
> 

Is there someone who can have a look at this ?

I won't push https://lore.kernel.org/r/20211020123947.2585572-1-narmstrong@baylibre.com until this is fixed
because with DRM_BRIDGE_ATTACH_NO_CONNECTOR we loose the ELD on the HDMI sound card,

Neil

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/bridge: dw-hdmi: handle ELD when DRM_BRIDGE_ATTACH_NO_CONNECTOR
@ 2021-11-09 13:35   ` Neil Armstrong
  0 siblings, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2021-11-09 13:35 UTC (permalink / raw)
  To: a.hajda, Laurent.pinchart, robert.foss, jonas, jernej.skrabec
  Cc: Martin Blumenstingl, dri-devel, linux-kernel

Hi drm/bridge maintainers,

On 29/10/2021 15:59, Neil Armstrong wrote:
> The current ELD handling takes the internal connector ELD buffer and
> shares it to the I2S and AHB sub-driver.
> 
> But with DRM_BRIDGE_ATTACH_NO_CONNECTOR, the connector is created
> elsewhere (not not), and an eventual connector is known only
> if the bridge chain up to a connector is enabled.
> 
> The current dw-hdmi code gets the current connector from
> atomic_enable() so use the already stored connector pointer and
> replace the buffer pointer with a callback returning the current
> connector ELD buffer.
> 
> Since a connector is not always available, either pass an empty
> ELD to the alsa HDMI driver or don't call snd_pcm_hw_constraint_eld()
> in AHB driver.
> 
> Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 10 +++++++---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h     |  4 ++--
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c |  9 ++++++++-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c           | 12 ++++++++++--
>  4 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> index d0db1acf11d7..7d2ed0ed2fe2 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> @@ -320,13 +320,17 @@ static int dw_hdmi_open(struct snd_pcm_substream *substream)
>  	struct snd_pcm_runtime *runtime = substream->runtime;
>  	struct snd_dw_hdmi *dw = substream->private_data;
>  	void __iomem *base = dw->data.base;
> +	u8 *eld;
>  	int ret;
>  
>  	runtime->hw = dw_hdmi_hw;
>  
> -	ret = snd_pcm_hw_constraint_eld(runtime, dw->data.eld);
> -	if (ret < 0)
> -		return ret;
> +	eld = dw->data.get_eld(dw->data.hdmi);
> +	if (eld) {
> +		ret = snd_pcm_hw_constraint_eld(runtime, eld);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	ret = snd_pcm_limit_hw_rates(runtime);
>  	if (ret < 0)
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> index cb07dc0da5a7..f72d27208ebe 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> @@ -9,15 +9,15 @@ struct dw_hdmi_audio_data {
>  	void __iomem *base;
>  	int irq;
>  	struct dw_hdmi *hdmi;
> -	u8 *eld;
> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>  };
>  
>  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);
> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>  };
>  
>  #endif
> 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 feb04f127b55..f50b47ac11a8 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> @@ -135,8 +135,15 @@ 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;
> +	u8 *eld;
> +
> +	eld = audio->get_eld(audio->hdmi);
> +	if (eld)
> +		memcpy(buf, eld, min_t(size_t, MAX_ELD_BYTES, len));
> +	else
> +		/* Pass en empty ELD if connector not available */
> +		memset(buf, 0, len);
>  
> -	memcpy(buf, audio->eld, min_t(size_t, MAX_ELD_BYTES, len));
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 62ae63565d3a..54d8fdad395f 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -757,6 +757,14 @@ static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable)
>  	hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
>  }
>  
> +static u8 *hdmi_audio_get_eld(struct dw_hdmi *hdmi)
> +{
> +	if (!hdmi->curr_conn)
> +		return NULL;
> +
> +	return hdmi->curr_conn->eld;
> +}
> +
>  static void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> @@ -3432,7 +3440,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>  		audio.base = hdmi->regs;
>  		audio.irq = irq;
>  		audio.hdmi = hdmi;
> -		audio.eld = hdmi->connector.eld;
> +		audio.get_eld = hdmi_audio_get_eld;
>  		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
>  		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
>  
> @@ -3445,7 +3453,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>  		struct dw_hdmi_i2s_audio_data audio;
>  
>  		audio.hdmi	= hdmi;
> -		audio.eld	= hdmi->connector.eld;
> +		audio.get_eld	= hdmi_audio_get_eld;
>  		audio.write	= hdmi_writeb;
>  		audio.read	= hdmi_readb;
>  		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
> 

Is there someone who can have a look at this ?

I won't push https://lore.kernel.org/r/20211020123947.2585572-1-narmstrong@baylibre.com until this is fixed
because with DRM_BRIDGE_ATTACH_NO_CONNECTOR we loose the ELD on the HDMI sound card,

Neil

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/bridge: dw-hdmi: handle ELD when DRM_BRIDGE_ATTACH_NO_CONNECTOR
  2021-10-29 13:59 [PATCH] drm/bridge: dw-hdmi: handle ELD when DRM_BRIDGE_ATTACH_NO_CONNECTOR Neil Armstrong
@ 2021-11-10 19:20   ` Jernej Škrabec
  2021-11-10 19:20   ` Jernej Škrabec
  1 sibling, 0 replies; 7+ messages in thread
From: Jernej Škrabec @ 2021-11-10 19:20 UTC (permalink / raw)
  To: a.hajda, Laurent.pinchart, robert.foss, jonas, Neil Armstrong
  Cc: daniel, dri-devel, linux-kernel, Neil Armstrong, Martin Blumenstingl

Hi Neil,

sorry for late response.

Dne petek, 29. oktober 2021 ob 15:59:47 CET je Neil Armstrong napisal(a):
> The current ELD handling takes the internal connector ELD buffer and
> shares it to the I2S and AHB sub-driver.
> 
> But with DRM_BRIDGE_ATTACH_NO_CONNECTOR, the connector is created
> elsewhere (not not), and an eventual connector is known only

^ typo, 2x "not"

Other than that, it looks good.

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> if the bridge chain up to a connector is enabled.
> 
> The current dw-hdmi code gets the current connector from
> atomic_enable() so use the already stored connector pointer and
> replace the buffer pointer with a callback returning the current
> connector ELD buffer.
> 
> Since a connector is not always available, either pass an empty
> ELD to the alsa HDMI driver or don't call snd_pcm_hw_constraint_eld()
> in AHB driver.
> 
> Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 10 +++++++---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h     |  4 ++--
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c |  9 ++++++++-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c           | 12 ++++++++++--
>  4 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/
gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> index d0db1acf11d7..7d2ed0ed2fe2 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> @@ -320,13 +320,17 @@ static int dw_hdmi_open(struct snd_pcm_substream 
*substream)
>  	struct snd_pcm_runtime *runtime = substream->runtime;
>  	struct snd_dw_hdmi *dw = substream->private_data;
>  	void __iomem *base = dw->data.base;
> +	u8 *eld;
>  	int ret;
>  
>  	runtime->hw = dw_hdmi_hw;
>  
> -	ret = snd_pcm_hw_constraint_eld(runtime, dw->data.eld);
> -	if (ret < 0)
> -		return ret;
> +	eld = dw->data.get_eld(dw->data.hdmi);
> +	if (eld) {
> +		ret = snd_pcm_hw_constraint_eld(runtime, eld);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	ret = snd_pcm_limit_hw_rates(runtime);
>  	if (ret < 0)
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/
drm/bridge/synopsys/dw-hdmi-audio.h
> index cb07dc0da5a7..f72d27208ebe 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> @@ -9,15 +9,15 @@ struct dw_hdmi_audio_data {
>  	void __iomem *base;
>  	int irq;
>  	struct dw_hdmi *hdmi;
> -	u8 *eld;
> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>  };
>  
>  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);
> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>  };
>  
>  #endif
> 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 feb04f127b55..f50b47ac11a8 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> @@ -135,8 +135,15 @@ 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;
> +	u8 *eld;
> +
> +	eld = audio->get_eld(audio->hdmi);
> +	if (eld)
> +		memcpy(buf, eld, min_t(size_t, MAX_ELD_BYTES, len));
> +	else
> +		/* Pass en empty ELD if connector not available */
> +		memset(buf, 0, len);
>  
> -	memcpy(buf, audio->eld, min_t(size_t, MAX_ELD_BYTES, len));
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/
bridge/synopsys/dw-hdmi.c
> index 62ae63565d3a..54d8fdad395f 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -757,6 +757,14 @@ static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, 
bool enable)
>  	hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
>  }
>  
> +static u8 *hdmi_audio_get_eld(struct dw_hdmi *hdmi)
> +{
> +	if (!hdmi->curr_conn)
> +		return NULL;
> +
> +	return hdmi->curr_conn->eld;
> +}
> +
>  static void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> @@ -3432,7 +3440,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device 
*pdev,
>  		audio.base = hdmi->regs;
>  		audio.irq = irq;
>  		audio.hdmi = hdmi;
> -		audio.eld = hdmi->connector.eld;
> +		audio.get_eld = hdmi_audio_get_eld;
>  		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
>  		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
>  
> @@ -3445,7 +3453,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device 
*pdev,
>  		struct dw_hdmi_i2s_audio_data audio;
>  
>  		audio.hdmi	= hdmi;
> -		audio.eld	= hdmi->connector.eld;
> +		audio.get_eld	= hdmi_audio_get_eld;
>  		audio.write	= hdmi_writeb;
>  		audio.read	= hdmi_readb;
>  		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
> -- 
> 2.25.1
> 
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/bridge: dw-hdmi: handle ELD when DRM_BRIDGE_ATTACH_NO_CONNECTOR
@ 2021-11-10 19:20   ` Jernej Škrabec
  0 siblings, 0 replies; 7+ messages in thread
From: Jernej Škrabec @ 2021-11-10 19:20 UTC (permalink / raw)
  To: a.hajda, Laurent.pinchart, robert.foss, jonas, Neil Armstrong
  Cc: Martin Blumenstingl, Neil Armstrong, dri-devel, linux-kernel

Hi Neil,

sorry for late response.

Dne petek, 29. oktober 2021 ob 15:59:47 CET je Neil Armstrong napisal(a):
> The current ELD handling takes the internal connector ELD buffer and
> shares it to the I2S and AHB sub-driver.
> 
> But with DRM_BRIDGE_ATTACH_NO_CONNECTOR, the connector is created
> elsewhere (not not), and an eventual connector is known only

^ typo, 2x "not"

Other than that, it looks good.

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> if the bridge chain up to a connector is enabled.
> 
> The current dw-hdmi code gets the current connector from
> atomic_enable() so use the already stored connector pointer and
> replace the buffer pointer with a callback returning the current
> connector ELD buffer.
> 
> Since a connector is not always available, either pass an empty
> ELD to the alsa HDMI driver or don't call snd_pcm_hw_constraint_eld()
> in AHB driver.
> 
> Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 10 +++++++---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h     |  4 ++--
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c |  9 ++++++++-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c           | 12 ++++++++++--
>  4 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/
gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> index d0db1acf11d7..7d2ed0ed2fe2 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> @@ -320,13 +320,17 @@ static int dw_hdmi_open(struct snd_pcm_substream 
*substream)
>  	struct snd_pcm_runtime *runtime = substream->runtime;
>  	struct snd_dw_hdmi *dw = substream->private_data;
>  	void __iomem *base = dw->data.base;
> +	u8 *eld;
>  	int ret;
>  
>  	runtime->hw = dw_hdmi_hw;
>  
> -	ret = snd_pcm_hw_constraint_eld(runtime, dw->data.eld);
> -	if (ret < 0)
> -		return ret;
> +	eld = dw->data.get_eld(dw->data.hdmi);
> +	if (eld) {
> +		ret = snd_pcm_hw_constraint_eld(runtime, eld);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	ret = snd_pcm_limit_hw_rates(runtime);
>  	if (ret < 0)
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/
drm/bridge/synopsys/dw-hdmi-audio.h
> index cb07dc0da5a7..f72d27208ebe 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
> @@ -9,15 +9,15 @@ struct dw_hdmi_audio_data {
>  	void __iomem *base;
>  	int irq;
>  	struct dw_hdmi *hdmi;
> -	u8 *eld;
> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>  };
>  
>  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);
> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>  };
>  
>  #endif
> 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 feb04f127b55..f50b47ac11a8 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> @@ -135,8 +135,15 @@ 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;
> +	u8 *eld;
> +
> +	eld = audio->get_eld(audio->hdmi);
> +	if (eld)
> +		memcpy(buf, eld, min_t(size_t, MAX_ELD_BYTES, len));
> +	else
> +		/* Pass en empty ELD if connector not available */
> +		memset(buf, 0, len);
>  
> -	memcpy(buf, audio->eld, min_t(size_t, MAX_ELD_BYTES, len));
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/
bridge/synopsys/dw-hdmi.c
> index 62ae63565d3a..54d8fdad395f 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -757,6 +757,14 @@ static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, 
bool enable)
>  	hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
>  }
>  
> +static u8 *hdmi_audio_get_eld(struct dw_hdmi *hdmi)
> +{
> +	if (!hdmi->curr_conn)
> +		return NULL;
> +
> +	return hdmi->curr_conn->eld;
> +}
> +
>  static void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> @@ -3432,7 +3440,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device 
*pdev,
>  		audio.base = hdmi->regs;
>  		audio.irq = irq;
>  		audio.hdmi = hdmi;
> -		audio.eld = hdmi->connector.eld;
> +		audio.get_eld = hdmi_audio_get_eld;
>  		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
>  		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
>  
> @@ -3445,7 +3453,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device 
*pdev,
>  		struct dw_hdmi_i2s_audio_data audio;
>  
>  		audio.hdmi	= hdmi;
> -		audio.eld	= hdmi->connector.eld;
> +		audio.get_eld	= hdmi_audio_get_eld;
>  		audio.write	= hdmi_writeb;
>  		audio.read	= hdmi_readb;
>  		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
> -- 
> 2.25.1
> 
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/bridge: dw-hdmi: handle ELD when DRM_BRIDGE_ATTACH_NO_CONNECTOR
  2021-11-10 19:20   ` Jernej Škrabec
@ 2021-11-12  9:01     ` Neil Armstrong
  -1 siblings, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2021-11-12  9:01 UTC (permalink / raw)
  To: Jernej Škrabec, a.hajda, Laurent.pinchart, robert.foss, jonas
  Cc: daniel, dri-devel, linux-kernel, Martin Blumenstingl

Hi Jernej,

On 10/11/2021 20:20, Jernej Škrabec wrote:
> Hi Neil,
> 
> sorry for late response.
> 
> Dne petek, 29. oktober 2021 ob 15:59:47 CET je Neil Armstrong napisal(a):
>> The current ELD handling takes the internal connector ELD buffer and
>> shares it to the I2S and AHB sub-driver.
>>
>> But with DRM_BRIDGE_ATTACH_NO_CONNECTOR, the connector is created
>> elsewhere (not not), and an eventual connector is known only
> 
> ^ typo, 2x "not"

thx, fixing while applying !

> 
> Other than that, it looks good.
> 
> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Applying to drm-misc-next

Thanks,
Neil

> 
> Best regards,
> Jernej
> 
>> if the bridge chain up to a connector is enabled.
>>
>> The current dw-hdmi code gets the current connector from
>> atomic_enable() so use the already stored connector pointer and
>> replace the buffer pointer with a callback returning the current
>> connector ELD buffer.
>>
>> Since a connector is not always available, either pass an empty
>> ELD to the alsa HDMI driver or don't call snd_pcm_hw_constraint_eld()
>> in AHB driver.
>>
>> Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 10 +++++++---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h     |  4 ++--
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c |  9 ++++++++-
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c           | 12 ++++++++++--
>>  4 files changed, 27 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/
> gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
>> index d0db1acf11d7..7d2ed0ed2fe2 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
>> @@ -320,13 +320,17 @@ static int dw_hdmi_open(struct snd_pcm_substream 
> *substream)
>>  	struct snd_pcm_runtime *runtime = substream->runtime;
>>  	struct snd_dw_hdmi *dw = substream->private_data;
>>  	void __iomem *base = dw->data.base;
>> +	u8 *eld;
>>  	int ret;
>>  
>>  	runtime->hw = dw_hdmi_hw;
>>  
>> -	ret = snd_pcm_hw_constraint_eld(runtime, dw->data.eld);
>> -	if (ret < 0)
>> -		return ret;
>> +	eld = dw->data.get_eld(dw->data.hdmi);
>> +	if (eld) {
>> +		ret = snd_pcm_hw_constraint_eld(runtime, eld);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>>  
>>  	ret = snd_pcm_limit_hw_rates(runtime);
>>  	if (ret < 0)
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/
> drm/bridge/synopsys/dw-hdmi-audio.h
>> index cb07dc0da5a7..f72d27208ebe 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
>> @@ -9,15 +9,15 @@ struct dw_hdmi_audio_data {
>>  	void __iomem *base;
>>  	int irq;
>>  	struct dw_hdmi *hdmi;
>> -	u8 *eld;
>> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>>  };
>>  
>>  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);
>> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>>  };
>>  
>>  #endif
>> 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 feb04f127b55..f50b47ac11a8 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>> @@ -135,8 +135,15 @@ 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;
>> +	u8 *eld;
>> +
>> +	eld = audio->get_eld(audio->hdmi);
>> +	if (eld)
>> +		memcpy(buf, eld, min_t(size_t, MAX_ELD_BYTES, len));
>> +	else
>> +		/* Pass en empty ELD if connector not available */
>> +		memset(buf, 0, len);
>>  
>> -	memcpy(buf, audio->eld, min_t(size_t, MAX_ELD_BYTES, len));
>>  	return 0;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/
> bridge/synopsys/dw-hdmi.c
>> index 62ae63565d3a..54d8fdad395f 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -757,6 +757,14 @@ static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, 
> bool enable)
>>  	hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
>>  }
>>  
>> +static u8 *hdmi_audio_get_eld(struct dw_hdmi *hdmi)
>> +{
>> +	if (!hdmi->curr_conn)
>> +		return NULL;
>> +
>> +	return hdmi->curr_conn->eld;
>> +}
>> +
>>  static void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>>  {
>>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
>> @@ -3432,7 +3440,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device 
> *pdev,
>>  		audio.base = hdmi->regs;
>>  		audio.irq = irq;
>>  		audio.hdmi = hdmi;
>> -		audio.eld = hdmi->connector.eld;
>> +		audio.get_eld = hdmi_audio_get_eld;
>>  		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
>>  		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
>>  
>> @@ -3445,7 +3453,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device 
> *pdev,
>>  		struct dw_hdmi_i2s_audio_data audio;
>>  
>>  		audio.hdmi	= hdmi;
>> -		audio.eld	= hdmi->connector.eld;
>> +		audio.get_eld	= hdmi_audio_get_eld;
>>  		audio.write	= hdmi_writeb;
>>  		audio.read	= hdmi_readb;
>>  		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
>> -- 
>> 2.25.1
>>
>>
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/bridge: dw-hdmi: handle ELD when DRM_BRIDGE_ATTACH_NO_CONNECTOR
@ 2021-11-12  9:01     ` Neil Armstrong
  0 siblings, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2021-11-12  9:01 UTC (permalink / raw)
  To: Jernej Škrabec, a.hajda, Laurent.pinchart, robert.foss, jonas
  Cc: Martin Blumenstingl, dri-devel, linux-kernel

Hi Jernej,

On 10/11/2021 20:20, Jernej Škrabec wrote:
> Hi Neil,
> 
> sorry for late response.
> 
> Dne petek, 29. oktober 2021 ob 15:59:47 CET je Neil Armstrong napisal(a):
>> The current ELD handling takes the internal connector ELD buffer and
>> shares it to the I2S and AHB sub-driver.
>>
>> But with DRM_BRIDGE_ATTACH_NO_CONNECTOR, the connector is created
>> elsewhere (not not), and an eventual connector is known only
> 
> ^ typo, 2x "not"

thx, fixing while applying !

> 
> Other than that, it looks good.
> 
> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Applying to drm-misc-next

Thanks,
Neil

> 
> Best regards,
> Jernej
> 
>> if the bridge chain up to a connector is enabled.
>>
>> The current dw-hdmi code gets the current connector from
>> atomic_enable() so use the already stored connector pointer and
>> replace the buffer pointer with a callback returning the current
>> connector ELD buffer.
>>
>> Since a connector is not always available, either pass an empty
>> ELD to the alsa HDMI driver or don't call snd_pcm_hw_constraint_eld()
>> in AHB driver.
>>
>> Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 10 +++++++---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h     |  4 ++--
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c |  9 ++++++++-
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c           | 12 ++++++++++--
>>  4 files changed, 27 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/
> gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
>> index d0db1acf11d7..7d2ed0ed2fe2 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
>> @@ -320,13 +320,17 @@ static int dw_hdmi_open(struct snd_pcm_substream 
> *substream)
>>  	struct snd_pcm_runtime *runtime = substream->runtime;
>>  	struct snd_dw_hdmi *dw = substream->private_data;
>>  	void __iomem *base = dw->data.base;
>> +	u8 *eld;
>>  	int ret;
>>  
>>  	runtime->hw = dw_hdmi_hw;
>>  
>> -	ret = snd_pcm_hw_constraint_eld(runtime, dw->data.eld);
>> -	if (ret < 0)
>> -		return ret;
>> +	eld = dw->data.get_eld(dw->data.hdmi);
>> +	if (eld) {
>> +		ret = snd_pcm_hw_constraint_eld(runtime, eld);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>>  
>>  	ret = snd_pcm_limit_hw_rates(runtime);
>>  	if (ret < 0)
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h b/drivers/gpu/
> drm/bridge/synopsys/dw-hdmi-audio.h
>> index cb07dc0da5a7..f72d27208ebe 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-audio.h
>> @@ -9,15 +9,15 @@ struct dw_hdmi_audio_data {
>>  	void __iomem *base;
>>  	int irq;
>>  	struct dw_hdmi *hdmi;
>> -	u8 *eld;
>> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>>  };
>>  
>>  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);
>> +	u8 *(*get_eld)(struct dw_hdmi *hdmi);
>>  };
>>  
>>  #endif
>> 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 feb04f127b55..f50b47ac11a8 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
>> @@ -135,8 +135,15 @@ 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;
>> +	u8 *eld;
>> +
>> +	eld = audio->get_eld(audio->hdmi);
>> +	if (eld)
>> +		memcpy(buf, eld, min_t(size_t, MAX_ELD_BYTES, len));
>> +	else
>> +		/* Pass en empty ELD if connector not available */
>> +		memset(buf, 0, len);
>>  
>> -	memcpy(buf, audio->eld, min_t(size_t, MAX_ELD_BYTES, len));
>>  	return 0;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/
> bridge/synopsys/dw-hdmi.c
>> index 62ae63565d3a..54d8fdad395f 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -757,6 +757,14 @@ static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, 
> bool enable)
>>  	hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
>>  }
>>  
>> +static u8 *hdmi_audio_get_eld(struct dw_hdmi *hdmi)
>> +{
>> +	if (!hdmi->curr_conn)
>> +		return NULL;
>> +
>> +	return hdmi->curr_conn->eld;
>> +}
>> +
>>  static void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>>  {
>>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
>> @@ -3432,7 +3440,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device 
> *pdev,
>>  		audio.base = hdmi->regs;
>>  		audio.irq = irq;
>>  		audio.hdmi = hdmi;
>> -		audio.eld = hdmi->connector.eld;
>> +		audio.get_eld = hdmi_audio_get_eld;
>>  		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
>>  		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
>>  
>> @@ -3445,7 +3453,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device 
> *pdev,
>>  		struct dw_hdmi_i2s_audio_data audio;
>>  
>>  		audio.hdmi	= hdmi;
>> -		audio.eld	= hdmi->connector.eld;
>> +		audio.get_eld	= hdmi_audio_get_eld;
>>  		audio.write	= hdmi_writeb;
>>  		audio.read	= hdmi_readb;
>>  		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
>> -- 
>> 2.25.1
>>
>>
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-11-12  9:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-29 13:59 [PATCH] drm/bridge: dw-hdmi: handle ELD when DRM_BRIDGE_ATTACH_NO_CONNECTOR Neil Armstrong
2021-11-09 13:35 ` Neil Armstrong
2021-11-09 13:35   ` Neil Armstrong
2021-11-10 19:20 ` Jernej Škrabec
2021-11-10 19:20   ` Jernej Škrabec
2021-11-12  9:01   ` Neil Armstrong
2021-11-12  9:01     ` Neil Armstrong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.