Re: [PATCH] drm/radeon: fix check order in radeon_bo_move
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 > > wrote: > > > Am 27.11.20 um 09:31 schrieb Dave Airlie: > > > > Oops sorry for delay LGTM > > > > > > > > Reviewed-by: Dave Airlie > > > Thanks. > > > > > > > On Fri, 27 Nov 2020 at 02:34, Daniel Vetter wrote: > > > > > On Wed, Nov 25, 2020 at 3:34 PM Christian König > > > > > 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 > > > > > 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 > > > Thanks, > > > Christian. > > > > -- 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
Re: [PATCH] drm/radeon: fix check order in radeon_bo_move
Am 27.11.20 um 15:46 schrieb Daniel Vetter: On Fri, Nov 27, 2020 at 3:10 PM Christian König wrote: Am 27.11.20 um 09:31 schrieb Dave Airlie: Oops sorry for delay LGTM Reviewed-by: Dave Airlie Thanks. On Fri, 27 Nov 2020 at 02:34, Daniel Vetter wrote: On Wed, Nov 25, 2020 at 3:34 PM Christian König 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 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 Thanks, Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/radeon: fix check order in radeon_bo_move
On Fri, Nov 27, 2020 at 3:10 PM Christian König wrote: > > Am 27.11.20 um 09:31 schrieb Dave Airlie: > > Oops sorry for delay LGTM > > > > Reviewed-by: Dave Airlie > > Thanks. > > > > > On Fri, 27 Nov 2020 at 02:34, Daniel Vetter wrote: > >> On Wed, Nov 25, 2020 at 3:34 PM Christian König > >> 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 > >> 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 > > 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 = >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, >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 =
Re: [PATCH] drm/radeon: fix check order in radeon_bo_move
Am 27.11.20 um 09:31 schrieb Dave Airlie: Oops sorry for delay LGTM Reviewed-by: Dave Airlie Thanks. On Fri, 27 Nov 2020 at 02:34, Daniel Vetter wrote: On Wed, Nov 25, 2020 at 3:34 PM Christian König 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 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 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 = >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, >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, >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-de...@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
Re: [PATCH] drm/radeon: fix check order in radeon_bo_move
Oops sorry for delay LGTM Reviewed-by: Dave Airlie On Fri, 27 Nov 2020 at 02:34, Daniel Vetter wrote: > > On Wed, Nov 25, 2020 at 3:34 PM Christian König > 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 > > 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 > > 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 = >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, >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, > > >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 >
Re: [PATCH] drm/radeon: fix check order in radeon_bo_move
On Wed, Nov 25, 2020 at 3:34 PM Christian König 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 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 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 = >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, >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, > >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-de...@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
Re: [PATCH] drm/radeon: fix check order in radeon_bo_move
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 --- 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 = >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, >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, >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) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/radeon: fix check order in radeon_bo_move
Reorder the code to fix checking if blitting is available. Signed-off-by: Christian König --- 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 = >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, >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, >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 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx