diff mbox

[Trusty] Fix framebuffer hangs with VMs using cirrus gfx

Message ID 1391076672-25385-1-git-send-email-stefan.bader@canonical.com
State New
Headers show

Commit Message

Stefan Bader Jan. 30, 2014, 10:11 a.m. UTC 
From 5eaa87a69bb40ffeec759b6e5aeec1a26bba1680 Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@canonical.com>
Date: Wed, 29 Jan 2014 16:45:12 +0000
Subject: [PATCH 1/2] UBUNTU: SAUCE: drm/cirrus: Ignore busy mem region when
 mapping VRAM

Since 29d274b8d3e2404cd1832b3a999b12f9d1e1d895 ("x86/simplefb: Mark
framebuffer mem-resources as IORESOURCE_BUSY to avoid bootup warning")
the messages for real hw drivers mapping the same region as the simple-
framebuffer are gone. But now the cirrus drm driver will always fail
to initialize its VRAM because the region is busy. Removing the
conflicting framebuffer only can stop simplefb from using the memory
and unmap it but it is the platform device which has the region assined
to itself.
As the comment of the above patch indicates there is currently no sane
way to get rid of the platform device. So at least for now accept that
the region can appear busy. It is usuable nonetheless.
This will fix VMs using the cirrus gfx emulation to appear locked up
when booting from GRUB gfx mode.

BugLink: http://bugs.launchpad.net/bugs/1269401

Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 drivers/gpu/drm/cirrus/cirrus_main.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Tim Gardner Jan. 31, 2014, 10:20 a.m. UTC | #1 
Ick.
Stefan Bader Feb. 3, 2014, 5:09 p.m. UTC | #2 
On 31.01.2014 11:20, Tim Gardner wrote:
> Ick.
> 

I got the following feedback from David Herrmann:

> This should be fixed by patches #1+#2 in this series:
>   http://lists.freedesktop.org/archives/dri-devel/2014-January/052584.html
>
> I hope we can get it applied this week. Just waiting for a rough
> review from airlied. As a workaround, you can always disable
> CONFIG_X86_SYSFB. There is no reason to activate it right now (until
> SimpleDRM gets merged..).

So we could either revert my patch and disable CONFIG_X86_SYSFB (was there a
special reason to enable it or just default policy?) or we wait until these two
patches end up in Linux tree and then revert mine in favour of those. I am not
sure whether they end up on a stable tree if simplefb is as "in development" as
it sounds to me.

-Stefan

[1] http://lists.freedesktop.org/archives/dri-devel/2014-January/052586.html
[2] http://lists.freedesktop.org/archives/dri-devel/2014-January/052585.html
Tim Gardner Feb. 3, 2014, 5:25 p.m. UTC | #3 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 02/03/2014 10:09 AM, Stefan Bader wrote:
> On 31.01.2014 11:20, Tim Gardner wrote:
>> Ick.
>> 
> 
> I got the following feedback from David Herrmann:
> 
>> This should be fixed by patches #1+#2 in this series: 
>> http://lists.freedesktop.org/archives/dri-devel/2014-January/052584.html
>>
>>
>> 
I hope we can get it applied this week. Just waiting for a rough
>> review from airlied. As a workaround, you can always disable 
>> CONFIG_X86_SYSFB. There is no reason to activate it right now
>> (until SimpleDRM gets merged..).
> 
> So we could either revert my patch and disable CONFIG_X86_SYSFB
> (was there a special reason to enable it or just default policy?)
> or we wait until these two patches end up in Linux tree and then
> revert mine in favour of those. I am not sure whether they end up
> on a stable tree if simplefb is as "in development" as it sounds to
> me.
> 
> -Stefan
> 
> [1]
> http://lists.freedesktop.org/archives/dri-devel/2014-January/052586.html
>
> 
[2] http://lists.freedesktop.org/archives/dri-devel/2014-January/052585.html
> 

CONFIG_X86_SYSFB likely was enabled as a matter of policy. The help
text suggests that if you don't know what you're doing then say "Y".

I'd be inclined to drop your patch and set CONFIG_X86_SYSFB=n.

rtg
- -- 
Tim Gardner tim.gardner@canonical.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBCgAGBQJS79EgAAoJED12yEX6FEfKYJ0P/1gmpS0UIlHg2lDWlwa10Ghq
afK4uS7xEX3ZPUYMf2DNOPwyBnM3iLgd0VIb64IBRm19P30qXrkv5sx7bYdLDW0R
icHRMjCtMPJi00fNPRDTrcyCtiAGl1IWW/CzpDFg2vjGmm44HnW2bl9Tcfq1Eo66
s+jXUPxIGMMmoed8lCfTYFGfGaBYp7IbffULFkDu22utENReOfDUSTYNDQ4hS903
zUnNM5g+xpEuybdnGyt4YXisJjDTo1FBVEYrFFIELhY1VtA3VWZjoves+4/xRZBZ
fXDA50Dea1FpJDRALplyNIf6DybPGKF13A2z2OXnOxHFi+aUnTeAMD2ueHRirTTk
RikQr24F2UPK4Qr7IxbHCPjff9BCu7/i4PHuY6bJ7uMCoGDM792yTbo7s9yXcAsL
UCZwU/SrFbDVCmb8kAQabW453j8OJ2EVDqVzx9AkWOGzlMnrpSaLeWnYhZ6NlMTL
ivL55Fv4EY8ymV46THy9UaI3L0xkwMrS7amucL5+E30QTCz3ytj/VDMBA8Gb1m9J
tO5wTLR7pVvMjFV3L1JI34ClIwYVaShe/wsfrvPbgve0myBZ90gTAHa2di01L1xW
yhPTR1ju1XZe4N18GnqnMOrxu0txPQ8YzZdVPym3vwNUFXl6ud2NYW6FbhAiWvfi
rl4/EBhlNbQDAeBhGEZu
=wj9U
-----END PGP SIGNATURE-----
Stefan Bader Feb. 3, 2014, 5:42 p.m. UTC | #4 
On 03.02.2014 18:25, Tim Gardner wrote:
> On 02/03/2014 10:09 AM, Stefan Bader wrote:
>> On 31.01.2014 11:20, Tim Gardner wrote:
>>> Ick.
>>>
> 
>> I got the following feedback from David Herrmann:
> 
>>> This should be fixed by patches #1+#2 in this series: 
>>> http://lists.freedesktop.org/archives/dri-devel/2014-January/052584.html
>>>
>>>
>>>
> I hope we can get it applied this week. Just waiting for a rough
>>> review from airlied. As a workaround, you can always disable 
>>> CONFIG_X86_SYSFB. There is no reason to activate it right now
>>> (until SimpleDRM gets merged..).
> 
>> So we could either revert my patch and disable CONFIG_X86_SYSFB
>> (was there a special reason to enable it or just default policy?)
>> or we wait until these two patches end up in Linux tree and then
>> revert mine in favour of those. I am not sure whether they end up
>> on a stable tree if simplefb is as "in development" as it sounds to
>> me.
> 
>> -Stefan
> 
>> [1]
>> http://lists.freedesktop.org/archives/dri-devel/2014-January/052586.html
> 
> 
> [2] http://lists.freedesktop.org/archives/dri-devel/2014-January/052585.html
> 
> 
> CONFIG_X86_SYSFB likely was enabled as a matter of policy. The help
> text suggests that if you don't know what you're doing then say "Y".
> 
> I'd be inclined to drop your patch and set CONFIG_X86_SYSFB=n.
> 

That would be my thinking as well. That will at least prevent unexpected
breakage that likely will happen when we do not revert it before or when those
upstream patches arrive.
We can simply flip it back to "y" should we really need it. Is that worth an
update in either the policy or the enforcer? At least I am prone to forget why
things were done...

-Stefan
Tim Gardner Feb. 3, 2014, 5:45 p.m. UTC | #5 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 02/03/2014 10:42 AM, Stefan Bader wrote:
> On 03.02.2014 18:25, Tim Gardner wrote:
>> On 02/03/2014 10:09 AM, Stefan Bader wrote:
>>> On 31.01.2014 11:20, Tim Gardner wrote:
>>>> Ick.
>>>> 
>> 
>>> I got the following feedback from David Herrmann:
>> 
>>>> This should be fixed by patches #1+#2 in this series: 
>>>> http://lists.freedesktop.org/archives/dri-devel/2014-January/052584.html
>>>>
>>>>
>>>>
>>
>>>> 
I hope we can get it applied this week. Just waiting for a rough
>>>> review from airlied. As a workaround, you can always disable
>>>>  CONFIG_X86_SYSFB. There is no reason to activate it right
>>>> now (until SimpleDRM gets merged..).
>> 
>>> So we could either revert my patch and disable
>>> CONFIG_X86_SYSFB (was there a special reason to enable it or
>>> just default policy?) or we wait until these two patches end up
>>> in Linux tree and then revert mine in favour of those. I am not
>>> sure whether they end up on a stable tree if simplefb is as "in
>>> development" as it sounds to me.
>> 
>>> -Stefan
>> 
>>> [1] 
>>> http://lists.freedesktop.org/archives/dri-devel/2014-January/052586.html
>>
>>
>>
>>> 
[2] http://lists.freedesktop.org/archives/dri-devel/2014-January/052585.html
>> 
>> 
>> CONFIG_X86_SYSFB likely was enabled as a matter of policy. The
>> help text suggests that if you don't know what you're doing then
>> say "Y".
>> 
>> I'd be inclined to drop your patch and set CONFIG_X86_SYSFB=n.
>> 
> 
> That would be my thinking as well. That will at least prevent
> unexpected breakage that likely will happen when we do not revert
> it before or when those upstream patches arrive. We can simply flip
> it back to "y" should we really need it. Is that worth an update in
> either the policy or the enforcer? At least I am prone to forget
> why things were done...
> 
> -Stefan
> 

I'll just note this email thread in the 'CONFIG_X86_SYSFB=n' patch.

- -- 
Tim Gardner tim.gardner@canonical.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBCgAGBQJS79WnAAoJED12yEX6FEfKuCUP/iF9yIiR43Q1DlsYouHNVP7T
OH5geyTMZs95pRWNEj/9oox7JClF5KY795BnXUgnFD1juCnyktf5ZJO5jfPKllZU
/D7Amedk+740GatkEhjClGCiAGQoXNPSVuzfymHue21swmG8pm8ZDS/FRLFgPJyW
ijbTGurcLC4EBdrtH9BRAAJ3YDDdMdNQ+bdRQbWDdKCGyt9Jp4qfGtqF8Mb42Bq6
xeVJzfTlpxI9i0QZrQwSKo+IJ2RZDMJJ+XjoDRIEOQ46NWH8GyqTGHtw1z55QDHH
HaY3r7kD7VDY8Ed8ExYdgfRFhzYkKKVvJixIE/8qzRlsGvjc2e5Mhr4rE6mszCER
Bg/l6ELzvqxAub8j10lytIchsU4mAn5I6xpso4zCeuUcf8NhEk3YDcLpQIubRZUE
DLGzQa2rtlqTAb0p//Udzu8+GITpeJ7XJ/S54lViYjPvp92yC1KOe0GqTqGpJLua
vKxRn9S5DJug9iw5Y7Yzjo+JEYhGteVTkfJxP30lM9Pk1zBNFGo1LgUTQNN9tHQZ
1U0zRJeHzS9ED6i1XbA8x1X2xBYpi/4zH9cPZU7AbIBV3EVGCZiLspOnzLKCAVF6
rRiQ8XPXoPNPbhJmt/DBbTC/dqMUZ5ve7j7sp9vimR3RaR7slFpq6zWsa6Puevvy
bG1qvLPw091JCyHXV71O
=ufex
-----END PGP SIGNATURE-----
Stefan Bader Feb. 3, 2014, 5:51 p.m. UTC | #6 
On 03.02.2014 18:45, Tim Gardner wrote:
> 
> I'll just note this email thread in the 'CONFIG_X86_SYSFB=n' patch.

ACK, works form me, too.

-Stefan
diff mbox

Patch

diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c
index 557d094..9f0193f 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -101,8 +101,15 @@  static int cirrus_vram_init(struct cirrus_device *cdev)
 
 	if (!request_mem_region(cdev->mc.vram_base, cdev->mc.vram_size,
 				"cirrusdrmfb_vram")) {
-		DRM_ERROR("can't reserve VRAM\n");
-		return -ENXIO;
+		/*
+		 * With the simple-framebuffer now marking the region
+		 * busy there is no way of this succeeding with having
+		 * the framebuffer active. Getting here means the old
+		 * one has been kicked and the region is unmapped but
+		 * there is no way to eject the platform device to get
+		 * the request_region undone.
+		 */
+		DRM_ERROR("can't reserve VRAM (ignored)\n");
 	}
 
 	return 0;