Hi Janusz,
...
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 25e97031d76e4..20deb01c0e5fe 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -1595,8 +1595,14 @@ int i915_vma_pin_ww(struct i915_vma *vma, struct
> i915_gem_ww_ctx *ww,
> err_vma_res:
> i915_vma_resource_free(vma_res);
> err_fence:
> - if (work)
> - dma_fence_work_commit_imm(&work->base);
> + if (work) {
> + /* don't risk lockdep splat against stop_machine() */
> + if (i915_vma_is_ggtt(vma) &&
> + intel_vm_no_concurrent_access_wa(vma->vm->i915))
> + dma_fence_work_commit(&work->base);
> + else
> + dma_fence_work_commit_imm(&work->base);
This looks a bit hacky to me. The proper solution would be to fix
the locking order and make sure we do not call stop_machine()
while holding the lock.
In the past we have added plenty of dirty and ugly hacks to
work around locking orders just to make lockdep happy. I would
rather live with a lockdep warning than carry a hack for many
years ahead.
On the other hand, we call stop_machine() for a specific case
caused by a hardware flaw in BXT, so doing a heavy refactoring
might be excessive.
To be honest, I do not have a clean solution in mind, and I do
not feel like either blocking or acking this approach.
It looks like you have achieved consensus here, so I can apply it
if you want (for maintainership's sake), unless someone objects.
Thanks,
Andi
> + }
> err_rpm:
> intel_runtime_pm_put(&vma->vm->i915->runtime_pm, wakeref);
>
> --
> 2.51.0
>
