[PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[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>
---
 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;
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] r600g, radeonsi: Inform the kernel if a BO will likely be accessed by the CPU

[Michel Dänzer]

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

Re: [PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[Christian König]

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;
> +		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;

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[Alex Deucher]

[-- Attachment #1: Type: text/plain, Size: 3222 bytes --]

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;
>> +               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;
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[-- Attachment #2: 0001-drm-radeon-add-RADEON_GEM_NO_CPU_ACCESS-BO-creation-.patch --]
[-- Type: text/x-diff, Size: 1902 bytes --]

From d6a65c62241fe2b3c1a0dd799f2af0802297446c Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Thu, 28 Aug 2014 10:59:05 -0400
Subject: [PATCH] drm/radeon: add RADEON_GEM_NO_CPU_ACCESS BO creation flag

Allows pinning of buffers in the non-CPU visible portion of
vram.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_object.c | 10 +++++++---
 include/uapi/drm/radeon_drm.h          |  2 ++
 2 files changed, 9 insertions(+), 3 deletions(-)

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;
+			else
+				lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;
+		} else {
 			lpfn = bo->rdev->mc.gtt_size >> PAGE_SHIFT; /* ??? */
+		}
 
 		if (max_offset)
 			lpfn = min (lpfn, (unsigned)(max_offset >> 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)
 
 struct drm_radeon_gem_create {
 	uint64_t	size;
-- 
1.8.3.1


[-- Attachment #3: Type: text/plain, Size: 156 bytes --]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[Michel Dänzer]

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;
> +			else
> +				lpfn = 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 */


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

Re: [Mesa-dev] [PATCH] r600g, radeonsi: Inform the kernel if a BO will likely be accessed by the CPU

[Marek Olšák]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[Alex Deucher]

[-- Attachment #1: Type: text/plain, Size: 2861 bytes --]

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;
>> +                     else
>> +                             lpfn = 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

[-- Attachment #2: 0001-drm-radeon-add-RADEON_GEM_NO_CPU_ACCESS-BO-creation-.patch --]
[-- Type: text/x-diff, Size: 1882 bytes --]

From 8e0fe1b090f75e5b7cadc9c316d1a9e3668c8ed2 Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Thu, 28 Aug 2014 10:59:05 -0400
Subject: [PATCH] drm/radeon: add RADEON_GEM_NO_CPU_ACCESS BO creation flag
 (v2)

Allows pinning of buffers in the non-CPU visible portion of
vram.

v2: incorporate Michel's comments.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_object.c | 8 +++++---
 include/uapi/drm/radeon_drm.h          | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index eef60aa..c7ad231d 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -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; /* ??? */
+		}
 
 		if (max_offset)
 			lpfn = min (lpfn, (unsigned)(max_offset >> PAGE_SHIFT));
diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h
index f755f20..50d0fb4 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)
+/* CPU access is not expected to work for this BO */
+#define RADEON_GEM_NO_CPU_ACCESS	(1 << 4)
 
 struct drm_radeon_gem_create {
 	uint64_t	size;
-- 
1.8.3.1


[-- Attachment #3: Type: text/plain, Size: 156 bytes --]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[Michel Dänzer]

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>


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

Re: [PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[Michel Dänzer]

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;
 	}
 
-- 
2.1.0


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

Re: [PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[Alex Deucher]

[-- Attachment #1: Type: text/plain, Size: 3375 bytes --]

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

[-- Attachment #2: 0001-drm-radeon-add-RADEON_GEM_NO_CPU_ACCESS-BO-creation-.patch --]
[-- Type: text/x-diff, Size: 1804 bytes --]

From 998fabb91851cef7c70e0f3920e45c70da301647 Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Tue, 9 Sep 2014 12:24:33 -0400
Subject: [PATCH] drm/radeon: add RADEON_GEM_NO_CPU_ACCESS BO creation flag
 (v3)

Allows pinning of buffers in the non-CPU visible portion of
vram.

v2: incorporate Michel's comments.
v3: rebase on Michel's patch

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_object.c | 3 ++-
 include/uapi/drm/radeon_drm.h          | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 68ab122..0d54993 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -312,7 +312,8 @@ 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++) {
 		/* force to pin into visible video ram */
-		if (bo->placements[i].flags & TTM_PL_FLAG_VRAM)
+		if ((bo->placements[i].flags & TTM_PL_FLAG_VRAM) &&
+		    (!(bo->flags & RADEON_GEM_NO_CPU_ACCESS)))
 			bo->placements[i].lpfn =
 				min(bo->rdev->mc.visible_vram_size, max_offset)
 				>> PAGE_SHIFT;
diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h
index f755f20..50d0fb4 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)
+/* CPU access is not expected to work for this BO */
+#define RADEON_GEM_NO_CPU_ACCESS	(1 << 4)
 
 struct drm_radeon_gem_create {
 	uint64_t	size;
-- 
1.8.3.1


[-- Attachment #3: Type: text/plain, Size: 156 bytes --]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[Michel Dänzer]

[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]

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.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-drm-radeon-Clean-up-assignment-of-TTM-placement-lpfn.patch --]
[-- Type: text/x-patch; name="0001-drm-radeon-Clean-up-assignment-of-TTM-placement-lpfn.patch", Size: 1842 bytes --]

>From 0428f396173e458288a5f0e1807033ebe9931ce0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michel=20D=C3=A4nzer?= <michel.daenzer@amd.com>
Date: Tue, 9 Sep 2014 10:09:23 +0900
Subject: [PATCH v2 1/2] 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

This sets the lpfn member to 0 instead of the full domain size. TTM uses
the full domain size when lpfn is 0.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---

v2: Handle max_offset == 0 correctly

 drivers/gpu/drm/radeon/radeon_object.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 908ea541..24c8772 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;
+		if ((bo->placements[i].flags & TTM_PL_FLAG_VRAM) &&
+		    (!max_offset || max_offset > bo->rdev->mc.visible_vram_size))
+			bo->placements[i].lpfn =
+				bo->rdev->mc.visible_vram_size >> 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-drm-radeon-add-RADEON_GEM_NO_CPU_ACCESS-BO-creation-.patch --]
[-- Type: text/x-patch; name="0002-drm-radeon-add-RADEON_GEM_NO_CPU_ACCESS-BO-creation-.patch", Size: 1965 bytes --]

>From 8e896486464526add633b6809b6d020ad810315c Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Thu, 28 Aug 2014 10:59:05 -0400
Subject: [PATCH 2/2] drm/radeon: add RADEON_GEM_NO_CPU_ACCESS BO creation flag
 (v4)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Allows pinning of buffers in the non-CPU visible portion of
vram.

v2: incorporate Michel's comments.
v3: rebase on Michel's patch
v4: rebase on Michel's v2 patch

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/radeon/radeon_object.c | 1 +
 include/uapi/drm/radeon_drm.h          | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 24c8772..d3f0a19 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -309,6 +309,7 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset,
 	for (i = 0; i < bo->placement.num_placement; i++) {
 		/* force to pin into visible video ram */
 		if ((bo->placements[i].flags & TTM_PL_FLAG_VRAM) &&
+		    !(bo->flags & RADEON_GEM_NO_CPU_ACCESS) &&
 		    (!max_offset || max_offset > bo->rdev->mc.visible_vram_size))
 			bo->placements[i].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 bf0067b..017f869 100644
--- a/include/uapi/drm/radeon_drm.h
+++ b/include/uapi/drm/radeon_drm.h
@@ -801,6 +801,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)
+/* CPU access is not expected to work for this BO */
+#define RADEON_GEM_NO_CPU_ACCESS	(1 << 4)
 
 struct drm_radeon_gem_create {
 	uint64_t	size;
-- 
2.1.0


[-- Attachment #4: Type: text/plain, Size: 156 bytes --]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[Alex Deucher]

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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev