From: Michel Dänzer michel.daenzer@amd.com
This flag is a hint that userspace expects the BO to be accessed by the CPU. We can use that hint to prevent such BOs from ever being stored in the CPU inaccessible part of VRAM.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com --- drivers/gpu/drm/radeon/radeon_object.c | 11 +++++++++-- include/uapi/drm/radeon_drm.h | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index dc74cc5..908ea541 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -143,7 +143,12 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
for (i = 0; i < c; ++i) { rbo->placements[i].fpfn = 0; - rbo->placements[i].lpfn = 0; + if ((rbo->flags & RADEON_GEM_CPU_ACCESS) && + (rbo->placements[i].flags & TTM_PL_FLAG_VRAM)) + rbo->placements[i].lpfn = + rbo->rdev->mc.visible_vram_size >> PAGE_SHIFT; + else + rbo->placements[i].lpfn = 0; }
/* @@ -151,7 +156,9 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain) * improve fragmentation quality. * 512kb was measured as the most optimal number. */ - if (rbo->tbo.mem.size > 512 * 1024) { + if (!((rbo->flags & RADEON_GEM_CPU_ACCESS) && + (rbo->placements[i].flags & TTM_PL_FLAG_VRAM)) && + rbo->tbo.mem.size > 512 * 1024) { for (i = 0; i < c; i++) { rbo->placements[i].flags |= TTM_PL_FLAG_TOPDOWN; } diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h index 509b2d7..bf0067b 100644 --- a/include/uapi/drm/radeon_drm.h +++ b/include/uapi/drm/radeon_drm.h @@ -799,6 +799,8 @@ struct drm_radeon_gem_info { #define RADEON_GEM_NO_BACKING_STORE (1 << 0) #define RADEON_GEM_GTT_UC (1 << 1) #define RADEON_GEM_GTT_WC (1 << 2) +/* BO is expected to be accessed by the CPU */ +#define RADEON_GEM_CPU_ACCESS (1 << 3)
struct drm_radeon_gem_create { uint64_t size;
From: Michel Dänzer michel.daenzer@amd.com
This allows the kernel to prevent such BOs from ever being stored in the CPU inaccessible part of VRAM.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com --- src/gallium/drivers/radeon/r600_buffer_common.c | 23 ++++++++++++++--------- src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 8 +++++++- src/gallium/winsys/radeon/drm/radeon_winsys.h | 3 ++- 3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/src/gallium/drivers/radeon/r600_buffer_common.c b/src/gallium/drivers/radeon/r600_buffer_common.c index acdabc0..1a6e97d 100644 --- a/src/gallium/drivers/radeon/r600_buffer_common.c +++ b/src/gallium/drivers/radeon/r600_buffer_common.c @@ -126,6 +126,7 @@ bool r600_init_resource(struct r600_common_screen *rscreen, flags = RADEON_FLAG_GTT_WC; break; } + flags = RADEON_FLAG_CPU_ACCESS; /* fall through */ case PIPE_USAGE_DEFAULT: case PIPE_USAGE_IMMUTABLE: @@ -136,23 +137,27 @@ bool r600_init_resource(struct r600_common_screen *rscreen, break; }
- /* Use GTT for all persistent mappings with older kernels, because they - * didn't always flush the HDP cache before CS execution. - * - * Write-combined CPU mappings are fine, the kernel ensures all CPU - * writes finish before the GPU executes a command stream. - */ - if (rscreen->info.drm_minor < 40 && - res->b.b.target == PIPE_BUFFER && + if (res->b.b.target == PIPE_BUFFER && res->b.b.flags & (PIPE_RESOURCE_FLAG_MAP_PERSISTENT | PIPE_RESOURCE_FLAG_MAP_COHERENT)) { - res->domains = RADEON_DOMAIN_GTT; + /* Use GTT for all persistent mappings with older kernels, + * because they didn't always flush the HDP cache before CS + * execution. + * + * Write-combined CPU mappings are fine, the kernel ensures all CPU + * writes finish before the GPU executes a command stream. + */ + if (rscreen->info.drm_minor < 40) + res->domains = RADEON_DOMAIN_GTT; + else if (res->domains & RADEON_DOMAIN_VRAM) + flags |= RADEON_FLAG_CPU_ACCESS; }
/* Tiled textures are unmappable. Always put them in VRAM. */ if (res->b.b.target != PIPE_BUFFER && rtex->surface.level[0].mode >= RADEON_SURF_MODE_1D) { res->domains = RADEON_DOMAIN_VRAM; + flags &= ~RADEON_FLAG_CPU_ACCESS; }
/* Allocate a new resource. */ diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c index 73f8d38..03b9b1d 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c @@ -478,7 +478,11 @@ const struct pb_vtbl radeon_bo_vtbl = { };
#ifndef RADEON_GEM_GTT_WC -#define RADEON_GEM_GTT_WC (1 << 2) +#define RADEON_GEM_GTT_WC (1 << 2) +#endif +#ifndef RADEON_GTM_CPU_ACCESS +/* BO is expected to be accessed by the CPU */ +#define RADEON_GEM_CPU_ACCESS (1 << 3) #endif
static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr, @@ -505,6 +509,8 @@ static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr,
if (rdesc->flags & RADEON_FLAG_GTT_WC) args.flags |= RADEON_GEM_GTT_WC; + if (rdesc->flags & RADEON_FLAG_CPU_ACCESS) + args.flags |= RADEON_GEM_CPU_ACCESS;
if (drmCommandWriteRead(rws->fd, DRM_RADEON_GEM_CREATE, &args, sizeof(args))) { diff --git a/src/gallium/winsys/radeon/drm/radeon_winsys.h b/src/gallium/winsys/radeon/drm/radeon_winsys.h index dbd58f1..69bf6ed 100644 --- a/src/gallium/winsys/radeon/drm/radeon_winsys.h +++ b/src/gallium/winsys/radeon/drm/radeon_winsys.h @@ -66,7 +66,8 @@ enum radeon_bo_domain { /* bitfield */ };
enum radeon_bo_flag { /* bitfield */ - RADEON_FLAG_GTT_WC = (1 << 0) + RADEON_FLAG_GTT_WC = (1 << 0), + RADEON_FLAG_CPU_ACCESS = (1 << 1), };
enum radeon_bo_usage { /* bitfield */
Reviewed-by: Marek Olšák marek.olsak@amd.com
Marek
On Thu, Aug 28, 2014 at 8:56 AM, Michel Dänzer michel@daenzer.net wrote:
From: Michel Dänzer michel.daenzer@amd.com
This allows the kernel to prevent such BOs from ever being stored in the CPU inaccessible part of VRAM.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
src/gallium/drivers/radeon/r600_buffer_common.c | 23 ++++++++++++++--------- src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 8 +++++++- src/gallium/winsys/radeon/drm/radeon_winsys.h | 3 ++- 3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/src/gallium/drivers/radeon/r600_buffer_common.c b/src/gallium/drivers/radeon/r600_buffer_common.c index acdabc0..1a6e97d 100644 --- a/src/gallium/drivers/radeon/r600_buffer_common.c +++ b/src/gallium/drivers/radeon/r600_buffer_common.c @@ -126,6 +126,7 @@ bool r600_init_resource(struct r600_common_screen *rscreen, flags = RADEON_FLAG_GTT_WC; break; }
flags = RADEON_FLAG_CPU_ACCESS; /* fall through */ case PIPE_USAGE_DEFAULT: case PIPE_USAGE_IMMUTABLE:@@ -136,23 +137,27 @@ bool r600_init_resource(struct r600_common_screen *rscreen, break; }
/* Use GTT for all persistent mappings with older kernels, because they* didn't always flush the HDP cache before CS execution.** Write-combined CPU mappings are fine, the kernel ensures all CPU* writes finish before the GPU executes a command stream.*/if (rscreen->info.drm_minor < 40 &&res->b.b.target == PIPE_BUFFER &&
if (res->b.b.target == PIPE_BUFFER && res->b.b.flags & (PIPE_RESOURCE_FLAG_MAP_PERSISTENT | PIPE_RESOURCE_FLAG_MAP_COHERENT)) {
res->domains = RADEON_DOMAIN_GTT;
/* Use GTT for all persistent mappings with older kernels,* because they didn't always flush the HDP cache before CS* execution.** Write-combined CPU mappings are fine, the kernel ensures all CPU* writes finish before the GPU executes a command stream.*/if (rscreen->info.drm_minor < 40)res->domains = RADEON_DOMAIN_GTT;else if (res->domains & RADEON_DOMAIN_VRAM)flags |= RADEON_FLAG_CPU_ACCESS; } /* Tiled textures are unmappable. Always put them in VRAM. */ if (res->b.b.target != PIPE_BUFFER && rtex->surface.level[0].mode >= RADEON_SURF_MODE_1D) { res->domains = RADEON_DOMAIN_VRAM;flags &= ~RADEON_FLAG_CPU_ACCESS; } /* Allocate a new resource. */diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c index 73f8d38..03b9b1d 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c @@ -478,7 +478,11 @@ const struct pb_vtbl radeon_bo_vtbl = { };
#ifndef RADEON_GEM_GTT_WC -#define RADEON_GEM_GTT_WC (1 << 2) +#define RADEON_GEM_GTT_WC (1 << 2) +#endif +#ifndef RADEON_GTM_CPU_ACCESS +/* BO is expected to be accessed by the CPU */ +#define RADEON_GEM_CPU_ACCESS (1 << 3) #endif
static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr, @@ -505,6 +509,8 @@ static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr,
if (rdesc->flags & RADEON_FLAG_GTT_WC) args.flags |= RADEON_GEM_GTT_WC;
if (rdesc->flags & RADEON_FLAG_CPU_ACCESS)
args.flags |= RADEON_GEM_CPU_ACCESS;if (drmCommandWriteRead(rws->fd, DRM_RADEON_GEM_CREATE, &args, sizeof(args))) {
diff --git a/src/gallium/winsys/radeon/drm/radeon_winsys.h b/src/gallium/winsys/radeon/drm/radeon_winsys.h index dbd58f1..69bf6ed 100644 --- a/src/gallium/winsys/radeon/drm/radeon_winsys.h +++ b/src/gallium/winsys/radeon/drm/radeon_winsys.h @@ -66,7 +66,8 @@ enum radeon_bo_domain { /* bitfield */ };
enum radeon_bo_flag { /* bitfield */
- RADEON_FLAG_GTT_WC = (1 << 0)
- RADEON_FLAG_GTT_WC = (1 << 0),
- RADEON_FLAG_CPU_ACCESS = (1 << 1),
};
enum radeon_bo_usage { /* bitfield */
2.1.0
mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Am 28.08.2014 um 08:56 schrieb Michel Dänzer:
From: Michel Dänzer michel.daenzer@amd.com
This flag is a hint that userspace expects the BO to be accessed by the CPU. We can use that hint to prevent such BOs from ever being stored in the CPU inaccessible part of VRAM.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
This patch is Reviewed-by: Christian König christian.koenig@amd.com
I think we need a similar negative flags as well, e.g. RADEON_GEM_NO_CPU_ACCESS.
This way we can stop forcing buffers into the visible VRAM while pinning them for scanout.
Regards, Christian.
drivers/gpu/drm/radeon/radeon_object.c | 11 +++++++++-- include/uapi/drm/radeon_drm.h | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index dc74cc5..908ea541 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -143,7 +143,12 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
for (i = 0; i < c; ++i) { rbo->placements[i].fpfn = 0;
rbo->placements[i].lpfn = 0;
if ((rbo->flags & RADEON_GEM_CPU_ACCESS) &&(rbo->placements[i].flags & TTM_PL_FLAG_VRAM))rbo->placements[i].lpfn =rbo->rdev->mc.visible_vram_size >> PAGE_SHIFT;elserbo->placements[i].lpfn = 0;}
/*
@@ -151,7 +156,9 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain) * improve fragmentation quality. * 512kb was measured as the most optimal number. */
- if (rbo->tbo.mem.size > 512 * 1024) {
- if (!((rbo->flags & RADEON_GEM_CPU_ACCESS) &&
(rbo->placements[i].flags & TTM_PL_FLAG_VRAM)) &&rbo->tbo.mem.size > 512 * 1024) {diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h index 509b2d7..bf0067b 100644 --- a/include/uapi/drm/radeon_drm.h +++ b/include/uapi/drm/radeon_drm.h @@ -799,6 +799,8 @@ struct drm_radeon_gem_info { #define RADEON_GEM_NO_BACKING_STORE (1 << 0) #define RADEON_GEM_GTT_UC (1 << 1) #define RADEON_GEM_GTT_WC (1 << 2) +/* BO is expected to be accessed by the CPU */ +#define RADEON_GEM_CPU_ACCESS (1 << 3)
struct drm_radeon_gem_create { uint64_t size;
On Thu, Aug 28, 2014 at 4:57 AM, Christian König deathsimple@vodafone.de wrote:
Am 28.08.2014 um 08:56 schrieb Michel Dänzer:
From: Michel Dänzer michel.daenzer@amd.com
This flag is a hint that userspace expects the BO to be accessed by the CPU. We can use that hint to prevent such BOs from ever being stored in the CPU inaccessible part of VRAM.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
This patch is Reviewed-by: Christian König christian.koenig@amd.com
Applied to my -next tree.
I think we need a similar negative flags as well, e.g. RADEON_GEM_NO_CPU_ACCESS.
This way we can stop forcing buffers into the visible VRAM while pinning them for scanout.
How about the attached patch?
Alex
Regards, Christian.
drivers/gpu/drm/radeon/radeon_object.c | 11 +++++++++-- include/uapi/drm/radeon_drm.h | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index dc74cc5..908ea541 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -143,7 +143,12 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain) for (i = 0; i < c; ++i) { rbo->placements[i].fpfn = 0;
rbo->placements[i].lpfn = 0;
if ((rbo->flags & RADEON_GEM_CPU_ACCESS) &&(rbo->placements[i].flags & TTM_PL_FLAG_VRAM))rbo->placements[i].lpfn =rbo->rdev->mc.visible_vram_size >>PAGE_SHIFT;
elserbo->placements[i].lpfn = 0; } /*@@ -151,7 +156,9 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain) * improve fragmentation quality. * 512kb was measured as the most optimal number. */
if (rbo->tbo.mem.size > 512 * 1024) {
if (!((rbo->flags & RADEON_GEM_CPU_ACCESS) &&(rbo->placements[i].flags & TTM_PL_FLAG_VRAM)) &&rbo->tbo.mem.size > 512 * 1024) { for (i = 0; i < c; i++) { rbo->placements[i].flags |= TTM_PL_FLAG_TOPDOWN; }diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h index 509b2d7..bf0067b 100644 --- a/include/uapi/drm/radeon_drm.h +++ b/include/uapi/drm/radeon_drm.h @@ -799,6 +799,8 @@ struct drm_radeon_gem_info { #define RADEON_GEM_NO_BACKING_STORE (1 << 0) #define RADEON_GEM_GTT_UC (1 << 1) #define RADEON_GEM_GTT_WC (1 << 2) +/* BO is expected to be accessed by the CPU */ +#define RADEON_GEM_CPU_ACCESS (1 << 3) struct drm_radeon_gem_create { uint64_t size;
mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 29.08.2014 00:01, Alex Deucher wrote:
On Thu, Aug 28, 2014 at 4:57 AM, Christian König deathsimple@vodafone.de wrote:
Am 28.08.2014 um 08:56 schrieb Michel Dänzer:
From: Michel Dänzer michel.daenzer@amd.com
This flag is a hint that userspace expects the BO to be accessed by the CPU. We can use that hint to prevent such BOs from ever being stored in the CPU inaccessible part of VRAM.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
This patch is Reviewed-by: Christian König christian.koenig@amd.com
Applied to my -next tree.
Thanks!
I think we need a similar negative flags as well, e.g. RADEON_GEM_NO_CPU_ACCESS.
This way we can stop forcing buffers into the visible VRAM while pinning them for scanout.
How about the attached patch?
[...]
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 09b039a..b71e8e0 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -314,10 +314,14 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, unsigned lpfn = 0;
/* force to pin into visible video ram */
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM)lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;else
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) {if (bo->flags & RADEON_GEM_NO_CPU_ACCESS)lpfn = bo->rdev->mc.real_vram_size >> PAGE_SHIFT;elselpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;
lpfn can be left at 0 if RADEON_GEM_NO_CPU_ACCESS is set, so this can be simplified to:
if (!(bo->flags & RADEON_GEM_NO_CPU_ACCESS)) lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;
diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h index f755f20..d2346fd 100644 --- a/include/uapi/drm/radeon_drm.h +++ b/include/uapi/drm/radeon_drm.h @@ -803,6 +803,8 @@ struct drm_radeon_gem_info { #define RADEON_GEM_GTT_WC (1 << 2) /* BO is expected to be accessed by the CPU */ #define RADEON_GEM_CPU_ACCESS (1 << 3) +/* BO is expected to not be accessed by the CPU */ +#define RADEON_GEM_NO_CPU_ACCESS (1 << 4)
I'd use stronger wording for this, e.g.
/* CPU access is not expected to work for this BO */
On Thu, Aug 28, 2014 at 9:46 PM, Michel Dänzer michel@daenzer.net wrote:
On 29.08.2014 00:01, Alex Deucher wrote:
On Thu, Aug 28, 2014 at 4:57 AM, Christian König deathsimple@vodafone.de wrote:
Am 28.08.2014 um 08:56 schrieb Michel Dänzer:
From: Michel Dänzer michel.daenzer@amd.com
This flag is a hint that userspace expects the BO to be accessed by the CPU. We can use that hint to prevent such BOs from ever being stored in the CPU inaccessible part of VRAM.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
This patch is Reviewed-by: Christian König christian.koenig@amd.com
Applied to my -next tree.
Thanks!
I think we need a similar negative flags as well, e.g. RADEON_GEM_NO_CPU_ACCESS.
This way we can stop forcing buffers into the visible VRAM while pinning them for scanout.
How about the attached patch?
[...]
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 09b039a..b71e8e0 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -314,10 +314,14 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, unsigned lpfn = 0;
/* force to pin into visible video ram */
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM)lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;else
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) {if (bo->flags & RADEON_GEM_NO_CPU_ACCESS)lpfn = bo->rdev->mc.real_vram_size >> PAGE_SHIFT;elselpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;lpfn can be left at 0 if RADEON_GEM_NO_CPU_ACCESS is set, so this can be simplified to:
if (!(bo->flags & RADEON_GEM_NO_CPU_ACCESS)) lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h index f755f20..d2346fd 100644 --- a/include/uapi/drm/radeon_drm.h +++ b/include/uapi/drm/radeon_drm.h @@ -803,6 +803,8 @@ struct drm_radeon_gem_info { #define RADEON_GEM_GTT_WC (1 << 2) /* BO is expected to be accessed by the CPU */ #define RADEON_GEM_CPU_ACCESS (1 << 3) +/* BO is expected to not be accessed by the CPU */ +#define RADEON_GEM_NO_CPU_ACCESS (1 << 4)
I'd use stronger wording for this, e.g.
/* CPU access is not expected to work for this BO */
Updated version with comments integrated.
Alex
On 09.09.2014 02:36, Alex Deucher wrote:
Updated version with comments integrated.
[...]
@@ -314,10 +314,12 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, unsigned lpfn = 0;
/* force to pin into visible video ram */
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM)lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;else
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) {if (!(bo->flags & RADEON_GEM_NO_CPU_ACCESS))lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;} else { lpfn = bo->rdev->mc.gtt_size >> PAGE_SHIFT; /* ??? */}
The else block can be removed as well, but that can be done in another patch.
Either way, v2 is
Reviewed-by: Michel Dänzer michel.daenzer@amd.com
On 09.09.2014 09:47, Michel Dänzer wrote:
On 09.09.2014 02:36, Alex Deucher wrote:
Updated version with comments integrated.
[...]
@@ -314,10 +314,12 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, unsigned lpfn = 0;
/* force to pin into visible video ram */
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM)lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;else
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) {if (!(bo->flags & RADEON_GEM_NO_CPU_ACCESS))lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;} else { lpfn = bo->rdev->mc.gtt_size >> PAGE_SHIFT; /* ??? */}The else block can be removed as well, but that can be done in another patch.
Actually, I just noticed a problem, the following if statement:
if (max_offset) lpfn = min (lpfn, (unsigned)(max_offset >> PAGE_SHIFT));
This will ignore max_offset if lpfn is 0. So either go with v1 of this hunk, or rebase on top of the patch below.
From: =?UTF-8?q?Michel=20D=C3=A4nzer?= michel.daenzer@amd.com Date: Tue, 9 Sep 2014 10:09:23 +0900 Subject: [PATCH] drm/radeon: Clean up assignment of TTM placement lpfn member for pinning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Signed-off-by: Michel Dänzer michel.daenzer@amd.com --- drivers/gpu/drm/radeon/radeon_object.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 908ea541..8ec8150 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -307,18 +307,14 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, } radeon_ttm_placement_from_domain(bo, domain); for (i = 0; i < bo->placement.num_placement; i++) { - unsigned lpfn = 0; - /* force to pin into visible video ram */ if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) - lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT; + bo->placements[i].lpfn = + min(bo->rdev->mc.visible_vram_size, max_offset) + >> PAGE_SHIFT; else - lpfn = bo->rdev->mc.gtt_size >> PAGE_SHIFT; /* ??? */ - - if (max_offset) - lpfn = min (lpfn, (unsigned)(max_offset >> PAGE_SHIFT)); + bo->placements[i].lpfn = max_offset >> PAGE_SHIFT;
- bo->placements[i].lpfn = lpfn; bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; }
On Mon, Sep 8, 2014 at 9:15 PM, Michel Dänzer michel@daenzer.net wrote:
On 09.09.2014 09:47, Michel Dänzer wrote:
On 09.09.2014 02:36, Alex Deucher wrote:
Updated version with comments integrated.
[...]
@@ -314,10 +314,12 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, unsigned lpfn = 0;
/* force to pin into visible video ram */
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM)lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;else
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) {if (!(bo->flags & RADEON_GEM_NO_CPU_ACCESS))lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;} else { lpfn = bo->rdev->mc.gtt_size >> PAGE_SHIFT; /* ??? */}The else block can be removed as well, but that can be done in another patch.
Actually, I just noticed a problem, the following if statement:
if (max_offset) lpfn = min (lpfn, (unsigned)(max_offset >> PAGE_SHIFT));This will ignore max_offset if lpfn is 0. So either go with v1 of this hunk, or rebase on top of the patch below.
Rebased on your patch and attached.
Alex
From: =?UTF-8?q?Michel=20D=C3=A4nzer?= michel.daenzer@amd.com Date: Tue, 9 Sep 2014 10:09:23 +0900 Subject: [PATCH] drm/radeon: Clean up assignment of TTM placement lpfn member for pinning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
drivers/gpu/drm/radeon/radeon_object.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 908ea541..8ec8150 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -307,18 +307,14 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, } radeon_ttm_placement_from_domain(bo, domain); for (i = 0; i < bo->placement.num_placement; i++) {
unsigned lpfn = 0;/* force to pin into visible video ram */ if (bo->placements[i].flags & TTM_PL_FLAG_VRAM)lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;
bo->placements[i].lpfn =min(bo->rdev->mc.visible_vram_size, max_offset)>> PAGE_SHIFT; else
lpfn = bo->rdev->mc.gtt_size >> PAGE_SHIFT; /* ??? */if (max_offset)lpfn = min (lpfn, (unsigned)(max_offset >> PAGE_SHIFT));
bo->placements[i].lpfn = max_offset >> PAGE_SHIFT;
bo->placements[i].lpfn = lpfn; bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; }-- 2.1.0
-- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
On 10.09.2014 01:28, Alex Deucher wrote:
On Mon, Sep 8, 2014 at 9:15 PM, Michel Dänzer michel@daenzer.net wrote:
On 09.09.2014 09:47, Michel Dänzer wrote:
On 09.09.2014 02:36, Alex Deucher wrote:
Updated version with comments integrated.
[...]
@@ -314,10 +314,12 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, unsigned lpfn = 0;
/* force to pin into visible video ram */
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM)lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;else
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) {if (!(bo->flags & RADEON_GEM_NO_CPU_ACCESS))lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;} else { lpfn = bo->rdev->mc.gtt_size >> PAGE_SHIFT; /* ??? */}The else block can be removed as well, but that can be done in another patch.
Actually, I just noticed a problem, the following if statement:
if (max_offset) lpfn = min (lpfn, (unsigned)(max_offset >> PAGE_SHIFT));This will ignore max_offset if lpfn is 0. So either go with v1 of this hunk, or rebase on top of the patch below.
Rebased on your patch and attached.
My patch didn't handle max_offset == 0 correctly. Attaching a fixed v2 patch and your patch rebased on top of that.
On Wed, Sep 10, 2014 at 12:03 AM, Michel Dänzer michel@daenzer.net wrote:
On 10.09.2014 01:28, Alex Deucher wrote:
On Mon, Sep 8, 2014 at 9:15 PM, Michel Dänzer michel@daenzer.net wrote:
On 09.09.2014 09:47, Michel Dänzer wrote:
On 09.09.2014 02:36, Alex Deucher wrote:
Updated version with comments integrated.
[...]
@@ -314,10 +314,12 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, unsigned lpfn = 0;
/* force to pin into visible video ram */
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM)lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;else
if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) {if (!(bo->flags & RADEON_GEM_NO_CPU_ACCESS))lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;} else { lpfn = bo->rdev->mc.gtt_size >> PAGE_SHIFT; /* ??? */}The else block can be removed as well, but that can be done in another patch.
Actually, I just noticed a problem, the following if statement:
if (max_offset) lpfn = min (lpfn, (unsigned)(max_offset >>PAGE_SHIFT));
This will ignore max_offset if lpfn is 0. So either go with v1 of this hunk, or rebase on top of the patch below.
Rebased on your patch and attached.
My patch didn't handle max_offset == 0 correctly. Attaching a fixed v2 patch and your patch rebased on top of that.
Applied. thanks!
Alex
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Christian König -
Marek Olšák -
Michel Dänzer