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

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

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