alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Yang Kuankuan <ykk@rock-chips.com>
To: Russell King <rmk+kernel@arm.linux.org.uk>,
	alsa-devel@alsa-project.org, dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org
Cc: Fabio Estevam <fabio.estevam@freescale.com>,
	David Airlie <airlied@linux.ie>, Mark Brown <broonie@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>
Subject: Re: [PATCH RFC 06/11] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio
Date: Tue, 31 Mar 2015 03:45:15 -0400	[thread overview]
Message-ID: <551A508B.1010106@rock-chips.com> (raw)
In-Reply-To: <E1YcfXu-0002v9-SB@rmk-PC.arm.linux.org.uk>

Hi Russell,

On 03/30/2015 03:40 PM, Russell King wrote:
> iMX6 devices from an errata (ERR005174) where the audio FIFO can be
> emptied while it is partially full, resulting in misalignment of the
> audio samples.
>
> To prevent this, the errata workaround recommends writing N as zero
> until the audio FIFO has been loaded by DMA.  Writing N=0 prevents the
> HDMI bridge from reading from the audio FIFO, effectively disabling
> audio.
>
> This means we need to provide the audio driver with a pair of functions
> to enable/disable audio.  These are dw_hdmi_audio_enable() and
> dw_hdmi_audio_disable().
>
> A spinlock is introduced to ensure that setting the CTS/N values can't
> race, ensuring that the audio driver calling the enable/disable
> functions (which are called in an atomic context) can't race with a
> modeset.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>   drivers/gpu/drm/bridge/dw_hdmi.c | 34 +++++++++++++++++++++++++++++++++-
>   include/drm/bridge/dw_hdmi.h     |  2 ++
>   2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
> index 245d17e58dec..67f07d15c12b 100644
> --- a/drivers/gpu/drm/bridge/dw_hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c
> @@ -18,6 +18,7 @@
>   #include <linux/hdmi.h>
>   #include <linux/mutex.h>
>   #include <linux/of_device.h>
> +#include <linux/spinlock.h>
>   
>   #include <drm/drm_of.h>
>   #include <drm/drmP.h>
> @@ -124,8 +125,12 @@ struct dw_hdmi {
>   	struct i2c_adapter *ddc;
>   	void __iomem *regs;
>   
> +	spinlock_t audio_lock;
>   	struct mutex audio_mutex;
>   	unsigned int sample_rate;
> +	unsigned int audio_cts;
> +	unsigned int audio_n;
> +	bool audio_enable;
>   	int ratio;
>   
>   	void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
> @@ -347,7 +352,11 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
>   	dev_dbg(hdmi->dev, "%s: samplerate=%ukHz ratio=%d pixelclk=%luMHz N=%d cts=%d\n",
>   		__func__, sample_rate, ratio, pixel_clk, n, cts);
>   
> -	hdmi_set_cts_n(hdmi, cts, n);
> +	spin_lock_irq(&hdmi->audio_lock);
> +	hdmi->audio_n = n;
> +	hdmi->audio_cts = cts;
> +	hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> +	spin_unlock_irq(&hdmi->audio_lock);
>   }
>   
>   static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> @@ -376,6 +385,28 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate)
>   }
>   EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
>   
> +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);
> +	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable);
> +
> +void dw_hdmi_audio_disable(struct dw_hdmi *hdmi)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&hdmi->audio_lock, flags);
> +	hdmi->audio_enable = false;
> +	hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0);
> +	spin_unlock_irqrestore(&hdmi->audio_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable);
> +

It is an smart way to operate audio status that do not relate to the audio
interfaces (AHB or I2S), but I am not sure whether this will work on 
rockchip
platform, I will do some experiment to test it as soon as possible :)

Best regards.
Yakir Yang

>   /*
>    * this submodule is responsible for the video data synchronization.
>    * for example, for RGB 4:4:4 input, the data map is defined as
> @@ -1579,6 +1610,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
>   	hdmi->encoder = encoder;
>   
>   	mutex_init(&hdmi->audio_mutex);
> +	spin_lock_init(&hdmi->audio_lock);
>   
>   	of_property_read_u32(np, "reg-io-width", &val);
>   
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 424b51f6a215..ab5d36e83ea2 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -62,5 +62,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master,
>   		 const struct dw_hdmi_plat_data *plat_data);
>   
>   void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
> +void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
> +void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
>   
>   #endif /* __IMX_HDMI_H__ */

  reply	other threads:[~2015-03-31  7:44 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-30 19:39 [RFC 0/11] dw_hdmi cleanups, audio preparation, helpers and ahb audio support Russell King - ARM Linux
2015-03-30 19:40 ` [PATCH RFC 01/11] drm: bridge/dw_hdmi: clean up hdmi_set_clk_regenerator() Russell King
2015-03-31  6:55   ` Yang Kuankuan
2015-03-31 10:35     ` Russell King - ARM Linux
2015-04-01  1:54       ` Yakir
2015-03-30 19:40 ` [PATCH RFC 02/11] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode() Russell King
2015-03-31  9:02   ` Yang Kuankuan
2015-03-31 11:57     ` Russell King - ARM Linux
2015-04-01  1:31       ` Yakir
2015-03-30 19:40 ` [PATCH RFC 03/11] drm: bridge/dw_hdmi: simplify hdmi_config_AVI() a little Russell King
2015-03-30 19:40 ` [PATCH RFC 04/11] drm: bridge/dw_hdmi: remove mhsyncpolarity/mvsyncpolarity/minterlaced Russell King
2015-03-30 19:40 ` [PATCH RFC 05/11] drm: bridge/dw_hdmi: introduce interface to setting sample rate Russell King
2015-03-30 19:40 ` [PATCH RFC 06/11] drm: bridge/dw_hdmi: introduce interfaces to enable and disable audio Russell King
2015-03-31  7:45   ` Yang Kuankuan [this message]
2015-03-31  9:15   ` Philipp Zabel
2015-03-30 19:40 ` [PATCH RFC 07/11] drm/edid: add function to help find SADs Russell King
2015-04-01 11:47   ` Jani Nikula
2015-04-01 11:56     ` Russell King - ARM Linux
2015-04-02 10:52       ` [PATCH] drm/edid: add #defines for ELD versions Jani Nikula
2015-03-30 19:40 ` [PATCH RFC 08/11] sound/core: add DRM ELD helper Russell King
2015-03-31  9:12   ` Philipp Zabel
2015-03-30 19:40 ` [PATCH RFC 09/11] sound/core: add IEC958 channel status helper Russell King
2015-03-31  8:30   ` Yang Kuankuan
2015-03-31  9:13     ` Russell King - ARM Linux
2015-04-01  2:04       ` Yakir
2015-04-01  7:58         ` Russell King - ARM Linux
2015-03-31  9:10   ` Philipp Zabel
2015-03-31  9:16     ` Russell King - ARM Linux
2015-03-30 19:40 ` [PATCH RFC 10/11] drm: bridge/dw_hdmi-ahb-audio: add audio driver Russell King
2015-03-30 19:40 ` [PATCH RFC 11/11] drm: bridge/dw_hdmi-ahb-audio: parse ELD from HDMI driver Russell King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=551A508B.1010106@rock-chips.com \
    --to=ykk@rock-chips.com \
    --cc=airlied@linux.ie \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=fabio.estevam@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=p.zabel@pengutronix.de \
    --cc=rmk+kernel@arm.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).