* BUG: 4.10 i915 drm display noise regression - bisected to a6a7cc4b7
@ 2017-01-09 6:32 lkml
2017-01-09 10:24 ` Chris Wilson
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: lkml @ 2017-01-09 6:32 UTC (permalink / raw)
To: linux-kernel; +Cc: dri-devel, chris
Hello all,
I'm experiencing display noise in the form of 8x1 pixel bars spuriously
appearing in random locations. This doesn't happen on 4.9, the machine
is an X61s, a Core2Duo 1.8Ghz w/XGA via LVDS.
I was able to bisect the issue to a6a7cc4b7:
commit a6a7cc4b7db6deaeca11cdd38844ea147a354c7a
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Fri Nov 18 21:17:46 2016 +0000
drm/i915: Always flush the dirty CPU cache when pinning the scanout
Currently we only clflush the scanout if it is in the CPU domain. Also
flush if we have a pending CPU clflush. We also want to treat the
dirtyfb path similar, and flush any pending writes there as well.
v2: Only send the fb flush message if flushing the dirt on flip
v3: Make flush-for-flip and dirtyfb look more alike since they serve
similar roles as end-of-frame marker.
Reproduction is simple, just run this native drm eye candy program:
https://github.com/vcaputo/rototiller
Thanks,
Vito Caputo
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH] drm/i915: Flush untouched framebuffers before display on !llc 2017-01-09 6:32 BUG: 4.10 i915 drm display noise regression - bisected to a6a7cc4b7 lkml @ 2017-01-09 10:24 ` Chris Wilson 2017-01-09 11:19 ` Chris Wilson ` (2 subsequent siblings) 3 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-01-09 10:24 UTC (permalink / raw) To: linux-kernel; +Cc: intel-gfx, lkml, # v4 . 10-rc1+ On a non-llc system, the objects are created with .cache_level = CACHE_NONE and so the transition to uncached for scanout is a no-op. However, if the object was never written to, it will still be in the CPU domain (having been zeroed out by shmemfs). Those cachelines need to be flushed prior to display. Reported-by: Vito Caputo Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ --- drivers/gpu/drm/i915/i915_gem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 76689b59fc90..e64d0ea6113d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2327,6 +2327,7 @@ static void i915_sg_trim(struct sg_table *orig_st) if (sg_alloc_table(&new_st, orig_st->nents, GFP_KERNEL | __GFP_NOWARN)) return; + new_st->orig_nents = orig_st->orig_nents; /* XXX lies for new_sg = new_st.sgl; for_each_sg(orig_st->sgl, sg, orig_st->nents, i) { sg_set_page(new_sg, sg_page(sg), sg->length, 0); @@ -3514,7 +3515,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, vma->display_alignment = max_t(u64, vma->display_alignment, alignment); /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ - if (obj->cache_dirty) { + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { i915_gem_clflush_object(obj, true); intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); } -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] drm/i915: Flush untouched framebuffers before display on !llc @ 2017-01-09 10:24 ` Chris Wilson 0 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-01-09 10:24 UTC (permalink / raw) To: linux-kernel; +Cc: intel-gfx, lkml, Chris Wilson, # v4 . 10-rc1+ On a non-llc system, the objects are created with .cache_level = CACHE_NONE and so the transition to uncached for scanout is a no-op. However, if the object was never written to, it will still be in the CPU domain (having been zeroed out by shmemfs). Those cachelines need to be flushed prior to display. Reported-by: Vito Caputo Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ --- drivers/gpu/drm/i915/i915_gem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 76689b59fc90..e64d0ea6113d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2327,6 +2327,7 @@ static void i915_sg_trim(struct sg_table *orig_st) if (sg_alloc_table(&new_st, orig_st->nents, GFP_KERNEL | __GFP_NOWARN)) return; + new_st->orig_nents = orig_st->orig_nents; /* XXX lies for new_sg = new_st.sgl; for_each_sg(orig_st->sgl, sg, orig_st->nents, i) { sg_set_page(new_sg, sg_page(sg), sg->length, 0); @@ -3514,7 +3515,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, vma->display_alignment = max_t(u64, vma->display_alignment, alignment); /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ - if (obj->cache_dirty) { + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { i915_gem_clflush_object(obj, true); intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); } -- 2.11.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Flush untouched framebuffers before display on !llc 2017-01-09 10:24 ` Chris Wilson @ 2017-01-09 10:52 ` Chris Wilson -1 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-01-09 10:52 UTC (permalink / raw) To: linux-kernel; +Cc: intel-gfx, lkml, # v4 . 10-rc1+ On Mon, Jan 09, 2017 at 10:24:01AM +0000, Chris Wilson wrote: > On a non-llc system, the objects are created with .cache_level = > CACHE_NONE and so the transition to uncached for scanout is a no-op. > However, if the object was never written to, it will still be in the CPU > domain (having been zeroed out by shmemfs). Those cachelines need to be > flushed prior to display. > > Reported-by: Vito Caputo > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ > --- > drivers/gpu/drm/i915/i915_gem.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 76689b59fc90..e64d0ea6113d 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2327,6 +2327,7 @@ static void i915_sg_trim(struct sg_table *orig_st) > if (sg_alloc_table(&new_st, orig_st->nents, GFP_KERNEL | __GFP_NOWARN)) > return; > > + new_st->orig_nents = orig_st->orig_nents; /* XXX lies for Oops. Ignore this chunk! > new_sg = new_st.sgl; > for_each_sg(orig_st->sgl, sg, orig_st->nents, i) { > sg_set_page(new_sg, sg_page(sg), sg->length, 0); > @@ -3514,7 +3515,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > vma->display_alignment = max_t(u64, vma->display_alignment, alignment); > > /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ > - if (obj->cache_dirty) { > + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { > i915_gem_clflush_object(obj, true); > intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); > } > -- > 2.11.0 > -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Flush untouched framebuffers before display on !llc @ 2017-01-09 10:52 ` Chris Wilson 0 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-01-09 10:52 UTC (permalink / raw) To: linux-kernel; +Cc: intel-gfx, lkml, # v4 . 10-rc1+ On Mon, Jan 09, 2017 at 10:24:01AM +0000, Chris Wilson wrote: > On a non-llc system, the objects are created with .cache_level = > CACHE_NONE and so the transition to uncached for scanout is a no-op. > However, if the object was never written to, it will still be in the CPU > domain (having been zeroed out by shmemfs). Those cachelines need to be > flushed prior to display. > > Reported-by: Vito Caputo > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ > --- > drivers/gpu/drm/i915/i915_gem.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 76689b59fc90..e64d0ea6113d 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2327,6 +2327,7 @@ static void i915_sg_trim(struct sg_table *orig_st) > if (sg_alloc_table(&new_st, orig_st->nents, GFP_KERNEL | __GFP_NOWARN)) > return; > > + new_st->orig_nents = orig_st->orig_nents; /* XXX lies for Oops. Ignore this chunk! > new_sg = new_st.sgl; > for_each_sg(orig_st->sgl, sg, orig_st->nents, i) { > sg_set_page(new_sg, sg_page(sg), sg->length, 0); > @@ -3514,7 +3515,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > vma->display_alignment = max_t(u64, vma->display_alignment, alignment); > > /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ > - if (obj->cache_dirty) { > + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { > i915_gem_clflush_object(obj, true); > intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); > } > -- > 2.11.0 > -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Flush untouched framebuffers before display on !llc 2017-01-09 10:24 ` Chris Wilson @ 2017-01-09 13:48 ` kbuild test robot -1 siblings, 0 replies; 20+ messages in thread From: kbuild test robot @ 2017-01-09 13:48 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx, lkml, # v4 . 10-rc1+, kbuild-all, linux-kernel [-- Attachment #1: Type: text/plain, Size: 6555 bytes --] Hi Chris, [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on v4.10-rc3 next-20170106] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-i915-Flush-untouched-framebuffers-before-display-on-llc/20170109-190816 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-s3-01092001 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/gpu/drm/i915/i915_gem.c: In function 'i915_sg_trim': drivers/gpu/drm/i915/i915_gem.c:2303:8: error: invalid type argument of '->' (have 'struct sg_table') new_st->orig_nents = orig_st->orig_nents; /* XXX lies for ^~ >> drivers/gpu/drm/i915/i915_gem.c:2307:3: error: "/*" within comment [-Werror=comment] /* called before being DMA mapped, no need to copy sg->dma_* */ >> drivers/gpu/drm/i915/i915_gem.c:2295:15: error: unused variable 'i' [-Werror=unused-variable] unsigned int i; ^ >> drivers/gpu/drm/i915/i915_gem.c:2294:22: error: unused variable 'sg' [-Werror=unused-variable] struct scatterlist *sg, *new_sg; ^~ In file included from include/linux/debug_locks.h:6:0, from include/linux/lockdep.h:25, from include/linux/spinlock_types.h:18, from include/linux/mutex.h:15, from include/linux/kernfs.h:13, from include/linux/sysfs.h:15, from include/linux/kobject.h:21, from include/linux/cdev.h:4, from include/drm/drmP.h:36, from drivers/gpu/drm/i915/i915_gem.c:28: drivers/gpu/drm/i915/i915_gem.c: At top level: include/linux/bug.h:45:35: error: expected identifier or '(' before 'void' #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e)))) ^ drivers/gpu/drm/i915/i915_gem.h:32:26: note: in expansion of macro 'BUILD_BUG_ON_INVALID' #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr) ^~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2310:2: note: in expansion of macro 'GEM_BUG_ON' GEM_BUG_ON(new_sg); /* Should walk exactly nents and hit the end */ ^~~~~~~~~~ include/linux/bug.h:45:40: error: expected ')' before '(' token #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e)))) ^ drivers/gpu/drm/i915/i915_gem.h:32:26: note: in expansion of macro 'BUILD_BUG_ON_INVALID' #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr) ^~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2310:2: note: in expansion of macro 'GEM_BUG_ON' GEM_BUG_ON(new_sg); /* Should walk exactly nents and hit the end */ ^~~~~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2312:2: error: data definition has no type or storage class [-Werror] sg_free_table(orig_st); ^~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2312:2: error: type defaults to 'int' in declaration of 'sg_free_table' [-Werror=implicit-int] drivers/gpu/drm/i915/i915_gem.c:2312:2: error: parameter names (without types) in function declaration [-Werror] drivers/gpu/drm/i915/i915_gem.c:2312:2: error: conflicting types for 'sg_free_table' In file included from include/linux/dma-mapping.h:10:0, from include/drm/drmP.h:37, from drivers/gpu/drm/i915/i915_gem.c:28: include/linux/scatterlist.h:260:6: note: previous declaration of 'sg_free_table' was here void sg_free_table(struct sg_table *); ^~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2314:2: error: data definition has no type or storage class [-Werror] *orig_st = new_st; ^ drivers/gpu/drm/i915/i915_gem.c:2314:3: error: type defaults to 'int' in declaration of 'orig_st' [-Werror=implicit-int] *orig_st = new_st; ^~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2314:13: error: 'new_st' undeclared here (not in a function) *orig_st = new_st; ^~~~~~ drivers/gpu/drm/i915/i915_gem.c:2315:1: error: expected identifier or '(' before '}' token } ^ cc1: all warnings being treated as errors vim +2307 drivers/gpu/drm/i915/i915_gem.c 871dfbd67 Chris Wilson 2016-10-11 2288 #endif 871dfbd67 Chris Wilson 2016-10-11 2289 } 871dfbd67 Chris Wilson 2016-10-11 2290 0c40ce130 Tvrtko Ursulin 2016-11-09 2291 static void i915_sg_trim(struct sg_table *orig_st) 0c40ce130 Tvrtko Ursulin 2016-11-09 2292 { 0c40ce130 Tvrtko Ursulin 2016-11-09 2293 struct sg_table new_st; 0c40ce130 Tvrtko Ursulin 2016-11-09 @2294 struct scatterlist *sg, *new_sg; 0c40ce130 Tvrtko Ursulin 2016-11-09 @2295 unsigned int i; 0c40ce130 Tvrtko Ursulin 2016-11-09 2296 0c40ce130 Tvrtko Ursulin 2016-11-09 2297 if (orig_st->nents == orig_st->orig_nents) 0c40ce130 Tvrtko Ursulin 2016-11-09 2298 return; 0c40ce130 Tvrtko Ursulin 2016-11-09 2299 8bfc478fa Chris Wilson 2016-12-23 2300 if (sg_alloc_table(&new_st, orig_st->nents, GFP_KERNEL | __GFP_NOWARN)) 0c40ce130 Tvrtko Ursulin 2016-11-09 2301 return; 0c40ce130 Tvrtko Ursulin 2016-11-09 2302 ccbf455f4 Chris Wilson 2017-01-09 @2303 new_st->orig_nents = orig_st->orig_nents; /* XXX lies for 0c40ce130 Tvrtko Ursulin 2016-11-09 2304 new_sg = new_st.sgl; 0c40ce130 Tvrtko Ursulin 2016-11-09 2305 for_each_sg(orig_st->sgl, sg, orig_st->nents, i) { 0c40ce130 Tvrtko Ursulin 2016-11-09 2306 sg_set_page(new_sg, sg_page(sg), sg->length, 0); 0c40ce130 Tvrtko Ursulin 2016-11-09 @2307 /* called before being DMA mapped, no need to copy sg->dma_* */ 0c40ce130 Tvrtko Ursulin 2016-11-09 2308 new_sg = sg_next(new_sg); 0c40ce130 Tvrtko Ursulin 2016-11-09 2309 } c2dc6cc94 Chris Wilson 2016-12-19 2310 GEM_BUG_ON(new_sg); /* Should walk exactly nents and hit the end */ :::::: The code at line 2307 was first introduced by commit :::::: 0c40ce130e38aeb9ddcee3ddcffbe5a79f27c080 drm/i915: Trim the object sg table :::::: TO: Tvrtko Ursulin <tvrtko.ursulin@intel.com> :::::: CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 27130 bytes --] [-- Attachment #3: Type: text/plain, Size: 160 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Flush untouched framebuffers before display on !llc @ 2017-01-09 13:48 ` kbuild test robot 0 siblings, 0 replies; 20+ messages in thread From: kbuild test robot @ 2017-01-09 13:48 UTC (permalink / raw) To: Chris Wilson; +Cc: kbuild-all, linux-kernel, intel-gfx, lkml, # v4 . 10-rc1+ [-- Attachment #1: Type: text/plain, Size: 6555 bytes --] Hi Chris, [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on v4.10-rc3 next-20170106] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-i915-Flush-untouched-framebuffers-before-display-on-llc/20170109-190816 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-s3-01092001 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/gpu/drm/i915/i915_gem.c: In function 'i915_sg_trim': drivers/gpu/drm/i915/i915_gem.c:2303:8: error: invalid type argument of '->' (have 'struct sg_table') new_st->orig_nents = orig_st->orig_nents; /* XXX lies for ^~ >> drivers/gpu/drm/i915/i915_gem.c:2307:3: error: "/*" within comment [-Werror=comment] /* called before being DMA mapped, no need to copy sg->dma_* */ >> drivers/gpu/drm/i915/i915_gem.c:2295:15: error: unused variable 'i' [-Werror=unused-variable] unsigned int i; ^ >> drivers/gpu/drm/i915/i915_gem.c:2294:22: error: unused variable 'sg' [-Werror=unused-variable] struct scatterlist *sg, *new_sg; ^~ In file included from include/linux/debug_locks.h:6:0, from include/linux/lockdep.h:25, from include/linux/spinlock_types.h:18, from include/linux/mutex.h:15, from include/linux/kernfs.h:13, from include/linux/sysfs.h:15, from include/linux/kobject.h:21, from include/linux/cdev.h:4, from include/drm/drmP.h:36, from drivers/gpu/drm/i915/i915_gem.c:28: drivers/gpu/drm/i915/i915_gem.c: At top level: include/linux/bug.h:45:35: error: expected identifier or '(' before 'void' #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e)))) ^ drivers/gpu/drm/i915/i915_gem.h:32:26: note: in expansion of macro 'BUILD_BUG_ON_INVALID' #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr) ^~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2310:2: note: in expansion of macro 'GEM_BUG_ON' GEM_BUG_ON(new_sg); /* Should walk exactly nents and hit the end */ ^~~~~~~~~~ include/linux/bug.h:45:40: error: expected ')' before '(' token #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e)))) ^ drivers/gpu/drm/i915/i915_gem.h:32:26: note: in expansion of macro 'BUILD_BUG_ON_INVALID' #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr) ^~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2310:2: note: in expansion of macro 'GEM_BUG_ON' GEM_BUG_ON(new_sg); /* Should walk exactly nents and hit the end */ ^~~~~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2312:2: error: data definition has no type or storage class [-Werror] sg_free_table(orig_st); ^~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2312:2: error: type defaults to 'int' in declaration of 'sg_free_table' [-Werror=implicit-int] drivers/gpu/drm/i915/i915_gem.c:2312:2: error: parameter names (without types) in function declaration [-Werror] drivers/gpu/drm/i915/i915_gem.c:2312:2: error: conflicting types for 'sg_free_table' In file included from include/linux/dma-mapping.h:10:0, from include/drm/drmP.h:37, from drivers/gpu/drm/i915/i915_gem.c:28: include/linux/scatterlist.h:260:6: note: previous declaration of 'sg_free_table' was here void sg_free_table(struct sg_table *); ^~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2314:2: error: data definition has no type or storage class [-Werror] *orig_st = new_st; ^ drivers/gpu/drm/i915/i915_gem.c:2314:3: error: type defaults to 'int' in declaration of 'orig_st' [-Werror=implicit-int] *orig_st = new_st; ^~~~~~~ drivers/gpu/drm/i915/i915_gem.c:2314:13: error: 'new_st' undeclared here (not in a function) *orig_st = new_st; ^~~~~~ drivers/gpu/drm/i915/i915_gem.c:2315:1: error: expected identifier or '(' before '}' token } ^ cc1: all warnings being treated as errors vim +2307 drivers/gpu/drm/i915/i915_gem.c 871dfbd67 Chris Wilson 2016-10-11 2288 #endif 871dfbd67 Chris Wilson 2016-10-11 2289 } 871dfbd67 Chris Wilson 2016-10-11 2290 0c40ce130 Tvrtko Ursulin 2016-11-09 2291 static void i915_sg_trim(struct sg_table *orig_st) 0c40ce130 Tvrtko Ursulin 2016-11-09 2292 { 0c40ce130 Tvrtko Ursulin 2016-11-09 2293 struct sg_table new_st; 0c40ce130 Tvrtko Ursulin 2016-11-09 @2294 struct scatterlist *sg, *new_sg; 0c40ce130 Tvrtko Ursulin 2016-11-09 @2295 unsigned int i; 0c40ce130 Tvrtko Ursulin 2016-11-09 2296 0c40ce130 Tvrtko Ursulin 2016-11-09 2297 if (orig_st->nents == orig_st->orig_nents) 0c40ce130 Tvrtko Ursulin 2016-11-09 2298 return; 0c40ce130 Tvrtko Ursulin 2016-11-09 2299 8bfc478fa Chris Wilson 2016-12-23 2300 if (sg_alloc_table(&new_st, orig_st->nents, GFP_KERNEL | __GFP_NOWARN)) 0c40ce130 Tvrtko Ursulin 2016-11-09 2301 return; 0c40ce130 Tvrtko Ursulin 2016-11-09 2302 ccbf455f4 Chris Wilson 2017-01-09 @2303 new_st->orig_nents = orig_st->orig_nents; /* XXX lies for 0c40ce130 Tvrtko Ursulin 2016-11-09 2304 new_sg = new_st.sgl; 0c40ce130 Tvrtko Ursulin 2016-11-09 2305 for_each_sg(orig_st->sgl, sg, orig_st->nents, i) { 0c40ce130 Tvrtko Ursulin 2016-11-09 2306 sg_set_page(new_sg, sg_page(sg), sg->length, 0); 0c40ce130 Tvrtko Ursulin 2016-11-09 @2307 /* called before being DMA mapped, no need to copy sg->dma_* */ 0c40ce130 Tvrtko Ursulin 2016-11-09 2308 new_sg = sg_next(new_sg); 0c40ce130 Tvrtko Ursulin 2016-11-09 2309 } c2dc6cc94 Chris Wilson 2016-12-19 2310 GEM_BUG_ON(new_sg); /* Should walk exactly nents and hit the end */ :::::: The code at line 2307 was first introduced by commit :::::: 0c40ce130e38aeb9ddcee3ddcffbe5a79f27c080 drm/i915: Trim the object sg table :::::: TO: Tvrtko Ursulin <tvrtko.ursulin@intel.com> :::::: CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 27130 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc 2017-01-09 6:32 BUG: 4.10 i915 drm display noise regression - bisected to a6a7cc4b7 lkml @ 2017-01-09 11:19 ` Chris Wilson 2017-01-09 11:19 ` Chris Wilson ` (2 subsequent siblings) 3 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-01-09 11:19 UTC (permalink / raw) To: linux-kernel; +Cc: intel-gfx, lkml, # v4 . 10-rc1+ On a non-llc system, the objects are created with .cache_level = CACHE_NONE and so the transition to uncached for scanout is a no-op. However, if the object was never written to, it will still be in the CPU domain (having been zeroed out by shmemfs). Those cachelines need to be flushed prior to display. Reported-by: Vito Caputo Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ --- drivers/gpu/drm/i915/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 76689b59fc90..bdb113ef8cfe 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, vma->display_alignment = max_t(u64, vma->display_alignment, alignment); /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ - if (obj->cache_dirty) { + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { i915_gem_clflush_object(obj, true); intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); } -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc @ 2017-01-09 11:19 ` Chris Wilson 0 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-01-09 11:19 UTC (permalink / raw) To: linux-kernel; +Cc: intel-gfx, lkml, Chris Wilson, # v4 . 10-rc1+ On a non-llc system, the objects are created with .cache_level = CACHE_NONE and so the transition to uncached for scanout is a no-op. However, if the object was never written to, it will still be in the CPU domain (having been zeroed out by shmemfs). Those cachelines need to be flushed prior to display. Reported-by: Vito Caputo Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ --- drivers/gpu/drm/i915/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 76689b59fc90..bdb113ef8cfe 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, vma->display_alignment = max_t(u64, vma->display_alignment, alignment); /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ - if (obj->cache_dirty) { + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { i915_gem_clflush_object(obj, true); intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); } -- 2.11.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc 2017-01-09 11:19 ` Chris Wilson @ 2017-01-12 21:17 ` Chris Wilson -1 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-01-12 21:17 UTC (permalink / raw) To: linux-kernel; +Cc: intel-gfx, lkml, # v4 . 10-rc1+ On Mon, Jan 09, 2017 at 11:19:32AM +0000, Chris Wilson wrote: > On a non-llc system, the objects are created with .cache_level = > CACHE_NONE and so the transition to uncached for scanout is a no-op. > However, if the object was never written to, it will still be in the CPU > domain (having been zeroed out by shmemfs). Those cachelines need to be > flushed prior to display. > > Reported-by: Vito Caputo > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ Ping? > --- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 76689b59fc90..bdb113ef8cfe 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > vma->display_alignment = max_t(u64, vma->display_alignment, alignment); > > /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ > - if (obj->cache_dirty) { > + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { > i915_gem_clflush_object(obj, true); > intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); > } > -- > 2.11.0 -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc @ 2017-01-12 21:17 ` Chris Wilson 0 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-01-12 21:17 UTC (permalink / raw) To: linux-kernel; +Cc: intel-gfx, lkml, # v4 . 10-rc1+ On Mon, Jan 09, 2017 at 11:19:32AM +0000, Chris Wilson wrote: > On a non-llc system, the objects are created with .cache_level = > CACHE_NONE and so the transition to uncached for scanout is a no-op. > However, if the object was never written to, it will still be in the CPU > domain (having been zeroed out by shmemfs). Those cachelines need to be > flushed prior to display. > > Reported-by: Vito Caputo > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ Ping? > --- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 76689b59fc90..bdb113ef8cfe 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > vma->display_alignment = max_t(u64, vma->display_alignment, alignment); > > /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ > - if (obj->cache_dirty) { > + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { > i915_gem_clflush_object(obj, true); > intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); > } > -- > 2.11.0 -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc 2017-01-12 21:17 ` Chris Wilson (?) @ 2017-01-12 22:24 ` lkml 2017-01-12 22:38 ` Chris Wilson -1 siblings, 1 reply; 20+ messages in thread From: lkml @ 2017-01-12 22:24 UTC (permalink / raw) To: Chris Wilson, linux-kernel, intel-gfx, # v4 . 10-rc1+ On Thu, Jan 12, 2017 at 09:17:06PM +0000, Chris Wilson wrote: > On Mon, Jan 09, 2017 at 11:19:32AM +0000, Chris Wilson wrote: > > On a non-llc system, the objects are created with .cache_level = > > CACHE_NONE and so the transition to uncached for scanout is a no-op. > > However, if the object was never written to, it will still be in the CPU > > domain (having been zeroed out by shmemfs). Those cachelines need to be > > flushed prior to display. > > > > Reported-by: Vito Caputo > > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ > > Ping? This patch fixes the problem for me, in case that's what the ping's for. Out of curiosity the bug I reported described here be getting fixed in 4.10? https://lists.freedesktop.org/archives/dri-devel/2017-January/128405.html Thanks. > > > --- > > drivers/gpu/drm/i915/i915_gem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index 76689b59fc90..bdb113ef8cfe 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > > vma->display_alignment = max_t(u64, vma->display_alignment, alignment); > > > > /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ > > - if (obj->cache_dirty) { > > + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { > > i915_gem_clflush_object(obj, true); > > intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); > > } > > -- > > 2.11.0 > > -- > Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc 2017-01-12 22:24 ` lkml @ 2017-01-12 22:38 ` Chris Wilson 0 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-01-12 22:38 UTC (permalink / raw) To: lkml; +Cc: intel-gfx, # v4 . 10-rc1+, linux-kernel On Thu, Jan 12, 2017 at 04:24:50PM -0600, lkml@pengaru.com wrote: > On Thu, Jan 12, 2017 at 09:17:06PM +0000, Chris Wilson wrote: > > On Mon, Jan 09, 2017 at 11:19:32AM +0000, Chris Wilson wrote: > > > On a non-llc system, the objects are created with .cache_level = > > > CACHE_NONE and so the transition to uncached for scanout is a no-op. > > > However, if the object was never written to, it will still be in the CPU > > > domain (having been zeroed out by shmemfs). Those cachelines need to be > > > flushed prior to display. > > > > > > Reported-by: Vito Caputo > > > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ > > > > Ping? > > This patch fixes the problem for me, in case that's what the ping's for. Partly that, and trying to catch CI + reviewers. > Out of curiosity the bug I reported described here be getting fixed in 4.10? > https://lists.freedesktop.org/archives/dri-devel/2017-January/128405.html It was fixed in the tree back in December, bit of a muddle to get that particular patch into 4.10. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc @ 2017-01-12 22:38 ` Chris Wilson 0 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-01-12 22:38 UTC (permalink / raw) To: lkml; +Cc: linux-kernel, intel-gfx, # v4 . 10-rc1+ On Thu, Jan 12, 2017 at 04:24:50PM -0600, lkml@pengaru.com wrote: > On Thu, Jan 12, 2017 at 09:17:06PM +0000, Chris Wilson wrote: > > On Mon, Jan 09, 2017 at 11:19:32AM +0000, Chris Wilson wrote: > > > On a non-llc system, the objects are created with .cache_level = > > > CACHE_NONE and so the transition to uncached for scanout is a no-op. > > > However, if the object was never written to, it will still be in the CPU > > > domain (having been zeroed out by shmemfs). Those cachelines need to be > > > flushed prior to display. > > > > > > Reported-by: Vito Caputo > > > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ > > > > Ping? > > This patch fixes the problem for me, in case that's what the ping's for. Partly that, and trying to catch CI + reviewers. > Out of curiosity the bug I reported described here be getting fixed in 4.10? > https://lists.freedesktop.org/archives/dri-devel/2017-January/128405.html It was fixed in the tree back in December, bit of a muddle to get that particular patch into 4.10. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc 2017-01-09 11:19 ` Chris Wilson @ 2017-02-01 10:24 ` Daniel Vetter -1 siblings, 0 replies; 20+ messages in thread From: Daniel Vetter @ 2017-02-01 10:24 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx, lkml, # v4 . 10-rc1+, linux-kernel On Mon, Jan 09, 2017 at 11:19:32AM +0000, Chris Wilson wrote: > On a non-llc system, the objects are created with .cache_level = > CACHE_NONE and so the transition to uncached for scanout is a no-op. > However, if the object was never written to, it will still be in the CPU > domain (having been zeroed out by shmemfs). Those cachelines need to be > flushed prior to display. > > Reported-by: Vito Caputo > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ > --- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 76689b59fc90..bdb113ef8cfe 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > vma->display_alignment = max_t(u64, vma->display_alignment, alignment); > > /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ > - if (obj->cache_dirty) { > + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { Alternatively, should we set cache_dirty when initially allocating an object? Ofc only if cpu_cache_is_coherent, like we do in other places. Anyway, up to you which one you like more I'd say, this one looks correct too. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > i915_gem_clflush_object(obj, true); > intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); > } > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc @ 2017-02-01 10:24 ` Daniel Vetter 0 siblings, 0 replies; 20+ messages in thread From: Daniel Vetter @ 2017-02-01 10:24 UTC (permalink / raw) To: Chris Wilson; +Cc: linux-kernel, intel-gfx, lkml, # v4 . 10-rc1+ On Mon, Jan 09, 2017 at 11:19:32AM +0000, Chris Wilson wrote: > On a non-llc system, the objects are created with .cache_level = > CACHE_NONE and so the transition to uncached for scanout is a no-op. > However, if the object was never written to, it will still be in the CPU > domain (having been zeroed out by shmemfs). Those cachelines need to be > flushed prior to display. > > Reported-by: Vito Caputo > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ > --- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 76689b59fc90..bdb113ef8cfe 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > vma->display_alignment = max_t(u64, vma->display_alignment, alignment); > > /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ > - if (obj->cache_dirty) { > + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { Alternatively, should we set cache_dirty when initially allocating an object? Ofc only if cpu_cache_is_coherent, like we do in other places. Anyway, up to you which one you like more I'd say, this one looks correct too. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > i915_gem_clflush_object(obj, true); > intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB); > } > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc 2017-02-01 10:24 ` [Intel-gfx] " Daniel Vetter @ 2017-02-01 10:48 ` Chris Wilson -1 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-02-01 10:48 UTC (permalink / raw) To: linux-kernel, intel-gfx, lkml, # v4 . 10-rc1+ On Wed, Feb 01, 2017 at 11:24:32AM +0100, Daniel Vetter wrote: > On Mon, Jan 09, 2017 at 11:19:32AM +0000, Chris Wilson wrote: > > On a non-llc system, the objects are created with .cache_level = > > CACHE_NONE and so the transition to uncached for scanout is a no-op. > > However, if the object was never written to, it will still be in the CPU > > domain (having been zeroed out by shmemfs). Those cachelines need to be > > flushed prior to display. > > > > Reported-by: Vito Caputo > > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ > > --- > > drivers/gpu/drm/i915/i915_gem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index 76689b59fc90..bdb113ef8cfe 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > > vma->display_alignment = max_t(u64, vma->display_alignment, alignment); > > > > /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ > > - if (obj->cache_dirty) { > > + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { > > Alternatively, should we set cache_dirty when initially allocating an > object? Ofc only if cpu_cache_is_coherent, like we do in other places. I thought about it and didn't come to any firm conclusion. Currently "cache_dirty" means omission of clflush, and that's been a source of confusion ever since. I've a patch/plan to do async clflushing which similarly impacts upon the meaning of obj->cache_dirty and interation with frontbuffer tracking, so that seems a reasonable point to which to defer further thought. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc @ 2017-02-01 10:48 ` Chris Wilson 0 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2017-02-01 10:48 UTC (permalink / raw) To: linux-kernel, intel-gfx, lkml, # v4 . 10-rc1+ On Wed, Feb 01, 2017 at 11:24:32AM +0100, Daniel Vetter wrote: > On Mon, Jan 09, 2017 at 11:19:32AM +0000, Chris Wilson wrote: > > On a non-llc system, the objects are created with .cache_level = > > CACHE_NONE and so the transition to uncached for scanout is a no-op. > > However, if the object was never written to, it will still be in the CPU > > domain (having been zeroed out by shmemfs). Those cachelines need to be > > flushed prior to display. > > > > Reported-by: Vito Caputo > > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.10-rc1+ > > --- > > drivers/gpu/drm/i915/i915_gem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index 76689b59fc90..bdb113ef8cfe 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > > vma->display_alignment = max_t(u64, vma->display_alignment, alignment); > > > > /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ > > - if (obj->cache_dirty) { > > + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) { > > Alternatively, should we set cache_dirty when initially allocating an > object? Ofc only if cpu_cache_is_coherent, like we do in other places. I thought about it and didn't come to any firm conclusion. Currently "cache_dirty" means omission of clflush, and that's been a source of confusion ever since. I've a patch/plan to do async clflushing which similarly impacts upon the meaning of obj->cache_dirty and interation with frontbuffer tracking, so that seems a reasonable point to which to defer further thought. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Flush untouched framebuffers before display on !llc 2017-01-09 6:32 BUG: 4.10 i915 drm display noise regression - bisected to a6a7cc4b7 lkml 2017-01-09 10:24 ` Chris Wilson 2017-01-09 11:19 ` Chris Wilson @ 2017-01-09 13:01 ` Patchwork 2017-01-30 2:04 ` BUG: 4.10 i915 drm display noise regression - bisected to a6a7cc4b7 lkml 3 siblings, 0 replies; 20+ messages in thread From: Patchwork @ 2017-01-09 13:01 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx == Series Details == Series: drm/i915: Flush untouched framebuffers before display on !llc URL : https://patchwork.freedesktop.org/series/17694/ State : failure == Summary == ^ drivers/gpu/drm/i915/i915_gem.c:2301:2: note: in expansion of macro ‘GEM_BUG_ON’ GEM_BUG_ON(new_sg); /* Should walk exactly nents and hit the end */ ^ drivers/gpu/drm/i915/i915_gem.c:2303:2: error: data definition has no type or storage class [-Werror] sg_free_table(orig_st); ^ drivers/gpu/drm/i915/i915_gem.c:2303:2: error: type defaults to ‘int’ in declaration of ‘sg_free_table’ [-Werror=implicit-int] drivers/gpu/drm/i915/i915_gem.c:2303:2: error: parameter names (without types) in function declaration [-Werror] drivers/gpu/drm/i915/i915_gem.c:2303:2: error: conflicting types for ‘sg_free_table’ In file included from ./include/linux/dma-mapping.h:10:0, from ./include/drm/drmP.h:37, from drivers/gpu/drm/i915/i915_gem.c:28: ./include/linux/scatterlist.h:260:6: note: previous declaration of ‘sg_free_table’ was here void sg_free_table(struct sg_table *); ^ drivers/gpu/drm/i915/i915_gem.c:2305:2: error: data definition has no type or storage class [-Werror] *orig_st = new_st; ^ drivers/gpu/drm/i915/i915_gem.c:2305:3: error: type defaults to ‘int’ in declaration of ‘orig_st’ [-Werror=implicit-int] *orig_st = new_st; ^ drivers/gpu/drm/i915/i915_gem.c:2305:13: error: ‘new_st’ undeclared here (not in a function) *orig_st = new_st; ^ drivers/gpu/drm/i915/i915_gem.c:2306:1: error: expected identifier or ‘(’ before ‘}’ token } ^ LD drivers/tty/serial/8250/8250.o LD drivers/iommu/built-in.o LD [M] drivers/ssb/ssb.o cc1: all warnings being treated as errors scripts/Makefile.build:293: recipe for target 'drivers/gpu/drm/i915/i915_gem.o' failed make[4]: *** [drivers/gpu/drm/i915/i915_gem.o] Error 1 make[4]: *** Waiting for unfinished jobs.... LD net/ipv6/ipv6.o LD drivers/usb/storage/usb-storage.o LD net/xfrm/built-in.o LD drivers/usb/storage/built-in.o LD drivers/pci/built-in.o LD net/ipv6/built-in.o LD [M] drivers/net/ethernet/intel/e1000/e1000.o LD [M] sound/pci/hda/snd-hda-codec-generic.o LD [M] drivers/usb/serial/usbserial.o LD drivers/video/fbdev/core/fb.o LD drivers/video/fbdev/core/built-in.o LD sound/pci/built-in.o LD drivers/tty/serial/8250/8250_base.o LD drivers/tty/serial/8250/built-in.o LD drivers/tty/serial/built-in.o LD drivers/thermal/thermal_sys.o LD drivers/video/fbdev/built-in.o LD drivers/thermal/built-in.o LD drivers/scsi/scsi_mod.o LD sound/built-in.o LD drivers/video/console/built-in.o AR lib/lib.a LD drivers/video/built-in.o EXPORTS lib/lib-ksyms.o LD drivers/usb/gadget/libcomposite.o LD lib/built-in.o LD fs/btrfs/btrfs.o LD drivers/spi/built-in.o LD drivers/usb/gadget/udc/udc-core.o LD drivers/usb/gadget/udc/built-in.o LD drivers/usb/gadget/built-in.o LD fs/btrfs/built-in.o LD [M] drivers/net/ethernet/intel/e1000e/e1000e.o LD [M] drivers/net/ethernet/intel/igb/igb.o LD net/ipv4/built-in.o LD drivers/scsi/sd_mod.o LD drivers/scsi/built-in.o CC arch/x86/kernel/cpu/capflags.o LD arch/x86/kernel/cpu/built-in.o LD arch/x86/kernel/built-in.o LD arch/x86/built-in.o LD drivers/usb/host/xhci-hcd.o LD drivers/md/md-mod.o LD drivers/md/built-in.o LD drivers/usb/core/usbcore.o LD drivers/usb/core/built-in.o LD drivers/tty/vt/built-in.o LD drivers/tty/built-in.o LD fs/ext4/ext4.o LD fs/ext4/built-in.o LD fs/built-in.o LD net/core/built-in.o LD drivers/usb/host/built-in.o LD net/built-in.o LD drivers/usb/built-in.o LD drivers/net/ethernet/built-in.o LD drivers/net/built-in.o scripts/Makefile.build:551: recipe for target 'drivers/gpu/drm/i915' failed make[3]: *** [drivers/gpu/drm/i915] Error 2 scripts/Makefile.build:551: recipe for target 'drivers/gpu/drm' failed make[2]: *** [drivers/gpu/drm] Error 2 scripts/Makefile.build:551: recipe for target 'drivers/gpu' failed make[1]: *** [drivers/gpu] Error 2 Makefile:988: recipe for target 'drivers' failed make: *** [drivers] Error 2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: BUG: 4.10 i915 drm display noise regression - bisected to a6a7cc4b7 2017-01-09 6:32 BUG: 4.10 i915 drm display noise regression - bisected to a6a7cc4b7 lkml ` (2 preceding siblings ...) 2017-01-09 13:01 ` ✗ Fi.CI.BAT: failure for " Patchwork @ 2017-01-30 2:04 ` lkml 3 siblings, 0 replies; 20+ messages in thread From: lkml @ 2017-01-30 2:04 UTC (permalink / raw) To: linux-kernel; +Cc: dri-devel, chris On Mon, Jan 09, 2017 at 12:32:40AM -0600, lkml@pengaru.com wrote: > Hello all, > > I'm experiencing display noise in the form of 8x1 pixel bars spuriously > appearing in random locations. This doesn't happen on 4.9, the machine > is an X61s, a Core2Duo 1.8Ghz w/XGA via LVDS. > > I was able to bisect the issue to a6a7cc4b7: > > commit a6a7cc4b7db6deaeca11cdd38844ea147a354c7a > Author: Chris Wilson <chris@chris-wilson.co.uk> > Date: Fri Nov 18 21:17:46 2016 +0000 > > drm/i915: Always flush the dirty CPU cache when pinning the scanout > > Currently we only clflush the scanout if it is in the CPU domain. Also > flush if we have a pending CPU clflush. We also want to treat the > dirtyfb path similar, and flush any pending writes there as well. > > v2: Only send the fb flush message if flushing the dirt on flip > v3: Make flush-for-flip and dirtyfb look more alike since they serve > similar roles as end-of-frame marker. > > Reproduction is simple, just run this native drm eye candy program: > https://github.com/vcaputo/rototiller > This regression still remains as of 4.10.0-rc6. Chris Wilson had posted a fix: https://www.spinics.net/lists/kernel/msg2420777.html But it seems to have been ignored so far. How do we get this fixed in 4.10 before it ships? Regards, Vito Caputo ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-02-01 10:49 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-09 6:32 BUG: 4.10 i915 drm display noise regression - bisected to a6a7cc4b7 lkml 2017-01-09 10:24 ` [PATCH] drm/i915: Flush untouched framebuffers before display on !llc Chris Wilson 2017-01-09 10:24 ` Chris Wilson 2017-01-09 10:52 ` Chris Wilson 2017-01-09 10:52 ` Chris Wilson 2017-01-09 13:48 ` kbuild test robot 2017-01-09 13:48 ` [Intel-gfx] " kbuild test robot 2017-01-09 11:19 ` [PATCH v2] " Chris Wilson 2017-01-09 11:19 ` Chris Wilson 2017-01-12 21:17 ` Chris Wilson 2017-01-12 21:17 ` Chris Wilson 2017-01-12 22:24 ` lkml 2017-01-12 22:38 ` Chris Wilson 2017-01-12 22:38 ` Chris Wilson 2017-02-01 10:24 ` Daniel Vetter 2017-02-01 10:24 ` [Intel-gfx] " Daniel Vetter 2017-02-01 10:48 ` Chris Wilson 2017-02-01 10:48 ` [Intel-gfx] " Chris Wilson 2017-01-09 13:01 ` ✗ Fi.CI.BAT: failure for " Patchwork 2017-01-30 2:04 ` BUG: 4.10 i915 drm display noise regression - bisected to a6a7cc4b7 lkml
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.