[PATCH v6 4/5] drm/i915: Implement end_cpu_access

[Chris Wilson]

On Fri, Dec 18, 2015 at 05:19:24PM -0200, Tiago Vignatti wrote:
> On 12/18/2015 05:02 PM, Tiago Vignatti wrote:
> >On 12/17/2015 06:01 AM, Chris Wilson wrote:
> >>On Wed, Dec 16, 2015 at 08:25:36PM -0200, Tiago Vignatti wrote:
> >>>This function is meant to be used with dma-buf mmap, when finishing
> >>>the CPU
> >>>access of the mapped pointer.
> >>>
> >>>+static void i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum
> >>>dma_data_direction direction)
> >>>+{
> >>>+    struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
> >>>+    struct drm_device *dev = obj->base.dev;
> >>>+    struct drm_i915_private *dev_priv = to_i915(dev);
> >>>+    bool was_interruptible, write = (direction == DMA_BIDIRECTIONAL
> >>>|| direction == DMA_TO_DEVICE);
> >>>+    int ret;
> >>>+
> >>>+    mutex_lock(&dev->struct_mutex);
> >>>+    was_interruptible = dev_priv->mm.interruptible;
> >>>+    dev_priv->mm.interruptible = false;
> >>>+
> >>>+    ret = i915_gem_object_set_to_gtt_domain(obj, write);
> >>
> >>This only needs to pass .write=false. The dma-buf direction is
> >>only for the period of the user access, and we are now flushing the
> >>caches. This is equivalent to the sw-finish ioctl and ideally we just
> >>want the i915_gem_object_flush_cpu_write_domain().
> >
> >in fact the only usage so far I found for end_cpu_access is when the
> >pinned buffer is scanout out. Should I pretty much copy sw-finish in
> >end_cpu_access then?
> 
> And do you think it's okay to declare
> i915_gem_object_flush_cpu_write_domain outside its file's only
> scope?

Whilst the simplicity of just doing the flush appeals, calling
set_gtt_domain(write=false) isn't that much heavier (the difference will
be lost in the noise of any clflushing) and is going to be always correct.
Whereas just flushing the CPU domain may be a hassle for us in future.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre