Op 30-10-2020 om 16:08 schreef Thomas Hellström:
>
> On 10/16/20 12:44 PM, Maarten Lankhorst wrote:
>> Pin in the caller, not in the work itself. This should also
>> work better for dma-fence annotations.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 15 +++++++--------
>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
>> index bc0223716906..daf9284ef1f5 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
>> @@ -27,15 +27,8 @@ static void __do_clflush(struct drm_i915_gem_object *obj)
>> static int clflush_work(struct dma_fence_work *base)
>> {
>> struct clflush *clflush = container_of(base, typeof(*clflush), base);
>> - struct drm_i915_gem_object *obj = clflush->obj;
>> - int err;
>> - err = i915_gem_object_pin_pages(obj);
>> - if (err)
>> - return err;
>> -
>> - __do_clflush(obj);
>> - i915_gem_object_unpin_pages(obj);
>> + __do_clflush(clflush->obj);
>> return 0;
>> }
>> @@ -44,6 +37,7 @@ static void clflush_release(struct dma_fence_work *base)
>> {
>> struct clflush *clflush = container_of(base, typeof(*clflush), base);
>> + i915_gem_object_unpin_pages(clflush->obj);
>
> Hmm, Could we do without pinning here? Pages present are protected first by the object lock, then by the fence?
>
> /Thomas
>
>
>
I felt the least chance of regressions was to copy the existing code. :)
At least until we have completed obj->mm.lock removal, we shouldn't try to be smart with these kind of things.
Likely, we've already pinned the pages, so it's a simple ref increase.
Later on, we can try to be smart, but whenever I tried to change behavior so far, I was forced to hunt down bugs.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
