| Message ID | 20201125143424.1434-1-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/radeon: fix check order in radeon_bo_move | expand |
Ping, Dave this is another fix for the Multihop patch set. Without it radeon is completely broken on drm-misc-next. Thanks, Christian. Am 25.11.20 um 15:34 schrieb Christian König: > Reorder the code to fix checking if blitting is available. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/radeon/radeon_ttm.c | 54 +++++++++++++---------------- > 1 file changed, 24 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c > index 0ca381b95d3d..2b598141225f 100644 > --- a/drivers/gpu/drm/radeon/radeon_ttm.c > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c > @@ -216,27 +216,15 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > struct ttm_resource *old_mem = &bo->mem; > int r; > > - if ((old_mem->mem_type == TTM_PL_SYSTEM && > - new_mem->mem_type == TTM_PL_VRAM) || > - (old_mem->mem_type == TTM_PL_VRAM && > - new_mem->mem_type == TTM_PL_SYSTEM)) { > - hop->fpfn = 0; > - hop->lpfn = 0; > - hop->mem_type = TTM_PL_TT; > - hop->flags = 0; > - return -EMULTIHOP; > - } > - > if (new_mem->mem_type == TTM_PL_TT) { > r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem); > if (r) > return r; > } > - radeon_bo_move_notify(bo, evict, new_mem); > > r = ttm_bo_wait_ctx(bo, ctx); > if (r) > - goto fail; > + return r; > > /* Can't move a pinned BO */ > rbo = container_of(bo, struct radeon_bo, tbo); > @@ -246,12 +234,12 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > rdev = radeon_get_rdev(bo->bdev); > if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { > ttm_bo_move_null(bo, new_mem); > - return 0; > + goto out; > } > if (old_mem->mem_type == TTM_PL_SYSTEM && > new_mem->mem_type == TTM_PL_TT) { > ttm_bo_move_null(bo, new_mem); > - return 0; > + goto out; > } > > if (old_mem->mem_type == TTM_PL_TT && > @@ -259,31 +247,37 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > radeon_ttm_tt_unbind(bo->bdev, bo->ttm); > ttm_resource_free(bo, &bo->mem); > ttm_bo_assign_mem(bo, new_mem); > - return 0; > + goto out; > } > - if (!rdev->ring[radeon_copy_ring_index(rdev)].ready || > - rdev->asic->copy.copy == NULL) { > - /* use memcpy */ > - goto memcpy; > + if (rdev->ring[radeon_copy_ring_index(rdev)].ready && > + rdev->asic->copy.copy != NULL) { > + if ((old_mem->mem_type == TTM_PL_SYSTEM && > + new_mem->mem_type == TTM_PL_VRAM) || > + (old_mem->mem_type == TTM_PL_VRAM && > + new_mem->mem_type == TTM_PL_SYSTEM)) { > + hop->fpfn = 0; > + hop->lpfn = 0; > + hop->mem_type = TTM_PL_TT; > + hop->flags = 0; > + return -EMULTIHOP; > + } > + > + r = radeon_move_blit(bo, evict, new_mem, old_mem); > + } else { > + r = -ENODEV; > } > > - r = radeon_move_blit(bo, evict, new_mem, old_mem); > if (r) { > -memcpy: > r = ttm_bo_move_memcpy(bo, ctx, new_mem); > - if (r) { > - goto fail; > - } > + if (r) > + return r; > } > > +out: > /* update statistics */ > atomic64_add((u64)bo->num_pages << PAGE_SHIFT, &rdev->num_bytes_moved); > + radeon_bo_move_notify(bo, evict, new_mem); > return 0; > -fail: > - swap(*new_mem, bo->mem); > - radeon_bo_move_notify(bo, false, new_mem); > - swap(*new_mem, bo->mem); > - return r; > } > > static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resource *mem)
On Wed, Nov 25, 2020 at 3:34 PM Christian König <ckoenig.leichtzumerken@gmail.com> wrote: > > Reorder the code to fix checking if blitting is available. Might be good to explain why blitting might not be available, e.g. suspend/resume and or chip death and stuff like that. > Signed-off-by: Christian König <christian.koenig@amd.com> Needs Fixes: 28a68f828266 ("drm/radeon/ttm: use multihop") Btw $ dim fixes [sha1] generates that for you plus nice cc list of offenders. With the Fixes line added: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> At least I'm hanging onto the illusion that I understand what you did here :-) -Daniel > --- > drivers/gpu/drm/radeon/radeon_ttm.c | 54 +++++++++++++---------------- > 1 file changed, 24 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c > index 0ca381b95d3d..2b598141225f 100644 > --- a/drivers/gpu/drm/radeon/radeon_ttm.c > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c > @@ -216,27 +216,15 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > struct ttm_resource *old_mem = &bo->mem; > int r; > > - if ((old_mem->mem_type == TTM_PL_SYSTEM && > - new_mem->mem_type == TTM_PL_VRAM) || > - (old_mem->mem_type == TTM_PL_VRAM && > - new_mem->mem_type == TTM_PL_SYSTEM)) { > - hop->fpfn = 0; > - hop->lpfn = 0; > - hop->mem_type = TTM_PL_TT; > - hop->flags = 0; > - return -EMULTIHOP; > - } > - > if (new_mem->mem_type == TTM_PL_TT) { > r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem); > if (r) > return r; > } > - radeon_bo_move_notify(bo, evict, new_mem); > > r = ttm_bo_wait_ctx(bo, ctx); > if (r) > - goto fail; > + return r; > > /* Can't move a pinned BO */ > rbo = container_of(bo, struct radeon_bo, tbo); > @@ -246,12 +234,12 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > rdev = radeon_get_rdev(bo->bdev); > if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { > ttm_bo_move_null(bo, new_mem); > - return 0; > + goto out; > } > if (old_mem->mem_type == TTM_PL_SYSTEM && > new_mem->mem_type == TTM_PL_TT) { > ttm_bo_move_null(bo, new_mem); > - return 0; > + goto out; > } > > if (old_mem->mem_type == TTM_PL_TT && > @@ -259,31 +247,37 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > radeon_ttm_tt_unbind(bo->bdev, bo->ttm); > ttm_resource_free(bo, &bo->mem); > ttm_bo_assign_mem(bo, new_mem); > - return 0; > + goto out; > } > - if (!rdev->ring[radeon_copy_ring_index(rdev)].ready || > - rdev->asic->copy.copy == NULL) { > - /* use memcpy */ > - goto memcpy; > + if (rdev->ring[radeon_copy_ring_index(rdev)].ready && > + rdev->asic->copy.copy != NULL) { > + if ((old_mem->mem_type == TTM_PL_SYSTEM && > + new_mem->mem_type == TTM_PL_VRAM) || > + (old_mem->mem_type == TTM_PL_VRAM && > + new_mem->mem_type == TTM_PL_SYSTEM)) { > + hop->fpfn = 0; > + hop->lpfn = 0; > + hop->mem_type = TTM_PL_TT; > + hop->flags = 0; > + return -EMULTIHOP; > + } > + > + r = radeon_move_blit(bo, evict, new_mem, old_mem); > + } else { > + r = -ENODEV; > } > > - r = radeon_move_blit(bo, evict, new_mem, old_mem); > if (r) { > -memcpy: > r = ttm_bo_move_memcpy(bo, ctx, new_mem); > - if (r) { > - goto fail; > - } > + if (r) > + return r; > } > > +out: > /* update statistics */ > atomic64_add((u64)bo->num_pages << PAGE_SHIFT, &rdev->num_bytes_moved); > + radeon_bo_move_notify(bo, evict, new_mem); > return 0; > -fail: > - swap(*new_mem, bo->mem); > - radeon_bo_move_notify(bo, false, new_mem); > - swap(*new_mem, bo->mem); > - return r; > } > > static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resource *mem) > -- > 2.25.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Oops sorry for delay LGTM Reviewed-by: Dave Airlie <airlied@redhat.com> On Fri, 27 Nov 2020 at 02:34, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Wed, Nov 25, 2020 at 3:34 PM Christian König > <ckoenig.leichtzumerken@gmail.com> wrote: > > > > Reorder the code to fix checking if blitting is available. > > Might be good to explain why blitting might not be available, e.g. > suspend/resume and or chip death and stuff like that. > > > Signed-off-by: Christian König <christian.koenig@amd.com> > > Needs Fixes: 28a68f828266 ("drm/radeon/ttm: use multihop") > > Btw > > $ dim fixes [sha1] > > generates that for you plus nice cc list of offenders. With the Fixes > line added: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > At least I'm hanging onto the illusion that I understand what you did here :-) > -Daniel > > --- > > drivers/gpu/drm/radeon/radeon_ttm.c | 54 +++++++++++++---------------- > > 1 file changed, 24 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c > > index 0ca381b95d3d..2b598141225f 100644 > > --- a/drivers/gpu/drm/radeon/radeon_ttm.c > > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c > > @@ -216,27 +216,15 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > > struct ttm_resource *old_mem = &bo->mem; > > int r; > > > > - if ((old_mem->mem_type == TTM_PL_SYSTEM && > > - new_mem->mem_type == TTM_PL_VRAM) || > > - (old_mem->mem_type == TTM_PL_VRAM && > > - new_mem->mem_type == TTM_PL_SYSTEM)) { > > - hop->fpfn = 0; > > - hop->lpfn = 0; > > - hop->mem_type = TTM_PL_TT; > > - hop->flags = 0; > > - return -EMULTIHOP; > > - } > > - > > if (new_mem->mem_type == TTM_PL_TT) { > > r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem); > > if (r) > > return r; > > } > > - radeon_bo_move_notify(bo, evict, new_mem); > > > > r = ttm_bo_wait_ctx(bo, ctx); > > if (r) > > - goto fail; > > + return r; > > > > /* Can't move a pinned BO */ > > rbo = container_of(bo, struct radeon_bo, tbo); > > @@ -246,12 +234,12 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > > rdev = radeon_get_rdev(bo->bdev); > > if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { > > ttm_bo_move_null(bo, new_mem); > > - return 0; > > + goto out; > > } > > if (old_mem->mem_type == TTM_PL_SYSTEM && > > new_mem->mem_type == TTM_PL_TT) { > > ttm_bo_move_null(bo, new_mem); > > - return 0; > > + goto out; > > } > > > > if (old_mem->mem_type == TTM_PL_TT && > > @@ -259,31 +247,37 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > > radeon_ttm_tt_unbind(bo->bdev, bo->ttm); > > ttm_resource_free(bo, &bo->mem); > > ttm_bo_assign_mem(bo, new_mem); > > - return 0; > > + goto out; > > } > > - if (!rdev->ring[radeon_copy_ring_index(rdev)].ready || > > - rdev->asic->copy.copy == NULL) { > > - /* use memcpy */ > > - goto memcpy; > > + if (rdev->ring[radeon_copy_ring_index(rdev)].ready && > > + rdev->asic->copy.copy != NULL) { > > + if ((old_mem->mem_type == TTM_PL_SYSTEM && > > + new_mem->mem_type == TTM_PL_VRAM) || > > + (old_mem->mem_type == TTM_PL_VRAM && > > + new_mem->mem_type == TTM_PL_SYSTEM)) { > > + hop->fpfn = 0; > > + hop->lpfn = 0; > > + hop->mem_type = TTM_PL_TT; > > + hop->flags = 0; > > + return -EMULTIHOP; > > + } > > + > > + r = radeon_move_blit(bo, evict, new_mem, old_mem); > > + } else { > > + r = -ENODEV; > > } > > > > - r = radeon_move_blit(bo, evict, new_mem, old_mem); > > if (r) { > > -memcpy: > > r = ttm_bo_move_memcpy(bo, ctx, new_mem); > > - if (r) { > > - goto fail; > > - } > > + if (r) > > + return r; > > } > > > > +out: > > /* update statistics */ > > atomic64_add((u64)bo->num_pages << PAGE_SHIFT, &rdev->num_bytes_moved); > > + radeon_bo_move_notify(bo, evict, new_mem); > > return 0; > > -fail: > > - swap(*new_mem, bo->mem); > > - radeon_bo_move_notify(bo, false, new_mem); > > - swap(*new_mem, bo->mem); > > - return r; > > } > > > > static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resource *mem) > > -- > > 2.25.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Am 27.11.20 um 09:31 schrieb Dave Airlie: > Oops sorry for delay LGTM > > Reviewed-by: Dave Airlie <airlied@redhat.com> Thanks. > > On Fri, 27 Nov 2020 at 02:34, Daniel Vetter <daniel@ffwll.ch> wrote: >> On Wed, Nov 25, 2020 at 3:34 PM Christian König >> <ckoenig.leichtzumerken@gmail.com> wrote: >>> Reorder the code to fix checking if blitting is available. >> Might be good to explain why blitting might not be available, e.g. >> suspend/resume and or chip death and stuff like that. >> >>> Signed-off-by: Christian König <christian.koenig@amd.com> >> Needs Fixes: 28a68f828266 ("drm/radeon/ttm: use multihop") Why does the subject of the patch needs to be in "()" ? I was already wondering why dim sometimes complains about my Fixes tag. >> >> Btw >> >> $ dim fixes [sha1] >> >> generates that for you plus nice cc list of offenders. With the Fixes >> line added: >> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Thanks, Christian. >> >> At least I'm hanging onto the illusion that I understand what you did here :-) >> -Daniel >>> --- >>> drivers/gpu/drm/radeon/radeon_ttm.c | 54 +++++++++++++---------------- >>> 1 file changed, 24 insertions(+), 30 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c >>> index 0ca381b95d3d..2b598141225f 100644 >>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c >>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c >>> @@ -216,27 +216,15 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, >>> struct ttm_resource *old_mem = &bo->mem; >>> int r; >>> >>> - if ((old_mem->mem_type == TTM_PL_SYSTEM && >>> - new_mem->mem_type == TTM_PL_VRAM) || >>> - (old_mem->mem_type == TTM_PL_VRAM && >>> - new_mem->mem_type == TTM_PL_SYSTEM)) { >>> - hop->fpfn = 0; >>> - hop->lpfn = 0; >>> - hop->mem_type = TTM_PL_TT; >>> - hop->flags = 0; >>> - return -EMULTIHOP; >>> - } >>> - >>> if (new_mem->mem_type == TTM_PL_TT) { >>> r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem); >>> if (r) >>> return r; >>> } >>> - radeon_bo_move_notify(bo, evict, new_mem); >>> >>> r = ttm_bo_wait_ctx(bo, ctx); >>> if (r) >>> - goto fail; >>> + return r; >>> >>> /* Can't move a pinned BO */ >>> rbo = container_of(bo, struct radeon_bo, tbo); >>> @@ -246,12 +234,12 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, >>> rdev = radeon_get_rdev(bo->bdev); >>> if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { >>> ttm_bo_move_null(bo, new_mem); >>> - return 0; >>> + goto out; >>> } >>> if (old_mem->mem_type == TTM_PL_SYSTEM && >>> new_mem->mem_type == TTM_PL_TT) { >>> ttm_bo_move_null(bo, new_mem); >>> - return 0; >>> + goto out; >>> } >>> >>> if (old_mem->mem_type == TTM_PL_TT && >>> @@ -259,31 +247,37 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, >>> radeon_ttm_tt_unbind(bo->bdev, bo->ttm); >>> ttm_resource_free(bo, &bo->mem); >>> ttm_bo_assign_mem(bo, new_mem); >>> - return 0; >>> + goto out; >>> } >>> - if (!rdev->ring[radeon_copy_ring_index(rdev)].ready || >>> - rdev->asic->copy.copy == NULL) { >>> - /* use memcpy */ >>> - goto memcpy; >>> + if (rdev->ring[radeon_copy_ring_index(rdev)].ready && >>> + rdev->asic->copy.copy != NULL) { >>> + if ((old_mem->mem_type == TTM_PL_SYSTEM && >>> + new_mem->mem_type == TTM_PL_VRAM) || >>> + (old_mem->mem_type == TTM_PL_VRAM && >>> + new_mem->mem_type == TTM_PL_SYSTEM)) { >>> + hop->fpfn = 0; >>> + hop->lpfn = 0; >>> + hop->mem_type = TTM_PL_TT; >>> + hop->flags = 0; >>> + return -EMULTIHOP; >>> + } >>> + >>> + r = radeon_move_blit(bo, evict, new_mem, old_mem); >>> + } else { >>> + r = -ENODEV; >>> } >>> >>> - r = radeon_move_blit(bo, evict, new_mem, old_mem); >>> if (r) { >>> -memcpy: >>> r = ttm_bo_move_memcpy(bo, ctx, new_mem); >>> - if (r) { >>> - goto fail; >>> - } >>> + if (r) >>> + return r; >>> } >>> >>> +out: >>> /* update statistics */ >>> atomic64_add((u64)bo->num_pages << PAGE_SHIFT, &rdev->num_bytes_moved); >>> + radeon_bo_move_notify(bo, evict, new_mem); >>> return 0; >>> -fail: >>> - swap(*new_mem, bo->mem); >>> - radeon_bo_move_notify(bo, false, new_mem); >>> - swap(*new_mem, bo->mem); >>> - return r; >>> } >>> >>> static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resource *mem) >>> -- >>> 2.25.1 >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> http://blog.ffwll.ch >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Fri, Nov 27, 2020 at 3:10 PM Christian König <ckoenig.leichtzumerken@gmail.com> wrote: > > Am 27.11.20 um 09:31 schrieb Dave Airlie: > > Oops sorry for delay LGTM > > > > Reviewed-by: Dave Airlie <airlied@redhat.com> > > Thanks. > > > > > On Fri, 27 Nov 2020 at 02:34, Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Wed, Nov 25, 2020 at 3:34 PM Christian König > >> <ckoenig.leichtzumerken@gmail.com> wrote: > >>> Reorder the code to fix checking if blitting is available. > >> Might be good to explain why blitting might not be available, e.g. > >> suspend/resume and or chip death and stuff like that. > >> > >>> Signed-off-by: Christian König <christian.koenig@amd.com> > >> Needs Fixes: 28a68f828266 ("drm/radeon/ttm: use multihop") > > Why does the subject of the patch needs to be in "()" ? I was already > wondering why dim sometimes complains about my Fixes tag. Hm I thought that's the official style. I kinda hacked around on it until linux-next stopped complaining about our Fixes: tags. Maybe it's not quite accurately reflecting the current bikeshed. Iirc checkpatch even complains when you leave out the commit before the sha1, at least in free-form text in the commit message. -Daniel > >> > >> Btw > >> > >> $ dim fixes [sha1] > >> > >> generates that for you plus nice cc list of offenders. With the Fixes > >> line added: > >> > >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Thanks, > Christian. > > >> > >> At least I'm hanging onto the illusion that I understand what you did here :-) > >> -Daniel > >>> --- > >>> drivers/gpu/drm/radeon/radeon_ttm.c | 54 +++++++++++++---------------- > >>> 1 file changed, 24 insertions(+), 30 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c > >>> index 0ca381b95d3d..2b598141225f 100644 > >>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c > >>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c > >>> @@ -216,27 +216,15 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > >>> struct ttm_resource *old_mem = &bo->mem; > >>> int r; > >>> > >>> - if ((old_mem->mem_type == TTM_PL_SYSTEM && > >>> - new_mem->mem_type == TTM_PL_VRAM) || > >>> - (old_mem->mem_type == TTM_PL_VRAM && > >>> - new_mem->mem_type == TTM_PL_SYSTEM)) { > >>> - hop->fpfn = 0; > >>> - hop->lpfn = 0; > >>> - hop->mem_type = TTM_PL_TT; > >>> - hop->flags = 0; > >>> - return -EMULTIHOP; > >>> - } > >>> - > >>> if (new_mem->mem_type == TTM_PL_TT) { > >>> r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem); > >>> if (r) > >>> return r; > >>> } > >>> - radeon_bo_move_notify(bo, evict, new_mem); > >>> > >>> r = ttm_bo_wait_ctx(bo, ctx); > >>> if (r) > >>> - goto fail; > >>> + return r; > >>> > >>> /* Can't move a pinned BO */ > >>> rbo = container_of(bo, struct radeon_bo, tbo); > >>> @@ -246,12 +234,12 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > >>> rdev = radeon_get_rdev(bo->bdev); > >>> if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { > >>> ttm_bo_move_null(bo, new_mem); > >>> - return 0; > >>> + goto out; > >>> } > >>> if (old_mem->mem_type == TTM_PL_SYSTEM && > >>> new_mem->mem_type == TTM_PL_TT) { > >>> ttm_bo_move_null(bo, new_mem); > >>> - return 0; > >>> + goto out; > >>> } > >>> > >>> if (old_mem->mem_type == TTM_PL_TT && > >>> @@ -259,31 +247,37 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, > >>> radeon_ttm_tt_unbind(bo->bdev, bo->ttm); > >>> ttm_resource_free(bo, &bo->mem); > >>> ttm_bo_assign_mem(bo, new_mem); > >>> - return 0; > >>> + goto out; > >>> } > >>> - if (!rdev->ring[radeon_copy_ring_index(rdev)].ready || > >>> - rdev->asic->copy.copy == NULL) { > >>> - /* use memcpy */ > >>> - goto memcpy; > >>> + if (rdev->ring[radeon_copy_ring_index(rdev)].ready && > >>> + rdev->asic->copy.copy != NULL) { > >>> + if ((old_mem->mem_type == TTM_PL_SYSTEM && > >>> + new_mem->mem_type == TTM_PL_VRAM) || > >>> + (old_mem->mem_type == TTM_PL_VRAM && > >>> + new_mem->mem_type == TTM_PL_SYSTEM)) { > >>> + hop->fpfn = 0; > >>> + hop->lpfn = 0; > >>> + hop->mem_type = TTM_PL_TT; > >>> + hop->flags = 0; > >>> + return -EMULTIHOP; > >>> + } > >>> + > >>> + r = radeon_move_blit(bo, evict, new_mem, old_mem); > >>> + } else { > >>> + r = -ENODEV; > >>> } > >>> > >>> - r = radeon_move_blit(bo, evict, new_mem, old_mem); > >>> if (r) { > >>> -memcpy: > >>> r = ttm_bo_move_memcpy(bo, ctx, new_mem); > >>> - if (r) { > >>> - goto fail; > >>> - } > >>> + if (r) > >>> + return r; > >>> } > >>> > >>> +out: > >>> /* update statistics */ > >>> atomic64_add((u64)bo->num_pages << PAGE_SHIFT, &rdev->num_bytes_moved); > >>> + radeon_bo_move_notify(bo, evict, new_mem); > >>> return 0; > >>> -fail: > >>> - swap(*new_mem, bo->mem); > >>> - radeon_bo_move_notify(bo, false, new_mem); > >>> - swap(*new_mem, bo->mem); > >>> - return r; > >>> } > >>> > >>> static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resource *mem) > >>> -- > >>> 2.25.1 > >>> > >>> _______________________________________________ > >>> dri-devel mailing list > >>> dri-devel@lists.freedesktop.org > >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >> > >> > >> -- > >> Daniel Vetter > >> Software Engineer, Intel Corporation > >> http://blog.ffwll.ch > >> _______________________________________________ > >> amd-gfx mailing list > >> amd-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >
Am 27.11.20 um 15:46 schrieb Daniel Vetter: > On Fri, Nov 27, 2020 at 3:10 PM Christian König > <ckoenig.leichtzumerken@gmail.com> wrote: >> Am 27.11.20 um 09:31 schrieb Dave Airlie: >>> Oops sorry for delay LGTM >>> >>> Reviewed-by: Dave Airlie <airlied@redhat.com> >> Thanks. >> >>> On Fri, 27 Nov 2020 at 02:34, Daniel Vetter <daniel@ffwll.ch> wrote: >>>> On Wed, Nov 25, 2020 at 3:34 PM Christian König >>>> <ckoenig.leichtzumerken@gmail.com> wrote: >>>>> Reorder the code to fix checking if blitting is available. >>>> Might be good to explain why blitting might not be available, e.g. >>>> suspend/resume and or chip death and stuff like that. >>>> >>>>> Signed-off-by: Christian König <christian.koenig@amd.com> >>>> Needs Fixes: 28a68f828266 ("drm/radeon/ttm: use multihop") >> Why does the subject of the patch needs to be in "()" ? I was already >> wondering why dim sometimes complains about my Fixes tag. > Hm I thought that's the official style. I kinda hacked around on it > until linux-next stopped complaining about our Fixes: tags. Maybe it's > not quite accurately reflecting the current bikeshed. Iirc checkpatch > even complains when you leave out the commit before the sha1, at least > in free-form text in the commit message. Well "git log -1 --oneline 28a68f828266" gives me: 28a68f828266 drm/radeon/ttm: use multihop Which is what I would naturally expect here, but no idea what the official format should be. Christian. > -Daniel > >>>> Btw >>>> >>>> $ dim fixes [sha1] >>>> >>>> generates that for you plus nice cc list of offenders. With the Fixes >>>> line added: >>>> >>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> Thanks, >> Christian. >>
On Fri, Nov 27, 2020 at 03:49:31PM +0100, Christian König wrote: > Am 27.11.20 um 15:46 schrieb Daniel Vetter: > > On Fri, Nov 27, 2020 at 3:10 PM Christian König > > <ckoenig.leichtzumerken@gmail.com> wrote: > > > Am 27.11.20 um 09:31 schrieb Dave Airlie: > > > > Oops sorry for delay LGTM > > > > > > > > Reviewed-by: Dave Airlie <airlied@redhat.com> > > > Thanks. > > > > > > > On Fri, 27 Nov 2020 at 02:34, Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > On Wed, Nov 25, 2020 at 3:34 PM Christian König > > > > > <ckoenig.leichtzumerken@gmail.com> wrote: > > > > > > Reorder the code to fix checking if blitting is available. > > > > > Might be good to explain why blitting might not be available, e.g. > > > > > suspend/resume and or chip death and stuff like that. > > > > > > > > > > > Signed-off-by: Christian König <christian.koenig@amd.com> > > > > > Needs Fixes: 28a68f828266 ("drm/radeon/ttm: use multihop") > > > Why does the subject of the patch needs to be in "()" ? I was already > > > wondering why dim sometimes complains about my Fixes tag. > > Hm I thought that's the official style. I kinda hacked around on it > > until linux-next stopped complaining about our Fixes: tags. Maybe it's > > not quite accurately reflecting the current bikeshed. Iirc checkpatch > > even complains when you leave out the commit before the sha1, at least > > in free-form text in the commit message. > > Well "git log -1 --oneline 28a68f828266" gives me: > > 28a68f828266 drm/radeon/ttm: use multihop > > Which is what I would naturally expect here, but no idea what the official > format should be. dim cite $sha1 is our attempt at modelling it. And yeah it's just bikeshedded differently for no good reason. And I just noticed that dim cite doesn't include the commit before the sha1. -Daniel > > Christian. > > > -Daniel > > > > > > > Btw > > > > > > > > > > $ dim fixes [sha1] > > > > > > > > > > generates that for you plus nice cc list of offenders. With the Fixes > > > > > line added: > > > > > > > > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > Thanks, > > > Christian. > > > >
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 0ca381b95d3d..2b598141225f 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -216,27 +216,15 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, struct ttm_resource *old_mem = &bo->mem; int r; - if ((old_mem->mem_type == TTM_PL_SYSTEM && - new_mem->mem_type == TTM_PL_VRAM) || - (old_mem->mem_type == TTM_PL_VRAM && - new_mem->mem_type == TTM_PL_SYSTEM)) { - hop->fpfn = 0; - hop->lpfn = 0; - hop->mem_type = TTM_PL_TT; - hop->flags = 0; - return -EMULTIHOP; - } - if (new_mem->mem_type == TTM_PL_TT) { r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem); if (r) return r; } - radeon_bo_move_notify(bo, evict, new_mem); r = ttm_bo_wait_ctx(bo, ctx); if (r) - goto fail; + return r; /* Can't move a pinned BO */ rbo = container_of(bo, struct radeon_bo, tbo); @@ -246,12 +234,12 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, rdev = radeon_get_rdev(bo->bdev); if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { ttm_bo_move_null(bo, new_mem); - return 0; + goto out; } if (old_mem->mem_type == TTM_PL_SYSTEM && new_mem->mem_type == TTM_PL_TT) { ttm_bo_move_null(bo, new_mem); - return 0; + goto out; } if (old_mem->mem_type == TTM_PL_TT && @@ -259,31 +247,37 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, radeon_ttm_tt_unbind(bo->bdev, bo->ttm); ttm_resource_free(bo, &bo->mem); ttm_bo_assign_mem(bo, new_mem); - return 0; + goto out; } - if (!rdev->ring[radeon_copy_ring_index(rdev)].ready || - rdev->asic->copy.copy == NULL) { - /* use memcpy */ - goto memcpy; + if (rdev->ring[radeon_copy_ring_index(rdev)].ready && + rdev->asic->copy.copy != NULL) { + if ((old_mem->mem_type == TTM_PL_SYSTEM && + new_mem->mem_type == TTM_PL_VRAM) || + (old_mem->mem_type == TTM_PL_VRAM && + new_mem->mem_type == TTM_PL_SYSTEM)) { + hop->fpfn = 0; + hop->lpfn = 0; + hop->mem_type = TTM_PL_TT; + hop->flags = 0; + return -EMULTIHOP; + } + + r = radeon_move_blit(bo, evict, new_mem, old_mem); + } else { + r = -ENODEV; } - r = radeon_move_blit(bo, evict, new_mem, old_mem); if (r) { -memcpy: r = ttm_bo_move_memcpy(bo, ctx, new_mem); - if (r) { - goto fail; - } + if (r) + return r; } +out: /* update statistics */ atomic64_add((u64)bo->num_pages << PAGE_SHIFT, &rdev->num_bytes_moved); + radeon_bo_move_notify(bo, evict, new_mem); return 0; -fail: - swap(*new_mem, bo->mem); - radeon_bo_move_notify(bo, false, new_mem); - swap(*new_mem, bo->mem); - return r; } static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resource *mem)
Reorder the code to fix checking if blitting is available. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/radeon/radeon_ttm.c | 54 +++++++++++++---------------- 1 file changed, 24 insertions(+), 30 deletions(-)