From: Michel Dänzer michel.daenzer@amd.com
I ran into the BUG_ON in ttm_tt_swapout, presumably the BO being swapped out was using a write-combined CPU mapping.
Instead of BUGging out, just set the caching mode to what's needed.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com --- drivers/gpu/drm/ttm/ttm_tt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 4e19d0f..c2794eb 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -334,7 +334,8 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage) int ret = -ENOMEM;
BUG_ON(ttm->state != tt_unbound && ttm->state != tt_unpopulated); - BUG_ON(ttm->caching_state != tt_cached); + + ttm_tt_set_caching(ttm, tt_cached);
if (!persistent_swap_storage) { swap_storage = shmem_file_setup("ttm swap",
Hi, Michel,
On 11/05/2015 09:08 AM, Michel Dänzer wrote:
From: Michel Dänzer michel.daenzer@amd.com
I ran into the BUG_ON in ttm_tt_swapout, presumably the BO being swapped out was using a write-combined CPU mapping.
Instead of BUGging out, just set the caching mode to what's needed.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
drivers/gpu/drm/ttm/ttm_tt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 4e19d0f..c2794eb 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -334,7 +334,8 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage) int ret = -ENOMEM;
BUG_ON(ttm->state != tt_unbound && ttm->state != tt_unpopulated);
- BUG_ON(ttm->caching_state != tt_cached);
ttm_tt_set_caching(ttm, tt_cached);
if (!persistent_swap_storage) { swap_storage = shmem_file_setup("ttm swap",
This *is* actually a bug somewhere, since before ttm_tt_swapout, ttm_bo_swapout should have moved out the bo to system and set the correct caching. A WARN_ON_ONCE instead of BUG_ON is ok, but this should never happen.
/Thomas
On 05.11.2015 17:47, Thomas Hellstrom wrote:
Hi, Michel,
On 11/05/2015 09:08 AM, Michel Dänzer wrote:
From: Michel Dänzer michel.daenzer@amd.com
I ran into the BUG_ON in ttm_tt_swapout, presumably the BO being swapped out was using a write-combined CPU mapping.
Instead of BUGging out, just set the caching mode to what's needed.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
drivers/gpu/drm/ttm/ttm_tt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 4e19d0f..c2794eb 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -334,7 +334,8 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage) int ret = -ENOMEM;
BUG_ON(ttm->state != tt_unbound && ttm->state != tt_unpopulated);
- BUG_ON(ttm->caching_state != tt_cached);
ttm_tt_set_caching(ttm, tt_cached);
if (!persistent_swap_storage) { swap_storage = shmem_file_setup("ttm swap",
This *is* actually a bug somewhere, since before ttm_tt_swapout, ttm_bo_swapout should have moved out the bo to system and set the correct caching.
Maybe ttm_bo_swapout needs to check ttm->caching_state explicitly? AFAICT it only checks the placement flags, but we allow all caching modes for GTT.
On 11/06/2015 03:33 AM, Michel Dänzer wrote:
On 05.11.2015 17:47, Thomas Hellstrom wrote:
Hi, Michel,
On 11/05/2015 09:08 AM, Michel Dänzer wrote:
From: Michel Dänzer michel.daenzer@amd.com
I ran into the BUG_ON in ttm_tt_swapout, presumably the BO being swapped out was using a write-combined CPU mapping.
Instead of BUGging out, just set the caching mode to what's needed.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
drivers/gpu/drm/ttm/ttm_tt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 4e19d0f..c2794eb 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -334,7 +334,8 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage) int ret = -ENOMEM;
BUG_ON(ttm->state != tt_unbound && ttm->state != tt_unpopulated);
- BUG_ON(ttm->caching_state != tt_cached);
ttm_tt_set_caching(ttm, tt_cached);
if (!persistent_swap_storage) { swap_storage = shmem_file_setup("ttm swap",
This *is* actually a bug somewhere, since before ttm_tt_swapout, ttm_bo_swapout should have moved out the bo to system and set the correct caching.
Maybe ttm_bo_swapout needs to check ttm->caching_state explicitly? AFAICT it only checks the placement flags, but we allow all caching modes for GTT.
bo->mem.placement should only have two flags set, one for the current memory type corresponding to bo->mem.mem_type, the other for the current caching. If there are more than one caching flag set at this point, that's a bug. The preferred caching mode is chosen from a set of modes in ttm_bo_select_caching(), and bo->mem is reassigned to incorporate the current caching mode after a successful move.
I quickly eyed through the TTM default move functions and they seem to do the correct thing from what I can tell. Is the radeon ttm driver assigning bo->mem in a move callback perhaps?
/Thomas
On 06.11.2015 22:02, Thomas Hellstrom wrote:
On 11/06/2015 03:33 AM, Michel Dänzer wrote:
On 05.11.2015 17:47, Thomas Hellstrom wrote:
Hi, Michel,
On 11/05/2015 09:08 AM, Michel Dänzer wrote:
From: Michel Dänzer michel.daenzer@amd.com
I ran into the BUG_ON in ttm_tt_swapout, presumably the BO being swapped out was using a write-combined CPU mapping.
Instead of BUGging out, just set the caching mode to what's needed.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
drivers/gpu/drm/ttm/ttm_tt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 4e19d0f..c2794eb 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -334,7 +334,8 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage) int ret = -ENOMEM;
BUG_ON(ttm->state != tt_unbound && ttm->state != tt_unpopulated);
- BUG_ON(ttm->caching_state != tt_cached);
ttm_tt_set_caching(ttm, tt_cached);
if (!persistent_swap_storage) { swap_storage = shmem_file_setup("ttm swap",
This *is* actually a bug somewhere, since before ttm_tt_swapout, ttm_bo_swapout should have moved out the bo to system and set the correct caching.
Maybe ttm_bo_swapout needs to check ttm->caching_state explicitly? AFAICT it only checks the placement flags, but we allow all caching modes for GTT.
bo->mem.placement should only have two flags set, one for the current memory type corresponding to bo->mem.mem_type, the other for the current caching. If there are more than one caching flag set at this point, that's a bug. The preferred caching mode is chosen from a set of modes in ttm_bo_select_caching(), and bo->mem is reassigned to incorporate the current caching mode after a successful move.
I quickly eyed through the TTM default move functions and they seem to do the correct thing from what I can tell. Is the radeon ttm driver assigning bo->mem in a move callback perhaps?
Only via ttm_bo_move_ttm / to the values passed into the move callback AFAICT.
dri-devel@lists.freedesktop.org
-
Michel Dänzer -
Thomas Hellstrom