* [PATCH v2 0/2] drm: dw-hdmi: various improvements @ 2017-04-14 8:31 Romain Perier 2017-04-14 8:31 ` [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling Romain Perier 2017-04-14 8:31 ` [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks Romain Perier 0 siblings, 2 replies; 10+ messages in thread From: Romain Perier @ 2017-04-14 8:31 UTC (permalink / raw) To: Archit Taneja, David Airlie Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel, Jose Abreu, Russell King, Neil Armstrong, 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 Changes in v2: - Add NULL check in dw_hdmi_audio_enable/dw_hdmi_audio_disable - Don't define dw_hdmi_i2s_audio_disable in PATCH 1/2 - Rebased onto drm-misc-next 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/synopsys/dw-hdmi.c | 46 +++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 8 deletions(-) -- 2.9.3 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling 2017-04-14 8:31 [PATCH v2 0/2] drm: dw-hdmi: various improvements Romain Perier @ 2017-04-14 8:31 ` Romain Perier 2017-04-14 9:05 ` Neil Armstrong 2017-04-19 4:49 ` Archit Taneja 2017-04-14 8:31 ` [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks Romain Perier 1 sibling, 2 replies; 10+ messages in thread From: Romain Perier @ 2017-04-14 8:31 UTC (permalink / raw) To: Archit Taneja, David Airlie Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel, Jose Abreu, Russell King, Neil Armstrong, 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/synopsys/dw-hdmi.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 4b6f216..5b328c0 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -173,6 +173,8 @@ struct dw_hdmi { unsigned int reg_shift; struct regmap *regm; + void (*enable_audio)(struct dw_hdmi *hdmi); + void (*disable_audio)(struct dw_hdmi *hdmi); }; #define HDMI_IH_PHY_STAT0_RX_SENSE \ @@ -542,13 +544,29 @@ 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_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); + if (hdmi->enable_audio) + hdmi->enable_audio(hdmi); spin_unlock_irqrestore(&hdmi->audio_lock, flags); } EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable); @@ -559,7 +577,8 @@ 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); + if (hdmi->disable_audio) + hdmi->disable_audio(hdmi); spin_unlock_irqrestore(&hdmi->audio_lock, flags); } EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable); @@ -2404,6 +2423,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; @@ -2416,6 +2437,7 @@ __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; pdevinfo.name = "dw-hdmi-i2s-audio"; pdevinfo.data = &audio; -- 2.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling 2017-04-14 8:31 ` [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling Romain Perier @ 2017-04-14 9:05 ` Neil Armstrong 2017-04-19 4:49 ` Archit Taneja 1 sibling, 0 replies; 10+ messages in thread From: Neil Armstrong @ 2017-04-14 9:05 UTC (permalink / raw) To: Romain Perier, Archit Taneja, David Airlie Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel, Jose Abreu, Russell King On 04/14/2017 10:31 AM, 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 > > Signed-off-by: Romain Perier <romain.perier@collabora.com> > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 4b6f216..5b328c0 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -173,6 +173,8 @@ struct dw_hdmi { > > unsigned int reg_shift; > struct regmap *regm; > + void (*enable_audio)(struct dw_hdmi *hdmi); > + void (*disable_audio)(struct dw_hdmi *hdmi); > }; > > #define HDMI_IH_PHY_STAT0_RX_SENSE \ > @@ -542,13 +544,29 @@ 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_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); > + if (hdmi->enable_audio) > + hdmi->enable_audio(hdmi); > spin_unlock_irqrestore(&hdmi->audio_lock, flags); > } > EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable); > @@ -559,7 +577,8 @@ 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); > + if (hdmi->disable_audio) > + hdmi->disable_audio(hdmi); > spin_unlock_irqrestore(&hdmi->audio_lock, flags); > } > EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable); > @@ -2404,6 +2423,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; > @@ -2416,6 +2437,7 @@ __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; > > pdevinfo.name = "dw-hdmi-i2s-audio"; > pdevinfo.data = &audio; > Hi Romain, Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling 2017-04-14 8:31 ` [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling Romain Perier 2017-04-14 9:05 ` Neil Armstrong @ 2017-04-19 4:49 ` Archit Taneja 1 sibling, 0 replies; 10+ messages in thread From: Archit Taneja @ 2017-04-19 4:49 UTC (permalink / raw) To: Romain Perier Cc: David Airlie, Jose Abreu, Neil Armstrong, Russell King, dri-devel, linux-kernel, linux-rockchip, linux-arm-kernel On 04/14/2017 02:01 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 > > Signed-off-by: Romain Perier <romain.perier@collabora.com> > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 4b6f216..5b328c0 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -173,6 +173,8 @@ struct dw_hdmi { > > unsigned int reg_shift; > struct regmap *regm; > + void (*enable_audio)(struct dw_hdmi *hdmi); > + void (*disable_audio)(struct dw_hdmi *hdmi); > }; > > #define HDMI_IH_PHY_STAT0_RX_SENSE \ > @@ -542,13 +544,29 @@ 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); > +} > + I get some sparse warnings asking for the above 3 to be static. Thanks, Archit > 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); > + if (hdmi->enable_audio) > + hdmi->enable_audio(hdmi); > spin_unlock_irqrestore(&hdmi->audio_lock, flags); > } > EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable); > @@ -559,7 +577,8 @@ 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); > + if (hdmi->disable_audio) > + hdmi->disable_audio(hdmi); > spin_unlock_irqrestore(&hdmi->audio_lock, flags); > } > EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable); > @@ -2404,6 +2423,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; > @@ -2416,6 +2437,7 @@ __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; > > pdevinfo.name = "dw-hdmi-i2s-audio"; > pdevinfo.data = &audio; > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks 2017-04-14 8:31 [PATCH v2 0/2] drm: dw-hdmi: various improvements Romain Perier 2017-04-14 8:31 ` [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling Romain Perier @ 2017-04-14 8:31 ` Romain Perier 2017-04-14 9:06 ` Neil Armstrong 2017-04-19 4:51 ` Archit Taneja 1 sibling, 2 replies; 10+ messages in thread From: Romain Perier @ 2017-04-14 8:31 UTC (permalink / raw) To: Archit Taneja, David Airlie Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel, Jose Abreu, Russell King, Neil Armstrong, 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/synopsys/dw-hdmi.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 5b328c0..a6da634 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -544,6 +544,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); @@ -557,6 +563,12 @@ 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) @@ -1592,11 +1604,6 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) HDMI_MC_FLOWCTRL); } -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) { @@ -1710,7 +1717,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 */ @@ -2438,6 +2445,7 @@ __dw_hdmi_probe(struct platform_device *pdev, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks 2017-04-14 8:31 ` [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks Romain Perier @ 2017-04-14 9:06 ` Neil Armstrong 2017-04-19 4:51 ` Archit Taneja 1 sibling, 0 replies; 10+ messages in thread From: Neil Armstrong @ 2017-04-14 9:06 UTC (permalink / raw) To: Romain Perier, Archit Taneja, David Airlie Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel, Jose Abreu, Russell King On 04/14/2017 10:31 AM, 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/synopsys/dw-hdmi.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 5b328c0..a6da634 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -544,6 +544,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); > @@ -557,6 +563,12 @@ 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) > @@ -1592,11 +1604,6 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) > HDMI_MC_FLOWCTRL); > } > > -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) > { > @@ -1710,7 +1717,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 */ > @@ -2438,6 +2445,7 @@ __dw_hdmi_probe(struct platform_device *pdev, > 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; > Hi Romain, Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks 2017-04-14 8:31 ` [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks Romain Perier 2017-04-14 9:06 ` Neil Armstrong @ 2017-04-19 4:51 ` Archit Taneja 2017-04-20 9:39 ` Archit Taneja 2017-04-24 6:44 ` Romain Perier 1 sibling, 2 replies; 10+ messages in thread From: Archit Taneja @ 2017-04-19 4:51 UTC (permalink / raw) To: Romain Perier, David Airlie Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel, Jose Abreu, Russell King, Neil Armstrong On 04/14/2017 02:01 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 s/Futhermore/Furthermore > variant need to gate/ungate the clock when the stream is s/need/needs > 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/synopsys/dw-hdmi.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 5b328c0..a6da634 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -544,6 +544,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); > @@ -557,6 +563,12 @@ 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); > } This should be static too. If you're okay with the suggestions, I can fix these myself and push. Let me know if that's okay. Thanks, Archit > > void dw_hdmi_audio_enable(struct dw_hdmi *hdmi) > @@ -1592,11 +1604,6 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) > HDMI_MC_FLOWCTRL); > } > > -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) > { > @@ -1710,7 +1717,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 */ > @@ -2438,6 +2445,7 @@ __dw_hdmi_probe(struct platform_device *pdev, > 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; > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks 2017-04-19 4:51 ` Archit Taneja @ 2017-04-20 9:39 ` Archit Taneja 2017-04-24 6:55 ` Romain Perier 2017-04-24 6:44 ` Romain Perier 1 sibling, 1 reply; 10+ messages in thread From: Archit Taneja @ 2017-04-20 9:39 UTC (permalink / raw) To: Romain Perier, David Airlie Cc: Jose Abreu, Neil Armstrong, Russell King, dri-devel, linux-kernel, linux-rockchip, linux-arm-kernel On 04/19/2017 10:21 AM, Archit Taneja wrote: > > > On 04/14/2017 02:01 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 > > s/Futhermore/Furthermore > >> variant need to gate/ungate the clock when the stream is > > s/need/needs > >> 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/synopsys/dw-hdmi.c | 20 ++++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> index 5b328c0..a6da634 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> @@ -544,6 +544,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); >> @@ -557,6 +563,12 @@ 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); >> } > > This should be static too. > > If you're okay with the suggestions, I can fix these myself and push. Let > me know if that's okay. Took the liberty of going ahead with the fixes. Queued both patches to drm-misc-next. Thanks, Archit > > Thanks, > Archit > >> >> void dw_hdmi_audio_enable(struct dw_hdmi *hdmi) >> @@ -1592,11 +1604,6 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) >> HDMI_MC_FLOWCTRL); >> } >> >> -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) >> { >> @@ -1710,7 +1717,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 */ >> @@ -2438,6 +2445,7 @@ __dw_hdmi_probe(struct platform_device *pdev, >> 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; >> > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks 2017-04-20 9:39 ` Archit Taneja @ 2017-04-24 6:55 ` Romain Perier 0 siblings, 0 replies; 10+ messages in thread From: Romain Perier @ 2017-04-24 6:55 UTC (permalink / raw) To: Archit Taneja, David Airlie Cc: Jose Abreu, Neil Armstrong, Russell King, dri-devel, linux-kernel, linux-rockchip, linux-arm-kernel Hello, > Took the liberty of going ahead with the fixes. Queued both patches to > drm-misc-next. > > Thanks, > Archit You were right. Sorry for the delay, I am back from vacations :) Regards, Romain ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks 2017-04-19 4:51 ` Archit Taneja 2017-04-20 9:39 ` Archit Taneja @ 2017-04-24 6:44 ` Romain Perier 1 sibling, 0 replies; 10+ messages in thread From: Romain Perier @ 2017-04-24 6:44 UTC (permalink / raw) To: Archit Taneja, David Airlie Cc: dri-devel, linux-rockchip, linux-arm-kernel, linux-kernel, Jose Abreu, Russell King, Neil Armstrong Hello, Le 19/04/2017 à 06:51, Archit Taneja a écrit : > > > On 04/14/2017 02:01 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 > > s/Futhermore/Furthermore > >> variant need to gate/ungate the clock when the stream is > > s/need/needs > >> 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/synopsys/dw-hdmi.c | 20 ++++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> index 5b328c0..a6da634 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> @@ -544,6 +544,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); >> @@ -557,6 +563,12 @@ 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); >> } > > This should be static too. > > If you're okay with the suggestions, I can fix these myself and push. Let > me know if that's okay. > > Thanks, > Archit > Yes, I completely agree. Thanks, Romain ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-04-24 6:55 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-14 8:31 [PATCH v2 0/2] drm: dw-hdmi: various improvements Romain Perier 2017-04-14 8:31 ` [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling Romain Perier 2017-04-14 9:05 ` Neil Armstrong 2017-04-19 4:49 ` Archit Taneja 2017-04-14 8:31 ` [PATCH v2 2/2] drm: dw-hdmi: gate audio clock from the I2S enablement callbacks Romain Perier 2017-04-14 9:06 ` Neil Armstrong 2017-04-19 4:51 ` Archit Taneja 2017-04-20 9:39 ` Archit Taneja 2017-04-24 6:55 ` Romain Perier 2017-04-24 6:44 ` Romain Perier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).