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

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

[Pavel Roskin]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Pavel Roskin]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Pavel Roskin]

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(-)

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

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

[Daniel Vetter]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Pavel Roskin]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ben Skeggs]

[-- Attachment #1.1.1: Type: text/plain, Size: 517 bytes --]

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
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel