Re: [Intel-gfx] [PATCH] drm/i915: Restore all GGTT VMAs on resume
On Wed, Jul 08, 2015 at 10:45:23AM +0100, Tvrtko Ursulin wrote: On 07/08/2015 10:32 AM, Daniel Vetter wrote: On Mon, Jul 06, 2015 at 04:19:01PM +0100, Chris Wilson wrote: On Mon, Jul 06, 2015 at 03:15:01PM +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com When rotated and partial views were added no one spotted the resume path which assumes only one GGTT VMA per object and hence is now skipping rebind of alternative views. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com --- Similarly to my recent debugfs patch, it would seem quicker path could be to walk GGTT active inactive lists, but I assume we want to call i915_gem_clflush_object only once per object which would make that problematic. Patch is missing offending commit that introduce this issue and which platforms are affected (I guess all due to partial view?). Also should be cc: stable I presume? Can you please supply this so I can ammend the commit message? Applied to -fixes meanwhile, thanks. Ha, offending commits. In the sense of that the bug only manifests when they are present they are either of these two: commit c5ad54cf7dd8922bd1cee2d5871aebf73dc9638e Author: Joonas Lahtinen joonas.lahti...@linux.intel.com Date: Wed May 6 14:36:09 2015 +0300 drm/i915: Use partial view in mmap fault handler commit 3b7a5119b5d2def1161226a4c6a643db537dff7e Author: Sonika Jindal sonika.jin...@intel.com Date: Fri Apr 10 14:37:29 2015 +0530 drm/i915/skl: Support for 90/270 rotation But they are just the tips of two longish streams of developments so they should not necessarily be viewed as offending towards the respective authors. They are just final feature enablements. All platforms are affected due to partial views I suppose, although I am not sure if they are actually used/triggered in the field. That's why I was not sure should stable be copied. Well commits are only in 4.2, so -fixes is ok. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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] drm/i915: Restore all GGTT VMAs on resume
On 07/08/2015 10:32 AM, Daniel Vetter wrote: On Mon, Jul 06, 2015 at 04:19:01PM +0100, Chris Wilson wrote: On Mon, Jul 06, 2015 at 03:15:01PM +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com When rotated and partial views were added no one spotted the resume path which assumes only one GGTT VMA per object and hence is now skipping rebind of alternative views. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com --- Similarly to my recent debugfs patch, it would seem quicker path could be to walk GGTT active inactive lists, but I assume we want to call i915_gem_clflush_object only once per object which would make that problematic. Patch is missing offending commit that introduce this issue and which platforms are affected (I guess all due to partial view?). Also should be cc: stable I presume? Can you please supply this so I can ammend the commit message? Applied to -fixes meanwhile, thanks. Ha, offending commits. In the sense of that the bug only manifests when they are present they are either of these two: commit c5ad54cf7dd8922bd1cee2d5871aebf73dc9638e Author: Joonas Lahtinen joonas.lahti...@linux.intel.com Date: Wed May 6 14:36:09 2015 +0300 drm/i915: Use partial view in mmap fault handler commit 3b7a5119b5d2def1161226a4c6a643db537dff7e Author: Sonika Jindal sonika.jin...@intel.com Date: Fri Apr 10 14:37:29 2015 +0530 drm/i915/skl: Support for 90/270 rotation But they are just the tips of two longish streams of developments so they should not necessarily be viewed as offending towards the respective authors. They are just final feature enablements. All platforms are affected due to partial views I suppose, although I am not sure if they are actually used/triggered in the field. That's why I was not sure should stable be copied. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Restore all GGTT VMAs on resume
On Mon, Jul 06, 2015 at 04:19:01PM +0100, Chris Wilson wrote: On Mon, Jul 06, 2015 at 03:15:01PM +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com When rotated and partial views were added no one spotted the resume path which assumes only one GGTT VMA per object and hence is now skipping rebind of alternative views. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com --- Similarly to my recent debugfs patch, it would seem quicker path could be to walk GGTT active inactive lists, but I assume we want to call i915_gem_clflush_object only once per object which would make that problematic. Patch is missing offending commit that introduce this issue and which platforms are affected (I guess all due to partial view?). Also should be cc: stable I presume? Can you please supply this so I can ammend the commit message? Applied to -fixes meanwhile, thanks. -Daniel It's a bit easier because there are no active entries after suspend, and we may not care too much for over clflushing. We should actually clear the GGTT of all but pinned entries on hibernate (which would save a bunch of work on early resume), and there are a bunch of non-objects inside the GGTT that should be evicted/restored properly. (Would have been trivial had we made them all objects in the first place.) The patch is in the right direction and the other issues are present irrespective of this patch. I guess a begrudingly Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Daniel Vetter Software Engineer, Intel Corporation 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] drm/i915: Restore all GGTT VMAs on resume
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6731 -Summary- Platform Delta drm-intel-nightly Series Applied ILK 302/302 302/302 SNB 312/316 312/316 IVB 345/345 345/345 BYT -2 289/289 287/289 HSW 382/382 382/382 -Detailed- Platform Testdrm-intel-nightly Series Applied *BYT igt@gem_partial_pwrite_pread@reads PASS(1) FAIL(1) *BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Restore all GGTT VMAs on resume
On 07/06/2015 04:19 PM, Chris Wilson wrote: On Mon, Jul 06, 2015 at 03:15:01PM +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com When rotated and partial views were added no one spotted the resume path which assumes only one GGTT VMA per object and hence is now skipping rebind of alternative views. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com --- Similarly to my recent debugfs patch, it would seem quicker path could be to walk GGTT active inactive lists, but I assume we want to call i915_gem_clflush_object only once per object which would make that problematic. It's a bit easier because there are no active entries after suspend, and we may not care too much for over clflushing. We should actually clear the GGTT of all but pinned entries on hibernate (which would save a bunch of work on early resume), and there are a bunch of non-objects inside the GGTT that should be evicted/restored properly. (Would have been trivial had we made them all objects in the first place.) What are these non-objects and where do I find the code with restores them improperly? The patch is in the right direction and the other issues are present irrespective of this patch. I guess a begrudingly Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Thanks! Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Restore all GGTT VMAs on resume
On Mon, Jul 06, 2015 at 03:15:01PM +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com When rotated and partial views were added no one spotted the resume path which assumes only one GGTT VMA per object and hence is now skipping rebind of alternative views. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com --- Similarly to my recent debugfs patch, it would seem quicker path could be to walk GGTT active inactive lists, but I assume we want to call i915_gem_clflush_object only once per object which would make that problematic. It's a bit easier because there are no active entries after suspend, and we may not care too much for over clflushing. We should actually clear the GGTT of all but pinned entries on hibernate (which would save a bunch of work on early resume), and there are a bunch of non-objects inside the GGTT that should be evicted/restored properly. (Would have been trivial had we made them all objects in the first place.) The patch is in the right direction and the other issues are present irrespective of this patch. I guess a begrudingly Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Restore all GGTT VMAs on resume
On Mon, Jul 06, 2015 at 04:29:12PM +0100, Tvrtko Ursulin wrote: On 07/06/2015 04:19 PM, Chris Wilson wrote: On Mon, Jul 06, 2015 at 03:15:01PM +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com When rotated and partial views were added no one spotted the resume path which assumes only one GGTT VMA per object and hence is now skipping rebind of alternative views. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com --- Similarly to my recent debugfs patch, it would seem quicker path could be to walk GGTT active inactive lists, but I assume we want to call i915_gem_clflush_object only once per object which would make that problematic. It's a bit easier because there are no active entries after suspend, and we may not care too much for over clflushing. We should actually clear the GGTT of all but pinned entries on hibernate (which would save a bunch of work on early resume), and there are a bunch of non-objects inside the GGTT that should be evicted/restored properly. (Would have been trivial had we made them all objects in the first place.) What are these non-objects and where do I find the code with restores them improperly? The PDEs are allocated directly in the GGTT and ignored completely across hibernation. All we ideally need to do is evict them on freeze. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
