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

["Michel Dänzer" <michel@daenzer.net>]

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