On pe, 2016-08-12 at 11:28 +0100, Chris Wilson wrote:
> @@ -1717,26 +1716,30 @@ int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
> }
>
> /* Use a partial view if the object is bigger than the aperture. */
Move this comment down to where partial view is actually created.
> - if (obj->base.size >= ggtt->mappable_end &&
> - !i915_gem_object_is_tiled(obj)) {
> + /* Now pin it into the GTT if needed */
> + vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
> + PIN_MAPPABLE | PIN_NONBLOCK);
> + if (IS_ERR(vma)) {
> + struct i915_ggtt_view partial;
'view' still makes more sense, less repeating of the word partial down.
> @@ -1754,26 +1757,7 @@ int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
> pfn = ggtt->mappable_base + i915_ggtt_offset(vma);
> pfn >>= PAGE_SHIFT;
>
> - if (unlikely(view.type == I915_GGTT_VIEW_PARTIAL)) {
> - /* Overriding existing pages in partial view does not cause
> - * us any trouble as TLBs are still valid because the fault
> - * is due to userspace losing part of the mapping or never
> - * having accessed it before (at this partials' range).
> - */
> - unsigned long base = area->vm_start +
> - (view.params.partial.offset << PAGE_SHIFT);
> - unsigned int i;
> -
> - for (i = 0; i < view.params.partial.size; i++) {
> - ret = vm_insert_pfn(area,
> - base + i * PAGE_SIZE,
> - pfn + i);
> - if (ret)
> - break;
> - }
> -
> - obj->fault_mappable = true;
> - } else {
> + if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) {
likely() ?
> if (!obj->fault_mappable) {
> unsigned long size =
> min_t(unsigned long,
> @@ -1789,13 +1773,31 @@ int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
> if (ret)
> break;
> }
> -
> - obj->fault_mappable = true;
> } else
> ret = vm_insert_pfn(area,
> (unsigned long)vmf->virtual_address,
> pfn + page_offset);
> + } else {
> + /* Overriding existing pages in partial view does not cause
> + * us any trouble as TLBs are still valid because the fault
> + * is due to userspace losing part of the mapping or never
> + * having accessed it before (at this partials' range).
> + */
> + const struct i915_ggtt_view *view = &vma->ggtt_view;
I now see why you did the rename. Do not have a better idea really, so;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
