diff mbox

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

Message ID CADnq5_PjfMrdw4oRZOGyRvU5BLbiVLHpSeLZ+-sC4ChEu8bf2w@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Deucher Sept. 8, 2014, 5:36 p.m. UTC 
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

Comments

Michel Dänzer Sept. 9, 2014, 12:47 a.m. UTC | #1 
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>
diff mbox

Patch

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