diff mbox

drm/i915: optimize the shmem_pwrite slowpath handling

Message ID 1352992849-11219-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Nov. 15, 2012, 3:20 p.m. UTC
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.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Chris Wilson Nov. 15, 2012, 3:37 p.m. UTC | #1
On Thu, 15 Nov 2012 16:20:49 +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.
> 
Getting clearer, certainly. How about reversing the order of the
checks so that it reads as
  if (used-fast-path && fast-path-no-longer-valid) do_fixup()?
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index eaaf095..ab66645 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -831,8 +831,9 @@  out:
 
 	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) {
+		 * cpu write domain anymore, and we haven't flushed it manually. */
+		if (obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
+		    !needs_clflush_after) {
 			i915_gem_clflush_object(obj);
 			i915_gem_chipset_flush(dev);
 		}