Re: [Intel-gfx] [PATCH 00/16] fix VLV S4 suspend/resume, unify S3/S4 handlers
On Wed, Sep 10, 2014 at 06:16:53PM +0300, Imre Deak wrote: The first part of the patchset (1-6) fixes an S4 bug on VLV introduced recently. The rest unifies the various S3/S4 handlers, which are in practice the same. The only real difference - besides some unused code - is that during S3 suspend we disable the PCI device whereas across an S4 freeze/thaw we keep it enabled. Given that we disable everything else anyway, we can just as well disable the PCI device there too. Doing so allows us to handle S3 suspend/resume and S4 freeze/thaw/restore/ power-off the same way, simplifying/clarifying things quite a bit. I smoke tested this on BDW, HSW, VLV (BYT-M), IVB, GEN45. Imre Deak (16): drm/i915: vlv: fix gunit HW state corruption during S4 suspend drm/i915: remove dead code from legacy suspend handler drm/i915: factor out i915_drm_suspend_late drm/i915: unify legacy S3 suspend and S4 freeze handlers drm/i915: propagate error from legacy resume handler drm/i915: vlv: fix switcheroo/legacy suspend/resume drm/i915: fix S4 suspend while switcheroo state is off drm/i915: remove unused restore_gtt_mappings optimization during suspend drm/i915: check for GT faults during S3 resume and S4 restore too drm/i915: enable output polling during S4 thaw drm/i915: disable/re-enable PCI device around S4 freeze/thaw drm/i915: unify S3 and S4 suspend/resume handlers drm/i915: sanitize suspend/resume helper function names drm/i915: add poweroff_late handler drm/i915: unify switcheroo and legacy suspend/resume handlers drm/i915: add comments on what stage a given PM handler is called It all looks pretty decent to me. You can slap Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com onto everything except 09/16. I was a bit lazy and didn't look at that one since Chris has already checked it. drivers/gpu/drm/i915/i915_dma.c | 4 +- drivers/gpu/drm/i915/i915_drv.c | 185 +++- drivers/gpu/drm/i915/i915_drv.h | 4 +- 3 files changed, 74 insertions(+), 119 deletions(-) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/16] fix VLV S4 suspend/resume, unify S3/S4 handlers
On Wed, Sep 10, 2014 at 09:38:50PM +0300, Imre Deak wrote: On Wed, 2014-09-10 at 17:52 +0200, Daniel Vetter wrote: On Wed, Sep 10, 2014 at 06:16:53PM +0300, Imre Deak wrote: The first part of the patchset (1-6) fixes an S4 bug on VLV introduced recently. The rest unifies the various S3/S4 handlers, which are in practice the same. The only real difference - besides some unused code - is that during S3 suspend we disable the PCI device whereas across an S4 freeze/thaw we keep it enabled. Given that we disable everything else anyway, we can just as well disable the PCI device there too. Doing so allows us to handle S3 suspend/resume and S4 freeze/thaw/restore/ power-off the same way, simplifying/clarifying things quite a bit. Hm, this might explain why we've seen random corruption on S4 on recent platforms. https://bugzilla.kernel.org/show_bug.cgi?id=59321 Can you please ask for retesting from reporters? Ok, can do, I also forgot to add https://bugs.freedesktop.org/show_bug.cgi?id=82842 which it fixes. I can't see immediately how platforms other than VLV would be fixed with these, but maybe I missed something. drm/i915: disable/re-enable PCI device around S4 freeze/thaw looks rather generic and not vlv specific, and could very well fix the kernel bz I've pasted. Or am I horribly blind? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/16] fix VLV S4 suspend/resume, unify S3/S4 handlers
On Thu, 2014-09-11 at 11:02 +0200, Daniel Vetter wrote: On Wed, Sep 10, 2014 at 09:38:50PM +0300, Imre Deak wrote: On Wed, 2014-09-10 at 17:52 +0200, Daniel Vetter wrote: On Wed, Sep 10, 2014 at 06:16:53PM +0300, Imre Deak wrote: The first part of the patchset (1-6) fixes an S4 bug on VLV introduced recently. The rest unifies the various S3/S4 handlers, which are in practice the same. The only real difference - besides some unused code - is that during S3 suspend we disable the PCI device whereas across an S4 freeze/thaw we keep it enabled. Given that we disable everything else anyway, we can just as well disable the PCI device there too. Doing so allows us to handle S3 suspend/resume and S4 freeze/thaw/restore/ power-off the same way, simplifying/clarifying things quite a bit. Hm, this might explain why we've seen random corruption on S4 on recent platforms. https://bugzilla.kernel.org/show_bug.cgi?id=59321 Can you please ask for retesting from reporters? Ok, can do, I also forgot to add https://bugs.freedesktop.org/show_bug.cgi?id=82842 which it fixes. I can't see immediately how platforms other than VLV would be fixed with these, but maybe I missed something. drm/i915: disable/re-enable PCI device around S4 freeze/thaw looks rather generic and not vlv specific, and could very well fix the kernel bz I've pasted. Or am I horribly blind? Yea it's generic, so possibly fixes something. Although by the time we disable the PCI device in freeze everything should be idle, so if simply disabling/re-enabling makes a difference then we failed to idle something. Or we depend on a HW reset (implicit in the disable/re-enable) before reinitializing things in thaw. Anyway we can clarify this more once we get feedback from the retesting. --Imre signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/16] fix VLV S4 suspend/resume, unify S3/S4 handlers
The first part of the patchset (1-6) fixes an S4 bug on VLV introduced recently. The rest unifies the various S3/S4 handlers, which are in practice the same. The only real difference - besides some unused code - is that during S3 suspend we disable the PCI device whereas across an S4 freeze/thaw we keep it enabled. Given that we disable everything else anyway, we can just as well disable the PCI device there too. Doing so allows us to handle S3 suspend/resume and S4 freeze/thaw/restore/ power-off the same way, simplifying/clarifying things quite a bit. I smoke tested this on BDW, HSW, VLV (BYT-M), IVB, GEN45. Imre Deak (16): drm/i915: vlv: fix gunit HW state corruption during S4 suspend drm/i915: remove dead code from legacy suspend handler drm/i915: factor out i915_drm_suspend_late drm/i915: unify legacy S3 suspend and S4 freeze handlers drm/i915: propagate error from legacy resume handler drm/i915: vlv: fix switcheroo/legacy suspend/resume drm/i915: fix S4 suspend while switcheroo state is off drm/i915: remove unused restore_gtt_mappings optimization during suspend drm/i915: check for GT faults during S3 resume and S4 restore too drm/i915: enable output polling during S4 thaw drm/i915: disable/re-enable PCI device around S4 freeze/thaw drm/i915: unify S3 and S4 suspend/resume handlers drm/i915: sanitize suspend/resume helper function names drm/i915: add poweroff_late handler drm/i915: unify switcheroo and legacy suspend/resume handlers drm/i915: add comments on what stage a given PM handler is called drivers/gpu/drm/i915/i915_dma.c | 4 +- drivers/gpu/drm/i915/i915_drv.c | 185 +++- drivers/gpu/drm/i915/i915_drv.h | 4 +- 3 files changed, 74 insertions(+), 119 deletions(-) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/16] fix VLV S4 suspend/resume, unify S3/S4 handlers
On Wed, Sep 10, 2014 at 06:16:53PM +0300, Imre Deak wrote: The first part of the patchset (1-6) fixes an S4 bug on VLV introduced recently. The rest unifies the various S3/S4 handlers, which are in practice the same. The only real difference - besides some unused code - is that during S3 suspend we disable the PCI device whereas across an S4 freeze/thaw we keep it enabled. Given that we disable everything else anyway, we can just as well disable the PCI device there too. Doing so allows us to handle S3 suspend/resume and S4 freeze/thaw/restore/ power-off the same way, simplifying/clarifying things quite a bit. Hm, this might explain why we've seen random corruption on S4 on recent platforms. https://bugzilla.kernel.org/show_bug.cgi?id=59321 Can you please ask for retesting from reporters? Thanks, Daniel I smoke tested this on BDW, HSW, VLV (BYT-M), IVB, GEN45. Imre Deak (16): drm/i915: vlv: fix gunit HW state corruption during S4 suspend drm/i915: remove dead code from legacy suspend handler drm/i915: factor out i915_drm_suspend_late drm/i915: unify legacy S3 suspend and S4 freeze handlers drm/i915: propagate error from legacy resume handler drm/i915: vlv: fix switcheroo/legacy suspend/resume drm/i915: fix S4 suspend while switcheroo state is off drm/i915: remove unused restore_gtt_mappings optimization during suspend drm/i915: check for GT faults during S3 resume and S4 restore too drm/i915: enable output polling during S4 thaw drm/i915: disable/re-enable PCI device around S4 freeze/thaw drm/i915: unify S3 and S4 suspend/resume handlers drm/i915: sanitize suspend/resume helper function names drm/i915: add poweroff_late handler drm/i915: unify switcheroo and legacy suspend/resume handlers drm/i915: add comments on what stage a given PM handler is called drivers/gpu/drm/i915/i915_dma.c | 4 +- drivers/gpu/drm/i915/i915_drv.c | 185 +++- drivers/gpu/drm/i915/i915_drv.h | 4 +- 3 files changed, 74 insertions(+), 119 deletions(-) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/16] fix VLV S4 suspend/resume, unify S3/S4 handlers
On Wed, 2014-09-10 at 17:52 +0200, Daniel Vetter wrote: On Wed, Sep 10, 2014 at 06:16:53PM +0300, Imre Deak wrote: The first part of the patchset (1-6) fixes an S4 bug on VLV introduced recently. The rest unifies the various S3/S4 handlers, which are in practice the same. The only real difference - besides some unused code - is that during S3 suspend we disable the PCI device whereas across an S4 freeze/thaw we keep it enabled. Given that we disable everything else anyway, we can just as well disable the PCI device there too. Doing so allows us to handle S3 suspend/resume and S4 freeze/thaw/restore/ power-off the same way, simplifying/clarifying things quite a bit. Hm, this might explain why we've seen random corruption on S4 on recent platforms. https://bugzilla.kernel.org/show_bug.cgi?id=59321 Can you please ask for retesting from reporters? Ok, can do, I also forgot to add https://bugs.freedesktop.org/show_bug.cgi?id=82842 which it fixes. I can't see immediately how platforms other than VLV would be fixed with these, but maybe I missed something. --Imre Thanks, Daniel I smoke tested this on BDW, HSW, VLV (BYT-M), IVB, GEN45. Imre Deak (16): drm/i915: vlv: fix gunit HW state corruption during S4 suspend drm/i915: remove dead code from legacy suspend handler drm/i915: factor out i915_drm_suspend_late drm/i915: unify legacy S3 suspend and S4 freeze handlers drm/i915: propagate error from legacy resume handler drm/i915: vlv: fix switcheroo/legacy suspend/resume drm/i915: fix S4 suspend while switcheroo state is off drm/i915: remove unused restore_gtt_mappings optimization during suspend drm/i915: check for GT faults during S3 resume and S4 restore too drm/i915: enable output polling during S4 thaw drm/i915: disable/re-enable PCI device around S4 freeze/thaw drm/i915: unify S3 and S4 suspend/resume handlers drm/i915: sanitize suspend/resume helper function names drm/i915: add poweroff_late handler drm/i915: unify switcheroo and legacy suspend/resume handlers drm/i915: add comments on what stage a given PM handler is called drivers/gpu/drm/i915/i915_dma.c | 4 +- drivers/gpu/drm/i915/i915_drv.c | 185 +++- drivers/gpu/drm/i915/i915_drv.h | 4 +- 3 files changed, 74 insertions(+), 119 deletions(-) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
