[PATCH 0/2] drm: dw-hdmi: various improvements

[PATCH 0/2] drm: dw-hdmi: various improvements

[Romain Perier]

This set of patches split the stream handling functions in two parts. It
introduces new callbacks that are specific to each variant, one for I2S
and one for AHB.

Then, as requested by the datasheet for the I2S variant, it adds support
for gating the audio sampler clock when the audio stream is enabled and
disabled.

This patches series is the continuity of the following discussion:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html

Romain Perier (2):
  drm: dw-hdmi: add specific I2S and AHB functions for stream handling
  drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks

 drivers/gpu/drm/bridge/dw-hdmi.c | 45 +++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 8 deletions(-)

-- 
2.9.3

[PATCH 0/2] drm: dw-hdmi: various improvements

[Romain Perier]

This set of patches split the stream handling functions in two parts. It
introduces new callbacks that are specific to each variant, one for I2S
and one for AHB.

Then, as requested by the datasheet for the I2S variant, it adds support
for gating the audio sampler clock when the audio stream is enabled and
disabled.

This patches series is the continuity of the following discussion:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html

Romain Perier (2):
  drm: dw-hdmi: add specific I2S and AHB functions for stream handling
  drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks

 drivers/gpu/drm/bridge/dw-hdmi.c | 45 +++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 8 deletions(-)

-- 
2.9.3

[PATCH 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling

[Romain Perier]

Currently, CTS+N is forced to zero as a workaround of the IP block for
i.MX platforms. This is requested in the datasheet of the corresponding
IP for AHB mode only. However, we have seen that it introduces glitches
or delays when playing a sound on HDMI for I2S mode. This proves that we
cannot keep the current functions for handling audio stream as-is if
these contain workaround that are specific to a mode.

This commit introduces two callbacks, one for each variant.
dw_hdmi_setup defines the right function depending on the detected
variant. Then, the exported functions dw_hdmi_audio_enable and
dw_hdmi_audio_disable calls the corresponding callbacks

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 542d198..d34e0a5 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -166,6 +166,8 @@ struct dw_hdmi {
 
 	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
 	u8 (*read)(struct dw_hdmi *hdmi, int offset);
+	void (*enable_audio)(struct dw_hdmi *hdmi);
+	void (*disable_audio)(struct dw_hdmi *hdmi);
 };
 
 #define HDMI_IH_PHY_STAT0_RX_SENSE \
@@ -557,13 +559,34 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
 
+void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
+{
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+}
+
+void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
+{
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
+}
+
+void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
+{
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+}
+
+
+void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
+{
+
+}
+
 void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&hdmi->audio_lock, flags);
 	hdmi->audio_enable = true;
-	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+	hdmi->enable_audio(hdmi);
 	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable);
@@ -574,7 +597,7 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi)
 
 	spin_lock_irqsave(&hdmi->audio_lock, flags);
 	hdmi->audio_enable = false;
-	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
+	hdmi->disable_audio(hdmi);
 	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);
@@ -2114,6 +2137,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		audio.irq = irq;
 		audio.hdmi = hdmi;
 		audio.eld = hdmi->connector.eld;
+		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
+		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
 
 		pdevinfo.name = "dw-hdmi-ahb-audio";
 		pdevinfo.data = &audio;
@@ -2126,6 +2151,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		audio.hdmi	= hdmi;
 		audio.write	= hdmi_writeb;
 		audio.read	= hdmi_readb;
+		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
+		hdmi->disable_audio = dw_hdmi_i2s_audio_disable;
 
 		pdevinfo.name = "dw-hdmi-i2s-audio";
 		pdevinfo.data = &audio;
-- 
2.9.3

[PATCH 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling

[Romain Perier]

Currently, CTS+N is forced to zero as a workaround of the IP block for
i.MX platforms. This is requested in the datasheet of the corresponding
IP for AHB mode only. However, we have seen that it introduces glitches
or delays when playing a sound on HDMI for I2S mode. This proves that we
cannot keep the current functions for handling audio stream as-is if
these contain workaround that are specific to a mode.

This commit introduces two callbacks, one for each variant.
dw_hdmi_setup defines the right function depending on the detected
variant. Then, the exported functions dw_hdmi_audio_enable and
dw_hdmi_audio_disable calls the corresponding callbacks

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 542d198..d34e0a5 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -166,6 +166,8 @@ struct dw_hdmi {
 
 	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
 	u8 (*read)(struct dw_hdmi *hdmi, int offset);
+	void (*enable_audio)(struct dw_hdmi *hdmi);
+	void (*disable_audio)(struct dw_hdmi *hdmi);
 };
 
 #define HDMI_IH_PHY_STAT0_RX_SENSE \
@@ -557,13 +559,34 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
 
+void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
+{
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+}
+
+void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
+{
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
+}
+
+void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
+{
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+}
+
+
+void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
+{
+
+}
+
 void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&hdmi->audio_lock, flags);
 	hdmi->audio_enable = true;
-	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+	hdmi->enable_audio(hdmi);
 	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable);
@@ -574,7 +597,7 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi)
 
 	spin_lock_irqsave(&hdmi->audio_lock, flags);
 	hdmi->audio_enable = false;
-	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
+	hdmi->disable_audio(hdmi);
 	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);
@@ -2114,6 +2137,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		audio.irq = irq;
 		audio.hdmi = hdmi;
 		audio.eld = hdmi->connector.eld;
+		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
+		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
 
 		pdevinfo.name = "dw-hdmi-ahb-audio";
 		pdevinfo.data = &audio;
@@ -2126,6 +2151,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		audio.hdmi	= hdmi;
 		audio.write	= hdmi_writeb;
 		audio.read	= hdmi_readb;
+		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
+		hdmi->disable_audio = dw_hdmi_i2s_audio_disable;
 
 		pdevinfo.name = "dw-hdmi-i2s-audio";
 		pdevinfo.data = &audio;
-- 
2.9.3

[PATCH 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling

[Romain Perier]

Currently, CTS+N is forced to zero as a workaround of the IP block for
i.MX platforms. This is requested in the datasheet of the corresponding
IP for AHB mode only. However, we have seen that it introduces glitches
or delays when playing a sound on HDMI for I2S mode. This proves that we
cannot keep the current functions for handling audio stream as-is if
these contain workaround that are specific to a mode.

This commit introduces two callbacks, one for each variant.
dw_hdmi_setup defines the right function depending on the detected
variant. Then, the exported functions dw_hdmi_audio_enable and
dw_hdmi_audio_disable calls the corresponding callbacks

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 542d198..d34e0a5 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -166,6 +166,8 @@ struct dw_hdmi {
 
 	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
 	u8 (*read)(struct dw_hdmi *hdmi, int offset);
+	void (*enable_audio)(struct dw_hdmi *hdmi);
+	void (*disable_audio)(struct dw_hdmi *hdmi);
 };
 
 #define HDMI_IH_PHY_STAT0_RX_SENSE \
@@ -557,13 +559,34 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
 
+void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
+{
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+}
+
+void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
+{
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
+}
+
+void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
+{
+	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+}
+
+
+void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
+{
+
+}
+
 void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&hdmi->audio_lock, flags);
 	hdmi->audio_enable = true;
-	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+	hdmi->enable_audio(hdmi);
 	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable);
@@ -574,7 +597,7 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi)
 
 	spin_lock_irqsave(&hdmi->audio_lock, flags);
 	hdmi->audio_enable = false;
-	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
+	hdmi->disable_audio(hdmi);
 	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);
@@ -2114,6 +2137,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		audio.irq = irq;
 		audio.hdmi = hdmi;
 		audio.eld = hdmi->connector.eld;
+		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
+		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
 
 		pdevinfo.name = "dw-hdmi-ahb-audio";
 		pdevinfo.data = &audio;
@@ -2126,6 +2151,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		audio.hdmi	= hdmi;
 		audio.write	= hdmi_writeb;
 		audio.read	= hdmi_readb;
+		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
+		hdmi->disable_audio = dw_hdmi_i2s_audio_disable;
 
 		pdevinfo.name = "dw-hdmi-i2s-audio";
 		pdevinfo.data = &audio;
-- 
2.9.3

Re: [PATCH 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling

[Neil Armstrong]

On 04/07/2017 04:19 PM, Romain Perier wrote:
> Currently, CTS+N is forced to zero as a workaround of the IP block for
> i.MX platforms. This is requested in the datasheet of the corresponding
> IP for AHB mode only. However, we have seen that it introduces glitches
> or delays when playing a sound on HDMI for I2S mode. This proves that we
> cannot keep the current functions for handling audio stream as-is if
> these contain workaround that are specific to a mode.
> 
> This commit introduces two callbacks, one for each variant.
> dw_hdmi_setup defines the right function depending on the detected
> variant. Then, the exported functions dw_hdmi_audio_enable and
> dw_hdmi_audio_disable calls the corresponding callbacks

Thanks for the patch, I'll test it on the Amlogic platform.

> 
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 542d198..d34e0a5 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -166,6 +166,8 @@ struct dw_hdmi {
>  
>  	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
>  	u8 (*read)(struct dw_hdmi *hdmi, int offset);
> +	void (*enable_audio)(struct dw_hdmi *hdmi);
> +	void (*disable_audio)(struct dw_hdmi *hdmi);
>  };
>  
>  #define HDMI_IH_PHY_STAT0_RX_SENSE \
> @@ -557,13 +559,34 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
>  
> +void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
> +{
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +}
> +
> +void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
> +{
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
> +}
> +
> +void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
> +{
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +}
> +
> +
> +void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
> +{
> +
> +}

For this one, it would be better to set it to NULL then check for NULL...

> +
>  void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&hdmi->audio_lock, flags);
>  	hdmi->audio_enable = true;
> -	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +	hdmi->enable_audio(hdmi);

if (hdmi->enable_audio)
	hdmi->enable_audio(hdmi);

for consistency...

>  	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable);
> @@ -574,7 +597,7 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi)
>  
>  	spin_lock_irqsave(&hdmi->audio_lock, flags);
>  	hdmi->audio_enable = false;
> -	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
> +	hdmi->disable_audio(hdmi);

if (hdmi->disable_audio)
	hdmi->disable_audio(hdmi);

>  	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);
> @@ -2114,6 +2137,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		audio.irq = irq;
>  		audio.hdmi = hdmi;
>  		audio.eld = hdmi->connector.eld;
> +		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
> +		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
>  
>  		pdevinfo.name = "dw-hdmi-ahb-audio";
>  		pdevinfo.data = &audio;
> @@ -2126,6 +2151,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		audio.hdmi	= hdmi;
>  		audio.write	= hdmi_writeb;
>  		audio.read	= hdmi_readb;
> +		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
> +		hdmi->disable_audio = dw_hdmi_i2s_audio_disable;
>  
>  		pdevinfo.name = "dw-hdmi-i2s-audio";
>  		pdevinfo.data = &audio;
> 

[PATCH 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling

[Neil Armstrong]

On 04/07/2017 04:19 PM, Romain Perier wrote:
> Currently, CTS+N is forced to zero as a workaround of the IP block for
> i.MX platforms. This is requested in the datasheet of the corresponding
> IP for AHB mode only. However, we have seen that it introduces glitches
> or delays when playing a sound on HDMI for I2S mode. This proves that we
> cannot keep the current functions for handling audio stream as-is if
> these contain workaround that are specific to a mode.
> 
> This commit introduces two callbacks, one for each variant.
> dw_hdmi_setup defines the right function depending on the detected
> variant. Then, the exported functions dw_hdmi_audio_enable and
> dw_hdmi_audio_disable calls the corresponding callbacks

Thanks for the patch, I'll test it on the Amlogic platform.

> 
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 542d198..d34e0a5 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -166,6 +166,8 @@ struct dw_hdmi {
>  
>  	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
>  	u8 (*read)(struct dw_hdmi *hdmi, int offset);
> +	void (*enable_audio)(struct dw_hdmi *hdmi);
> +	void (*disable_audio)(struct dw_hdmi *hdmi);
>  };
>  
>  #define HDMI_IH_PHY_STAT0_RX_SENSE \
> @@ -557,13 +559,34 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
>  
> +void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
> +{
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +}
> +
> +void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
> +{
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
> +}
> +
> +void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
> +{
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +}
> +
> +
> +void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
> +{
> +
> +}

For this one, it would be better to set it to NULL then check for NULL...

> +
>  void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&hdmi->audio_lock, flags);
>  	hdmi->audio_enable = true;
> -	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +	hdmi->enable_audio(hdmi);

if (hdmi->enable_audio)
	hdmi->enable_audio(hdmi);

for consistency...

>  	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable);
> @@ -574,7 +597,7 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi)
>  
>  	spin_lock_irqsave(&hdmi->audio_lock, flags);
>  	hdmi->audio_enable = false;
> -	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
> +	hdmi->disable_audio(hdmi);

if (hdmi->disable_audio)
	hdmi->disable_audio(hdmi);

>  	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);
> @@ -2114,6 +2137,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		audio.irq = irq;
>  		audio.hdmi = hdmi;
>  		audio.eld = hdmi->connector.eld;
> +		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
> +		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
>  
>  		pdevinfo.name = "dw-hdmi-ahb-audio";
>  		pdevinfo.data = &audio;
> @@ -2126,6 +2151,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		audio.hdmi	= hdmi;
>  		audio.write	= hdmi_writeb;
>  		audio.read	= hdmi_readb;
> +		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
> +		hdmi->disable_audio = dw_hdmi_i2s_audio_disable;
>  
>  		pdevinfo.name = "dw-hdmi-i2s-audio";
>  		pdevinfo.data = &audio;
> 

Re: [PATCH 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling

[Neil Armstrong]

On 04/07/2017 04:19 PM, Romain Perier wrote:
> Currently, CTS+N is forced to zero as a workaround of the IP block for
> i.MX platforms. This is requested in the datasheet of the corresponding
> IP for AHB mode only. However, we have seen that it introduces glitches
> or delays when playing a sound on HDMI for I2S mode. This proves that we
> cannot keep the current functions for handling audio stream as-is if
> these contain workaround that are specific to a mode.
> 
> This commit introduces two callbacks, one for each variant.
> dw_hdmi_setup defines the right function depending on the detected
> variant. Then, the exported functions dw_hdmi_audio_enable and
> dw_hdmi_audio_disable calls the corresponding callbacks

Thanks for the patch, I'll test it on the Amlogic platform.

> 
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 542d198..d34e0a5 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -166,6 +166,8 @@ struct dw_hdmi {
>  
>  	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
>  	u8 (*read)(struct dw_hdmi *hdmi, int offset);
> +	void (*enable_audio)(struct dw_hdmi *hdmi);
> +	void (*disable_audio)(struct dw_hdmi *hdmi);
>  };
>  
>  #define HDMI_IH_PHY_STAT0_RX_SENSE \
> @@ -557,13 +559,34 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
>  
> +void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
> +{
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +}
> +
> +void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
> +{
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
> +}
> +
> +void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
> +{
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +}
> +
> +
> +void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
> +{
> +
> +}

For this one, it would be better to set it to NULL then check for NULL...

> +
>  void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&hdmi->audio_lock, flags);
>  	hdmi->audio_enable = true;
> -	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +	hdmi->enable_audio(hdmi);

if (hdmi->enable_audio)
	hdmi->enable_audio(hdmi);

for consistency...

>  	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable);
> @@ -574,7 +597,7 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi)
>  
>  	spin_lock_irqsave(&hdmi->audio_lock, flags);
>  	hdmi->audio_enable = false;
> -	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
> +	hdmi->disable_audio(hdmi);

if (hdmi->disable_audio)
	hdmi->disable_audio(hdmi);

>  	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);
> @@ -2114,6 +2137,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		audio.irq = irq;
>  		audio.hdmi = hdmi;
>  		audio.eld = hdmi->connector.eld;
> +		hdmi->enable_audio = dw_hdmi_ahb_audio_enable;
> +		hdmi->disable_audio = dw_hdmi_ahb_audio_disable;
>  
>  		pdevinfo.name = "dw-hdmi-ahb-audio";
>  		pdevinfo.data = &audio;
> @@ -2126,6 +2151,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		audio.hdmi	= hdmi;
>  		audio.write	= hdmi_writeb;
>  		audio.read	= hdmi_readb;
> +		hdmi->enable_audio = dw_hdmi_i2s_audio_enable;
> +		hdmi->disable_audio = dw_hdmi_i2s_audio_disable;
>  
>  		pdevinfo.name = "dw-hdmi-i2s-audio";
>  		pdevinfo.data = &audio;
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks

[Romain Perier]

Currently, the audio sampler clock is enabled from dw_hdmi_setup() at
step E. and is kept enabled for later use. This clock should be enabled
and disabled along with the actual audio stream and not always on (that
is bad for PM). Futhermore, as described by the datasheet, the I2S
variant need to gate/ungate the clock when the stream is
enabled/disabled.

This commit adds a parameter to hdmi_audio_enable_clk() that controls
when the audio sample clock must be enabled or disabled. Then, it adds
the call to this function from dw_hdmi_i2s_audio_enable() and
dw_hdmi_i2s_audio_disable().

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index d34e0a5..3bd0807 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -559,6 +559,12 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
 
+static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable)
+{
+	hdmi_modb(hdmi, enable ? 0 : HDMI_MC_CLKDIS_AUDCLK_DISABLE,
+		  HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
+}
+
 void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
 {
 	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
@@ -572,12 +578,13 @@ void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
 void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
 {
 	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+	hdmi_enable_audio_clk(hdmi, true);
 }
 
 
 void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
 {
-
+	hdmi_enable_audio_clk(hdmi, false);
 }
 
 void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
@@ -1365,11 +1372,6 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi)
 	}
 }
 
-static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi)
-{
-	hdmi_modb(hdmi, 0, HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
-}
-
 /* Workaround to clear the overflow condition */
 static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
 {
@@ -1471,7 +1473,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 
 		/* HDMI Initialization Step E - Configure audio */
 		hdmi_clk_regenerator_update_pixel_clock(hdmi);
-		hdmi_enable_audio_clk(hdmi);
+		hdmi_enable_audio_clk(hdmi, true);
 	}
 
 	/* not for DVI mode */
-- 
2.9.3

[PATCH 2/2] drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks

[Romain Perier]

Currently, the audio sampler clock is enabled from dw_hdmi_setup() at
step E. and is kept enabled for later use. This clock should be enabled
and disabled along with the actual audio stream and not always on (that
is bad for PM). Futhermore, as described by the datasheet, the I2S
variant need to gate/ungate the clock when the stream is
enabled/disabled.

This commit adds a parameter to hdmi_audio_enable_clk() that controls
when the audio sample clock must be enabled or disabled. Then, it adds
the call to this function from dw_hdmi_i2s_audio_enable() and
dw_hdmi_i2s_audio_disable().

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 drivers/gpu/drm/bridge/dw-hdmi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index d34e0a5..3bd0807 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -559,6 +559,12 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
 
+static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable)
+{
+	hdmi_modb(hdmi, enable ? 0 : HDMI_MC_CLKDIS_AUDCLK_DISABLE,
+		  HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
+}
+
 void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
 {
 	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
@@ -572,12 +578,13 @@ void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
 void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
 {
 	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
+	hdmi_enable_audio_clk(hdmi, true);
 }
 
 
 void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
 {
-
+	hdmi_enable_audio_clk(hdmi, false);
 }
 
 void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
@@ -1365,11 +1372,6 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi)
 	}
 }
 
-static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi)
-{
-	hdmi_modb(hdmi, 0, HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
-}
-
 /* Workaround to clear the overflow condition */
 static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
 {
@@ -1471,7 +1473,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 
 		/* HDMI Initialization Step E - Configure audio */
 		hdmi_clk_regenerator_update_pixel_clock(hdmi);
-		hdmi_enable_audio_clk(hdmi);
+		hdmi_enable_audio_clk(hdmi, true);
 	}
 
 	/* not for DVI mode */
-- 
2.9.3

Re: [PATCH 2/2] drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks

[Neil Armstrong]

On 04/07/2017 04:19 PM, Romain Perier wrote:
> Currently, the audio sampler clock is enabled from dw_hdmi_setup() at
> step E. and is kept enabled for later use. This clock should be enabled
> and disabled along with the actual audio stream and not always on (that
> is bad for PM). Futhermore, as described by the datasheet, the I2S
> variant need to gate/ungate the clock when the stream is
> enabled/disabled.
> 
> This commit adds a parameter to hdmi_audio_enable_clk() that controls
> when the audio sample clock must be enabled or disabled. Then, it adds
> the call to this function from dw_hdmi_i2s_audio_enable() and
> dw_hdmi_i2s_audio_disable().
> 
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> index d34e0a5..3bd0807 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -559,6 +559,12 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
>  
> +static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable)
> +{
> +	hdmi_modb(hdmi, enable ? 0 : HDMI_MC_CLKDIS_AUDCLK_DISABLE,
> +		  HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
> +}
> +
>  void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> @@ -572,12 +578,13 @@ void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
>  void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +	hdmi_enable_audio_clk(hdmi, true);
>  }
>  
>  
>  void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
>  {
> -
> +	hdmi_enable_audio_clk(hdmi, false);
>  }

I think the NULL check is still valid if you fill this function, because the IP also
support other modes (SPDIF and GPA).

>  
>  void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
> @@ -1365,11 +1372,6 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi)
>  	}
>  }
>  
> -static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi)
> -{
> -	hdmi_modb(hdmi, 0, HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
> -}
> -
>  /* Workaround to clear the overflow condition */
>  static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
>  {
> @@ -1471,7 +1473,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>  
>  		/* HDMI Initialization Step E - Configure audio */
>  		hdmi_clk_regenerator_update_pixel_clock(hdmi);
> -		hdmi_enable_audio_clk(hdmi);
> +		hdmi_enable_audio_clk(hdmi, true);
>  	}
>  
>  	/* not for DVI mode */
> 

[PATCH 2/2] drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks

[Neil Armstrong]

On 04/07/2017 04:19 PM, Romain Perier wrote:
> Currently, the audio sampler clock is enabled from dw_hdmi_setup() at
> step E. and is kept enabled for later use. This clock should be enabled
> and disabled along with the actual audio stream and not always on (that
> is bad for PM). Futhermore, as described by the datasheet, the I2S
> variant need to gate/ungate the clock when the stream is
> enabled/disabled.
> 
> This commit adds a parameter to hdmi_audio_enable_clk() that controls
> when the audio sample clock must be enabled or disabled. Then, it adds
> the call to this function from dw_hdmi_i2s_audio_enable() and
> dw_hdmi_i2s_audio_disable().
> 
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> index d34e0a5..3bd0807 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -559,6 +559,12 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
>  
> +static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable)
> +{
> +	hdmi_modb(hdmi, enable ? 0 : HDMI_MC_CLKDIS_AUDCLK_DISABLE,
> +		  HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
> +}
> +
>  void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> @@ -572,12 +578,13 @@ void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
>  void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +	hdmi_enable_audio_clk(hdmi, true);
>  }
>  
>  
>  void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
>  {
> -
> +	hdmi_enable_audio_clk(hdmi, false);
>  }

I think the NULL check is still valid if you fill this function, because the IP also
support other modes (SPDIF and GPA).

>  
>  void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
> @@ -1365,11 +1372,6 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi)
>  	}
>  }
>  
> -static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi)
> -{
> -	hdmi_modb(hdmi, 0, HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
> -}
> -
>  /* Workaround to clear the overflow condition */
>  static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
>  {
> @@ -1471,7 +1473,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>  
>  		/* HDMI Initialization Step E - Configure audio */
>  		hdmi_clk_regenerator_update_pixel_clock(hdmi);
> -		hdmi_enable_audio_clk(hdmi);
> +		hdmi_enable_audio_clk(hdmi, true);
>  	}
>  
>  	/* not for DVI mode */
> 

Re: [PATCH 2/2] drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks

[Neil Armstrong]

On 04/07/2017 04:19 PM, Romain Perier wrote:
> Currently, the audio sampler clock is enabled from dw_hdmi_setup() at
> step E. and is kept enabled for later use. This clock should be enabled
> and disabled along with the actual audio stream and not always on (that
> is bad for PM). Futhermore, as described by the datasheet, the I2S
> variant need to gate/ungate the clock when the stream is
> enabled/disabled.
> 
> This commit adds a parameter to hdmi_audio_enable_clk() that controls
> when the audio sample clock must be enabled or disabled. Then, it adds
> the call to this function from dw_hdmi_i2s_audio_enable() and
> dw_hdmi_i2s_audio_disable().
> 
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> index d34e0a5..3bd0807 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -559,6 +559,12 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
>  
> +static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable)
> +{
> +	hdmi_modb(hdmi, enable ? 0 : HDMI_MC_CLKDIS_AUDCLK_DISABLE,
> +		  HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
> +}
> +
>  void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> @@ -572,12 +578,13 @@ void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
>  void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
>  {
>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
> +	hdmi_enable_audio_clk(hdmi, true);
>  }
>  
>  
>  void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
>  {
> -
> +	hdmi_enable_audio_clk(hdmi, false);
>  }

I think the NULL check is still valid if you fill this function, because the IP also
support other modes (SPDIF and GPA).

>  
>  void dw_hdmi_audio_enable(struct dw_hdmi *hdmi)
> @@ -1365,11 +1372,6 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi)
>  	}
>  }
>  
> -static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi)
> -{
> -	hdmi_modb(hdmi, 0, HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
> -}
> -
>  /* Workaround to clear the overflow condition */
>  static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
>  {
> @@ -1471,7 +1473,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>  
>  		/* HDMI Initialization Step E - Configure audio */
>  		hdmi_clk_regenerator_update_pixel_clock(hdmi);
> -		hdmi_enable_audio_clk(hdmi);
> +		hdmi_enable_audio_clk(hdmi, true);
>  	}
>  
>  	/* not for DVI mode */
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks

[Romain Perier]

Hello,


Le 07/04/2017 à 16:23, Neil Armstrong a écrit :
> On 04/07/2017 04:19 PM, Romain Perier wrote:
>> Currently, the audio sampler clock is enabled from dw_hdmi_setup() at
>> step E. and is kept enabled for later use. This clock should be enabled
>> and disabled along with the actual audio stream and not always on (that
>> is bad for PM). Futhermore, as described by the datasheet, the I2S
>> variant need to gate/ungate the clock when the stream is
>> enabled/disabled.
>>
>> This commit adds a parameter to hdmi_audio_enable_clk() that controls
>> when the audio sample clock must be enabled or disabled. Then, it adds
>> the call to this function from dw_hdmi_i2s_audio_enable() and
>> dw_hdmi_i2s_audio_disable().
>>
>> Signed-off-by: Romain Perier <romain.perier@collabora.com>
>> ---
>>  drivers/gpu/drm/bridge/dw-hdmi.c | 16 +++++++++-------
>>  1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
>> index d34e0a5..3bd0807 100644
>> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
>> @@ -559,6 +559,12 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
>>  }
>>  EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
>>  
>> +static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable)
>> +{
>> +	hdmi_modb(hdmi, enable ? 0 : HDMI_MC_CLKDIS_AUDCLK_DISABLE,
>> +		  HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
>> +}
>> +
>>  void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>>  {
>>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
>> @@ -572,12 +578,13 @@ void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
>>  void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
>>  {
>>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
>> +	hdmi_enable_audio_clk(hdmi, true);
>>  }
>>  
>>  
>>  void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
>>  {
>> -
>> +	hdmi_enable_audio_clk(hdmi, false);
>>  }
> I think the NULL check is still valid if you fill this function, because the IP also
> support other modes (SPDIF and GPA).

Ah, good point!

Thanks,
Romain

[PATCH 2/2] drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks

[Romain Perier]

Hello,


Le 07/04/2017 ? 16:23, Neil Armstrong a ?crit :
> On 04/07/2017 04:19 PM, Romain Perier wrote:
>> Currently, the audio sampler clock is enabled from dw_hdmi_setup() at
>> step E. and is kept enabled for later use. This clock should be enabled
>> and disabled along with the actual audio stream and not always on (that
>> is bad for PM). Futhermore, as described by the datasheet, the I2S
>> variant need to gate/ungate the clock when the stream is
>> enabled/disabled.
>>
>> This commit adds a parameter to hdmi_audio_enable_clk() that controls
>> when the audio sample clock must be enabled or disabled. Then, it adds
>> the call to this function from dw_hdmi_i2s_audio_enable() and
>> dw_hdmi_i2s_audio_disable().
>>
>> Signed-off-by: Romain Perier <romain.perier@collabora.com>
>> ---
>>  drivers/gpu/drm/bridge/dw-hdmi.c | 16 +++++++++-------
>>  1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
>> index d34e0a5..3bd0807 100644
>> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
>> @@ -559,6 +559,12 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
>>  }
>>  EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
>>  
>> +static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable)
>> +{
>> +	hdmi_modb(hdmi, enable ? 0 : HDMI_MC_CLKDIS_AUDCLK_DISABLE,
>> +		  HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
>> +}
>> +
>>  void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
>>  {
>>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
>> @@ -572,12 +578,13 @@ void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi)
>>  void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi)
>>  {
>>  	hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n);
>> +	hdmi_enable_audio_clk(hdmi, true);
>>  }
>>  
>>  
>>  void dw_hdmi_i2s_audio_disable(struct dw_hdmi *hdmi)
>>  {
>> -
>> +	hdmi_enable_audio_clk(hdmi, false);
>>  }
> I think the NULL check is still valid if you fill this function, because the IP also
> support other modes (SPDIF and GPA).

Ah, good point!

Thanks,
Romain

Re: [PATCH 0/2] drm: dw-hdmi: various improvements

[Archit Taneja]

Hi,

On 04/07/2017 07:49 PM, Romain Perier wrote:
> This set of patches split the stream handling functions in two parts. It
> introduces new callbacks that are specific to each variant, one for I2S
> and one for AHB.
>
> Then, as requested by the datasheet for the I2S variant, it adds support
> for gating the audio sampler clock when the audio stream is enabled and
> disabled.
>
> This patches series is the continuity of the following discussion:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html

Since these aren't fixes, could you make sure to redo the patches over:

git://anongit.freedesktop.org/drm-misc drm-misc-next

The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/

Thanks,
Archit

>
> Romain Perier (2):
>   drm: dw-hdmi: add specific I2S and AHB functions for stream handling
>   drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks
>
>  drivers/gpu/drm/bridge/dw-hdmi.c | 45 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 37 insertions(+), 8 deletions(-)
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 0/2] drm: dw-hdmi: various improvements

[Archit Taneja]

Hi,

On 04/07/2017 07:49 PM, Romain Perier wrote:
> This set of patches split the stream handling functions in two parts. It
> introduces new callbacks that are specific to each variant, one for I2S
> and one for AHB.
>
> Then, as requested by the datasheet for the I2S variant, it adds support
> for gating the audio sampler clock when the audio stream is enabled and
> disabled.
>
> This patches series is the continuity of the following discussion:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html

Since these aren't fixes, could you make sure to redo the patches over:

git://anongit.freedesktop.org/drm-misc drm-misc-next

The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/

Thanks,
Archit

>
> Romain Perier (2):
>   drm: dw-hdmi: add specific I2S and AHB functions for stream handling
>   drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks
>
>  drivers/gpu/drm/bridge/dw-hdmi.c | 45 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 37 insertions(+), 8 deletions(-)
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 0/2] drm: dw-hdmi: various improvements

[Archit Taneja]

Hi,

On 04/07/2017 07:49 PM, Romain Perier wrote:
> This set of patches split the stream handling functions in two parts. It
> introduces new callbacks that are specific to each variant, one for I2S
> and one for AHB.
>
> Then, as requested by the datasheet for the I2S variant, it adds support
> for gating the audio sampler clock when the audio stream is enabled and
> disabled.
>
> This patches series is the continuity of the following discussion:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html

Since these aren't fixes, could you make sure to redo the patches over:

git://anongit.freedesktop.org/drm-misc drm-misc-next

The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/

Thanks,
Archit

>
> Romain Perier (2):
>   drm: dw-hdmi: add specific I2S and AHB functions for stream handling
>   drm: dw-hdmi: Gate audio clock from the I2S enablement callbacks
>
>  drivers/gpu/drm/bridge/dw-hdmi.c | 45 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 37 insertions(+), 8 deletions(-)
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] drm: dw-hdmi: various improvements

[Russell King - ARM Linux]

On Mon, Apr 10, 2017 at 10:49:18AM +0530, Archit Taneja wrote:
> Hi,
> 
> On 04/07/2017 07:49 PM, Romain Perier wrote:
> >This set of patches split the stream handling functions in two parts. It
> >introduces new callbacks that are specific to each variant, one for I2S
> >and one for AHB.
> >
> >Then, as requested by the datasheet for the I2S variant, it adds support
> >for gating the audio sampler clock when the audio stream is enabled and
> >disabled.
> >
> >This patches series is the continuity of the following discussion:
> >http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html
> 
> Since these aren't fixes, could you make sure to redo the patches over:
> 
> git://anongit.freedesktop.org/drm-misc drm-misc-next
> 
> The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/

This is annoying as it makes submission of CEC support for dw-hdmi
rather difficult due to the now horrid cross-tree dependencies:

