* Re: [PATCH] ARC: add negative dependency for VGA_CONSOLE
From: Tomi Valkeinen @ 2015-08-26 11:34 UTC (permalink / raw)
To: Vineet Gupta, Geert Uytterhoeven, Yuriy Kolerov
Cc: linux-kernel@vger.kernel.org, Linux-Arch,
Linux Fbdev development list, Arnd Bergmann, Andrew Morton,
Jean-Christophe PLAGNIOL-VILLARD, Alexey Brodkin,
Anton.Kolesov@synopsys.com, Francois Bedard
In-Reply-To: <C2D7FE5348E1B147BCA15975FBA23075CFB85748@IN01WEMBXB.internal.synopsys.com>
[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]
On 25/08/15 10:47, Vineet Gupta wrote:
> On Monday 17 August 2015 09:51 PM, Geert Uytterhoeven wrote:
>> On Mon, Aug 17, 2015 at 5:52 PM, Yuriy Kolerov
>> <yuriy.kolerov@synopsys.com> wrote:
>>> Architectures which support VGA console must define screen_info
>>> structurture from "uapi/linux/screen_info.h". Otherwise undefined
>>> symbol error occurs. Usually it's defined in "setup.c" for each
>>> architecture.
>>>
>>> If an architecture does not support VGA console (ARC's case)
>>> there are 2 ways: define a dummy instance of screen_info or add
>>> a negative dependency for VGA_CONSOLE in to prevent selecting
>>> this option.
>>>
>>> I've implemented the second way. However the best solution is to
>>> add HAVE_VGA_CONSOLE option for targets which support VGA
>>> console. Then turn off VGA_CONSOLE by default and add dependency
>>> to HAVE_VGA_CONSOLE. But right now it's better to just add a
>>> negative dependency for ARC and then consider how to collaborate
>>> about this issue with maintainers of other architectures.
>> +1
>>
>>> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
>> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
> Hi Tomi,
>
> Is anything else needed for this to be merged.
This is already in linux-next, merged by Andrew Morton.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2] pwm-backlight: Avoid backlight flicker when probed from DT
From: Lee Jones @ 2015-08-26 11:58 UTC (permalink / raw)
To: Philipp Zabel
Cc: Thierry Reding, Jingoo Han, Ajay Kumar, linux-pwm, linux-fbdev,
linux-kernel, kernel
In-Reply-To: <1440580842.3190.26.camel@pengutronix.de>
On Wed, 26 Aug 2015, Philipp Zabel wrote:
> Hi Thierry, Jingoo, Lee,
>
> Am Donnerstag, den 30.07.2015, 19:22 +0200 schrieb Philipp Zabel:
> > If the driver is probed from the device tree, and there is a phandle
> > property set on it, and the enable GPIO is already configured as output,
> > and the backlight is currently disabled, keep it disabled.
> > If all these conditions are met, assume there will be some other driver
> > that can enable the backlight at the appropriate time.
>
> do you have any comments on this?
>
> This patch fixes premature backlight illumination issues for DT probed
> devices with enable gpio or regulator without changing the behavior of
> the non-DT case or DT cases where the enable gpio and regulator are kept
> initially enabled or where no other device node points to the backlight
> via phandle.
I need Thierry's review to move forward on this.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH 0/2] OMAPDSS: Couple of HDMI audio fixes
From: Jyri Sarha @ 2015-08-26 13:11 UTC (permalink / raw)
To: alsa-devel, linux-fbdev, linux-omap
Cc: peter.ujfalusi, liam.r.girdwood, tomi.valkeinen, broonie,
Jyri Sarha
The first fix is a hairy one, but I think the locking is fool proof
now. It is needed because there is no telling in which order user
space starts an audio and video stream playback. If the audio is
started first and the video mode is changed when video playback starts
the audio setup needs to survive display turning off and back on
again. After this patch the audio streams should survive a
suspend-resume cycle too.
The second one is a trivial work-around to a problem in ALSA constraint
resolver code.
The two fixes are totally independent and the video and audio side
patches applied separately trough their own trees.
Jyri Sarha (2):
OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
ASoC: omap-hdmi-audio: Set buffer bytes step constraint to 128
drivers/video/fbdev/omap2/dss/hdmi.h | 10 +++-
drivers/video/fbdev/omap2/dss/hdmi4.c | 65 ++++++++++++++++++++-----
drivers/video/fbdev/omap2/dss/hdmi5.c | 89 +++++++++++++++++++++++++++++------
sound/soc/omap/omap-hdmi-audio.c | 10 +++-
4 files changed, 146 insertions(+), 28 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH 1/2] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
From: Jyri Sarha @ 2015-08-26 13:11 UTC (permalink / raw)
To: alsa-devel, linux-fbdev, linux-omap
Cc: peter.ujfalusi, liam.r.girdwood, tomi.valkeinen, broonie,
Jyri Sarha
In-Reply-To: <cover.1440594174.git.jsarha@ti.com>
Reconfigure and restart audio when display is enabled, if audio
playback was active before. The audio configuration is stored when is
is successfully applied and a boolean is set when playback has started
and unset when stopped. This data is used to reconfigure the audio
when display is re-enabled. Abort audio playback if reconfiguration
fails.
A new spin lock is introduced to in order protect state variables
related to audio playback status. The wp_idlemode protection relies on
PM not touching it after the original initialization. A power
management API for controlling the idlemode would be needed for proper
synchronization.
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
drivers/video/fbdev/omap2/dss/hdmi.h | 10 +++-
drivers/video/fbdev/omap2/dss/hdmi4.c | 65 ++++++++++++++++++++-----
drivers/video/fbdev/omap2/dss/hdmi5.c | 89 +++++++++++++++++++++++++++++------
3 files changed, 137 insertions(+), 27 deletions(-)
diff --git a/drivers/video/fbdev/omap2/dss/hdmi.h b/drivers/video/fbdev/omap2/dss/hdmi.h
index e4a32fe..1e45b84 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi.h
+++ b/drivers/video/fbdev/omap2/dss/hdmi.h
@@ -351,12 +351,20 @@ struct omap_hdmi {
struct regulator *vdda_reg;
bool core_enabled;
- bool display_enabled;
struct omap_dss_device output;
struct platform_device *audio_pdev;
void (*audio_abort_cb)(struct device *dev);
+
+ bool audio_configured;
+ struct omap_dss_audio audio_config;
+
+ /* This lock should be taken when any one of the three
+ state variables bellow are touched. */
+ spinlock_t audio_playing_lock;
+ bool audio_playing;
+ bool display_enabled;
int wp_idlemode;
};
diff --git a/drivers/video/fbdev/omap2/dss/hdmi4.c b/drivers/video/fbdev/omap2/dss/hdmi4.c
index 6d3aa3f..ea1fa64 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi4.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi4.c
@@ -324,6 +324,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");
@@ -342,7 +343,26 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
goto err0;
}
+ if (hdmi.audio_configured) {
+ hdmi4_audio_stop(&hdmi.core, &hdmi.wp);
+ hdmi_wp_audio_enable(&hdmi.wp, false);
+
+ r = hdmi4_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) {
+ hdmi_wp_audio_enable(&hdmi.wp, true);
+ hdmi4_audio_start(&hdmi.core, &hdmi.wp);
+ }
hdmi.display_enabled = true;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
mutex_unlock(&hdmi.lock);
return 0;
@@ -354,17 +374,18 @@ err0:
static void hdmi_display_disable(struct omap_dss_device *dssdev)
{
+ unsigned long flags;
+
DSSDBG("Enter hdmi_display_disable\n");
mutex_lock(&hdmi.lock);
- if (hdmi.audio_pdev && hdmi.audio_abort_cb)
- hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
+ spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+ hdmi.display_enabled = false;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
hdmi_power_off_full(dssdev);
- hdmi.display_enabled = false;
-
mutex_unlock(&hdmi.lock);
}
@@ -565,9 +586,14 @@ out:
static int hdmi_audio_shutdown(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
mutex_lock(&hd->lock);
hd->audio_abort_cb = NULL;
+ hd->audio_configured = false;
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+ hd->audio_playing = false;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
mutex_unlock(&hd->lock);
return 0;
@@ -576,25 +602,38 @@ static int hdmi_audio_shutdown(struct device *dev)
static int hdmi_audio_start(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi_wp_audio_enable(&hd->wp, true);
- hdmi4_audio_start(&hd->core, &hd->wp);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled) {
+ hdmi_wp_audio_enable(&hd->wp, true);
+ hdmi4_audio_start(&hd->core, &hd->wp);
+ }
+ hd->audio_playing = true;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
return 0;
}
static void hdmi_audio_stop(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi4_audio_stop(&hd->core, &hd->wp);
- hdmi_wp_audio_enable(&hd->wp, false);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled) {
+ hdmi4_audio_stop(&hd->core, &hd->wp);
+ hdmi_wp_audio_enable(&hd->wp, false);
+ }
+ hd->audio_playing = false;
+
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
}
static int hdmi_audio_config(struct device *dev,
@@ -612,7 +651,10 @@ static int hdmi_audio_config(struct device *dev,
ret = hdmi4_audio_config(&hd->core, &hd->wp, dss_audio,
hd->cfg.timings.pixelclock);
-
+ if (!ret) {
+ hd->audio_configured = true;
+ hd->audio_config = *dss_audio;
+ }
out:
mutex_unlock(&hd->lock);
@@ -657,6 +699,7 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
dev_set_drvdata(&pdev->dev, &hdmi);
mutex_init(&hdmi.lock);
+ spin_lock_init(&hdmi.audio_playing_lock);
if (pdev->dev.of_node) {
r = hdmi_probe_of(pdev);
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);
+
+ 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);
mutex_unlock(&hdmi.lock);
return 0;
@@ -382,17 +413,18 @@ err0:
static void hdmi_display_disable(struct omap_dss_device *dssdev)
{
+ unsigned long flags;
+
DSSDBG("Enter hdmi_display_disable\n");
mutex_lock(&hdmi.lock);
- if (hdmi.audio_pdev && hdmi.audio_abort_cb)
- hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
+ spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+ hdmi.display_enabled = false;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
hdmi_power_off_full(dssdev);
- hdmi.display_enabled = false;
-
mutex_unlock(&hdmi.lock);
}
@@ -593,9 +625,14 @@ out:
static int hdmi_audio_shutdown(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
mutex_lock(&hd->lock);
hd->audio_abort_cb = NULL;
+ hd->audio_configured = false;
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+ hd->audio_playing = false;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
mutex_unlock(&hd->lock);
return 0;
@@ -604,32 +641,48 @@ static int hdmi_audio_shutdown(struct device *dev)
static int hdmi_audio_start(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- /* No-idle while playing audio, store the old value */
- hd->wp_idlemode = REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
- REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
- hdmi_wp_audio_enable(&hd->wp, true);
- hdmi_wp_audio_core_req_enable(&hd->wp, true);
+ if (hd->display_enabled) {
+ /* No-idle while playing audio, store the old value */
+ hd->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(&hd->wp, true);
+ hdmi_wp_audio_core_req_enable(&hd->wp, true);
+ }
+ hd->audio_playing = true;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
return 0;
}
static void hdmi_audio_stop(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi_wp_audio_core_req_enable(&hd->wp, false);
- hdmi_wp_audio_enable(&hd->wp, false);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
- /* Playback stopped, restore original idlemode */
- REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
+ if (hd->display_enabled) {
+ hdmi_wp_audio_core_req_enable(&hd->wp, false);
+ hdmi_wp_audio_enable(&hd->wp, false);
+
+ /* Playback stopped, restore original idlemode */
+ REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode,
+ 3, 2);
+ hd->wp_idlemode = -1;
+ }
+ hd->audio_playing = false;
+
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
}
static int hdmi_audio_config(struct device *dev,
@@ -648,6 +701,10 @@ static int hdmi_audio_config(struct device *dev,
ret = hdmi5_audio_config(&hd->core, &hd->wp, dss_audio,
hd->cfg.timings.pixelclock);
+ if (!ret) {
+ hd->audio_configured = true;
+ hd->audio_config = *dss_audio;
+ }
out:
mutex_unlock(&hd->lock);
@@ -692,6 +749,8 @@ static int hdmi5_bind(struct device *dev, struct device *master, void *data)
dev_set_drvdata(&pdev->dev, &hdmi);
mutex_init(&hdmi.lock);
+ spin_lock_init(&hdmi.audio_playing_lock);
+ hdmi.wp_idlemode = -1;
if (pdev->dev.of_node) {
r = hdmi_probe_of(pdev);
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] ASoC: omap-hdmi-audio: Set buffer bytes step constraint to 128
From: Jyri Sarha @ 2015-08-26 13:11 UTC (permalink / raw)
To: alsa-devel, linux-fbdev, linux-omap
Cc: peter.ujfalusi, liam.r.girdwood, tomi.valkeinen, broonie,
Jyri Sarha
In-Reply-To: <cover.1440594174.git.jsarha@ti.com>
Set buffer bytes step constraint to 128. A matching constraint has
already been set to period size. This helps PCM setup to tolerate ALSA
clients that set the PCM hw params in unusual order.
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
sound/soc/omap/omap-hdmi-audio.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/soc/omap/omap-hdmi-audio.c b/sound/soc/omap/omap-hdmi-audio.c
index aeef25c..584b237 100644
--- a/sound/soc/omap/omap-hdmi-audio.c
+++ b/sound/soc/omap/omap-hdmi-audio.c
@@ -81,7 +81,15 @@ static int hdmi_dai_startup(struct snd_pcm_substream *substream,
ret = snd_pcm_hw_constraint_step(substream->runtime, 0,
SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 128);
if (ret < 0) {
- dev_err(dai->dev, "could not apply constraint\n");
+ dev_err(dai->dev, "Could not apply period constraint: %d\n",
+ ret);
+ return ret;
+ }
+ ret = snd_pcm_hw_constraint_step(substream->runtime, 0,
+ SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 128);
+ if (ret < 0) {
+ dev_err(dai->dev, "Could not apply buffer constraint: %d\n",
+ ret);
return ret;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 0/2] OMAPDSS: Couple of HDMI audio fixes
From: Mark Brown @ 2015-08-26 17:42 UTC (permalink / raw)
To: Jyri Sarha
Cc: liam.r.girdwood, alsa-devel, linux-fbdev, peter.ujfalusi,
tomi.valkeinen, linux-omap
In-Reply-To: <cover.1440594174.git.jsarha@ti.com>
[-- Attachment #1: Type: text/plain, Size: 376 bytes --]
On Wed, Aug 26, 2015 at 04:11:38PM +0300, Jyri Sarha wrote:
> The two fixes are totally independent and the video and audio side
> patches applied separately trough their own trees.
In general if patches aren't related to each other either through code
overlap or through content it's better to just send them as individual
changes, that avoids any potential for confusion.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH -next] drivers/video/fbdev: Add dependencies on !S390
From: Luis R. Rodriguez @ 2015-08-26 19:48 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel, Borislav Petkov, Ingo Molnar, Fengguang Wu,
Andrew Morton, Steven Rostedt
In-Reply-To: <1440588770-17774-1-git-send-email-linux@roeck-us.net>
Hey Guenter,
thanks so much for your patch, my feedback below! I'll proactively
Cc Andrew and Steven as they're the other ones who I've seen as
hawks to compile issues on linux-next so I expect they'll soon
run into this as well if they are also testing against s390.
On Wed, Aug 26, 2015 at 04:32:50AM -0700, Guenter Roeck wrote:
> s390:allmodconfig fails to build with:
>
> ERROR: "pci_iomap_wc" [drivers/video/fbdev/vt8623fb.ko] undefined!
> ERROR: "pci_iomap_wc" [drivers/video/fbdev/s3fb.ko] undefined!
> ERROR: "pci_iomap_wc" [drivers/video/fbdev/arkfb.ko] undefined!
>
> Those functions are currently only available in generic PCI iomap code,
> which is not used on S390.
>
> Fixes: 81bdef04d3bc ("drivers/video/fbdev/vt8623fb: Use arch_phys_wc_add() and pci_iomap_wc()")
> Fixes: 4edcd2ab1255 ("drivers/video/fbdev/s3fb: Use arch_phys_wc_add() and pci_iomap_wc()")
> Fixes: c823a48ac47f ("drivers/video/fbdev/arkfb.c: Use arch_phys_wc_add() and pci_iomap_wc()")
Those commit IDs are only sepcific to linux-next and as such are
ephemeral.
> Cc: Luis R. Rodriguez <mcgrof@suse.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/video/fbdev/Kconfig | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index 8b1d371b5404..6b1893e5c769 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -1442,6 +1442,7 @@ config FB_ATY_BACKLIGHT
> config FB_S3
> tristate "S3 Trio/Virge support"
> depends on FB && PCI
> + depends on !S390
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> @@ -1649,6 +1650,7 @@ config FB_VOODOO1
> config FB_VT8623
> tristate "VIA VT8623 support"
> depends on FB && PCI
> + depends on !S390
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> @@ -1683,6 +1685,7 @@ config FB_TRIDENT
> config FB_ARK
> tristate "ARK 2000PV support"
> depends on FB && PCI
> + depends on !S390
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
Although this is one way to resolve it, the patch does not explain why
its needed, and this would also mean its a reactive solution, we either
test-compile and find issues or expect and hope developers will also
annotate this themselves when using this new API. I rather we define
it S390, which I do below.
The reason S390 requires its own S390 requires its own pci_iomap*()
calls is because it has its "BAR spaces are not disjunctive on s390
so we need the bar parameter of pci_iomap to find the corresponding
device and create the mapping cookie." but in summary, it has its own
lookup/lock solution. It does not include asm-generic/pci_iomap.h
and requires implementing the generic solutoins on its own then.
As for write-combining, it has no specific solution for this so
its ioremap_wc() is a direct mapping, so we can just do a 1-1
mapping to the non-wc calls as well then. If we do this then we
don't have to go on fixing drivers when they use this new API and
if and when S390 gets some form of WC it can go and update its
implementation but right now it has none.
I've tested the patch below on linux-next with make.cross ARCH=s390
with allyesconfig and it fixes the compile issue. I'll post this as
a patch fix. Let me know if this is agreeable to you.
diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
index cb5fdf3a78fc..437e9af96688 100644
--- a/arch/s390/include/asm/io.h
+++ b/arch/s390/include/asm/io.h
@@ -57,6 +57,8 @@ static inline void ioport_unmap(void __iomem *p)
*/
#define pci_iomap pci_iomap
#define pci_iounmap pci_iounmap
+#define pci_iomap_wc pci_iomap
+#define pci_iomap_wc_range pci_iomap_range
#define memcpy_fromio(dst, src, count) zpci_memcpy_fromio(dst, src, count)
#define memcpy_toio(dst, src, count) zpci_memcpy_toio(dst, src, count)
^ permalink raw reply related
* Re: [PATCH -next] drivers/video/fbdev: Add dependencies on !S390
From: Guenter Roeck @ 2015-08-26 20:06 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel, Borislav Petkov, Ingo Molnar, Fengguang Wu,
Andrew Morton, Steven Rostedt
In-Reply-To: <20150826194813.GY8051@wotan.suse.de>
Hi Luis,
On 08/26/2015 12:48 PM, Luis R. Rodriguez wrote:
>
> Hey Guenter,
>
> thanks so much for your patch, my feedback below! I'll proactively
> Cc Andrew and Steven as they're the other ones who I've seen as
> hawks to compile issues on linux-next so I expect they'll soon
> run into this as well if they are also testing against s390.
>
> On Wed, Aug 26, 2015 at 04:32:50AM -0700, Guenter Roeck wrote:
>> s390:allmodconfig fails to build with:
>>
>> ERROR: "pci_iomap_wc" [drivers/video/fbdev/vt8623fb.ko] undefined!
>> ERROR: "pci_iomap_wc" [drivers/video/fbdev/s3fb.ko] undefined!
>> ERROR: "pci_iomap_wc" [drivers/video/fbdev/arkfb.ko] undefined!
>>
>> Those functions are currently only available in generic PCI iomap code,
>> which is not used on S390.
>>
>> Fixes: 81bdef04d3bc ("drivers/video/fbdev/vt8623fb: Use arch_phys_wc_add() and pci_iomap_wc()")
>> Fixes: 4edcd2ab1255 ("drivers/video/fbdev/s3fb: Use arch_phys_wc_add() and pci_iomap_wc()")
>> Fixes: c823a48ac47f ("drivers/video/fbdev/arkfb.c: Use arch_phys_wc_add() and pci_iomap_wc()")
>
> Those commit IDs are only sepcific to linux-next and as such are
> ephemeral.
>
Depends on the introducing tree.
>> Cc: Luis R. Rodriguez <mcgrof@suse.com>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> drivers/video/fbdev/Kconfig | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>> index 8b1d371b5404..6b1893e5c769 100644
>> --- a/drivers/video/fbdev/Kconfig
>> +++ b/drivers/video/fbdev/Kconfig
>> @@ -1442,6 +1442,7 @@ config FB_ATY_BACKLIGHT
>> config FB_S3
>> tristate "S3 Trio/Virge support"
>> depends on FB && PCI
>> + depends on !S390
>> select FB_CFB_FILLRECT
>> select FB_CFB_COPYAREA
>> select FB_CFB_IMAGEBLIT
>> @@ -1649,6 +1650,7 @@ config FB_VOODOO1
>> config FB_VT8623
>> tristate "VIA VT8623 support"
>> depends on FB && PCI
>> + depends on !S390
>> select FB_CFB_FILLRECT
>> select FB_CFB_COPYAREA
>> select FB_CFB_IMAGEBLIT
>> @@ -1683,6 +1685,7 @@ config FB_TRIDENT
>> config FB_ARK
>> tristate "ARK 2000PV support"
>> depends on FB && PCI
>> + depends on !S390
>> select FB_CFB_FILLRECT
>> select FB_CFB_COPYAREA
>> select FB_CFB_IMAGEBLIT
>
> Although this is one way to resolve it, the patch does not explain why
> its needed, and this would also mean its a reactive solution, we either
> test-compile and find issues or expect and hope developers will also
> annotate this themselves when using this new API. I rather we define
> it S390, which I do below.
>
> The reason S390 requires its own S390 requires its own pci_iomap*()
> calls is because it has its "BAR spaces are not disjunctive on s390
> so we need the bar parameter of pci_iomap to find the corresponding
> device and create the mapping cookie." but in summary, it has its own
> lookup/lock solution. It does not include asm-generic/pci_iomap.h
> and requires implementing the generic solutoins on its own then.
>
> As for write-combining, it has no specific solution for this so
> its ioremap_wc() is a direct mapping, so we can just do a 1-1
> mapping to the non-wc calls as well then. If we do this then we
> don't have to go on fixing drivers when they use this new API and
> if and when S390 gets some form of WC it can go and update its
> implementation but right now it has none.
>
> I've tested the patch below on linux-next with make.cross ARCH=s390
> with allyesconfig and it fixes the compile issue. I'll post this as
> a patch fix. Let me know if this is agreeable to you.
>
Sure, go ahead. It is a much better solution than mine, after all.
Guenter
> diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
> index cb5fdf3a78fc..437e9af96688 100644
> --- a/arch/s390/include/asm/io.h
> +++ b/arch/s390/include/asm/io.h
> @@ -57,6 +57,8 @@ static inline void ioport_unmap(void __iomem *p)
> */
> #define pci_iomap pci_iomap
> #define pci_iounmap pci_iounmap
> +#define pci_iomap_wc pci_iomap
> +#define pci_iomap_wc_range pci_iomap_range
>
> #define memcpy_fromio(dst, src, count) zpci_memcpy_fromio(dst, src, count)
> #define memcpy_toio(dst, src, count) zpci_memcpy_toio(dst, src, count)
>
^ permalink raw reply
* [PATCH] S390: add pci_iomap_wc() and pci_iomap_wc_range()
From: Luis R. Rodriguez @ 2015-08-26 23:34 UTC (permalink / raw)
To: mingo, schwidefsky, heiko.carstens, linux-s390
Cc: bp, linux, plagnioj, tomi.valkeinen, linux-fbdev, linux-kernel,
fengguang.wu, akpm, rostedt, Luis R. Rodriguez
From: "Luis R. Rodriguez" <mcgrof@suse.com>
S390 requires its own implementation of pcio_iomap*() calls
is because it has its "BAR spaces are not disjunctive on s390
so we need the bar parameter of pci_iomap to find the corresponding
device and create the mapping cookie" -- in summary, it has its own
lookup/lock solution.
It does not include asm-generic/pci_iomap.h
Since it currenty maps ioremap_wc() to ioremap_nocache() and that's
the architecture default we can easily just map the wc calls to
the default calls as well.
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Borislav Petkov <bp@suse.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Fengguang Wu <fengguang.wu@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Reported-by: 0 day bot
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
This broke through some series that went into Ingo's tip tree which
I added. As such I *think* this should go through Ingo's tip tree.
Let me know. Up to you guys.
arch/s390/include/asm/io.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
index cb5fdf3a78fc..437e9af96688 100644
--- a/arch/s390/include/asm/io.h
+++ b/arch/s390/include/asm/io.h
@@ -57,6 +57,8 @@ static inline void ioport_unmap(void __iomem *p)
*/
#define pci_iomap pci_iomap
#define pci_iounmap pci_iounmap
+#define pci_iomap_wc pci_iomap
+#define pci_iomap_wc_range pci_iomap_range
#define memcpy_fromio(dst, src, count) zpci_memcpy_fromio(dst, src, count)
#define memcpy_toio(dst, src, count) zpci_memcpy_toio(dst, src, count)
--
2.4.3
^ permalink raw reply related
* Re: [PATCH] staging: sm750fb: fix improper typedef usage
From: Sudip Mukherjee @ 2015-08-27 12:23 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20150819022745.GA5960@brian_archtop.Home>
On Tue, Aug 18, 2015 at 10:27:45PM -0400, Brian Stottler wrote:
> Fix typedef usage which does not comply with kernel style guidelines.
>
> Signed-off-by: Brian Stottler <brianstottler@gmail.com>
> ---
I think you need to update your tree. This patch will not apply due to
changes already done in some of the files.
regards
sudip
^ permalink raw reply
* Re: [PATCH 1/2] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
From: Tomi Valkeinen @ 2015-08-27 14:30 UTC (permalink / raw)
To: Jyri Sarha, linux-fbdev, linux-omap
Cc: peter.ujfalusi, liam.r.girdwood, alsa-devel, broonie
In-Reply-To: <44089dc819903c71e1d0ed0c14dd3ab060ed8c68.1440594174.git.jsarha@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3284 bytes --]
Hi,
On 26/08/15 16:11, Jyri Sarha wrote:
I few comments, for the parts I had time to review:
> 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;
> }
>
I think you could refactor parts of the code below into small helper
functions:
> + 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;
The above looks like it's disabling audio output, the same that's done
in hdmi_audio_stop(). Adding a helper func for that makes the code more
readable.
For the wp_idlemode, I think hdmi.wp_idlemode could be initialized to a
valid value, so that it can be set at any time without the if check above.
> + spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
> +
> + 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);
And here maybe a helper func to enable the audio output.
> + }
> hdmi.display_enabled = true;
> + spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
>
> mutex_unlock(&hdmi.lock);
> return 0;
> @@ -382,17 +413,18 @@ err0:
>
> static void hdmi_display_disable(struct omap_dss_device *dssdev)
> {
> + unsigned long flags;
> +
> DSSDBG("Enter hdmi_display_disable\n");
>
> mutex_lock(&hdmi.lock);
>
> - if (hdmi.audio_pdev && hdmi.audio_abort_cb)
> - hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
> + spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
> + hdmi.display_enabled = false;
> + spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
Shouldn't something be done here in hdmi_display_disable about audio?
You probably want to keep the state flag enabled, so that we know the
user is playing audio, but you could still disable the HDMI audio HW.
I'm sure the audio output dies when the video is disabled, but being
more explicit here makes the code logic easier to follow.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] backlight: lp855x: Make sure props struct is zeroed
From: Bjorn Andersson @ 2015-08-27 17:41 UTC (permalink / raw)
To: Milo Kim, Jingoo Han, Lee Jones, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
Cc: linux-fbdev, linux-kernel, Werner Johansson
From: Werner Johansson <werner.johansson@sonymobile.com>
The driver occasionally got stuck in suspend mode or other strange
states as those parts of the props struct were never initialized.
Signed-off-by: Werner Johansson <werner.johansson@sonymobile.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
drivers/video/backlight/lp855x_bl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index 88116b493f3b..0a7f88ce1ab0 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -282,6 +282,7 @@ static int lp855x_backlight_register(struct lp855x *lp)
struct lp855x_platform_data *pdata = lp->pdata;
const char *name = pdata->name ? : DEFAULT_BL_NAME;
+ memset(&props, 0, sizeof(props));
props.type = BACKLIGHT_PLATFORM;
props.max_brightness = MAX_BRIGHTNESS;
--
1.8.2.2
^ permalink raw reply related
* Re: [PATCH] backlight: lp855x: Make sure props struct is zeroed
From: Kim, Milo @ 2015-08-27 22:44 UTC (permalink / raw)
To: Bjorn Andersson, Werner Johansson
Cc: Jingoo Han, Lee Jones, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1440697275-4469-1-git-send-email-bjorn.andersson@sonymobile.com>
On 8/28/2015 2:41 AM, Bjorn Andersson wrote:
> From: Werner Johansson <werner.johansson@sonymobile.com>
>
> The driver occasionally got stuck in suspend mode or other strange
> states as those parts of the props struct were never initialized.
Acked-by: Milo Kim <milo.kim@ti.com>
Thanks for catching this.
^ permalink raw reply
* RE: [PATCH] backlight: lp855x: Make sure props struct is zeroed
From: Johansson, Werner @ 2015-08-27 22:51 UTC (permalink / raw)
To: Kim, Milo, "Andersson, Björn"
Cc: Jingoo Han, Lee Jones, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <55DF92BF.7030703@ti.com>
> > From: Werner Johansson <werner.johansson@sonymobile.com>
> >
> > The driver occasionally got stuck in suspend mode or other strange
> > states as those parts of the props struct were never initialized.
>
> Acked-by: Milo Kim <milo.kim@ti.com>
>
> Thanks for catching this.
You're most welcome! These issues can be difficult to track down, the stack is unpredictable..
/wj
^ permalink raw reply
* Re: [PATCH] S390: add pci_iomap_wc() and pci_iomap_wc_range()
From: Ingo Molnar @ 2015-08-28 6:06 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: schwidefsky, heiko.carstens, linux-s390, bp, linux, plagnioj,
tomi.valkeinen, linux-fbdev, linux-kernel, fengguang.wu, akpm,
rostedt, Luis R. Rodriguez
In-Reply-To: <1440632050-23648-1-git-send-email-mcgrof@do-not-panic.com>
* Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> S390 requires its own implementation of pcio_iomap*() calls
> is because it has its "BAR spaces are not disjunctive on s390
> so we need the bar parameter of pci_iomap to find the corresponding
> device and create the mapping cookie" -- in summary, it has its own
> lookup/lock solution.
>
> It does not include asm-generic/pci_iomap.h
>
> Since it currenty maps ioremap_wc() to ioremap_nocache() and that's
> the architecture default we can easily just map the wc calls to
> the default calls as well.
>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Fengguang Wu <fengguang.wu@intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Reported-by: 0 day bot
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>
> This broke through some series that went into Ingo's tip tree which
> I added. As such I *think* this should go through Ingo's tip tree.
> Let me know. Up to you guys.
I fixed the changelog to explain all this.
Also, is there any other architecture that got broken by:
1b3d4200c1e0 PCI: Add pci_iomap_wc() variants
?
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH] S390: add pci_iomap_wc() and pci_iomap_wc_range()
From: Guenter Roeck @ 2015-08-28 8:22 UTC (permalink / raw)
To: Ingo Molnar, Luis R. Rodriguez
Cc: schwidefsky, heiko.carstens, linux-s390, bp, plagnioj,
tomi.valkeinen, linux-fbdev, linux-kernel, fengguang.wu, akpm,
rostedt, Luis R. Rodriguez
In-Reply-To: <20150828060605.GF26741@gmail.com>
On 08/27/2015 11:06 PM, Ingo Molnar wrote:
>
> * Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
>
>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>
>> S390 requires its own implementation of pcio_iomap*() calls
>> is because it has its "BAR spaces are not disjunctive on s390
>> so we need the bar parameter of pci_iomap to find the corresponding
>> device and create the mapping cookie" -- in summary, it has its own
>> lookup/lock solution.
>>
>> It does not include asm-generic/pci_iomap.h
>>
>> Since it currenty maps ioremap_wc() to ioremap_nocache() and that's
>> the architecture default we can easily just map the wc calls to
>> the default calls as well.
>>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: linux-fbdev@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Fengguang Wu <fengguang.wu@intel.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Reported-by: 0 day bot
>> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
>> ---
>>
>> This broke through some series that went into Ingo's tip tree which
>> I added. As such I *think* this should go through Ingo's tip tree.
>> Let me know. Up to you guys.
>
> I fixed the changelog to explain all this.
>
> Also, is there any other architecture that got broken by:
>
> 1b3d4200c1e0 PCI: Add pci_iomap_wc() variants
>
Should be the only one. There are only two implementations of pci_iomap(),
one in s390 code and the generic implementation.
Guenter
^ permalink raw reply
* Re: [PATCH 1/2] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
From: Tomi Valkeinen @ 2015-08-28 10:37 UTC (permalink / raw)
To: Jyri Sarha
Cc: liam.r.girdwood, alsa-devel, linux-fbdev, peter.ujfalusi, broonie,
linux-omap
In-Reply-To: <44089dc819903c71e1d0ed0c14dd3ab060ed8c68.1440594174.git.jsarha@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2267 bytes --]
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?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
From: Jyri Sarha @ 2015-08-28 11:35 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: liam.r.girdwood, alsa-devel, linux-fbdev, peter.ujfalusi, broonie,
linux-omap
In-Reply-To: <55E039FC.8090707@ti.com>
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
^ permalink raw reply
* [PATCH] efifb: Add support for 64-bit frame buffer addresses
From: Matt Fleming @ 2015-08-28 12:12 UTC (permalink / raw)
To: Peter Jones
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Matt Fleming, Pete Hawkins,
Matthew Garrett, Chad Page
From: Matt Fleming <matt.fleming@intel.com>
The EFI Graphics Output Protocol uses 64-bit frame buffer addresses
but these get truncated to 32-bit by the EFI boot stub when storing
the address in the 'lfb_base' field of 'struct screen_info'.
Add a 'ext_lfb_base' field for the upper 32-bits of the frame buffer
address and set VIDEO_TYPE_CAPABILITY_64BIT_BASE when the field is
useable.
It turns out that the reason no one has required this support so far
is that there's actually code in tianocore to "downgrade" PCI
resources that have option ROMs and 64-bit BARS from 64-bit to 32-bit
to cope with legacy option ROMs that can't handle 64-bit addresses.
The upshot is that basically all GOP devices in the wild use a 32-bit
frame buffer address.
Still, it is possible to build firmware that uses a full 64-bit GOP
frame buffer address. Chad did, which led to him reporting this issue.
Add support in anticipation of GOP devices using 64-bit addresses more
widely, and so that efifb works out of the box when that happens.
Reported-by: Chad Page <chad.page@znyx.com>
Cc: Pete Hawkins <pete.hawkins@znyx.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
arch/x86/boot/compressed/eboot.c | 24 ++++++++++++++++++++----
drivers/video/fbdev/efifb.c | 24 +++++++++++++++++++++++-
include/uapi/linux/screen_info.h | 5 +++--
3 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 7d69afd8b6fa..43473bb86ac0 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -624,7 +624,7 @@ setup_pixel_info(struct screen_info *si, u32 pixels_per_scan_line,
static efi_status_t
__gop_query32(struct efi_graphics_output_protocol_32 *gop32,
struct efi_graphics_output_mode_info **info,
- unsigned long *size, u32 *fb_base)
+ unsigned long *size, u64 *fb_base)
{
struct efi_graphics_output_protocol_mode_32 *mode;
efi_status_t status;
@@ -650,7 +650,8 @@ setup_gop32(struct screen_info *si, efi_guid_t *proto,
unsigned long nr_gops;
u16 width, height;
u32 pixels_per_scan_line;
- u32 fb_base;
+ u32 ext_lfb_base;
+ u64 fb_base;
struct efi_pixel_bitmask pixel_info;
int pixel_format;
efi_status_t status;
@@ -713,6 +714,13 @@ setup_gop32(struct screen_info *si, efi_guid_t *proto,
si->lfb_width = width;
si->lfb_height = height;
si->lfb_base = fb_base;
+
+ ext_lfb_base = (u64)(unsigned long)fb_base >> 32;
+ if (ext_lfb_base) {
+ si->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
+ si->ext_lfb_base = ext_lfb_base;
+ }
+
si->pages = 1;
setup_pixel_info(si, pixels_per_scan_line, pixel_info, pixel_format);
@@ -727,7 +735,7 @@ out:
static efi_status_t
__gop_query64(struct efi_graphics_output_protocol_64 *gop64,
struct efi_graphics_output_mode_info **info,
- unsigned long *size, u32 *fb_base)
+ unsigned long *size, u64 *fb_base)
{
struct efi_graphics_output_protocol_mode_64 *mode;
efi_status_t status;
@@ -753,7 +761,8 @@ setup_gop64(struct screen_info *si, efi_guid_t *proto,
unsigned long nr_gops;
u16 width, height;
u32 pixels_per_scan_line;
- u32 fb_base;
+ u32 ext_lfb_base;
+ u64 fb_base;
struct efi_pixel_bitmask pixel_info;
int pixel_format;
efi_status_t status;
@@ -816,6 +825,13 @@ setup_gop64(struct screen_info *si, efi_guid_t *proto,
si->lfb_width = width;
si->lfb_height = height;
si->lfb_base = fb_base;
+
+ ext_lfb_base = (u64)(unsigned long)fb_base >> 32;
+ if (ext_lfb_base) {
+ si->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
+ si->ext_lfb_base = ext_lfb_base;
+ }
+
si->pages = 1;
setup_pixel_info(si, pixels_per_scan_line, pixel_info, pixel_format);
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff349b1fb..95d293b7445a 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -114,6 +114,20 @@ static int efifb_setup(char *options)
return 0;
}
+static inline bool fb_base_is_valid(void)
+{
+ if (screen_info.lfb_base)
+ return true;
+
+ if (!(screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE))
+ return false;
+
+ if (screen_info.ext_lfb_base)
+ return true;
+
+ return false;
+}
+
static int efifb_probe(struct platform_device *dev)
{
struct fb_info *info;
@@ -141,7 +155,7 @@ static int efifb_probe(struct platform_device *dev)
screen_info.lfb_depth = 32;
if (!screen_info.pages)
screen_info.pages = 1;
- if (!screen_info.lfb_base) {
+ if (!fb_base_is_valid()) {
printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
return -ENODEV;
}
@@ -160,6 +174,14 @@ static int efifb_probe(struct platform_device *dev)
}
efifb_fix.smem_start = screen_info.lfb_base;
+
+ if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE) {
+ u64 ext_lfb_base;
+
+ ext_lfb_base = (u64)(unsigned long)screen_info.ext_lfb_base << 32;
+ efifb_fix.smem_start |= ext_lfb_base;
+ }
+
efifb_defined.bits_per_pixel = screen_info.lfb_depth;
efifb_defined.xres = screen_info.lfb_width;
efifb_defined.yres = screen_info.lfb_height;
diff --git a/include/uapi/linux/screen_info.h b/include/uapi/linux/screen_info.h
index 7530e7447620..8b8d39dfb67f 100644
--- a/include/uapi/linux/screen_info.h
+++ b/include/uapi/linux/screen_info.h
@@ -43,7 +43,8 @@ struct screen_info {
__u16 pages; /* 0x32 */
__u16 vesa_attributes; /* 0x34 */
__u32 capabilities; /* 0x36 */
- __u8 _reserved[6]; /* 0x3a */
+ __u32 ext_lfb_base; /* 0x3a */
+ __u8 _reserved[2]; /* 0x3e */
} __attribute__((packed));
#define VIDEO_TYPE_MDA 0x10 /* Monochrome Text Display */
@@ -69,6 +70,6 @@ struct screen_info {
#define VIDEO_FLAGS_NOCURSOR (1 << 0) /* The video mode has no cursor set */
#define VIDEO_CAPABILITY_SKIP_QUIRKS (1 << 0)
-
+#define VIDEO_CAPABILITY_64BIT_BASE (1 << 1) /* Frame buffer base is 64-bit */
#endif /* _UAPI_SCREEN_INFO_H */
--
2.1.0
^ permalink raw reply related
* [PATCH v2] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
From: Jyri Sarha @ 2015-08-28 12:24 UTC (permalink / raw)
To: alsa-devel, linux-fbdev, linux-omap
Cc: peter.ujfalusi, tomi.valkeinen, Jyri Sarha
Reconfigure and restart audio when display is enabled, if audio
playback was active before. The audio configuration is stored when is
is successfully applied and a boolean is set when playback has started
and unset when stopped. This data is used to reconfigure the audio
when display is re-enabled. Abort audio playback if reconfiguration
fails.
A new spin lock is introduced in order to protect state variables
related to audio playback status. 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 with the start/stop audio commands.
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 hdmi.audio_playing 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.
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
I dropped the ASoC maintainers from the recipient list as this patch
hardly concerns them.
drivers/video/fbdev/omap2/dss/hdmi.h | 9 +++-
drivers/video/fbdev/omap2/dss/hdmi4.c | 69 +++++++++++++++++++++++++-----
drivers/video/fbdev/omap2/dss/hdmi5.c | 79 ++++++++++++++++++++++++++++-------
3 files changed, 130 insertions(+), 27 deletions(-)
diff --git a/drivers/video/fbdev/omap2/dss/hdmi.h b/drivers/video/fbdev/omap2/dss/hdmi.h
index e4a32fe..e48aefd 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi.h
+++ b/drivers/video/fbdev/omap2/dss/hdmi.h
@@ -351,13 +351,20 @@ struct omap_hdmi {
struct regulator *vdda_reg;
bool core_enabled;
- bool display_enabled;
struct omap_dss_device output;
struct platform_device *audio_pdev;
void (*audio_abort_cb)(struct device *dev);
int wp_idlemode;
+
+ bool audio_configured;
+ struct omap_dss_audio audio_config;
+
+ /* This lock should be taken when booleas bellow is touched. */
+ spinlock_t audio_playing_lock;
+ bool audio_playing;
+ bool display_enabled;
};
#endif
diff --git a/drivers/video/fbdev/omap2/dss/hdmi4.c b/drivers/video/fbdev/omap2/dss/hdmi4.c
index 6d3aa3f..6b9817a 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi4.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi4.c
@@ -321,9 +321,22 @@ static int read_edid(u8 *buf, int len)
return r;
}
+static void hdmi_start_audio_stream(struct omap_hdmi *hd)
+{
+ hdmi_wp_audio_enable(&hd->wp, true);
+ hdmi4_audio_start(&hd->core, &hd->wp);
+}
+
+static void hdmi_stop_audio_stream(struct omap_hdmi *hd)
+{
+ hdmi4_audio_stop(&hd->core, &hd->wp);
+ hdmi_wp_audio_enable(&hd->wp, false);
+}
+
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");
@@ -342,7 +355,21 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
goto err0;
}
+ if (hdmi.audio_configured) {
+ r = hdmi4_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)
+ hdmi_start_audio_stream(&hdmi);
hdmi.display_enabled = true;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
mutex_unlock(&hdmi.lock);
return 0;
@@ -354,17 +381,19 @@ err0:
static void hdmi_display_disable(struct omap_dss_device *dssdev)
{
+ unsigned long flags;
+
DSSDBG("Enter hdmi_display_disable\n");
mutex_lock(&hdmi.lock);
- if (hdmi.audio_pdev && hdmi.audio_abort_cb)
- hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
+ spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+ hdmi_stop_audio_stream(&hdmi);
+ hdmi.display_enabled = false;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
hdmi_power_off_full(dssdev);
- hdmi.display_enabled = false;
-
mutex_unlock(&hdmi.lock);
}
@@ -565,9 +594,14 @@ out:
static int hdmi_audio_shutdown(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
mutex_lock(&hd->lock);
hd->audio_abort_cb = NULL;
+ hd->audio_configured = false;
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+ hd->audio_playing = false;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
mutex_unlock(&hd->lock);
return 0;
@@ -576,25 +610,34 @@ static int hdmi_audio_shutdown(struct device *dev)
static int hdmi_audio_start(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi_wp_audio_enable(&hd->wp, true);
- hdmi4_audio_start(&hd->core, &hd->wp);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled)
+ hdmi_start_audio_stream(hd);
+ hd->audio_playing = true;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
return 0;
}
static void hdmi_audio_stop(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi4_audio_stop(&hd->core, &hd->wp);
- hdmi_wp_audio_enable(&hd->wp, false);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled)
+ hdmi_stop_audio_stream(hd);
+ hd->audio_playing = false;
+
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
}
static int hdmi_audio_config(struct device *dev,
@@ -612,7 +655,10 @@ static int hdmi_audio_config(struct device *dev,
ret = hdmi4_audio_config(&hd->core, &hd->wp, dss_audio,
hd->cfg.timings.pixelclock);
-
+ if (!ret) {
+ hd->audio_configured = true;
+ hd->audio_config = *dss_audio;
+ }
out:
mutex_unlock(&hd->lock);
@@ -657,6 +703,7 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
dev_set_drvdata(&pdev->dev, &hdmi);
mutex_init(&hdmi.lock);
+ spin_lock_init(&hdmi.audio_playing_lock);
if (pdev->dev.of_node) {
r = hdmi_probe_of(pdev);
diff --git a/drivers/video/fbdev/omap2/dss/hdmi5.c b/drivers/video/fbdev/omap2/dss/hdmi5.c
index 7f87578..85e70af 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi5.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi5.c
@@ -349,9 +349,24 @@ static int read_edid(u8 *buf, int len)
return r;
}
+static void hdmi_start_audio_stream(struct omap_hdmi *hd)
+{
+ REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
+ hdmi_wp_audio_enable(&hd->wp, true);
+ hdmi_wp_audio_core_req_enable(&hd->wp, true);
+}
+
+static void hdmi_stop_audio_stream(struct omap_hdmi *hd)
+{
+ hdmi_wp_audio_core_req_enable(&hd->wp, false);
+ hdmi_wp_audio_enable(&hd->wp, false);
+ REG_FLD_MOD(hd->wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
+}
+
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 +385,21 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
goto err0;
}
+ if (hdmi.audio_configured) {
+ 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)
+ hdmi_start_audio_stream(&hdmi);
hdmi.display_enabled = true;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
mutex_unlock(&hdmi.lock);
return 0;
@@ -382,17 +411,19 @@ err0:
static void hdmi_display_disable(struct omap_dss_device *dssdev)
{
+ unsigned long flags;
+
DSSDBG("Enter hdmi_display_disable\n");
mutex_lock(&hdmi.lock);
- if (hdmi.audio_pdev && hdmi.audio_abort_cb)
- hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
+ spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+ hdmi_stop_audio_stream(&hdmi);
+ hdmi.display_enabled = false;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
hdmi_power_off_full(dssdev);
- hdmi.display_enabled = false;
-
mutex_unlock(&hdmi.lock);
}
@@ -593,9 +624,14 @@ out:
static int hdmi_audio_shutdown(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
mutex_lock(&hd->lock);
hd->audio_abort_cb = NULL;
+ hd->audio_configured = false;
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+ hd->audio_playing = false;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
mutex_unlock(&hd->lock);
return 0;
@@ -604,32 +640,35 @@ static int hdmi_audio_shutdown(struct device *dev)
static int hdmi_audio_start(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- /* No-idle while playing audio, store the old value */
- hd->wp_idlemode = REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
- REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
- hdmi_wp_audio_enable(&hd->wp, true);
- hdmi_wp_audio_core_req_enable(&hd->wp, true);
+ if (hd->display_enabled) {
+ hdmi_start_audio_stream(hd);
+ }
+ hd->audio_playing = true;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
return 0;
}
static void hdmi_audio_stop(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi_wp_audio_core_req_enable(&hd->wp, false);
- hdmi_wp_audio_enable(&hd->wp, false);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled)
+ hdmi_stop_audio_stream(hd);
+ hd->audio_playing = false;
- /* Playback stopped, restore original idlemode */
- REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
}
static int hdmi_audio_config(struct device *dev,
@@ -648,6 +687,10 @@ static int hdmi_audio_config(struct device *dev,
ret = hdmi5_audio_config(&hd->core, &hd->wp, dss_audio,
hd->cfg.timings.pixelclock);
+ if (!ret) {
+ hd->audio_configured = true;
+ hd->audio_config = *dss_audio;
+ }
out:
mutex_unlock(&hd->lock);
@@ -678,6 +721,11 @@ static int hdmi_audio_register(struct device *dev)
if (IS_ERR(hdmi.audio_pdev))
return PTR_ERR(hdmi.audio_pdev);
+ hdmi_runtime_get();
+ hdmi.wp_idlemode + REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
+ hdmi_runtime_put();
+
return 0;
}
@@ -692,6 +740,7 @@ static int hdmi5_bind(struct device *dev, struct device *master, void *data)
dev_set_drvdata(&pdev->dev, &hdmi);
mutex_init(&hdmi.lock);
+ spin_lock_init(&hdmi.audio_playing_lock);
if (pdev->dev.of_node) {
r = hdmi_probe_of(pdev);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
From: Jyri Sarha @ 2015-08-28 12:57 UTC (permalink / raw)
To: alsa-devel, linux-fbdev, linux-omap; +Cc: peter.ujfalusi, tomi.valkeinen
In-Reply-To: <1440764660-10417-1-git-send-email-jsarha@ti.com>
On 08/28/15 15:24, Jyri Sarha wrote:
> @@ -565,9 +594,14 @@ out:
> static int hdmi_audio_shutdown(struct device *dev)
> {
> struct omap_hdmi *hd = dev_get_drvdata(dev);
> + unsigned long flags;
>
> mutex_lock(&hd->lock);
> hd->audio_abort_cb = NULL;
> + hd->audio_configured = false;
> + spin_lock_irqsave(&hd->audio_playing_lock, flags);
> + hd->audio_playing = false;
> + spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
>
BTW, This extra locking (and the corresponding change in hdmi5.c) should
not be needed. But it does not harm either. I'll fix that, but I'll wait
first if there is anything else fix.
Best regards,
Jyri
^ permalink raw reply
* Re: [PATCH v2] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
From: Tomi Valkeinen @ 2015-08-28 13:04 UTC (permalink / raw)
To: Jyri Sarha, alsa-devel, linux-fbdev, linux-omap; +Cc: peter.ujfalusi
In-Reply-To: <1440764660-10417-1-git-send-email-jsarha@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3250 bytes --]
Hi,
On 28/08/15 15:24, Jyri Sarha wrote:
> Reconfigure and restart audio when display is enabled, if audio
> playback was active before. The audio configuration is stored when is
'is' -> 'it' above
> is successfully applied and a boolean is set when playback has started
> and unset when stopped. This data is used to reconfigure the audio
> when display is re-enabled. Abort audio playback if reconfiguration
> fails.
It would be good to start the description by telling what the current
problem is. And probably the subject could also be better... This fixes
the audio playback when a video mode change happens (or such), right?
> A new spin lock is introduced in order to protect state variables
> related to audio playback status. 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 with the start/stop audio commands.
>
> 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 hdmi.audio_playing 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.
The question is (which was my point in the earlier mail), we already
have mutex, so why a new spinlock?
I think the answer is that audio start/stop (anything else?) are called
in atomic context, so mutex cannot be used.
Also (not exactly related to this patch), if the audio callbacks must be
atomic, could we use a workqueue to run the audio start/stop work in
non-atomic context? Protecting the whole hdmi state with a single mutex
would be much nicer.
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
> I dropped the ASoC maintainers from the recipient list as this patch
> hardly concerns them.
>
> drivers/video/fbdev/omap2/dss/hdmi.h | 9 +++-
> drivers/video/fbdev/omap2/dss/hdmi4.c | 69 +++++++++++++++++++++++++-----
> drivers/video/fbdev/omap2/dss/hdmi5.c | 79 ++++++++++++++++++++++++++++-------
> 3 files changed, 130 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/video/fbdev/omap2/dss/hdmi.h b/drivers/video/fbdev/omap2/dss/hdmi.h
> index e4a32fe..e48aefd 100644
> --- a/drivers/video/fbdev/omap2/dss/hdmi.h
> +++ b/drivers/video/fbdev/omap2/dss/hdmi.h
> @@ -351,13 +351,20 @@ struct omap_hdmi {
> struct regulator *vdda_reg;
>
> bool core_enabled;
> - bool display_enabled;
>
> struct omap_dss_device output;
>
> struct platform_device *audio_pdev;
> void (*audio_abort_cb)(struct device *dev);
> int wp_idlemode;
> +
> + bool audio_configured;
> + struct omap_dss_audio audio_config;
> +
> + /* This lock should be taken when booleas bellow is touched. */
typo above.
Otherwise, looks much cleaner than the previous one. I tested it and
worked fine for me: I could play audio while turning on and off the
video output, and the audio would resume, except when the video was off
for long enough.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
From: Jyri Sarha @ 2015-08-28 13:46 UTC (permalink / raw)
To: Tomi Valkeinen, alsa-devel, linux-fbdev, linux-omap; +Cc: peter.ujfalusi
In-Reply-To: <55E05C60.2090400@ti.com>
On 08/28/15 16:04, Tomi Valkeinen wrote:
...
> The question is (which was my point in the earlier mail), we already
> have mutex, so why a new spinlock?
>
> I think the answer is that audio start/stop (anything else?) are called
> in atomic context, so mutex cannot be used.
>
Yes, that is correct.
> Also (not exactly related to this patch), if the audio callbacks must be
> atomic, could we use a workqueue to run the audio start/stop work in
> non-atomic context? Protecting the whole hdmi state with a single mutex
> would be much nicer.
The reason why the audio start and stop are done in atomic context is
for audio synchronization to other audio devices or video stream to be
more accurate.
I guess in theory a work queue could be used to serialize the audio
commands, but that would ruin the whole point of why the start and stop
commands are atomic in the first place.
Cheers,
Jyri
^ permalink raw reply
* [PATCH v3] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
From: Jyri Sarha @ 2015-08-28 14:10 UTC (permalink / raw)
To: alsa-devel, linux-fbdev, linux-omap
Cc: peter.ujfalusi, tomi.valkeinen, Jyri Sarha
Reconfigure and restart audio when display is enabled, if audio
playback was active before. This is needed in a situation when an
audio+video stream application opens the audio stream before the
video. When video stream is opened the display mode may change and
that would aborted audio playback, because the display is momentarily
turned off.
The audio configuration is stored when it is successfully applied and
a boolean is set when the audio playback is started and unset when
stopped. This data is used to reconfigure the audio when display is
re-enabled. The audio playback is aborted if the reconfiguration fails.
A new spin lock is introduced in order to protect state variables
related to audio playback status. This is needed for the transition
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 with the start/stop audio commands. The already
existing mutex can not be used, because the audio start and stop
commands are executed in atomic context.
For example: when display is turned back on we take the spinlock and
we can be sure that the audio start/stop status will not change while
we update the HW according to hdmi.audio_playing 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.
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
drivers/video/fbdev/omap2/dss/hdmi.h | 9 ++++-
drivers/video/fbdev/omap2/dss/hdmi4.c | 66 +++++++++++++++++++++++++-----
drivers/video/fbdev/omap2/dss/hdmi5.c | 76 ++++++++++++++++++++++++++++-------
3 files changed, 124 insertions(+), 27 deletions(-)
diff --git a/drivers/video/fbdev/omap2/dss/hdmi.h b/drivers/video/fbdev/omap2/dss/hdmi.h
index e4a32fe..53616b0 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi.h
+++ b/drivers/video/fbdev/omap2/dss/hdmi.h
@@ -351,13 +351,20 @@ struct omap_hdmi {
struct regulator *vdda_reg;
bool core_enabled;
- bool display_enabled;
struct omap_dss_device output;
struct platform_device *audio_pdev;
void (*audio_abort_cb)(struct device *dev);
int wp_idlemode;
+
+ bool audio_configured;
+ struct omap_dss_audio audio_config;
+
+ /* This lock should be taken when booleans bellow are touched. */
+ spinlock_t audio_playing_lock;
+ bool audio_playing;
+ bool display_enabled;
};
#endif
diff --git a/drivers/video/fbdev/omap2/dss/hdmi4.c b/drivers/video/fbdev/omap2/dss/hdmi4.c
index 6d3aa3f..94c8d55 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi4.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi4.c
@@ -321,9 +321,22 @@ static int read_edid(u8 *buf, int len)
return r;
}
+static void hdmi_start_audio_stream(struct omap_hdmi *hd)
+{
+ hdmi_wp_audio_enable(&hd->wp, true);
+ hdmi4_audio_start(&hd->core, &hd->wp);
+}
+
+static void hdmi_stop_audio_stream(struct omap_hdmi *hd)
+{
+ hdmi4_audio_stop(&hd->core, &hd->wp);
+ hdmi_wp_audio_enable(&hd->wp, false);
+}
+
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");
@@ -342,7 +355,21 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
goto err0;
}
+ if (hdmi.audio_configured) {
+ r = hdmi4_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)
+ hdmi_start_audio_stream(&hdmi);
hdmi.display_enabled = true;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
mutex_unlock(&hdmi.lock);
return 0;
@@ -354,17 +381,19 @@ err0:
static void hdmi_display_disable(struct omap_dss_device *dssdev)
{
+ unsigned long flags;
+
DSSDBG("Enter hdmi_display_disable\n");
mutex_lock(&hdmi.lock);
- if (hdmi.audio_pdev && hdmi.audio_abort_cb)
- hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
+ spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+ hdmi_stop_audio_stream(&hdmi);
+ hdmi.display_enabled = false;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
hdmi_power_off_full(dssdev);
- hdmi.display_enabled = false;
-
mutex_unlock(&hdmi.lock);
}
@@ -568,6 +597,8 @@ static int hdmi_audio_shutdown(struct device *dev)
mutex_lock(&hd->lock);
hd->audio_abort_cb = NULL;
+ hd->audio_configured = false;
+ hd->audio_playing = false;
mutex_unlock(&hd->lock);
return 0;
@@ -576,25 +607,34 @@ static int hdmi_audio_shutdown(struct device *dev)
static int hdmi_audio_start(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi_wp_audio_enable(&hd->wp, true);
- hdmi4_audio_start(&hd->core, &hd->wp);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled)
+ hdmi_start_audio_stream(hd);
+ hd->audio_playing = true;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
return 0;
}
static void hdmi_audio_stop(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi4_audio_stop(&hd->core, &hd->wp);
- hdmi_wp_audio_enable(&hd->wp, false);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled)
+ hdmi_stop_audio_stream(hd);
+ hd->audio_playing = false;
+
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
}
static int hdmi_audio_config(struct device *dev,
@@ -612,7 +652,10 @@ static int hdmi_audio_config(struct device *dev,
ret = hdmi4_audio_config(&hd->core, &hd->wp, dss_audio,
hd->cfg.timings.pixelclock);
-
+ if (!ret) {
+ hd->audio_configured = true;
+ hd->audio_config = *dss_audio;
+ }
out:
mutex_unlock(&hd->lock);
@@ -657,6 +700,7 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
dev_set_drvdata(&pdev->dev, &hdmi);
mutex_init(&hdmi.lock);
+ spin_lock_init(&hdmi.audio_playing_lock);
if (pdev->dev.of_node) {
r = hdmi_probe_of(pdev);
diff --git a/drivers/video/fbdev/omap2/dss/hdmi5.c b/drivers/video/fbdev/omap2/dss/hdmi5.c
index 7f87578..b6c767b 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi5.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi5.c
@@ -349,9 +349,24 @@ static int read_edid(u8 *buf, int len)
return r;
}
+static void hdmi_start_audio_stream(struct omap_hdmi *hd)
+{
+ REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
+ hdmi_wp_audio_enable(&hd->wp, true);
+ hdmi_wp_audio_core_req_enable(&hd->wp, true);
+}
+
+static void hdmi_stop_audio_stream(struct omap_hdmi *hd)
+{
+ hdmi_wp_audio_core_req_enable(&hd->wp, false);
+ hdmi_wp_audio_enable(&hd->wp, false);
+ REG_FLD_MOD(hd->wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
+}
+
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 +385,21 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
goto err0;
}
+ if (hdmi.audio_configured) {
+ 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)
+ hdmi_start_audio_stream(&hdmi);
hdmi.display_enabled = true;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
mutex_unlock(&hdmi.lock);
return 0;
@@ -382,17 +411,19 @@ err0:
static void hdmi_display_disable(struct omap_dss_device *dssdev)
{
+ unsigned long flags;
+
DSSDBG("Enter hdmi_display_disable\n");
mutex_lock(&hdmi.lock);
- if (hdmi.audio_pdev && hdmi.audio_abort_cb)
- hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
+ spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+ hdmi_stop_audio_stream(&hdmi);
+ hdmi.display_enabled = false;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
hdmi_power_off_full(dssdev);
- hdmi.display_enabled = false;
-
mutex_unlock(&hdmi.lock);
}
@@ -596,6 +627,8 @@ static int hdmi_audio_shutdown(struct device *dev)
mutex_lock(&hd->lock);
hd->audio_abort_cb = NULL;
+ hd->audio_configured = false;
+ hd->audio_playing = false;
mutex_unlock(&hd->lock);
return 0;
@@ -604,32 +637,35 @@ static int hdmi_audio_shutdown(struct device *dev)
static int hdmi_audio_start(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- /* No-idle while playing audio, store the old value */
- hd->wp_idlemode = REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
- REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
- hdmi_wp_audio_enable(&hd->wp, true);
- hdmi_wp_audio_core_req_enable(&hd->wp, true);
+ if (hd->display_enabled) {
+ hdmi_start_audio_stream(hd);
+ }
+ hd->audio_playing = true;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
return 0;
}
static void hdmi_audio_stop(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi_wp_audio_core_req_enable(&hd->wp, false);
- hdmi_wp_audio_enable(&hd->wp, false);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled)
+ hdmi_stop_audio_stream(hd);
+ hd->audio_playing = false;
- /* Playback stopped, restore original idlemode */
- REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
}
static int hdmi_audio_config(struct device *dev,
@@ -648,6 +684,10 @@ static int hdmi_audio_config(struct device *dev,
ret = hdmi5_audio_config(&hd->core, &hd->wp, dss_audio,
hd->cfg.timings.pixelclock);
+ if (!ret) {
+ hd->audio_configured = true;
+ hd->audio_config = *dss_audio;
+ }
out:
mutex_unlock(&hd->lock);
@@ -678,6 +718,11 @@ static int hdmi_audio_register(struct device *dev)
if (IS_ERR(hdmi.audio_pdev))
return PTR_ERR(hdmi.audio_pdev);
+ hdmi_runtime_get();
+ hdmi.wp_idlemode + REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
+ hdmi_runtime_put();
+
return 0;
}
@@ -692,6 +737,7 @@ static int hdmi5_bind(struct device *dev, struct device *master, void *data)
dev_set_drvdata(&pdev->dev, &hdmi);
mutex_init(&hdmi.lock);
+ spin_lock_init(&hdmi.audio_playing_lock);
if (pdev->dev.of_node) {
r = hdmi_probe_of(pdev);
--
1.9.1
^ permalink raw reply related
* [PATCH v4] OMAPDSS: hdmi: Reconfigure and restart audio when display is enabled
From: Jyri Sarha @ 2015-08-28 14:21 UTC (permalink / raw)
To: alsa-devel, linux-fbdev, linux-omap
Cc: peter.ujfalusi, tomi.valkeinen, Jyri Sarha
Reconfigure and restart audio when display is enabled, if audio
playback was active before. This is needed in a situation when an
audio+video stream application opens the audio stream before the
video. When video stream is opened the display mode may change and
that aborts audio playback, because the display is momentarily
turned off.
The audio configuration is stored when it is successfully applied and
a boolean is set when the audio playback is started and unset when
stopped. This data is used to reconfigure the audio when display is
re-enabled. The audio playback is aborted if the reconfiguration fails.
A new spin lock is introduced in order to protect state variables
related to audio playback status. This is needed for the transition
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 with the start/stop audio commands. The already
existing mutex can not be used, because the audio start and stop
commands are executed in atomic context.
For example: when display is turned back on we take the spinlock and
we can be sure that the audio start/stop status will not change while
we update the HW according to hdmi.audio_playing 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.
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
Fixed one bad sentece in the description.
drivers/video/fbdev/omap2/dss/hdmi.h | 9 ++++-
drivers/video/fbdev/omap2/dss/hdmi4.c | 66 +++++++++++++++++++++++++-----
drivers/video/fbdev/omap2/dss/hdmi5.c | 76 ++++++++++++++++++++++++++++-------
3 files changed, 124 insertions(+), 27 deletions(-)
diff --git a/drivers/video/fbdev/omap2/dss/hdmi.h b/drivers/video/fbdev/omap2/dss/hdmi.h
index e4a32fe..53616b0 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi.h
+++ b/drivers/video/fbdev/omap2/dss/hdmi.h
@@ -351,13 +351,20 @@ struct omap_hdmi {
struct regulator *vdda_reg;
bool core_enabled;
- bool display_enabled;
struct omap_dss_device output;
struct platform_device *audio_pdev;
void (*audio_abort_cb)(struct device *dev);
int wp_idlemode;
+
+ bool audio_configured;
+ struct omap_dss_audio audio_config;
+
+ /* This lock should be taken when booleans bellow are touched. */
+ spinlock_t audio_playing_lock;
+ bool audio_playing;
+ bool display_enabled;
};
#endif
diff --git a/drivers/video/fbdev/omap2/dss/hdmi4.c b/drivers/video/fbdev/omap2/dss/hdmi4.c
index 6d3aa3f..94c8d55 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi4.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi4.c
@@ -321,9 +321,22 @@ static int read_edid(u8 *buf, int len)
return r;
}
+static void hdmi_start_audio_stream(struct omap_hdmi *hd)
+{
+ hdmi_wp_audio_enable(&hd->wp, true);
+ hdmi4_audio_start(&hd->core, &hd->wp);
+}
+
+static void hdmi_stop_audio_stream(struct omap_hdmi *hd)
+{
+ hdmi4_audio_stop(&hd->core, &hd->wp);
+ hdmi_wp_audio_enable(&hd->wp, false);
+}
+
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");
@@ -342,7 +355,21 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
goto err0;
}
+ if (hdmi.audio_configured) {
+ r = hdmi4_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)
+ hdmi_start_audio_stream(&hdmi);
hdmi.display_enabled = true;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
mutex_unlock(&hdmi.lock);
return 0;
@@ -354,17 +381,19 @@ err0:
static void hdmi_display_disable(struct omap_dss_device *dssdev)
{
+ unsigned long flags;
+
DSSDBG("Enter hdmi_display_disable\n");
mutex_lock(&hdmi.lock);
- if (hdmi.audio_pdev && hdmi.audio_abort_cb)
- hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
+ spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+ hdmi_stop_audio_stream(&hdmi);
+ hdmi.display_enabled = false;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
hdmi_power_off_full(dssdev);
- hdmi.display_enabled = false;
-
mutex_unlock(&hdmi.lock);
}
@@ -568,6 +597,8 @@ static int hdmi_audio_shutdown(struct device *dev)
mutex_lock(&hd->lock);
hd->audio_abort_cb = NULL;
+ hd->audio_configured = false;
+ hd->audio_playing = false;
mutex_unlock(&hd->lock);
return 0;
@@ -576,25 +607,34 @@ static int hdmi_audio_shutdown(struct device *dev)
static int hdmi_audio_start(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi_wp_audio_enable(&hd->wp, true);
- hdmi4_audio_start(&hd->core, &hd->wp);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled)
+ hdmi_start_audio_stream(hd);
+ hd->audio_playing = true;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
return 0;
}
static void hdmi_audio_stop(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi4_audio_stop(&hd->core, &hd->wp);
- hdmi_wp_audio_enable(&hd->wp, false);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled)
+ hdmi_stop_audio_stream(hd);
+ hd->audio_playing = false;
+
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
}
static int hdmi_audio_config(struct device *dev,
@@ -612,7 +652,10 @@ static int hdmi_audio_config(struct device *dev,
ret = hdmi4_audio_config(&hd->core, &hd->wp, dss_audio,
hd->cfg.timings.pixelclock);
-
+ if (!ret) {
+ hd->audio_configured = true;
+ hd->audio_config = *dss_audio;
+ }
out:
mutex_unlock(&hd->lock);
@@ -657,6 +700,7 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
dev_set_drvdata(&pdev->dev, &hdmi);
mutex_init(&hdmi.lock);
+ spin_lock_init(&hdmi.audio_playing_lock);
if (pdev->dev.of_node) {
r = hdmi_probe_of(pdev);
diff --git a/drivers/video/fbdev/omap2/dss/hdmi5.c b/drivers/video/fbdev/omap2/dss/hdmi5.c
index 7f87578..b6c767b 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi5.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi5.c
@@ -349,9 +349,24 @@ static int read_edid(u8 *buf, int len)
return r;
}
+static void hdmi_start_audio_stream(struct omap_hdmi *hd)
+{
+ REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
+ hdmi_wp_audio_enable(&hd->wp, true);
+ hdmi_wp_audio_core_req_enable(&hd->wp, true);
+}
+
+static void hdmi_stop_audio_stream(struct omap_hdmi *hd)
+{
+ hdmi_wp_audio_core_req_enable(&hd->wp, false);
+ hdmi_wp_audio_enable(&hd->wp, false);
+ REG_FLD_MOD(hd->wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
+}
+
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 +385,21 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
goto err0;
}
+ if (hdmi.audio_configured) {
+ 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)
+ hdmi_start_audio_stream(&hdmi);
hdmi.display_enabled = true;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
mutex_unlock(&hdmi.lock);
return 0;
@@ -382,17 +411,19 @@ err0:
static void hdmi_display_disable(struct omap_dss_device *dssdev)
{
+ unsigned long flags;
+
DSSDBG("Enter hdmi_display_disable\n");
mutex_lock(&hdmi.lock);
- if (hdmi.audio_pdev && hdmi.audio_abort_cb)
- hdmi.audio_abort_cb(&hdmi.audio_pdev->dev);
+ spin_lock_irqsave(&hdmi.audio_playing_lock, flags);
+ hdmi_stop_audio_stream(&hdmi);
+ hdmi.display_enabled = false;
+ spin_unlock_irqrestore(&hdmi.audio_playing_lock, flags);
hdmi_power_off_full(dssdev);
- hdmi.display_enabled = false;
-
mutex_unlock(&hdmi.lock);
}
@@ -596,6 +627,8 @@ static int hdmi_audio_shutdown(struct device *dev)
mutex_lock(&hd->lock);
hd->audio_abort_cb = NULL;
+ hd->audio_configured = false;
+ hd->audio_playing = false;
mutex_unlock(&hd->lock);
return 0;
@@ -604,32 +637,35 @@ static int hdmi_audio_shutdown(struct device *dev)
static int hdmi_audio_start(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- /* No-idle while playing audio, store the old value */
- hd->wp_idlemode = REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
- REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, 1, 3, 2);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
- hdmi_wp_audio_enable(&hd->wp, true);
- hdmi_wp_audio_core_req_enable(&hd->wp, true);
+ if (hd->display_enabled) {
+ hdmi_start_audio_stream(hd);
+ }
+ hd->audio_playing = true;
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
return 0;
}
static void hdmi_audio_stop(struct device *dev)
{
struct omap_hdmi *hd = dev_get_drvdata(dev);
+ unsigned long flags;
WARN_ON(!hdmi_mode_has_audio(&hd->cfg));
- WARN_ON(!hd->display_enabled);
- hdmi_wp_audio_core_req_enable(&hd->wp, false);
- hdmi_wp_audio_enable(&hd->wp, false);
+ spin_lock_irqsave(&hd->audio_playing_lock, flags);
+
+ if (hd->display_enabled)
+ hdmi_stop_audio_stream(hd);
+ hd->audio_playing = false;
- /* Playback stopped, restore original idlemode */
- REG_FLD_MOD(hdmi.wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
+ spin_unlock_irqrestore(&hd->audio_playing_lock, flags);
}
static int hdmi_audio_config(struct device *dev,
@@ -648,6 +684,10 @@ static int hdmi_audio_config(struct device *dev,
ret = hdmi5_audio_config(&hd->core, &hd->wp, dss_audio,
hd->cfg.timings.pixelclock);
+ if (!ret) {
+ hd->audio_configured = true;
+ hd->audio_config = *dss_audio;
+ }
out:
mutex_unlock(&hd->lock);
@@ -678,6 +718,11 @@ static int hdmi_audio_register(struct device *dev)
if (IS_ERR(hdmi.audio_pdev))
return PTR_ERR(hdmi.audio_pdev);
+ hdmi_runtime_get();
+ hdmi.wp_idlemode + REG_GET(hdmi.wp.base, HDMI_WP_SYSCONFIG, 3, 2);
+ hdmi_runtime_put();
+
return 0;
}
@@ -692,6 +737,7 @@ static int hdmi5_bind(struct device *dev, struct device *master, void *data)
dev_set_drvdata(&pdev->dev, &hdmi);
mutex_init(&hdmi.lock);
+ spin_lock_init(&hdmi.audio_playing_lock);
if (pdev->dev.of_node) {
r = hdmi_probe_of(pdev);
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox