[Intel-gfx] [PATCH 07/10] drm/i915: Support for pread/pwrite from/to non shmem backed objects

[Tvrtko Ursulin]

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...

Regards,

Tvrtko