On 08/28/15 13:37, Tomi Valkeinen wrote:
On 26/08/15 16:11, Jyri Sarha wrote:
diff --git a/drivers/video/fbdev/omap2/dss/hdmi5.c b/drivers/video/fbdev/omap2/dss/hdmi5.c index 7f87578..f352c4b 100644 --- a/drivers/video/fbdev/omap2/dss/hdmi5.c +++ b/drivers/video/fbdev/omap2/dss/hdmi5.c @@ -352,6 +352,7 @@ static int read_edid(u8 *buf, int len) static int hdmi_display_enable(struct omap_dss_device *dssdev) { struct omap_dss_device *out = &hdmi.output;
unsigned long flags; int r = 0;
DSSDBG("ENTER hdmi_display_enable\n");
@@ -370,7 +371,37 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev) goto err0; }
- if (hdmi.audio_configured) {
spin_lock_irqsave(&hdmi.audio_playing_lock, flags);hdmi_wp_audio_core_req_enable(&hdmi.wp, false);hdmi_wp_audio_enable(&hdmi.wp, false);if (hdmi.wp_idlemode > 0)REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG,hdmi.wp_idlemode, 3, 2);hdmi.wp_idlemode = -1;spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);Here I think the audio HW is always disabled already. It has to be, because the whole HDMI IP has been off. So the above should not be needed.
r = hdmi5_audio_config(&hdmi.core, &hdmi.wp, &hdmi.audio_config,hdmi.cfg.timings.pixelclock);if (r) {DSSERR("Error restoring audio configuration: %d", r);hdmi.audio_abort_cb(&hdmi.pdev->dev);hdmi.audio_configured = false;}- }
- spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
- if (hdmi.audio_configured && hdmi.audio_playing) {
/* No-idle while playing audio, store the old value */hdmi.wp_idlemode =REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);hdmi_wp_audio_enable(&hdmi.wp, true);hdmi_wp_audio_core_req_enable(&hdmi.wp, true);- } hdmi.display_enabled = true;
- spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
Maybe you've looked at the locking carefully, but it's not obvious to me. So is hdmi_audio_start and hdmi_audio_stop the only functions that are called from atomic context? Every other function is protected with the mutex?
The idea is for the spinlock to make audio start, audio stop, and updates to hdmi.display_enabled and hdmi.audio_playing variable atomic.
This is needed for the transitions from display enabled state (when audio start/stop commands can be written to HW) to display disabled state (when audio start/stop commands update only the hdmi.audio_playing variable) to always serialize correctly.
IOW, the idea is to make sure the hdmi.audio_playing variable is always in sync with what is in the HW when hdmi.display_enabled == true.
For example: when display is turned back on we take the spinlock and we can be sure that the audio start/stop status won't change while we update the HW according to current state and set hdmi.display_enabled to true. After releasing the lock hdmi.display_enabled is true and all audio_start and audio_stop commands write their stuff directly to HW.
In theory, just making hdmi.display_enabled and hdmi.audio_playing atomic-variables and touching them always in correct oreder should be enough, but explaining the mechanism would then be even trickier.
Cheers, Jyri