wsdisplay(4) burner bugs

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

wsdisplay(4) burner bugs

Artturi Alm
Hi,


was going after another bug when i found these, the "on" is kind of wrongly
named, i guess? (fwiw., i only compared against dev/ic/vga.c)

the meaningful variable from sys/dev/wsconsc/wsdisplay.c:
..
179         int     sc_burnman;             /* nonzero if screen blanked */
..
and how it's used in wsdisplay_burner():
..
2357                 (*sc->sc_accessops->burn_screen)(sc->sc_accesscookie,
2358                     sc->sc_burnman, sc->sc_burnflags);
...


minimal diff below for what i think is right, even if drm_fb_helper.c
somehow might possibly achieve the same behaviour(?) by some other means.

idk., i'm still just guessing here, the whole burner(/wsdisplay.c as is)
seems a bit overdesigned to me:]

-Artturi


diff --git a/sys/dev/pci/drm/amd/amdgpu/amdgpu_fb.c b/sys/dev/pci/drm/amd/amdgpu/amdgpu_fb.c
index 9d1c158f17e..736aeb71db1 100644
--- a/sys/dev/pci/drm/amd/amdgpu/amdgpu_fb.c
+++ b/sys/dev/pci/drm/amd/amdgpu/amdgpu_fb.c
@@ -446,7 +446,7 @@ amdgpu_burner(void *v, u_int on, u_int flags)
 
  task_del(systq, &adev->burner_task);
 
- if (on)
+ if (!on)
  adev->burner_fblank = FB_BLANK_UNBLANK;
  else {
  if (flags & WSDISPLAY_BURN_VBLANK)
diff --git a/sys/dev/pci/drm/radeon/radeon_fb.c b/sys/dev/pci/drm/radeon/radeon_fb.c
index d43d99ad8b3..9049daff4d1 100644
--- a/sys/dev/pci/drm/radeon/radeon_fb.c
+++ b/sys/dev/pci/drm/radeon/radeon_fb.c
@@ -495,7 +495,7 @@ radeondrm_burner(void *v, u_int on, u_int flags)
 
  task_del(systq, &rdev->burner_task);
 
- if (on)
+ if (!on)
  rdev->burner_fblank = FB_BLANK_UNBLANK;
  else {
  if (flags & WSDISPLAY_BURN_VBLANK)

Reply | Threaded
Open this post in threaded view
|

Re: wsdisplay(4) burner bugs

Mark Kettenis
> Date: Sun, 16 Feb 2020 11:29:03 +0200
> From: Artturi Alm <[hidden email]>
>
> Hi,
>
> was going after another bug when i found these, the "on" is kind of wrongly
> named, i guess? (fwiw., i only compared against dev/ic/vga.c)
>
> the meaningful variable from sys/dev/wsconsc/wsdisplay.c:
> ..
> 179         int     sc_burnman;             /* nonzero if screen blanked */
> ..
> and how it's used in wsdisplay_burner():
> ..
> 2357                 (*sc->sc_accessops->burn_screen)(sc->sc_accesscookie,
> 2358                     sc->sc_burnman, sc->sc_burnflags);
> ...
>
>
> minimal diff below for what i think is right, even if drm_fb_helper.c
> somehow might possibly achieve the same behaviour(?) by some other means.
>
> idk., i'm still just guessing here, the whole burner(/wsdisplay.c as is)
> seems a bit overdesigned to me:]

Maybe, but the code works as intended.  Maybe the documentation is
just wrong?

> diff --git a/sys/dev/pci/drm/amd/amdgpu/amdgpu_fb.c b/sys/dev/pci/drm/amd/amdgpu/amdgpu_fb.c
> index 9d1c158f17e..736aeb71db1 100644
> --- a/sys/dev/pci/drm/amd/amdgpu/amdgpu_fb.c
> +++ b/sys/dev/pci/drm/amd/amdgpu/amdgpu_fb.c
> @@ -446,7 +446,7 @@ amdgpu_burner(void *v, u_int on, u_int flags)
>  
>   task_del(systq, &adev->burner_task);
>  
> - if (on)
> + if (!on)
>   adev->burner_fblank = FB_BLANK_UNBLANK;
>   else {
>   if (flags & WSDISPLAY_BURN_VBLANK)
> diff --git a/sys/dev/pci/drm/radeon/radeon_fb.c b/sys/dev/pci/drm/radeon/radeon_fb.c
> index d43d99ad8b3..9049daff4d1 100644
> --- a/sys/dev/pci/drm/radeon/radeon_fb.c
> +++ b/sys/dev/pci/drm/radeon/radeon_fb.c
> @@ -495,7 +495,7 @@ radeondrm_burner(void *v, u_int on, u_int flags)
>  
>   task_del(systq, &rdev->burner_task);
>  
> - if (on)
> + if (!on)
>   rdev->burner_fblank = FB_BLANK_UNBLANK;
>   else {
>   if (flags & WSDISPLAY_BURN_VBLANK)
>
>