Skip to content

Commit

Permalink
drm/i915: Simplify flush_cpu_write_domain
Browse files Browse the repository at this point in the history
We can push down the decision whether to force flushing into the
implementation since in all places that matter obj->pin_display is
accurate already. The only place where the optimization really matters
is the sw_finish_ioctl, and that already checks for obj->pin_display
on its own.

I suspect that this was simply an artifact of how

commit 2c22569
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Aug 9 12:26:45 2013 +0100

    drm/i915: Update rules for writing through the LLC with the cpu

evolved - only v2 added the pin_display tracking.

Note that we still retain the gist of this logic from the above commit
with the explicit force argument for the low-level clflush function.

Ville noted in his review that there's a slight behavioural change in
the set_to_gtt_domain function, which now also will flush display
plane data. This opens-open the potential for userspace to start doing
buggy things by omitting the sw_finish_ioctl, which is why I've
rejected a functional equivalent patch from Ville a while ago:

http://lists.freedesktop.org/archives/intel-gfx/2013-November/036421.html

But on second consideration it's not that evil, and in any case the
justification here is more clarity, not allowing crazy userspace.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
  • Loading branch information
Daniel Vetter committed Jan 27, 2015
1 parent d9806c9 commit e62b59e
Showing 1 changed file with 6 additions and 8 deletions.
14 changes: 6 additions & 8 deletions drivers/gpu/drm/i915/i915_gem.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@
#include <linux/dma-buf.h>

static void i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj);
static void i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj,
bool force);
static void i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj);
static __must_check int
i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
bool readonly);
Expand Down Expand Up @@ -1516,7 +1515,7 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,

/* Pinned buffers may be scanout, so flush the cache */
if (obj->pin_display)
i915_gem_object_flush_cpu_write_domain(obj, true);
i915_gem_object_flush_cpu_write_domain(obj);

drm_gem_object_unreference(&obj->base);
unlock:
Expand Down Expand Up @@ -3680,15 +3679,14 @@ i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj)

/** Flushes the CPU write domain for the object if it's dirty. */
static void
i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj,
bool force)
i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj)
{
uint32_t old_write_domain;

if (obj->base.write_domain != I915_GEM_DOMAIN_CPU)
return;

if (i915_gem_clflush_object(obj, force))
if (i915_gem_clflush_object(obj, obj->pin_display))
i915_gem_chipset_flush(obj->base.dev);

old_write_domain = obj->base.write_domain;
Expand Down Expand Up @@ -3735,7 +3733,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
if (ret)
return ret;

i915_gem_object_flush_cpu_write_domain(obj, false);
i915_gem_object_flush_cpu_write_domain(obj);

/* Serialise direct access to this object with the barriers for
* coherent writes from the GPU, by effectively invalidating the
Expand Down Expand Up @@ -3981,7 +3979,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
if (ret)
goto err_unpin_display;

i915_gem_object_flush_cpu_write_domain(obj, true);
i915_gem_object_flush_cpu_write_domain(obj);

old_write_domain = obj->base.write_domain;
old_read_domains = obj->base.read_domains;
Expand Down

0 comments on commit e62b59e

Please sign in to comment.