Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Peter Zijlstra]

On Fri, Jul 14, 2017 at 11:20:01AM -0400, Ilia Mirkin wrote:
> On Fri, Jul 14, 2017 at 11:19 AM, Tobias Klausmann
>  wrote:
> > The conversion is a nice catch, but i'd like to have a bit more context, see
> > below!
> >
> > With a better description:
> >
> > Tobias Klausmann 
> 
> I don't think it was meant as a serious patch. WARN_ON_ONCE should
> work. The fix isn't to remove all instances of WARN_ON_ONCE. The fix
> is to fix WARN_ON_ONCE.

Quite so. Clearly I buggered it for modules; that really wasn't the
plan.

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Peter Zijlstra]

On Fri, Jul 14, 2017 at 11:20:01AM -0400, Ilia Mirkin wrote:
> On Fri, Jul 14, 2017 at 11:19 AM, Tobias Klausmann
>  wrote:
> > The conversion is a nice catch, but i'd like to have a bit more context, see
> > below!
> >
> > With a better description:
> >
> > Tobias Klausmann 
> 
> I don't think it was meant as a serious patch. WARN_ON_ONCE should
> work. The fix isn't to remove all instances of WARN_ON_ONCE. The fix
> is to fix WARN_ON_ONCE.

Quite so. Clearly I buggered it for modules; that really wasn't the
plan.

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Ilia Mirkin]

On Fri, Jul 14, 2017 at 11:19 AM, Tobias Klausmann
 wrote:
> The conversion is a nice catch, but i'd like to have a bit more context, see
> below!
>
> With a better description:
>
> Tobias Klausmann 

I don't think it was meant as a serious patch. WARN_ON_ONCE should
work. The fix isn't to remove all instances of WARN_ON_ONCE. The fix
is to fix WARN_ON_ONCE.

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Ilia Mirkin]

On Fri, Jul 14, 2017 at 11:19 AM, Tobias Klausmann
 wrote:
> The conversion is a nice catch, but i'd like to have a bit more context, see
> below!
>
> With a better description:
>
> Tobias Klausmann 

I don't think it was meant as a serious patch. WARN_ON_ONCE should
work. The fix isn't to remove all instances of WARN_ON_ONCE. The fix
is to fix WARN_ON_ONCE.

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Tobias Klausmann]

The conversion is a nice catch, but i'd like to have a bit more context, 
see below!


With a better description:

Tobias Klausmann 


On 7/14/17 5:10 PM, Karol Herbst wrote:

Yeah, we shouldn't let the machine die. Are there more WARN_ON_ONCE
usage we could convert to WARN_ONCE?

Reviewed-By: Karol Herbst 

On Fri, Jul 14, 2017 at 5:05 PM, Tobias Klausmann
 wrote:

On 7/14/17 3:41 PM, Mike Galbraith wrote:

On Fri, 2017-07-14 at 15:36 +0200, Mike Galbraith wrote:

   All DRM did was to slip a
WARN_ON_ONCE() that nouveau triggers into a kernel module where such
things no longer warn, they blow the box out of the water.

BTW, turn that irksome WARN_ON_ONCE() in drivers/gpu/drm/drm_vblank.c
into a WARN_ONCE(), and all is peachy, you get the warning, box lives.

---
   drivers/gpu/drm/drm_vblank.c |3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -605,7 +605,8 @@ bool drm_calc_vbltimestamp_from_scanoutp
  */
 if (mode->crtc_clock == 0) {
 DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n",
pipe);
-   WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev));
+   WARN_ONCE(drm_drv_uses_atomic_modeset(dev), "%s: report
me.\n",


"report me" seems a bit odd, maybe just uninitialized mode?



+ dev->driver->name);
 return false;
 }



Hey,

confirmed this helps saving the box, but we still have to find the root
cause! Backtrace with the above fix applied (and the one which came in with
the latest drm-fixes merge)!


[1] https://hastebin.com/uyoqifijed.http

Thanks,

Tobias
Reviewed-By: Karol Herbst 
___
Nouveau mailing list
nouv...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Tobias Klausmann]

The conversion is a nice catch, but i'd like to have a bit more context, 
see below!


With a better description:

Tobias Klausmann 


On 7/14/17 5:10 PM, Karol Herbst wrote:

Yeah, we shouldn't let the machine die. Are there more WARN_ON_ONCE
usage we could convert to WARN_ONCE?

Reviewed-By: Karol Herbst 

On Fri, Jul 14, 2017 at 5:05 PM, Tobias Klausmann
 wrote:

On 7/14/17 3:41 PM, Mike Galbraith wrote:

On Fri, 2017-07-14 at 15:36 +0200, Mike Galbraith wrote:

   All DRM did was to slip a
WARN_ON_ONCE() that nouveau triggers into a kernel module where such
things no longer warn, they blow the box out of the water.

BTW, turn that irksome WARN_ON_ONCE() in drivers/gpu/drm/drm_vblank.c
into a WARN_ONCE(), and all is peachy, you get the warning, box lives.

---
   drivers/gpu/drm/drm_vblank.c |3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -605,7 +605,8 @@ bool drm_calc_vbltimestamp_from_scanoutp
  */
 if (mode->crtc_clock == 0) {
 DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n",
pipe);
-   WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev));
+   WARN_ONCE(drm_drv_uses_atomic_modeset(dev), "%s: report
me.\n",


"report me" seems a bit odd, maybe just uninitialized mode?



+ dev->driver->name);
 return false;
 }



Hey,

confirmed this helps saving the box, but we still have to find the root
cause! Backtrace with the above fix applied (and the one which came in with
the latest drm-fixes merge)!


[1] https://hastebin.com/uyoqifijed.http

Thanks,

Tobias
Reviewed-By: Karol Herbst 
___
Nouveau mailing list
nouv...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Ilia Mirkin]

On Fri, Jul 14, 2017 at 11:15 AM, Mike Galbraith  wrote:
> On Fri, 2017-07-14 at 17:10 +0200, Karol Herbst wrote:
>> Yeah, we shouldn't let the machine die. Are there more WARN_ON_ONCE
>> usage we could convert to WARN_ONCE?
>
> Shooting the messenger is generally considered uncool :)

That's never stopped it from being a popular practice...

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Ilia Mirkin]

On Fri, Jul 14, 2017 at 11:15 AM, Mike Galbraith  wrote:
> On Fri, 2017-07-14 at 17:10 +0200, Karol Herbst wrote:
>> Yeah, we shouldn't let the machine die. Are there more WARN_ON_ONCE
>> usage we could convert to WARN_ONCE?
>
> Shooting the messenger is generally considered uncool :)

That's never stopped it from being a popular practice...

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Mike Galbraith]

On Fri, 2017-07-14 at 17:10 +0200, Karol Herbst wrote:
> Yeah, we shouldn't let the machine die. Are there more WARN_ON_ONCE
> usage we could convert to WARN_ONCE?

Shooting the messenger is generally considered uncool :)

-Mike

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Mike Galbraith]

On Fri, 2017-07-14 at 17:10 +0200, Karol Herbst wrote:
> Yeah, we shouldn't let the machine die. Are there more WARN_ON_ONCE
> usage we could convert to WARN_ONCE?

Shooting the messenger is generally considered uncool :)

-Mike

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Karol Herbst]

Yeah, we shouldn't let the machine die. Are there more WARN_ON_ONCE
usage we could convert to WARN_ONCE?

Reviewed-By: Karol Herbst 

On Fri, Jul 14, 2017 at 5:05 PM, Tobias Klausmann
 wrote:
> On 7/14/17 3:41 PM, Mike Galbraith wrote:
>>
>> On Fri, 2017-07-14 at 15:36 +0200, Mike Galbraith wrote:
>>>
>>>   All DRM did was to slip a
>>> WARN_ON_ONCE() that nouveau triggers into a kernel module where such
>>> things no longer warn, they blow the box out of the water.
>>
>> BTW, turn that irksome WARN_ON_ONCE() in drivers/gpu/drm/drm_vblank.c
>> into a WARN_ONCE(), and all is peachy, you get the warning, box lives.
>>
>> ---
>>   drivers/gpu/drm/drm_vblank.c |3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -605,7 +605,8 @@ bool drm_calc_vbltimestamp_from_scanoutp
>>  */
>> if (mode->crtc_clock == 0) {
>> DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n",
>> pipe);
>> -   WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev));
>> +   WARN_ONCE(drm_drv_uses_atomic_modeset(dev), "%s: report
>> me.\n",
>> + dev->driver->name);
>> return false;
>> }
>
>
>
> Hey,
>
> confirmed this helps saving the box, but we still have to find the root
> cause! Backtrace with the above fix applied (and the one which came in with
> the latest drm-fixes merge)!
>
>
> [1] https://hastebin.com/uyoqifijed.http
>
> Thanks,
>
> Tobias
>Reviewed-By: Karol Herbst 
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [regression drm/noveau] suspend to ram -> BOOM: exception RIP: drm_calc_vbltimestamp_from_scanoutpos+335

[Karol Herbst]

Yeah, we shouldn't let the machine die. Are there more WARN_ON_ONCE
usage we could convert to WARN_ONCE?

Reviewed-By: Karol Herbst 

On Fri, Jul 14, 2017 at 5:05 PM, Tobias Klausmann
 wrote:
> On 7/14/17 3:41 PM, Mike Galbraith wrote:
>>
>> On Fri, 2017-07-14 at 15:36 +0200, Mike Galbraith wrote:
>>>
>>>   All DRM did was to slip a
>>> WARN_ON_ONCE() that nouveau triggers into a kernel module where such
>>> things no longer warn, they blow the box out of the water.
>>
>> BTW, turn that irksome WARN_ON_ONCE() in drivers/gpu/drm/drm_vblank.c
>> into a WARN_ONCE(), and all is peachy, you get the warning, box lives.
>>
>> ---
>>   drivers/gpu/drm/drm_vblank.c |3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -605,7 +605,8 @@ bool drm_calc_vbltimestamp_from_scanoutp
>>  */
>> if (mode->crtc_clock == 0) {
>> DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n",
>> pipe);
>> -   WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev));
>> +   WARN_ONCE(drm_drv_uses_atomic_modeset(dev), "%s: report
>> me.\n",
>> + dev->driver->name);
>> return false;
>> }
>
>
>
> Hey,
>
> confirmed this helps saving the box, but we still have to find the root
> cause! Backtrace with the above fix applied (and the one which came in with
> the latest drm-fixes merge)!
>
>
> [1] https://hastebin.com/uyoqifijed.http
>
> Thanks,
>
> Tobias
>Reviewed-By: Karol Herbst 
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau