On Mon, Jan 11, 2016 at 03:11:07PM +0000, Tvrtko Ursulin wrote:
>
> On 11/01/16 14:45, Chris Wilson wrote:
> >On Mon, Jan 11, 2016 at 02:21:33PM +0000, Tvrtko Ursulin wrote:
> >>
> >>On 22/12/15 17:40, Chris Wilson wrote:
> >>>On Tue, Dec 22, 2015 at 11:58:33AM +0000, Tvrtko Ursulin wrote:
> >>>>Maybe:
> >>>>
> >>>> if (!obj->base.filp || cpu_write_needs_clflush(obj))
> >>>> ret = i915_gem_gtt_pwrite_fast(...);
> >>>>
> >>>> if (ret == -EFAULT && !obj->base.filp) {
> >>>> ret = i915_gem_gtt_pwrite_slow(...) /* New function, doing the
> >>>>slow_user_access loop for !filp objects, extracted from
> >>>>gtt_pwrite_fast above. */
> >>>
> >>>The point is that "gtt_pwrite_slow" is going to be preferrable in the
> >>>cases where it is possible. It just wasn't the full fallback patch for
> >>>all objects previously, so we didn't bother to write a partial fallback
> >>>handler.
> >>
> >>Maybe I don't get this - is fast_user_write expected always to fail
> >>for non shmem backed objects? And so revert to the slow_user_path
> >>always and immediately? Because fast_user_write is still the primary
> >>choice for everything.
> >
> >If we already have a GTT mapping available, then WC writes into the
> >object are about as fast as we can get, especially if we don't have
> >direct page access. They also have the benefit of not polluting the
> >cache further - though that maybe a downside as well, in which case
> >pwrite/pread was the wrong interface to use.
> >
> >fast_user_write is no more likely to fail for stolen objs than for
> >shmemfs obj, it is just that we cannot fallback to direct page access
> >for stolen objs and so need a fallback path that writes through the GTT.
> >That fallback path would also be preferrable to falling back from the
> >middle of a GTT write to the direct page paths. The issue was simply
> >that the GTT paths cannot be assumed to be universally available,
> >whereas historically the direct page access paths were. *That* changes,
> >and now we cannot rely on either path being universally available.
>
> So it sounds that we don't need to have code which falls back in the
> middle of the write but could be written cleaner as separate
> helpers?
>
> Because I really dislike that new loop...
What new loop? We don't need a new loop...
i915_gem_gtt_pwrite():
/* Important and exceedingly complex setup/teardown code
* removed for brevity.
*/
for_each_page() {
... get limits of operation in page...
if (fast_gtt_write(##args)) {
/* Beware dragons */
mutex_unlock();
hit_slow_path = 1;
slow_gtt_write(##args);
mutex_lock();
}
}
if (hit_slow_path) {
/* Beware dragons that bite */
ret = i915_gem_object_set_to_gtt_domain(obj, true);
}
Is that not what was written? I take it my telepathy isn't working
again.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
