[PATCH] drm/nouveau/fbcon: fix oops without fbdev emulation

List overview All Threads
Download

newer

older

[PATCH v2 00/12] of: overlay:...

[Bug 60857] New: Unstable display...

Pavel Roskin

23 Sep 2017 23 Sep '17

8:10 p.m.

This is similar to an earlier commit 52dfcc5ccfbb ("drm/nouveau: fix for disabled fbdev emulation"), but protects all occurrences of helper.fbdev in the source.

I see oops in nouveau_fbcon_accel_save_disable() called from nouveau_fbcon_set_suspend_work() on Linux 3.13 when CONFIG_DRM_FBDEV_EMULATION option is disabled.

Signed-off-by: Pavel Roskin plroskin@gmail.com --- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index f7707849bb53..698b8b10b646 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -223,7 +223,7 @@ void nouveau_fbcon_accel_save_disable(struct drm_device *dev) { struct nouveau_drm *drm = nouveau_drm(dev); - if (drm->fbcon) { + if (drm->fbcon && drm->fbcon->helper.fbdev) { drm->fbcon->saved_flags = drm->fbcon->helper.fbdev->flags; drm->fbcon->helper.fbdev->flags |= FBINFO_HWACCEL_DISABLED; } @@ -233,9 +233,8 @@ void nouveau_fbcon_accel_restore(struct drm_device *dev) { struct nouveau_drm *drm = nouveau_drm(dev); - if (drm->fbcon) { + if (drm->fbcon && drm->fbcon->helper.fbdev) drm->fbcon->helper.fbdev->flags = drm->fbcon->saved_flags; - } }

static void @@ -245,7 +244,8 @@ nouveau_fbcon_accel_fini(struct drm_device *dev) struct nouveau_fbdev *fbcon = drm->fbcon; if (fbcon && drm->channel) { console_lock(); - fbcon->helper.fbdev->flags |= FBINFO_HWACCEL_DISABLED; + if (drm->fbcon->helper.fbdev) + fbcon->helper.fbdev->flags |= FBINFO_HWACCEL_DISABLED; console_unlock(); nouveau_channel_idle(drm->channel); nvif_object_fini(&fbcon->twod);

-- 2.14.1

Show replies by date

Daniel Vetter

26 Sep 26 Sep

8:05 a.m.

On Sat, Sep 23, 2017 at 01:10:33PM -0700, Pavel Roskin wrote:

...

This is similar to an earlier commit 52dfcc5ccfbb ("drm/nouveau: fix for disabled fbdev emulation"), but protects all occurrences of helper.fbdev in the source.

I see oops in nouveau_fbcon_accel_save_disable() called from nouveau_fbcon_set_suspend_work() on Linux 3.13 when CONFIG_DRM_FBDEV_EMULATION option is disabled.

3.13 is _very_ old. Can you pls retest this with 4.14-rc kernels first? I think we've fixed these all, but not sure.

Thanks, Daneil

...

Signed-off-by: Pavel Roskin plroskin@gmail.com

drivers/gpu/drm/nouveau/nouveau_fbcon.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index f7707849bb53..698b8b10b646 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -223,7 +223,7 @@ void nouveau_fbcon_accel_save_disable(struct drm_device *dev) { struct nouveau_drm *drm = nouveau_drm(dev);

if (drm->fbcon) {

if (drm->fbcon && drm->fbcon->helper.fbdev) { drm->fbcon->saved_flags = drm->fbcon->helper.fbdev->flags; drm->fbcon->helper.fbdev->flags |= FBINFO_HWACCEL_DISABLED; }

@@ -233,9 +233,8 @@ void nouveau_fbcon_accel_restore(struct drm_device *dev) { struct nouveau_drm *drm = nouveau_drm(dev);

if (drm->fbcon) {

if (drm->fbcon && drm->fbcon->helper.fbdev) drm->fbcon->helper.fbdev->flags = drm->fbcon->saved_flags;

}

}

static void @@ -245,7 +244,8 @@ nouveau_fbcon_accel_fini(struct drm_device *dev) struct nouveau_fbdev *fbcon = drm->fbcon; if (fbcon && drm->channel) { console_lock();
   fbcon->helper.fbdev->flags |= FBINFO_HWACCEL_DISABLED;
   if (drm->fbcon->helper.fbdev)
       fbcon->helper.fbdev->flags |= FBINFO_HWACCEL_DISABLED;
   console_unlock();
   nouveau_channel_idle(drm->channel);
   nvif_object_fini(&fbcon->twod);
-- 2.14.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

Pavel Roskin

5:36 p.m.

Hi Daniel,

I meant Linux 4.13 of course, sorry! Can you fix the description for me please? If not, I can resend the patch with the updated description.

Let me add some context to my patch. My laptop is Dell Precision 7510 with dual graphics (Intel and Nvidia). The dock is connected to two monitors with DisplayPort cables. It used to be two identical Dell P2416D monitors with 2560x1440 resolution. One of the monitors is in the vertical position. I've been using Fedora 26 with 4.11 and 4.12 kernels for a long time with no problems. The only issue was that the outputs would change names on reboot (e.g. DP-1-1 would become DP-1-4). I solved it by configuring all combinations in Gnome display settings. I also copied the resulting monitors.xml to the gdm repository to have the correct orientation in gdm.

A few days ago, I changed one of the 24-inch monitors (the horizontal one) with a wide Dell U3417W monitor with 3440x1440 resolution. The monitor would come up blank on some reboots (the monitor would show that it was not getting any signal and would go to power saving more). So I decided to update to Fedora 27 prerelease with 4.13 kernel. Things got even worse. The blank screen issue was not resolved, but I also started getting messages about circular dependencies:

====================================================== WARNING: possible circular locking dependency detected 4.13.3-300.fc27.x86_64 #1 Not tainted ------------------------------------------------------ Xorg/1032 is trying to acquire lock: (&dev->struct_mutex){+.+.+.}, at: [<ffffffffc00bddfa>] drm_gem_object_put_unlocked+0xa/0xb0 [drm] but task is already holding lock: (reservation_ww_class_mutex){+.+.+.}, at: [<ffffffffc0329cc2>] validate_init.isra.5+0x202/0x740 [nouveau] which lock already depends on the new lock.

I tried the current mainline kernel (4.14-rc1 at that time), and it started making oopses in nouveau_fbcon_set_suspend_work(). It that point, I wasn't using the dock. Then I realized that the 4.14-rc1 kernel wasn't showing "nouveaufb" in the log, whereas 4.13 was. The 4.14-rc1 kernel registers nouveaufb only if I use the dock.

I tracked the issue to nouveau_fbcon_accel_save_disable() trying to dereference drm->fbcon->helper.fbdev which was NULL. Once I fixed nouveau_fbcon_accel_save_disable(), oops started happening in nouveau_fbcon_accel_restore(), which I fixed as well. I haven't seen oopses in nouveau_fbcon_accel_fini(), but maybe it's just because that code wasn't called, so I added a check there as well.

Then I read the description of commit 52dfcc5ccfbb ("drm/nouveau: fix for disabled fbdev emulation") and decided to try Linux 4.13 with CONFIG_DRM_FBDEV_EMULATION disabled. As expected, it was oopsing without my patch (without the dock).

I've been running the patched Linux kernels (now 4.14-rc2) for a day with no issues other than the blank screen. I work it around by changing the display orientation in display properties to portrait and back to landscape. I hope to get it fixed, but let's deal with the oops for now.

I'm thinking of bisecting the circular dependency issue to get it backported to Linux 4.13. That would benefit other Fedora 27 users. If you know what patch fixed it, you would save me some time.

Pavel

Pavel Roskin

7:28 p.m.

This is similar to an earlier commit 52dfcc5ccfbb ("drm/nouveau: fix for disabled fbdev emulation"), but protects all other occurrences of helper.fbdev in the source.

I see an oops in nouveau_fbcon_accel_save_disable() called from nouveau_fbcon_set_suspend_work() on Linux 4.13 and 4.14-rc2 when CONFIG_DRM_FBDEV_EMULATION option is disabled. It happens during gdm login.

Linux 4.14-rc2 is affected even if CONFIG_DRM_FBDEV_EMULATION is enabled, but no monitors are connected to the dock on a laptop with a discrete NVIDIA card.

Signed-off-by: Pavel Roskin plroskin@gmail.com --- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

-- 2.14.1

Daniel Vetter

27 Sep 27 Sep

8:51 a.m.

On Tue, Sep 26, 2017 at 12:28:27PM -0700, Pavel Roskin wrote:

...

This is similar to an earlier commit 52dfcc5ccfbb ("drm/nouveau: fix for disabled fbdev emulation"), but protects all other occurrences of helper.fbdev in the source.

I see an oops in nouveau_fbcon_accel_save_disable() called from nouveau_fbcon_set_suspend_work() on Linux 4.13 and 4.14-rc2 when CONFIG_DRM_FBDEV_EMULATION option is disabled. It happens during gdm login.

Linux 4.14-rc2 is affected even if CONFIG_DRM_FBDEV_EMULATION is enabled, but no monitors are connected to the dock on a laptop with a discrete NVIDIA card.

Signed-off-by: Pavel Roskin plroskin@gmail.com

Ok, this actually makes sens now.

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

I'll leave it to Ben or Dave to pick this up. I guess there's still a regression somewhere with the monitor detection itself, but this should stop the oops.

Longer-term we maybe want to rip the fbdev hw accel stuff out, not sure there's any point in this really. -Daniel

...

drivers/gpu/drm/nouveau/nouveau_fbcon.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index f7707849bb53..698b8b10b646 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -223,7 +223,7 @@ void nouveau_fbcon_accel_save_disable(struct drm_device *dev) { struct nouveau_drm *drm = nouveau_drm(dev);

if (drm->fbcon) {

if (drm->fbcon && drm->fbcon->helper.fbdev) { drm->fbcon->saved_flags = drm->fbcon->helper.fbdev->flags; drm->fbcon->helper.fbdev->flags |= FBINFO_HWACCEL_DISABLED; }

@@ -233,9 +233,8 @@ void nouveau_fbcon_accel_restore(struct drm_device *dev) { struct nouveau_drm *drm = nouveau_drm(dev);

if (drm->fbcon) {

if (drm->fbcon && drm->fbcon->helper.fbdev) drm->fbcon->helper.fbdev->flags = drm->fbcon->saved_flags;

}

}

static void @@ -245,7 +244,8 @@ nouveau_fbcon_accel_fini(struct drm_device *dev) struct nouveau_fbdev *fbcon = drm->fbcon; if (fbcon && drm->channel) { console_lock();
fbcon->helper.fbdev->flags |= FBINFO_HWACCEL_DISABLED;
if (drm->fbcon->helper.fbdev)
	fbcon->helper.fbdev->flags |= FBINFO_HWACCEL_DISABLED;
console_unlock(); nouveau_channel_idle(drm->channel); nvif_object_fini(&fbcon->twod);
-- 2.14.1

-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

Pavel Roskin

17 Oct 17 Oct

6:06 p.m.

Ben, David,

I would hate if Linux 4.14 is released without this fix, as it would be a regression for my machine. As I mentioned, Linux 4.13 registers nouveaufb even without the dock, but Linux 4.14 doesn't. And that would cause an oops even if CONFIG_DRM_FBDEV_EMULATION is enabled.

Please let me know if any assistance is needed on my part to get the patch merged.

Pavel

Ben Skeggs

6:18 p.m.

On 10/18/2017 04:06 AM, Pavel Roskin wrote:

...

Ben, David,

I would hate if Linux 4.14 is released without this fix, as it would be a regression for my machine. As I mentioned, Linux 4.13 registers nouveaufb even without the dock, but Linux 4.14 doesn't. And that would cause an oops even if CONFIG_DRM_FBDEV_EMULATION is enabled.

Please let me know if any assistance is needed on my part to get the patch merged.

None necessary, I will pick it up.

Thank you! Ben.

...

Pavel

2752

Age (days ago)

2776

Last active (days ago)

dri-devel@lists.freedesktop.org

6 comments

3 participants

tags (0)

participants (3)

Ben Skeggs
Daniel Vetter
Pavel Roskin