On Wed, 2012-03-28 at 16:38 -0600, Ricardo Neri wrote:
> Implement the DSS device driver audio support interface in the HDMI
> panel driver and generic driver. The implementation relies on the
> IP-specific functions that are defined at DSS probe time.
>
> A HW-safe spinlock is used to protect the audio functions. This is because
What is a "HW-safe spinlock"?
> the audio functions may be called while holding a lock; in such case,
> the panel's driver mutex is not suitable. Functions should be used
> to set registers and should not wait for any other event.
Are you sure this is the only option? What lock is being held? While a
spinlock may be ok for now, quite often enabling/disabling things do not
happen immediately, and it's much easier to do the wait synchronously.
> Signed-off-by: Ricardo Neri <ricardo.neri@xxxxxx>
> ---
> drivers/video/omap2/dss/dss.h | 7 +++
> drivers/video/omap2/dss/hdmi.c | 33 +++++++++++++++
> drivers/video/omap2/dss/hdmi_panel.c | 76 ++++++++++++++++++++++++++++++++++
> 3 files changed, 116 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index 32ff69f..fca4490 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -520,6 +520,13 @@ int omapdss_hdmi_read_edid(u8 *buf, int len);
> bool omapdss_hdmi_detect(void);
> int hdmi_panel_init(void);
> void hdmi_panel_exit(void);
> +#ifdef CONFIG_OMAP4_DSS_HDMI_AUDIO
> +int hdmi_audio_enable(bool enable);
> +int hdmi_audio_start(bool start);
> +bool hdmi_mode_has_audio(void);
> +int hdmi_audio_config(struct snd_aes_iec958 *iec,
> + struct snd_cea_861_aud_if *aud_if);
> +#endif
>
> /* RFBI */
> #ifdef CONFIG_OMAP2_DSS_RFBI
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index bd44891..880509d 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -698,6 +698,39 @@ int hdmi_compute_acr(u32 sample_freq, u32 *n, u32 *cts)
>
> return 0;
> }
> +
> +int hdmi_audio_enable(bool enable)
> +{
> + DSSDBG("audio_enable\n");
> +
> + hdmi.ip_data.ops->audio_enable(&hdmi.ip_data, enable);
Shouldn't this, and the others below, return the value from the called
function, instead of always returning 0?
> +
> + return 0;
> +}
> +
> +int hdmi_audio_start(bool start)
> +{
> + DSSDBG("audio_enable\n");
> +
> + hdmi.ip_data.ops->audio_start(&hdmi.ip_data, start);
> +
> + return 0;
> +}
> +
> +bool hdmi_mode_has_audio(void)
> +{
> + if (hdmi.ip_data.cfg.cm.mode == HDMI_HDMI)
> + return true;
> + else
> + return false;
> +}
> +
> +int hdmi_audio_config(struct snd_aes_iec958 *iec,
> + struct snd_cea_861_aud_if *aud_if)
> +{
> + return hdmi.ip_data.ops->audio_config(&hdmi.ip_data, iec, aud_if);
> +}
> +
> #endif
>
> /* HDMI HW IP initialisation */
> diff --git a/drivers/video/omap2/dss/hdmi_panel.c b/drivers/video/omap2/dss/hdmi_panel.c
> index 533d5dc..dac1ac2 100644
> --- a/drivers/video/omap2/dss/hdmi_panel.c
> +++ b/drivers/video/omap2/dss/hdmi_panel.c
> @@ -31,6 +31,10 @@
>
> static struct {
> struct mutex hdmi_lock;
> +#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
> + /* protects calls to HDMI driver audio functionality */
> + spinlock_t hdmi_sp_lock;
What does "sp" stand for? Spinlock? Perhaps a better name would be
"audio_lock", if it's audio specific. And probably no reason to prefix
it with "hdmi", as it's inside "hdmi" struct already.
Tomi
Attachment:
signature.asc
Description: This is a digitally signed message part
