* [PATCH 2/4] drm/qxl: clean up qxl_bo_move_notify
2021-02-11 13:16 [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify Christian König
@ 2021-02-11 13:16 ` Christian König
2021-02-11 14:53 ` Thomas Zimmermann
2021-02-11 13:16 ` [PATCH 3/4] drm/nouveau: clean up nouveau_bo_move_ntfy Christian König
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2021-02-11 13:16 UTC (permalink / raw)
To: tzimmermann, airlied, daniel, kraxel, zackr, dri-devel
Remove the unused evict parameter and drop swapping bo->mem.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/qxl/qxl_ttm.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index b7f77eb685cb..47afe95d04a1 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -121,7 +121,6 @@ static struct ttm_tt *qxl_ttm_tt_create(struct ttm_buffer_object *bo,
}
static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
- bool evict,
struct ttm_resource *new_mem)
{
struct qxl_bo *qbo;
@@ -144,29 +143,22 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict,
struct ttm_resource *old_mem = &bo->mem;
int ret;
- qxl_bo_move_notify(bo, evict, new_mem);
+ qxl_bo_move_notify(bo, new_mem);
ret = ttm_bo_wait_ctx(bo, ctx);
if (ret)
- goto out;
+ return ret;
if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) {
ttm_bo_move_null(bo, new_mem);
return 0;
}
- ret = ttm_bo_move_memcpy(bo, ctx, new_mem);
-out:
- if (ret) {
- swap(*new_mem, bo->mem);
- qxl_bo_move_notify(bo, false, new_mem);
- swap(*new_mem, bo->mem);
- }
- return ret;
+ return ttm_bo_move_memcpy(bo, ctx, new_mem);
}
static void qxl_bo_delete_mem_notify(struct ttm_buffer_object *bo)
{
- qxl_bo_move_notify(bo, false, NULL);
+ qxl_bo_move_notify(bo, NULL);
}
static struct ttm_device_funcs qxl_bo_driver = {
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] drm/qxl: clean up qxl_bo_move_notify
2021-02-11 13:16 ` [PATCH 2/4] drm/qxl: clean up qxl_bo_move_notify Christian König
@ 2021-02-11 14:53 ` Thomas Zimmermann
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2021-02-11 14:53 UTC (permalink / raw)
To: Christian König, airlied, daniel, kraxel, zackr, dri-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 2035 bytes --]
Hi
Am 11.02.21 um 14:16 schrieb Christian König:
> Remove the unused evict parameter and drop swapping bo->mem.
Could you provide the same patch for vram-helpers as well?
Best regards
Thomas
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/qxl/qxl_ttm.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index b7f77eb685cb..47afe95d04a1 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -121,7 +121,6 @@ static struct ttm_tt *qxl_ttm_tt_create(struct ttm_buffer_object *bo,
> }
>
> static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
> - bool evict,
> struct ttm_resource *new_mem)
> {
> struct qxl_bo *qbo;
> @@ -144,29 +143,22 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict,
> struct ttm_resource *old_mem = &bo->mem;
> int ret;
>
> - qxl_bo_move_notify(bo, evict, new_mem);
> + qxl_bo_move_notify(bo, new_mem);
>
> ret = ttm_bo_wait_ctx(bo, ctx);
> if (ret)
> - goto out;
> + return ret;
>
> if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) {
> ttm_bo_move_null(bo, new_mem);
> return 0;
> }
> - ret = ttm_bo_move_memcpy(bo, ctx, new_mem);
> -out:
> - if (ret) {
> - swap(*new_mem, bo->mem);
> - qxl_bo_move_notify(bo, false, new_mem);
> - swap(*new_mem, bo->mem);
> - }
> - return ret;
> + return ttm_bo_move_memcpy(bo, ctx, new_mem);
> }
>
> static void qxl_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> {
> - qxl_bo_move_notify(bo, false, NULL);
> + qxl_bo_move_notify(bo, NULL);
> }
>
> static struct ttm_device_funcs qxl_bo_driver = {
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] drm/nouveau: clean up nouveau_bo_move_ntfy
2021-02-11 13:16 [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify Christian König
2021-02-11 13:16 ` [PATCH 2/4] drm/qxl: clean up qxl_bo_move_notify Christian König
@ 2021-02-11 13:16 ` Christian König
2021-02-11 13:16 ` [PATCH 4/4] drm/vmwgfx: clean up vmw_move_notify Christian König
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2021-02-11 13:16 UTC (permalink / raw)
To: tzimmermann, airlied, daniel, kraxel, zackr, dri-devel
Just another leftover from a TTM cleanup.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index ca2a8ae1938e..9bb8cee3df40 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -861,9 +861,8 @@ nouveau_bo_move_init(struct nouveau_drm *drm)
NV_INFO(drm, "MM: using %s for buffer copies\n", name);
}
-static void
-nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
- struct ttm_resource *new_reg)
+static void nouveau_bo_move_ntfy(struct ttm_buffer_object *bo,
+ struct ttm_resource *new_reg)
{
struct nouveau_mem *mem = new_reg ? nouveau_mem(new_reg) : NULL;
struct nouveau_bo *nvbo = nouveau_bo(bo);
@@ -949,7 +948,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
return ret;
}
- nouveau_bo_move_ntfy(bo, evict, new_reg);
+ nouveau_bo_move_ntfy(bo, new_reg);
ret = ttm_bo_wait_ctx(bo, ctx);
if (ret)
goto out_ntfy;
@@ -1014,9 +1013,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
}
out_ntfy:
if (ret) {
- swap(*new_reg, bo->mem);
- nouveau_bo_move_ntfy(bo, false, new_reg);
- swap(*new_reg, bo->mem);
+ nouveau_bo_move_ntfy(bo, &bo->mem);
}
return ret;
}
@@ -1290,7 +1287,7 @@ nouveau_bo_fence(struct nouveau_bo *nvbo, struct nouveau_fence *fence, bool excl
static void
nouveau_bo_delete_mem_notify(struct ttm_buffer_object *bo)
{
- nouveau_bo_move_ntfy(bo, false, NULL);
+ nouveau_bo_move_ntfy(bo, NULL);
}
struct ttm_device_funcs nouveau_bo_driver = {
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] drm/vmwgfx: clean up vmw_move_notify
2021-02-11 13:16 [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify Christian König
2021-02-11 13:16 ` [PATCH 2/4] drm/qxl: clean up qxl_bo_move_notify Christian König
2021-02-11 13:16 ` [PATCH 3/4] drm/nouveau: clean up nouveau_bo_move_ntfy Christian König
@ 2021-02-11 13:16 ` Christian König
2021-02-11 14:59 ` Alex Deucher
2021-02-11 14:52 ` [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify Thomas Zimmermann
2021-02-12 8:26 ` Thomas Zimmermann
4 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2021-02-11 13:16 UTC (permalink / raw)
To: tzimmermann, airlied, daniel, kraxel, zackr, dri-devel
Instead of swapping bo->mem just give old and new as parameters.
Also drop unused parameters and code.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +++++---
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 26 +++++++---------------
3 files changed, 16 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index e65b00f8336d..21a72fa7b9fd 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -775,7 +775,8 @@ extern void vmw_resource_unreserve(struct vmw_resource *res,
struct vmw_buffer_object *new_backup,
unsigned long new_backup_offset);
extern void vmw_query_move_notify(struct ttm_buffer_object *bo,
- struct ttm_resource *mem);
+ struct ttm_resource *old_mem,
+ struct ttm_resource *new_mem);
extern int vmw_query_readback_all(struct vmw_buffer_object *dx_query_mob);
extern void vmw_resource_evict_all(struct vmw_private *dev_priv);
extern void vmw_resource_unbind_list(struct vmw_buffer_object *vbo);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index c3a724e37104..35f02958ee2c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -847,13 +847,15 @@ int vmw_query_readback_all(struct vmw_buffer_object *dx_query_mob)
* vmw_query_move_notify - Read back cached query states
*
* @bo: The TTM buffer object about to move.
- * @mem: The memory region @bo is moving to.
+ * @old_mem: The memory region @bo is moving from.
+ * @new_mem: The memory region @bo is moving to.
*
* Called before the query MOB is swapped out to read back cached query
* states from the device.
*/
void vmw_query_move_notify(struct ttm_buffer_object *bo,
- struct ttm_resource *mem)
+ struct ttm_resource *old_mem,
+ struct ttm_resource *new_mem)
{
struct vmw_buffer_object *dx_query_mob;
struct ttm_device *bdev = bo->bdev;
@@ -871,7 +873,8 @@ void vmw_query_move_notify(struct ttm_buffer_object *bo,
}
/* If BO is being moved from MOB to system memory */
- if (mem->mem_type == TTM_PL_SYSTEM && bo->mem.mem_type == VMW_PL_MOB) {
+ if (new_mem->mem_type == TTM_PL_SYSTEM &&
+ old_mem->mem_type == VMW_PL_MOB) {
struct vmw_fence_obj *fence;
(void) vmw_query_readback_all(dx_query_mob);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 63f10c865061..38801462134e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -692,20 +692,19 @@ static int vmw_ttm_io_mem_reserve(struct ttm_device *bdev, struct ttm_resource *
*
* @bo: The TTM buffer object about to move.
* @evict: Unused
- * @mem: The struct ttm_resource indicating to what memory
+ * @old_mem: The old memory where we move from
+ * @new_mem: The struct ttm_resource indicating to what memory
* region the move is taking place.
*
* Calls move_notify for all subsystems needing it.
* (currently only resources).
*/
static void vmw_move_notify(struct ttm_buffer_object *bo,
- bool evict,
- struct ttm_resource *mem)
+ struct ttm_resource *old_mem,
+ struct ttm_resource *new_mem)
{
- if (!mem)
- return;
- vmw_bo_move_notify(bo, mem);
- vmw_query_move_notify(bo, mem);
+ vmw_bo_move_notify(bo, new_mem);
+ vmw_query_move_notify(bo, old_mem, new_mem);
}
@@ -736,7 +735,7 @@ static int vmw_move(struct ttm_buffer_object *bo,
return ret;
}
- vmw_move_notify(bo, evict, new_mem);
+ vmw_move_notify(bo, &bo->mem, new_mem);
if (old_man->use_tt && new_man->use_tt) {
if (bo->mem.mem_type == TTM_PL_SYSTEM) {
@@ -758,18 +757,10 @@ static int vmw_move(struct ttm_buffer_object *bo,
}
return 0;
fail:
- swap(*new_mem, bo->mem);
- vmw_move_notify(bo, false, new_mem);
- swap(*new_mem, bo->mem);
+ vmw_move_notify(bo, new_mem, &bo->mem);
return ret;
}
-static void
-vmw_delete_mem_notify(struct ttm_buffer_object *bo)
-{
- vmw_move_notify(bo, false, NULL);
-}
-
struct ttm_device_funcs vmw_bo_driver = {
.ttm_tt_create = &vmw_ttm_tt_create,
.ttm_tt_populate = &vmw_ttm_populate,
@@ -779,7 +770,6 @@ struct ttm_device_funcs vmw_bo_driver = {
.evict_flags = vmw_evict_flags,
.move = vmw_move,
.verify_access = vmw_verify_access,
- .delete_mem_notify = vmw_delete_mem_notify,
.swap_notify = vmw_swap_notify,
.io_mem_reserve = &vmw_ttm_io_mem_reserve,
};
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/4] drm/vmwgfx: clean up vmw_move_notify
2021-02-11 13:16 ` [PATCH 4/4] drm/vmwgfx: clean up vmw_move_notify Christian König
@ 2021-02-11 14:59 ` Alex Deucher
0 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2021-02-11 14:59 UTC (permalink / raw)
To: Christian König
Cc: Thomas Zimmermann, Dave Airlie, Maling list - DRI developers,
Gerd Hoffmann
On Thu, Feb 11, 2021 at 8:17 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Instead of swapping bo->mem just give old and new as parameters.
>
> Also drop unused parameters and code.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 3 ++-
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +++++---
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 26 +++++++---------------
> 3 files changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index e65b00f8336d..21a72fa7b9fd 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -775,7 +775,8 @@ extern void vmw_resource_unreserve(struct vmw_resource *res,
> struct vmw_buffer_object *new_backup,
> unsigned long new_backup_offset);
> extern void vmw_query_move_notify(struct ttm_buffer_object *bo,
> - struct ttm_resource *mem);
> + struct ttm_resource *old_mem,
> + struct ttm_resource *new_mem);
> extern int vmw_query_readback_all(struct vmw_buffer_object *dx_query_mob);
> extern void vmw_resource_evict_all(struct vmw_private *dev_priv);
> extern void vmw_resource_unbind_list(struct vmw_buffer_object *vbo);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index c3a724e37104..35f02958ee2c 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -847,13 +847,15 @@ int vmw_query_readback_all(struct vmw_buffer_object *dx_query_mob)
> * vmw_query_move_notify - Read back cached query states
> *
> * @bo: The TTM buffer object about to move.
> - * @mem: The memory region @bo is moving to.
> + * @old_mem: The memory region @bo is moving from.
> + * @new_mem: The memory region @bo is moving to.
> *
> * Called before the query MOB is swapped out to read back cached query
> * states from the device.
> */
> void vmw_query_move_notify(struct ttm_buffer_object *bo,
> - struct ttm_resource *mem)
> + struct ttm_resource *old_mem,
> + struct ttm_resource *new_mem)
> {
> struct vmw_buffer_object *dx_query_mob;
> struct ttm_device *bdev = bo->bdev;
> @@ -871,7 +873,8 @@ void vmw_query_move_notify(struct ttm_buffer_object *bo,
> }
>
> /* If BO is being moved from MOB to system memory */
> - if (mem->mem_type == TTM_PL_SYSTEM && bo->mem.mem_type == VMW_PL_MOB) {
> + if (new_mem->mem_type == TTM_PL_SYSTEM &&
> + old_mem->mem_type == VMW_PL_MOB) {
> struct vmw_fence_obj *fence;
>
> (void) vmw_query_readback_all(dx_query_mob);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 63f10c865061..38801462134e 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -692,20 +692,19 @@ static int vmw_ttm_io_mem_reserve(struct ttm_device *bdev, struct ttm_resource *
> *
> * @bo: The TTM buffer object about to move.
> * @evict: Unused
Evict is removed now. Can be dropped from the documentation.
Alex
> - * @mem: The struct ttm_resource indicating to what memory
> + * @old_mem: The old memory where we move from
> + * @new_mem: The struct ttm_resource indicating to what memory
> * region the move is taking place.
> *
> * Calls move_notify for all subsystems needing it.
> * (currently only resources).
> */
> static void vmw_move_notify(struct ttm_buffer_object *bo,
> - bool evict,
> - struct ttm_resource *mem)
> + struct ttm_resource *old_mem,
> + struct ttm_resource *new_mem)
> {
> - if (!mem)
> - return;
> - vmw_bo_move_notify(bo, mem);
> - vmw_query_move_notify(bo, mem);
> + vmw_bo_move_notify(bo, new_mem);
> + vmw_query_move_notify(bo, old_mem, new_mem);
> }
>
>
> @@ -736,7 +735,7 @@ static int vmw_move(struct ttm_buffer_object *bo,
> return ret;
> }
>
> - vmw_move_notify(bo, evict, new_mem);
> + vmw_move_notify(bo, &bo->mem, new_mem);
>
> if (old_man->use_tt && new_man->use_tt) {
> if (bo->mem.mem_type == TTM_PL_SYSTEM) {
> @@ -758,18 +757,10 @@ static int vmw_move(struct ttm_buffer_object *bo,
> }
> return 0;
> fail:
> - swap(*new_mem, bo->mem);
> - vmw_move_notify(bo, false, new_mem);
> - swap(*new_mem, bo->mem);
> + vmw_move_notify(bo, new_mem, &bo->mem);
> return ret;
> }
>
> -static void
> -vmw_delete_mem_notify(struct ttm_buffer_object *bo)
> -{
> - vmw_move_notify(bo, false, NULL);
> -}
> -
> struct ttm_device_funcs vmw_bo_driver = {
> .ttm_tt_create = &vmw_ttm_tt_create,
> .ttm_tt_populate = &vmw_ttm_populate,
> @@ -779,7 +770,6 @@ struct ttm_device_funcs vmw_bo_driver = {
> .evict_flags = vmw_evict_flags,
> .move = vmw_move,
> .verify_access = vmw_verify_access,
> - .delete_mem_notify = vmw_delete_mem_notify,
> .swap_notify = vmw_swap_notify,
> .io_mem_reserve = &vmw_ttm_io_mem_reserve,
> };
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify
2021-02-11 13:16 [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify Christian König
` (2 preceding siblings ...)
2021-02-11 13:16 ` [PATCH 4/4] drm/vmwgfx: clean up vmw_move_notify Christian König
@ 2021-02-11 14:52 ` Thomas Zimmermann
2021-02-11 15:05 ` Christian König
2021-02-12 8:26 ` Thomas Zimmermann
4 siblings, 1 reply; 10+ messages in thread
From: Thomas Zimmermann @ 2021-02-11 14:52 UTC (permalink / raw)
To: Christian König, airlied, daniel, kraxel, zackr, dri-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 2571 bytes --]
Hi
Am 11.02.21 um 14:16 schrieb Christian König:
> Swapping bo->mem was completely unecessary. Cleanup the function which
> is just a leftover from a TTM cleanup.
Yes this was introduced in a recent cleanup effort. Can you explain what
the code intends to do? It seems as if it tries to "re-unmap the BO" if
the move_memcpy fails.
If the move_memcpy fails now, it seems like we can live without
reverting that call to drm_gem_vram_bo_driver_move_notify(). (?)
Best regards
Thomas
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/drm_gem_vram_helper.c | 18 ++++--------------
> 1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index a0992f0b8afd..0c2233ee6029 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -564,9 +564,7 @@ static void drm_gem_vram_bo_driver_evict_flags(struct drm_gem_vram_object *gbo,
> *pl = gbo->placement;
> }
>
> -static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo,
> - bool evict,
> - struct ttm_resource *new_mem)
> +static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo)
> {
> struct ttm_buffer_object *bo = &gbo->bo;
> struct drm_device *dev = bo->base.dev;
> @@ -582,16 +580,8 @@ static int drm_gem_vram_bo_driver_move(struct drm_gem_vram_object *gbo,
> struct ttm_operation_ctx *ctx,
> struct ttm_resource *new_mem)
> {
> - int ret;
> -
> - drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
> - ret = ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
> - if (ret) {
> - swap(*new_mem, gbo->bo.mem);
> - drm_gem_vram_bo_driver_move_notify(gbo, false, new_mem);
> - swap(*new_mem, gbo->bo.mem);
> - }
> - return ret;
> + drm_gem_vram_bo_driver_move_notify(gbo);
> + return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
> }
>
> /*
> @@ -947,7 +937,7 @@ static void bo_driver_delete_mem_notify(struct ttm_buffer_object *bo)
>
> gbo = drm_gem_vram_of_bo(bo);
>
> - drm_gem_vram_bo_driver_move_notify(gbo, false, NULL);
> + drm_gem_vram_bo_driver_move_notify(gbo);
> }
>
> static int bo_driver_move(struct ttm_buffer_object *bo,
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify
2021-02-11 14:52 ` [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify Thomas Zimmermann
@ 2021-02-11 15:05 ` Christian König
2021-02-11 15:12 ` Thomas Zimmermann
0 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2021-02-11 15:05 UTC (permalink / raw)
To: Thomas Zimmermann, airlied, daniel, kraxel, zackr, dri-devel
Am 11.02.21 um 15:52 schrieb Thomas Zimmermann:
> Hi
>
> Am 11.02.21 um 14:16 schrieb Christian König:
>> Swapping bo->mem was completely unecessary. Cleanup the function which
>> is just a leftover from a TTM cleanup.
>
> Yes this was introduced in a recent cleanup effort. Can you explain
> what the code intends to do? It seems as if it tries to "re-unmap the
> BO" if the move_memcpy fails.
>
> If the move_memcpy fails now, it seems like we can live without
> reverting that call to drm_gem_vram_bo_driver_move_notify(). (?)
I think so, but I'm not 100% sure either.
The swap() -> notify -> swap() was just how TTM did it and that was
moved into the drivers.
I'm now just trying to remove all the hard write accesses to bo->mem
from drivers and stumbled over this here.
Thanks for the comments,
Christian.
>
> Best regards
> Thomas
>
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>> drivers/gpu/drm/drm_gem_vram_helper.c | 18 ++++--------------
>> 1 file changed, 4 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index a0992f0b8afd..0c2233ee6029 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -564,9 +564,7 @@ static void
>> drm_gem_vram_bo_driver_evict_flags(struct drm_gem_vram_object *gbo,
>> *pl = gbo->placement;
>> }
>> -static void drm_gem_vram_bo_driver_move_notify(struct
>> drm_gem_vram_object *gbo,
>> - bool evict,
>> - struct ttm_resource *new_mem)
>> +static void drm_gem_vram_bo_driver_move_notify(struct
>> drm_gem_vram_object *gbo)
>> {
>> struct ttm_buffer_object *bo = &gbo->bo;
>> struct drm_device *dev = bo->base.dev;
>> @@ -582,16 +580,8 @@ static int drm_gem_vram_bo_driver_move(struct
>> drm_gem_vram_object *gbo,
>> struct ttm_operation_ctx *ctx,
>> struct ttm_resource *new_mem)
>> {
>> - int ret;
>> -
>> - drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
>> - ret = ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
>> - if (ret) {
>> - swap(*new_mem, gbo->bo.mem);
>> - drm_gem_vram_bo_driver_move_notify(gbo, false, new_mem);
>> - swap(*new_mem, gbo->bo.mem);
>> - }
>> - return ret;
>> + drm_gem_vram_bo_driver_move_notify(gbo);
>> + return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
>> }
>> /*
>> @@ -947,7 +937,7 @@ static void bo_driver_delete_mem_notify(struct
>> ttm_buffer_object *bo)
>> gbo = drm_gem_vram_of_bo(bo);
>> - drm_gem_vram_bo_driver_move_notify(gbo, false, NULL);
>> + drm_gem_vram_bo_driver_move_notify(gbo);
>> }
>> static int bo_driver_move(struct ttm_buffer_object *bo,
>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify
2021-02-11 15:05 ` Christian König
@ 2021-02-11 15:12 ` Thomas Zimmermann
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2021-02-11 15:12 UTC (permalink / raw)
To: Christian König, airlied, daniel, kraxel, zackr, dri-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 3879 bytes --]
Hi
Am 11.02.21 um 16:05 schrieb Christian König:
>
>
> Am 11.02.21 um 15:52 schrieb Thomas Zimmermann:
>> Hi
>>
>> Am 11.02.21 um 14:16 schrieb Christian König:
>>> Swapping bo->mem was completely unecessary. Cleanup the function which
>>> is just a leftover from a TTM cleanup.
>>
>> Yes this was introduced in a recent cleanup effort. Can you explain
>> what the code intends to do? It seems as if it tries to "re-unmap the
>> BO" if the move_memcpy fails.
>>
>> If the move_memcpy fails now, it seems like we can live without
>> reverting that call to drm_gem_vram_bo_driver_move_notify(). (?)
>
> I think so, but I'm not 100% sure either.
>
> The swap() -> notify -> swap() was just how TTM did it and that was
> moved into the drivers.
>
> I'm now just trying to remove all the hard write accesses to bo->mem
> from drivers and stumbled over this here.
We already have a vmap count of zero; so unmapping the BO pages is fine
at any time. The next caller of vmap will simple instantiate a new mapping.
Let me give this patch a test run tomorrow, but it seems correct.
Best regards
Thomas
>
> Thanks for the comments,
> Christian.
>
>>
>> Best regards
>> Thomas
>>
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>> drivers/gpu/drm/drm_gem_vram_helper.c | 18 ++++--------------
>>> 1 file changed, 4 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>>> index a0992f0b8afd..0c2233ee6029 100644
>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>>> @@ -564,9 +564,7 @@ static void
>>> drm_gem_vram_bo_driver_evict_flags(struct drm_gem_vram_object *gbo,
>>> *pl = gbo->placement;
>>> }
>>> -static void drm_gem_vram_bo_driver_move_notify(struct
>>> drm_gem_vram_object *gbo,
>>> - bool evict,
>>> - struct ttm_resource *new_mem)
>>> +static void drm_gem_vram_bo_driver_move_notify(struct
>>> drm_gem_vram_object *gbo)
>>> {
>>> struct ttm_buffer_object *bo = &gbo->bo;
>>> struct drm_device *dev = bo->base.dev;
>>> @@ -582,16 +580,8 @@ static int drm_gem_vram_bo_driver_move(struct
>>> drm_gem_vram_object *gbo,
>>> struct ttm_operation_ctx *ctx,
>>> struct ttm_resource *new_mem)
>>> {
>>> - int ret;
>>> -
>>> - drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
>>> - ret = ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
>>> - if (ret) {
>>> - swap(*new_mem, gbo->bo.mem);
>>> - drm_gem_vram_bo_driver_move_notify(gbo, false, new_mem);
>>> - swap(*new_mem, gbo->bo.mem);
>>> - }
>>> - return ret;
>>> + drm_gem_vram_bo_driver_move_notify(gbo);
>>> + return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
>>> }
>>> /*
>>> @@ -947,7 +937,7 @@ static void bo_driver_delete_mem_notify(struct
>>> ttm_buffer_object *bo)
>>> gbo = drm_gem_vram_of_bo(bo);
>>> - drm_gem_vram_bo_driver_move_notify(gbo, false, NULL);
>>> + drm_gem_vram_bo_driver_move_notify(gbo);
>>> }
>>> static int bo_driver_move(struct ttm_buffer_object *bo,
>>>
>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify
2021-02-11 13:16 [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify Christian König
` (3 preceding siblings ...)
2021-02-11 14:52 ` [PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify Thomas Zimmermann
@ 2021-02-12 8:26 ` Thomas Zimmermann
4 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2021-02-12 8:26 UTC (permalink / raw)
To: Christian König, airlied, daniel, kraxel, zackr, dri-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 2366 bytes --]
Hi
Am 11.02.21 um 14:16 schrieb Christian König:
> Swapping bo->mem was completely unecessary. Cleanup the function which
> is just a leftover from a TTM cleanup.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
Appears to work.
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/drm_gem_vram_helper.c | 18 ++++--------------
> 1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index a0992f0b8afd..0c2233ee6029 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -564,9 +564,7 @@ static void drm_gem_vram_bo_driver_evict_flags(struct drm_gem_vram_object *gbo,
> *pl = gbo->placement;
> }
>
> -static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo,
> - bool evict,
> - struct ttm_resource *new_mem)
> +static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo)
> {
> struct ttm_buffer_object *bo = &gbo->bo;
> struct drm_device *dev = bo->base.dev;
> @@ -582,16 +580,8 @@ static int drm_gem_vram_bo_driver_move(struct drm_gem_vram_object *gbo,
> struct ttm_operation_ctx *ctx,
> struct ttm_resource *new_mem)
> {
> - int ret;
> -
> - drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
> - ret = ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
> - if (ret) {
> - swap(*new_mem, gbo->bo.mem);
> - drm_gem_vram_bo_driver_move_notify(gbo, false, new_mem);
> - swap(*new_mem, gbo->bo.mem);
> - }
> - return ret;
> + drm_gem_vram_bo_driver_move_notify(gbo);
> + return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
> }
>
> /*
> @@ -947,7 +937,7 @@ static void bo_driver_delete_mem_notify(struct ttm_buffer_object *bo)
>
> gbo = drm_gem_vram_of_bo(bo);
>
> - drm_gem_vram_bo_driver_move_notify(gbo, false, NULL);
> + drm_gem_vram_bo_driver_move_notify(gbo);
> }
>
> static int bo_driver_move(struct ttm_buffer_object *bo,
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread