| Message ID | 20221014081553.114899-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/sched: add DRM_SCHED_FENCE_DONT_PIPELINE flag | expand |
Hi Christian, On 2022-10-14 04:15, Christian König wrote: > Make sure that we always have a CPU round trip to let the submission > code correctly decide if a TLB flush is necessary or not. > > Signed-off-by: Christian König <christian.koenig@amd.com> > CC: stable@vger.kernel.org # 5.19+ > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c > index 2b0669c464f6..69e105fa41f6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c > @@ -116,8 +116,15 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p, > DMA_RESV_USAGE_BOOKKEEP); > } > > - if (fence && !p->immediate) > + if (fence && !p->immediate) { > + /* > + * Most hw generations now have a separate queue for page table > + * updates, but when the queue is shared with userspace we need > + * the extra CPU round trip to correctly flush the TLB. > + */ > + set_bit(DRM_SCHED_FENCE_DONT_PIPELINE, &f->flags); > swap(*fence, f); > + } Do you ever turn that bit off? Regards Luben > dma_fence_put(f); > return 0; >
Am 17.10.22 um 07:29 schrieb Luben Tuikov: > Hi Christian, > > On 2022-10-14 04:15, Christian König wrote: >> Make sure that we always have a CPU round trip to let the submission >> code correctly decide if a TLB flush is necessary or not. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> CC: stable@vger.kernel.org # 5.19+ >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c >> index 2b0669c464f6..69e105fa41f6 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c >> @@ -116,8 +116,15 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p, >> DMA_RESV_USAGE_BOOKKEEP); >> } >> >> - if (fence && !p->immediate) >> + if (fence && !p->immediate) { >> + /* >> + * Most hw generations now have a separate queue for page table >> + * updates, but when the queue is shared with userspace we need >> + * the extra CPU round trip to correctly flush the TLB. >> + */ >> + set_bit(DRM_SCHED_FENCE_DONT_PIPELINE, &f->flags); >> swap(*fence, f); >> + } > Do you ever turn that bit off? No, I just rely on the fact that the flags are zero initialized. Regards, Christian. > > Regards > Luben > >> dma_fence_put(f); >> return 0; >>
On 2022-10-17 02:27, Christian König wrote: > Am 17.10.22 um 07:29 schrieb Luben Tuikov: >> Hi Christian, >> >> On 2022-10-14 04:15, Christian König wrote: >>> Make sure that we always have a CPU round trip to let the submission >>> code correctly decide if a TLB flush is necessary or not. >>> >>> Signed-off-by: Christian König <christian.koenig@amd.com> >>> CC: stable@vger.kernel.org # 5.19+ >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 9 ++++++++- >>> 1 file changed, 8 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c >>> index 2b0669c464f6..69e105fa41f6 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c >>> @@ -116,8 +116,15 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p, >>> DMA_RESV_USAGE_BOOKKEEP); >>> } >>> >>> - if (fence && !p->immediate) >>> + if (fence && !p->immediate) { >>> + /* >>> + * Most hw generations now have a separate queue for page table >>> + * updates, but when the queue is shared with userspace we need >>> + * the extra CPU round trip to correctly flush the TLB. >>> + */ >>> + set_bit(DRM_SCHED_FENCE_DONT_PIPELINE, &f->flags); >>> swap(*fence, f); >>> + } >> Do you ever turn that bit off? > > No, I just rely on the fact that the flags are zero initialized. Acked-by: Luben Tuikov <luben.tuikov@amd.com> Regards, Luben
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c index 2b0669c464f6..69e105fa41f6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c @@ -116,8 +116,15 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p, DMA_RESV_USAGE_BOOKKEEP); } - if (fence && !p->immediate) + if (fence && !p->immediate) { + /* + * Most hw generations now have a separate queue for page table + * updates, but when the queue is shared with userspace we need + * the extra CPU round trip to correctly flush the TLB. + */ + set_bit(DRM_SCHED_FENCE_DONT_PIPELINE, &f->flags); swap(*fence, f); + } dma_fence_put(f); return 0;
Make sure that we always have a CPU round trip to let the submission code correctly decide if a TLB flush is necessary or not. Signed-off-by: Christian König <christian.koenig@amd.com> CC: stable@vger.kernel.org # 5.19+ --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)