| Message ID | 1352994838-15156-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 15 Nov 2012 16:53:58 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > Since we drop dev->struct_mutex when going through the slowpath, the > object might have been moved out of the cpu domain. Hence we need to > clflush the entire object to ensure that after the ioctl returns, > everything is coherent again (interwoven writes are ill-defined > anyway). > > But we only need to do this if we start in the cpu domain and the > object requires flushing for coherency. So don't do the flushing if > the object is coherent anyway or if we've done in-line clfushing > already. > > v2: i915_gem_clflush_object already checks whether the object is > coherent and if so, drops the flushing. Hence we don't need to check > that ourselves, simplifying the condition. > > v3: Reorder the checks for better clarify (and adjust the comment > accordingly), suggested by Chris Wilson. > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Yup, the comment makes much more sense now. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Thu, Nov 15, 2012 at 04:07:45PM +0000, Chris Wilson wrote: > On Thu, 15 Nov 2012 16:53:58 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > Since we drop dev->struct_mutex when going through the slowpath, the > > object might have been moved out of the cpu domain. Hence we need to > > clflush the entire object to ensure that after the ioctl returns, > > everything is coherent again (interwoven writes are ill-defined > > anyway). > > > > But we only need to do this if we start in the cpu domain and the > > object requires flushing for coherency. So don't do the flushing if > > the object is coherent anyway or if we've done in-line clfushing > > already. > > > > v2: i915_gem_clflush_object already checks whether the object is > > coherent and if so, drops the flushing. Hence we don't need to check > > that ourselves, simplifying the condition. > > > > v3: Reorder the checks for better clarify (and adjust the comment > > accordingly), suggested by Chris Wilson. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Yup, the comment makes much more sense now. > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Both patches merged to dinq, thanks a lot for the review comments. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index eaaf095..851f787 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -830,9 +830,13 @@ out: i915_gem_object_unpin_pages(obj); if (hit_slowpath) { - /* Fixup: Flush dirty cachelines in case the object isn't in the - * cpu write domain anymore. */ - if (obj->base.write_domain != I915_GEM_DOMAIN_CPU) { + /* + * Fixup: Flush cpu caches in case we didn't flush the dirty + * cachelines in-line while writing and the object moved + * out of the cpu write domain while we've dropped the lock. + */ + if (!needs_clflush_after && + obj->base.write_domain != I915_GEM_DOMAIN_CPU) { i915_gem_clflush_object(obj); i915_gem_chipset_flush(dev); }
Since we drop dev->struct_mutex when going through the slowpath, the object might have been moved out of the cpu domain. Hence we need to clflush the entire object to ensure that after the ioctl returns, everything is coherent again (interwoven writes are ill-defined anyway). But we only need to do this if we start in the cpu domain and the object requires flushing for coherency. So don't do the flushing if the object is coherent anyway or if we've done in-line clfushing already. v2: i915_gem_clflush_object already checks whether the object is coherent and if so, drops the flushing. Hence we don't need to check that ourselves, simplifying the condition. v3: Reorder the checks for better clarify (and adjust the comment accordingly), suggested by Chris Wilson. Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_gem.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)