* if I submit it to the DRM tree, the DRM tree will build break because
  you don't have the necessary CEC changes which have recently been
  merged into the media tree.

* if I submit it to the media tree, the new files will be placed into
  drivers/gpu/drm/bridge and when stuff gets merged into linux-next
  and/or Linus' tree, things will need quite a large fixup (someone
  will have to rename the files and fix the Kconfig/Makefiles.)

So, I'll hold it back for another cycle to avoid the mess that would
result from trying to get it merged during this cycle.

-- 
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.

[PATCH 0/2] drm: dw-hdmi: various improvements

[Russell King - ARM Linux]

On Mon, Apr 10, 2017 at 10:49:18AM +0530, Archit Taneja wrote:
> Hi,
> 
> On 04/07/2017 07:49 PM, Romain Perier wrote:
> >This set of patches split the stream handling functions in two parts. It
> >introduces new callbacks that are specific to each variant, one for I2S
> >and one for AHB.
> >
> >Then, as requested by the datasheet for the I2S variant, it adds support
> >for gating the audio sampler clock when the audio stream is enabled and
> >disabled.
> >
> >This patches series is the continuity of the following discussion:
> >http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html
> 
> Since these aren't fixes, could you make sure to redo the patches over:
> 
> git://anongit.freedesktop.org/drm-misc drm-misc-next
> 
> The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/

This is annoying as it makes submission of CEC support for dw-hdmi
rather difficult due to the now horrid cross-tree dependencies:

* if I submit it to the DRM tree, the DRM tree will build break because
  you don't have the necessary CEC changes which have recently been
  merged into the media tree.

* if I submit it to the media tree, the new files will be placed into
  drivers/gpu/drm/bridge and when stuff gets merged into linux-next
  and/or Linus' tree, things will need quite a large fixup (someone
  will have to rename the files and fix the Kconfig/Makefiles.)

So, I'll hold it back for another cycle to avoid the mess that would
result from trying to get it merged during this cycle.

-- 
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 0/2] drm: dw-hdmi: various improvements

[Neil Armstrong]

On 04/10/2017 12:08 PM, Russell King - ARM Linux wrote:
> On Mon, Apr 10, 2017 at 10:49:18AM +0530, Archit Taneja wrote:
>> Hi,
>>
>> On 04/07/2017 07:49 PM, Romain Perier wrote:
>>> This set of patches split the stream handling functions in two parts. It
>>> introduces new callbacks that are specific to each variant, one for I2S
>>> and one for AHB.
>>>
>>> Then, as requested by the datasheet for the I2S variant, it adds support
>>> for gating the audio sampler clock when the audio stream is enabled and
>>> disabled.
>>>
>>> This patches series is the continuity of the following discussion:
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html
>>
>> Since these aren't fixes, could you make sure to redo the patches over:
>>
>> git://anongit.freedesktop.org/drm-misc drm-misc-next
>>
>> The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/
> 
> This is annoying as it makes submission of CEC support for dw-hdmi
> rather difficult due to the now horrid cross-tree dependencies:
> 
> * if I submit it to the DRM tree, the DRM tree will build break because
>   you don't have the necessary CEC changes which have recently been
>   merged into the media tree.
> 
> * if I submit it to the media tree, the new files will be placed into
>   drivers/gpu/drm/bridge and when stuff gets merged into linux-next
>   and/or Linus' tree, things will need quite a large fixup (someone
>   will have to rename the files and fix the Kconfig/Makefiles.)
> 
> So, I'll hold it back for another cycle to avoid the mess that would
> result from trying to get it merged during this cycle.
> 

Hi Russell,

Cross tree with media has already been done on this cycle with the
dw-hdmi formats changes.

Nevertheless, please push your updated dw-hdmi cec patchset for tests
and review based on the latest drm-misc-next and hverkuil's HPD notifier
release.

I would like to run it on some Amlogic boards.

Neil

[PATCH 0/2] drm: dw-hdmi: various improvements

[Neil Armstrong]

On 04/10/2017 12:08 PM, Russell King - ARM Linux wrote:
> On Mon, Apr 10, 2017 at 10:49:18AM +0530, Archit Taneja wrote:
>> Hi,
>>
>> On 04/07/2017 07:49 PM, Romain Perier wrote:
>>> This set of patches split the stream handling functions in two parts. It
>>> introduces new callbacks that are specific to each variant, one for I2S
>>> and one for AHB.
>>>
>>> Then, as requested by the datasheet for the I2S variant, it adds support
>>> for gating the audio sampler clock when the audio stream is enabled and
>>> disabled.
>>>
>>> This patches series is the continuity of the following discussion:
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html
>>
>> Since these aren't fixes, could you make sure to redo the patches over:
>>
>> git://anongit.freedesktop.org/drm-misc drm-misc-next
>>
>> The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/
> 
> This is annoying as it makes submission of CEC support for dw-hdmi
> rather difficult due to the now horrid cross-tree dependencies:
> 
> * if I submit it to the DRM tree, the DRM tree will build break because
>   you don't have the necessary CEC changes which have recently been
>   merged into the media tree.
> 
> * if I submit it to the media tree, the new files will be placed into
>   drivers/gpu/drm/bridge and when stuff gets merged into linux-next
>   and/or Linus' tree, things will need quite a large fixup (someone
>   will have to rename the files and fix the Kconfig/Makefiles.)
> 
> So, I'll hold it back for another cycle to avoid the mess that would
> result from trying to get it merged during this cycle.
> 

Hi Russell,

Cross tree with media has already been done on this cycle with the
dw-hdmi formats changes.

Nevertheless, please push your updated dw-hdmi cec patchset for tests
and review based on the latest drm-misc-next and hverkuil's HPD notifier
release.

I would like to run it on some Amlogic boards.

Neil

Re: [PATCH 0/2] drm: dw-hdmi: various improvements

[Neil Armstrong]

On 04/10/2017 12:08 PM, Russell King - ARM Linux wrote:
> On Mon, Apr 10, 2017 at 10:49:18AM +0530, Archit Taneja wrote:
>> Hi,
>>
>> On 04/07/2017 07:49 PM, Romain Perier wrote:
>>> This set of patches split the stream handling functions in two parts. It
>>> introduces new callbacks that are specific to each variant, one for I2S
>>> and one for AHB.
>>>
>>> Then, as requested by the datasheet for the I2S variant, it adds support
>>> for gating the audio sampler clock when the audio stream is enabled and
>>> disabled.
>>>
>>> This patches series is the continuity of the following discussion:
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html
>>
>> Since these aren't fixes, could you make sure to redo the patches over:
>>
>> git://anongit.freedesktop.org/drm-misc drm-misc-next
>>
>> The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/
> 
> This is annoying as it makes submission of CEC support for dw-hdmi
> rather difficult due to the now horrid cross-tree dependencies:
> 
> * if I submit it to the DRM tree, the DRM tree will build break because
>   you don't have the necessary CEC changes which have recently been
>   merged into the media tree.
> 
> * if I submit it to the media tree, the new files will be placed into
>   drivers/gpu/drm/bridge and when stuff gets merged into linux-next
>   and/or Linus' tree, things will need quite a large fixup (someone
>   will have to rename the files and fix the Kconfig/Makefiles.)
> 
> So, I'll hold it back for another cycle to avoid the mess that would
> result from trying to get it merged during this cycle.
> 

Hi Russell,

Cross tree with media has already been done on this cycle with the
dw-hdmi formats changes.

Nevertheless, please push your updated dw-hdmi cec patchset for tests
and review based on the latest drm-misc-next and hverkuil's HPD notifier
release.

I would like to run it on some Amlogic boards.

Neil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] drm: dw-hdmi: various improvements

[Russell King - ARM Linux]

On Mon, Apr 10, 2017 at 12:35:43PM +0200, Neil Armstrong wrote:
> Hi Russell,
> 
> Cross tree with media has already been done on this cycle with the
> dw-hdmi formats changes.
> 
> Nevertheless, please push your updated dw-hdmi cec patchset for tests
> and review based on the latest drm-misc-next and hverkuil's HPD notifier
> release.

You can get my current version at:

 git://git.armlinux.org.uk/~rmk/linux-arm.git cec-v18

As it's taken me over a week to validate both dw-hdmi and tda998x against
the latest version of Hans patches (despite me wanting to get it out to
Hans), I don't have the bandwidth to rebase it onto some other tree and
re-test at the moment.  As we're at -rc6 today, I doubt that'll happen
before the merge window, sorry.

-- 
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.

[PATCH 0/2] drm: dw-hdmi: various improvements

[Russell King - ARM Linux]

On Mon, Apr 10, 2017 at 12:35:43PM +0200, Neil Armstrong wrote:
> Hi Russell,
> 
> Cross tree with media has already been done on this cycle with the
> dw-hdmi formats changes.
> 
> Nevertheless, please push your updated dw-hdmi cec patchset for tests
> and review based on the latest drm-misc-next and hverkuil's HPD notifier
> release.

You can get my current version at:

 git://git.armlinux.org.uk/~rmk/linux-arm.git cec-v18

As it's taken me over a week to validate both dw-hdmi and tda998x against
the latest version of Hans patches (despite me wanting to get it out to
Hans), I don't have the bandwidth to rebase it onto some other tree and
re-test at the moment.  As we're at -rc6 today, I doubt that'll happen
before the merge window, sorry.

-- 
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 0/2] drm: dw-hdmi: various improvements

[Daniel Vetter]

On Mon, Apr 10, 2017 at 12:35:43PM +0200, Neil Armstrong wrote:
> On 04/10/2017 12:08 PM, Russell King - ARM Linux wrote:
> > On Mon, Apr 10, 2017 at 10:49:18AM +0530, Archit Taneja wrote:
> >> Hi,
> >>
> >> On 04/07/2017 07:49 PM, Romain Perier wrote:
> >>> This set of patches split the stream handling functions in two parts. It
> >>> introduces new callbacks that are specific to each variant, one for I2S
> >>> and one for AHB.
> >>>
> >>> Then, as requested by the datasheet for the I2S variant, it adds support
> >>> for gating the audio sampler clock when the audio stream is enabled and
> >>> disabled.
> >>>
> >>> This patches series is the continuity of the following discussion:
> >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html
> >>
> >> Since these aren't fixes, could you make sure to redo the patches over:
> >>
> >> git://anongit.freedesktop.org/drm-misc drm-misc-next
> >>
> >> The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/
> > 
> > This is annoying as it makes submission of CEC support for dw-hdmi
> > rather difficult due to the now horrid cross-tree dependencies:
> > 
> > * if I submit it to the DRM tree, the DRM tree will build break because
> >   you don't have the necessary CEC changes which have recently been
> >   merged into the media tree.
> > 
> > * if I submit it to the media tree, the new files will be placed into
> >   drivers/gpu/drm/bridge and when stuff gets merged into linux-next
> >   and/or Linus' tree, things will need quite a large fixup (someone
> >   will have to rename the files and fix the Kconfig/Makefiles.)
> > 
> > So, I'll hold it back for another cycle to avoid the mess that would
> > result from trying to get it merged during this cycle.
> > 
> 
> Hi Russell,
> 
> Cross tree with media has already been done on this cycle with the
> dw-hdmi formats changes.
> 
> Nevertheless, please push your updated dw-hdmi cec patchset for tests
> and review based on the latest drm-misc-next and hverkuil's HPD notifier
> release.
> 
> I would like to run it on some Amlogic boards.

Yeah, cross subsystem topic branches aren't rocket science, we have the
entire process documented and scripted on the drm side. Hairy depencies
really aren't a reason to not submit patches, we've got this :-)

But 4.12 is done anyway on the drm side, so will get delayed to 4.13 no
matter what. Still would be good to get the patches reviewed as early as
possible.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 0/2] drm: dw-hdmi: various improvements

[Daniel Vetter]

On Mon, Apr 10, 2017 at 12:35:43PM +0200, Neil Armstrong wrote:
> On 04/10/2017 12:08 PM, Russell King - ARM Linux wrote:
> > On Mon, Apr 10, 2017 at 10:49:18AM +0530, Archit Taneja wrote:
> >> Hi,
> >>
> >> On 04/07/2017 07:49 PM, Romain Perier wrote:
> >>> This set of patches split the stream handling functions in two parts. It
> >>> introduces new callbacks that are specific to each variant, one for I2S
> >>> and one for AHB.
> >>>
> >>> Then, as requested by the datasheet for the I2S variant, it adds support
> >>> for gating the audio sampler clock when the audio stream is enabled and
> >>> disabled.
> >>>
> >>> This patches series is the continuity of the following discussion:
> >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html
> >>
> >> Since these aren't fixes, could you make sure to redo the patches over:
> >>
> >> git://anongit.freedesktop.org/drm-misc drm-misc-next
> >>
> >> The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/
> > 
> > This is annoying as it makes submission of CEC support for dw-hdmi
> > rather difficult due to the now horrid cross-tree dependencies:
> > 
> > * if I submit it to the DRM tree, the DRM tree will build break because
> >   you don't have the necessary CEC changes which have recently been
> >   merged into the media tree.
> > 
> > * if I submit it to the media tree, the new files will be placed into
> >   drivers/gpu/drm/bridge and when stuff gets merged into linux-next
> >   and/or Linus' tree, things will need quite a large fixup (someone
> >   will have to rename the files and fix the Kconfig/Makefiles.)
> > 
> > So, I'll hold it back for another cycle to avoid the mess that would
> > result from trying to get it merged during this cycle.
> > 
> 
> Hi Russell,
> 
> Cross tree with media has already been done on this cycle with the
> dw-hdmi formats changes.
> 
> Nevertheless, please push your updated dw-hdmi cec patchset for tests
> and review based on the latest drm-misc-next and hverkuil's HPD notifier
> release.
> 
> I would like to run it on some Amlogic boards.

Yeah, cross subsystem topic branches aren't rocket science, we have the
entire process documented and scripted on the drm side. Hairy depencies
really aren't a reason to not submit patches, we've got this :-)

But 4.12 is done anyway on the drm side, so will get delayed to 4.13 no
matter what. Still would be good to get the patches reviewed as early as
possible.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/2] drm: dw-hdmi: various improvements

[Russell King - ARM Linux]

On Mon, Apr 10, 2017 at 02:08:44PM +0200, Daniel Vetter wrote:
> On Mon, Apr 10, 2017 at 12:35:43PM +0200, Neil Armstrong wrote:
> > On 04/10/2017 12:08 PM, Russell King - ARM Linux wrote:
> > > On Mon, Apr 10, 2017 at 10:49:18AM +0530, Archit Taneja wrote:
> > >> Hi,
> > >>
> > >> On 04/07/2017 07:49 PM, Romain Perier wrote:
> > >>> This set of patches split the stream handling functions in two parts. It
> > >>> introduces new callbacks that are specific to each variant, one for I2S
> > >>> and one for AHB.
> > >>>
> > >>> Then, as requested by the datasheet for the I2S variant, it adds support
> > >>> for gating the audio sampler clock when the audio stream is enabled and
> > >>> disabled.
> > >>>
> > >>> This patches series is the continuity of the following discussion:
> > >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html
> > >>
> > >> Since these aren't fixes, could you make sure to redo the patches over:
> > >>
> > >> git://anongit.freedesktop.org/drm-misc drm-misc-next
> > >>
> > >> The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/
> > > 
> > > This is annoying as it makes submission of CEC support for dw-hdmi
> > > rather difficult due to the now horrid cross-tree dependencies:
> > > 
> > > * if I submit it to the DRM tree, the DRM tree will build break because
> > >   you don't have the necessary CEC changes which have recently been
> > >   merged into the media tree.
> > > 
> > > * if I submit it to the media tree, the new files will be placed into
> > >   drivers/gpu/drm/bridge and when stuff gets merged into linux-next
> > >   and/or Linus' tree, things will need quite a large fixup (someone
> > >   will have to rename the files and fix the Kconfig/Makefiles.)
> > > 
> > > So, I'll hold it back for another cycle to avoid the mess that would
> > > result from trying to get it merged during this cycle.
> > > 
> > 
> > Hi Russell,
> > 
> > Cross tree with media has already been done on this cycle with the
> > dw-hdmi formats changes.
> > 
> > Nevertheless, please push your updated dw-hdmi cec patchset for tests
> > and review based on the latest drm-misc-next and hverkuil's HPD notifier
> > release.
> > 
> > I would like to run it on some Amlogic boards.
> 
> Yeah, cross subsystem topic branches aren't rocket science, we have the
> entire process documented and scripted on the drm side. Hairy depencies
> really aren't a reason to not submit patches, we've got this :-)
> 
> But 4.12 is done anyway on the drm side, so will get delayed to 4.13 no
> matter what. Still would be good to get the patches reviewed as early as
> possible.

Maybe if I had infinite bandwidth, then that would happen, but I don't.
That makes it very low priority for me, as there's no benefit to me to
spending time on it before the merge window, and if I wait until after
the merge window, things get easier, and I don't have to pollute my tree
with unmerged changes from other trees.  So there's actually beneficial
reasons for me to do nothing on this right now.

-- 
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.

[PATCH 0/2] drm: dw-hdmi: various improvements

[Russell King - ARM Linux]

On Mon, Apr 10, 2017 at 02:08:44PM +0200, Daniel Vetter wrote:
> On Mon, Apr 10, 2017 at 12:35:43PM +0200, Neil Armstrong wrote:
> > On 04/10/2017 12:08 PM, Russell King - ARM Linux wrote:
> > > On Mon, Apr 10, 2017 at 10:49:18AM +0530, Archit Taneja wrote:
> > >> Hi,
> > >>
> > >> On 04/07/2017 07:49 PM, Romain Perier wrote:
> > >>> This set of patches split the stream handling functions in two parts. It
> > >>> introduces new callbacks that are specific to each variant, one for I2S
> > >>> and one for AHB.
> > >>>
> > >>> Then, as requested by the datasheet for the I2S variant, it adds support
> > >>> for gating the audio sampler clock when the audio stream is enabled and
> > >>> disabled.
> > >>>
> > >>> This patches series is the continuity of the following discussion:
> > >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/493550.html
> > >>
> > >> Since these aren't fixes, could you make sure to redo the patches over:
> > >>
> > >> git://anongit.freedesktop.org/drm-misc drm-misc-next
> > >>
> > >> The dw-hdmi driver is now under drivers/gpu/drm/bridge/synopsis/
> > > 
> > > This is annoying as it makes submission of CEC support for dw-hdmi
> > > rather difficult due to the now horrid cross-tree dependencies:
> > > 
> > > * if I submit it to the DRM tree, the DRM tree will build break because
> > >   you don't have the necessary CEC changes which have recently been
> > >   merged into the media tree.
> > > 
> > > * if I submit it to the media tree, the new files will be placed into
> > >   drivers/gpu/drm/bridge and when stuff gets merged into linux-next
> > >   and/or Linus' tree, things will need quite a large fixup (someone
> > >   will have to rename the files and fix the Kconfig/Makefiles.)
> > > 
> > > So, I'll hold it back for another cycle to avoid the mess that would
> > > result from trying to get it merged during this cycle.
> > > 
> > 
> > Hi Russell,
> > 
> > Cross tree with media has already been done on this cycle with the
> > dw-hdmi formats changes.
> > 
> > Nevertheless, please push your updated dw-hdmi cec patchset for tests
> > and review based on the latest drm-misc-next and hverkuil's HPD notifier
> > release.
> > 
> > I would like to run it on some Amlogic boards.
> 
> Yeah, cross subsystem topic branches aren't rocket science, we have the
> entire process documented and scripted on the drm side. Hairy depencies
> really aren't a reason to not submit patches, we've got this :-)
> 
> But 4.12 is done anyway on the drm side, so will get delayed to 4.13 no
> matter what. Still would be good to get the patches reviewed as early as
> possible.

Maybe if I had infinite bandwidth, then that would happen, but I don't.
That makes it very low priority for me, as there's no benefit to me to
spending time on it before the merge window, and if I wait until after
the merge window, things get easier, and I don't have to pollute my tree
with unmerged changes from other trees.  So there's actually beneficial
reasons for me to do nothing on this right now.

-- 
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.