Re: [PATCH] drm/fbdev: Make skip_vt_switch the default
On Wed, Nov 28, 2018 at 8:17 AM Maarten Lankhorst wrote: > > Op 27-11-18 om 18:34 schreef Daniel Vetter: > > KMS drivers really should all be able to restore their display state > > on resume without fbcon helping out. So make this the default. > > > > Since I'm not entirely foolish, make it only a default, which drivers > > can still override. That way when the inevitable regression report > > happens I can fix things up with a one-liner plus FIXME comment that > > someone should fix up the suspend/resume code in that driver. > > > > But at least all new drivers won't be broken by accident as soon as > > you turn off fbcon because "suspend/resume worked when I tested it". > > > > v2: Keep this for radeon because of > > > > commit 18c437caa5b18a235dd65cec224eab54bebcee65 > > Author: Alex Deucher > > Date: Tue Nov 14 17:19:29 2017 -0500 > > > > Revert "drm/radeon: dont switch vt on suspend" > > > > Thanks to Michel Dänzer for pointing this one out. > > Maybe just reload the gamma lut on resume for radeon, instead of relying on > fbcon? > > Otherwise patch looks sane, would be nice if radeon was fixed instead of > worked around. > > Reviewed-by: Maarten Lankhorst Noticed this old patch, finally gotten around to merging it. Thanks everyone for reviewing. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fbdev: Make skip_vt_switch the default
Reviewed-by: Samuel Li On 2018-11-28 3:20 a.m., Daniel Vetter wrote: > On Wed, Nov 28, 2018 at 08:17:04AM +0100, Maarten Lankhorst wrote: >> Op 27-11-18 om 18:34 schreef Daniel Vetter: >>> KMS drivers really should all be able to restore their display state >>> on resume without fbcon helping out. So make this the default. >>> >>> Since I'm not entirely foolish, make it only a default, which drivers >>> can still override. That way when the inevitable regression report >>> happens I can fix things up with a one-liner plus FIXME comment that >>> someone should fix up the suspend/resume code in that driver. >>> >>> But at least all new drivers won't be broken by accident as soon as >>> you turn off fbcon because "suspend/resume worked when I tested it". >>> >>> v2: Keep this for radeon because of >>> >>> commit 18c437caa5b18a235dd65cec224eab54bebcee65 >>> Author: Alex Deucher >>> Date: Tue Nov 14 17:19:29 2017 -0500 >>> >>> Revert "drm/radeon: dont switch vt on suspend" >>> >>> Thanks to Michel Dänzer for pointing this one out. >> >> Maybe just reload the gamma lut on resume for radeon, instead of relying on >> fbcon? > > It also caused random sound issues and other fun. Also, afaiui radeon does > load the lut. Except because of timing or bad luck, it doesn't work > reliably, and the vt switch doing it a 2nd time helps out. > -Daniel > >> Otherwise patch looks sane, would be nice if radeon was fixed instead of >> worked around. >> >> Reviewed-by: Maarten Lankhorst ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fbdev: Make skip_vt_switch the default
Am Dienstag, 27. November 2018, 18:34:24 CET schrieb Daniel Vetter: > KMS drivers really should all be able to restore their display state > on resume without fbcon helping out. So make this the default. > > Since I'm not entirely foolish, make it only a default, which drivers > can still override. That way when the inevitable regression report > happens I can fix things up with a one-liner plus FIXME comment that > someone should fix up the suspend/resume code in that driver. > > But at least all new drivers won't be broken by accident as soon as > you turn off fbcon because "suspend/resume worked when I tested it". > > v2: Keep this for radeon because of > > commit 18c437caa5b18a235dd65cec224eab54bebcee65 > Author: Alex Deucher > Date: Tue Nov 14 17:19:29 2017 -0500 > > Revert "drm/radeon: dont switch vt on suspend" > > Thanks to Michel Dänzer for pointing this one out. > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c index > e6650553f5d6..361604e51361 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > @@ -111,8 +111,6 @@ static int rockchip_drm_fbdev_create(struct > drm_fb_helper *helper, rk_obj->kvaddr, > offset, size); > > - fbi->skip_vt_switch = true; > - > return 0; > > out: for the Rockchip-part Acked-by: Heiko Stuebner It looks somewhat obvious for that, as the Rockchip setting was true from the beginning, but I still gave it some suspend-spins on rk3399 so as well Tested-by: Heiko Stuebner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fbdev: Make skip_vt_switch the default
On Wed, Nov 28, 2018 at 08:17:04AM +0100, Maarten Lankhorst wrote: > Op 27-11-18 om 18:34 schreef Daniel Vetter: > > KMS drivers really should all be able to restore their display state > > on resume without fbcon helping out. So make this the default. > > > > Since I'm not entirely foolish, make it only a default, which drivers > > can still override. That way when the inevitable regression report > > happens I can fix things up with a one-liner plus FIXME comment that > > someone should fix up the suspend/resume code in that driver. > > > > But at least all new drivers won't be broken by accident as soon as > > you turn off fbcon because "suspend/resume worked when I tested it". > > > > v2: Keep this for radeon because of > > > > commit 18c437caa5b18a235dd65cec224eab54bebcee65 > > Author: Alex Deucher > > Date: Tue Nov 14 17:19:29 2017 -0500 > > > > Revert "drm/radeon: dont switch vt on suspend" > > > > Thanks to Michel Dänzer for pointing this one out. > > Maybe just reload the gamma lut on resume for radeon, instead of relying on > fbcon? It also caused random sound issues and other fun. Also, afaiui radeon does load the lut. Except because of timing or bad luck, it doesn't work reliably, and the vt switch doing it a 2nd time helps out. -Daniel > Otherwise patch looks sane, would be nice if radeon was fixed instead of > worked around. > > Reviewed-by: Maarten Lankhorst -- 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/fbdev: Make skip_vt_switch the default
Op 27-11-18 om 18:34 schreef Daniel Vetter: > KMS drivers really should all be able to restore their display state > on resume without fbcon helping out. So make this the default. > > Since I'm not entirely foolish, make it only a default, which drivers > can still override. That way when the inevitable regression report > happens I can fix things up with a one-liner plus FIXME comment that > someone should fix up the suspend/resume code in that driver. > > But at least all new drivers won't be broken by accident as soon as > you turn off fbcon because "suspend/resume worked when I tested it". > > v2: Keep this for radeon because of > > commit 18c437caa5b18a235dd65cec224eab54bebcee65 > Author: Alex Deucher > Date: Tue Nov 14 17:19:29 2017 -0500 > > Revert "drm/radeon: dont switch vt on suspend" > > Thanks to Michel Dänzer for pointing this one out. Maybe just reload the gamma lut on resume for radeon, instead of relying on fbcon? Otherwise patch looks sane, would be nice if radeon was fixed instead of worked around. Reviewed-by: Maarten Lankhorst ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/fbdev: Make skip_vt_switch the default
KMS drivers really should all be able to restore their display state on resume without fbcon helping out. So make this the default. Since I'm not entirely foolish, make it only a default, which drivers can still override. That way when the inevitable regression report happens I can fix things up with a one-liner plus FIXME comment that someone should fix up the suspend/resume code in that driver. But at least all new drivers won't be broken by accident as soon as you turn off fbcon because "suspend/resume worked when I tested it". v2: Keep this for radeon because of commit 18c437caa5b18a235dd65cec224eab54bebcee65 Author: Alex Deucher Date: Tue Nov 14 17:19:29 2017 -0500 Revert "drm/radeon: dont switch vt on suspend" Thanks to Michel Dänzer for pointing this one out. Signed-off-by: Daniel Vetter Cc: Michel Dänzer Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: Ben Skeggs Cc: Sandy Huang Cc: "Heiko Stübner" Cc: Alex Deucher Cc: "Christian König" Cc: Samuel Li Cc: "Michel Dänzer" Cc: Daniel Vetter Cc: Junwei Zhang Cc: Huang Rui Cc: Shirish S Cc: Daniel Stone Cc: "Noralf Trønnes" Cc: intel-...@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-rockc...@lists.infradead.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c| 1 - drivers/gpu/drm/drm_fb_helper.c | 1 + drivers/gpu/drm/i915/intel_fbdev.c| 3 --- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 1 - drivers/gpu/drm/radeon/radeon_fb.c| 3 +++ drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 2 -- 6 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c index 5cbde74b97dd..24890d8f9ee4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c @@ -234,7 +234,6 @@ static int amdgpufb_create(struct drm_fb_helper *helper, } info->par = rfbdev; - info->skip_vt_switch = true; ret = amdgpu_display_framebuffer_init(adev->ddev, &rfbdev->rfb, &mode_cmd, gobj); diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 5e9ca6f96379..41f37704e0a3 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -934,6 +934,7 @@ struct fb_info *drm_fb_helper_alloc_fbi(struct drm_fb_helper *fb_helper) } fb_helper->fbdev = info; + info->skip_vt_switch = true; return info; diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 2480c7d6edee..d6f8d4bbc9fc 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -257,9 +257,6 @@ static int intelfb_create(struct drm_fb_helper *helper, info->screen_base = vaddr; info->screen_size = vma->node.size; - /* This driver doesn't need a VT switch to restore the mode on resume */ - info->skip_vt_switch = true; - drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth); drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height); diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 032317c81bf0..67572408d9ae 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -365,7 +365,6 @@ nouveau_fbcon_create(struct drm_fb_helper *helper, ret = PTR_ERR(info); goto out_unlock; } - info->skip_vt_switch = 1; info->par = fbcon; diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index 1179034024ae..d50bff20f7de 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -244,6 +244,9 @@ static int radeonfb_create(struct drm_fb_helper *helper, goto out; } + /* radeon resume is fragile and needs a vt switch to help it along */ + info->skip_vt_switch = false; + info->par = rfbdev; ret = radeon_framebuffer_init(rdev->ddev, &rfbdev->fb, &mode_cmd, gobj); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c index e6650553f5d6..361604e51361 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c @@ -111,8 +111,6 @@ static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper, rk_obj->kvaddr, offset, size); - fbi->skip_vt_switch = true; - return 0; out: -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fbdev: Make skip_vt_switch the default
On 2018-11-27 6:09 p.m., Daniel Vetter wrote: > KMS drivers really should all be able to restore their display state > on resume without fbcon helping out. So make this the default. > > Since I'm not entirely foolish, make it only a default, which drivers > can still override. That way when the inevitable regression report > happens I can fix things up with a one-liner plus FIXME comment that > someone should fix up the suspend/resume code in that driver. > > But at least all new drivers won't be broken by accident because > "suspend/resume worked when I tested it" as soon as you turn off > fbcon. The radeon driver probably needs to override this, per the bug reports referenced in commit 18c437caa5b1 'Revert "drm/radeon: dont switch vt on suspend"'. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/fbdev: Make skip_vt_switch the default
KMS drivers really should all be able to restore their display state on resume without fbcon helping out. So make this the default. Since I'm not entirely foolish, make it only a default, which drivers can still override. That way when the inevitable regression report happens I can fix things up with a one-liner plus FIXME comment that someone should fix up the suspend/resume code in that driver. But at least all new drivers won't be broken by accident because "suspend/resume worked when I tested it" as soon as you turn off fbcon. Signed-off-by: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: Ben Skeggs Cc: Sandy Huang Cc: "Heiko Stübner" Cc: Alex Deucher Cc: "Christian König" Cc: Samuel Li Cc: "Michel Dänzer" Cc: Daniel Vetter Cc: Junwei Zhang Cc: Huang Rui Cc: Shirish S Cc: Daniel Stone Cc: "Noralf Trønnes" Cc: intel-...@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-rockc...@lists.infradead.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c| 1 - drivers/gpu/drm/drm_fb_helper.c | 1 + drivers/gpu/drm/i915/intel_fbdev.c| 3 --- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 1 - drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 2 -- 5 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c index 5cbde74b97dd..24890d8f9ee4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c @@ -234,7 +234,6 @@ static int amdgpufb_create(struct drm_fb_helper *helper, } info->par = rfbdev; - info->skip_vt_switch = true; ret = amdgpu_display_framebuffer_init(adev->ddev, &rfbdev->rfb, &mode_cmd, gobj); diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 5e9ca6f96379..41f37704e0a3 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -934,6 +934,7 @@ struct fb_info *drm_fb_helper_alloc_fbi(struct drm_fb_helper *fb_helper) } fb_helper->fbdev = info; + info->skip_vt_switch = true; return info; diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 2480c7d6edee..d6f8d4bbc9fc 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -257,9 +257,6 @@ static int intelfb_create(struct drm_fb_helper *helper, info->screen_base = vaddr; info->screen_size = vma->node.size; - /* This driver doesn't need a VT switch to restore the mode on resume */ - info->skip_vt_switch = true; - drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth); drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height); diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 032317c81bf0..67572408d9ae 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -365,7 +365,6 @@ nouveau_fbcon_create(struct drm_fb_helper *helper, ret = PTR_ERR(info); goto out_unlock; } - info->skip_vt_switch = 1; info->par = fbcon; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c index e6650553f5d6..361604e51361 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c @@ -111,8 +111,6 @@ static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper, rk_obj->kvaddr, offset, size); - fbi->skip_vt_switch = true; - return 0; out: -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel