| Message ID | 20230125155023.105584-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/ttm: revert "stop allocating a dummy resource for pipelined gutting" | expand |
On Wed, 25 Jan 2023 at 15:50, Christian König <ckoenig.leichtzumerken@gmail.com> wrote: > > This reverts commit 00984ad39599bb2a1e6ec5d4e9c75a749f7f45c9. > > It seems to still breka i915. We also need to revert the third patch: b49323aa35d5 drm/ttm: prevent moving of pinned BOs It introduces the side effect of no longer calling tt_create(true) in ttm_bo_validate(), and I'm 99% sure that will break object clearing. We rely on having a ttm_tt for the initial dummy placement, with FLAG_ZERO_ALLOC set if clear is needed. Also I'm not sure who even creates the ttm_tt now, if ttm_bo_validate() doesn't, and we don't have the dummy move, like with this patch. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/ttm/ttm_bo.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 33471e363ff4..9baccb2f6e99 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -957,6 +957,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, > struct sg_table *sg, struct dma_resv *resv, > void (*destroy) (struct ttm_buffer_object *)) > { > + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; > int ret; > > kref_init(&bo->kref); > @@ -973,6 +974,12 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, > bo->base.resv = &bo->base._resv; > atomic_inc(&ttm_glob.bo_count); > > + ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); > + if (unlikely(ret)) { > + ttm_bo_put(bo); > + return ret; > + } > + > /* > * For ttm_bo_type_device buffers, allocate > * address space from the device. > -- > 2.34.1 >
Am 25.01.23 um 17:13 schrieb Matthew Auld: > On Wed, 25 Jan 2023 at 15:50, Christian König > <ckoenig.leichtzumerken@gmail.com> wrote: >> This reverts commit 00984ad39599bb2a1e6ec5d4e9c75a749f7f45c9. >> >> It seems to still breka i915. > We also need to revert the third patch: > > b49323aa35d5 drm/ttm: prevent moving of pinned BOs > > It introduces the side effect of no longer calling tt_create(true) in > ttm_bo_validate(), and I'm 99% sure that will break object clearing. > We rely on having a ttm_tt for the initial dummy placement, with > FLAG_ZERO_ALLOC set if clear is needed. Also I'm not sure who even > creates the ttm_tt now, if ttm_bo_validate() doesn't, and we don't > have the dummy move, like with this patch. Oh, yes of course. Can I add your Acked-by to reverting all three? Thanks, Christian. > >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/ttm/ttm_bo.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >> index 33471e363ff4..9baccb2f6e99 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >> @@ -957,6 +957,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, >> struct sg_table *sg, struct dma_resv *resv, >> void (*destroy) (struct ttm_buffer_object *)) >> { >> + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; >> int ret; >> >> kref_init(&bo->kref); >> @@ -973,6 +974,12 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, >> bo->base.resv = &bo->base._resv; >> atomic_inc(&ttm_glob.bo_count); >> >> + ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); >> + if (unlikely(ret)) { >> + ttm_bo_put(bo); >> + return ret; >> + } >> + >> /* >> * For ttm_bo_type_device buffers, allocate >> * address space from the device. >> -- >> 2.34.1 >>
On Wed, 25 Jan 2023 at 16:15, Christian König <christian.koenig@amd.com> wrote: > > Am 25.01.23 um 17:13 schrieb Matthew Auld: > > On Wed, 25 Jan 2023 at 15:50, Christian König > > <ckoenig.leichtzumerken@gmail.com> wrote: > >> This reverts commit 00984ad39599bb2a1e6ec5d4e9c75a749f7f45c9. > >> > >> It seems to still breka i915. > > We also need to revert the third patch: > > > > b49323aa35d5 drm/ttm: prevent moving of pinned BOs > > > > It introduces the side effect of no longer calling tt_create(true) in > > ttm_bo_validate(), and I'm 99% sure that will break object clearing. > > We rely on having a ttm_tt for the initial dummy placement, with > > FLAG_ZERO_ALLOC set if clear is needed. Also I'm not sure who even > > creates the ttm_tt now, if ttm_bo_validate() doesn't, and we don't > > have the dummy move, like with this patch. > > Oh, yes of course. Can I add your Acked-by to reverting all three? Yeah, feel free to add. I can then resend your series with the extra stuff we need for i915. > > Thanks, > Christian. > > > > >> Signed-off-by: Christian König <christian.koenig@amd.com> > >> --- > >> drivers/gpu/drm/ttm/ttm_bo.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > >> index 33471e363ff4..9baccb2f6e99 100644 > >> --- a/drivers/gpu/drm/ttm/ttm_bo.c > >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c > >> @@ -957,6 +957,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, > >> struct sg_table *sg, struct dma_resv *resv, > >> void (*destroy) (struct ttm_buffer_object *)) > >> { > >> + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; > >> int ret; > >> > >> kref_init(&bo->kref); > >> @@ -973,6 +974,12 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, > >> bo->base.resv = &bo->base._resv; > >> atomic_inc(&ttm_glob.bo_count); > >> > >> + ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); > >> + if (unlikely(ret)) { > >> + ttm_bo_put(bo); > >> + return ret; > >> + } > >> + > >> /* > >> * For ttm_bo_type_device buffers, allocate > >> * address space from the device. > >> -- > >> 2.34.1 > >> >
On Wed, 25 Jan 2023 at 16:24, Matthew Auld <matthew.william.auld@gmail.com> wrote: > > On Wed, 25 Jan 2023 at 16:15, Christian König <christian.koenig@amd.com> wrote: > > > > Am 25.01.23 um 17:13 schrieb Matthew Auld: > > > On Wed, 25 Jan 2023 at 15:50, Christian König > > > <ckoenig.leichtzumerken@gmail.com> wrote: > > >> This reverts commit 00984ad39599bb2a1e6ec5d4e9c75a749f7f45c9. > > >> > > >> It seems to still breka i915. > > > We also need to revert the third patch: > > > > > > b49323aa35d5 drm/ttm: prevent moving of pinned BOs > > > > > > It introduces the side effect of no longer calling tt_create(true) in > > > ttm_bo_validate(), and I'm 99% sure that will break object clearing. > > > We rely on having a ttm_tt for the initial dummy placement, with > > > FLAG_ZERO_ALLOC set if clear is needed. Also I'm not sure who even > > > creates the ttm_tt now, if ttm_bo_validate() doesn't, and we don't > > > have the dummy move, like with this patch. > > > > Oh, yes of course. Can I add your Acked-by to reverting all three? > > Yeah, feel free to add. I can then resend your series with the extra > stuff we need for i915. https://patchwork.freedesktop.org/series/113484/ CI appears to be happy now. Feel free to merge the series. > > > > > Thanks, > > Christian. > > > > > > > >> Signed-off-by: Christian König <christian.koenig@amd.com> > > >> --- > > >> drivers/gpu/drm/ttm/ttm_bo.c | 7 +++++++ > > >> 1 file changed, 7 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > > >> index 33471e363ff4..9baccb2f6e99 100644 > > >> --- a/drivers/gpu/drm/ttm/ttm_bo.c > > >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c > > >> @@ -957,6 +957,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, > > >> struct sg_table *sg, struct dma_resv *resv, > > >> void (*destroy) (struct ttm_buffer_object *)) > > >> { > > >> + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; > > >> int ret; > > >> > > >> kref_init(&bo->kref); > > >> @@ -973,6 +974,12 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, > > >> bo->base.resv = &bo->base._resv; > > >> atomic_inc(&ttm_glob.bo_count); > > >> > > >> + ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); > > >> + if (unlikely(ret)) { > > >> + ttm_bo_put(bo); > > >> + return ret; > > >> + } > > >> + > > >> /* > > >> * For ttm_bo_type_device buffers, allocate > > >> * address space from the device. > > >> -- > > >> 2.34.1 > > >> > >
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 33471e363ff4..9baccb2f6e99 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -957,6 +957,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *)) { + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; int ret; kref_init(&bo->kref); @@ -973,6 +974,12 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, bo->base.resv = &bo->base._resv; atomic_inc(&ttm_glob.bo_count); + ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); + if (unlikely(ret)) { + ttm_bo_put(bo); + return ret; + } + /* * For ttm_bo_type_device buffers, allocate * address space from the device.
This reverts commit 00984ad39599bb2a1e6ec5d4e9c75a749f7f45c9. It seems to still breka i915. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/ttm/ttm_bo.c | 7 +++++++ 1 file changed, 7 insertions(+)