[PATCH RFC 1/2] drm/amdgpu: Add AMDGPU_BO_FLAG_CPU_ACCESS

[Christian König]

Am 30.06.2017 um 17:18 schrieb John Brooks:
> For userspace BO allocations, replace AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED
> with a new AMDGPU_BO_FLAG_CPU_ACCESS flag. This flag will be used to
> indicate that a BO should currently be CPU accessible. Unlike the
> CPU_ACCESS_REQUIRED flag, it is meant to be an ephemeral rather than a
> permanent constraint. Currently, however, it is treated no differently.
>
> Signed-off-by: John Brooks <john at fastquake.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 3 +++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 ++++++++-
>   include/uapi/drm/amdgpu_drm.h              | 1 +
>   3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 12d61ed..a724e4f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -411,6 +411,9 @@ struct amdgpu_bo_va {
>   
>   #define AMDGPU_GEM_DOMAIN_MAX		0x3
>   
> +/* BO internal flags */
> +#define AMDGPU_BO_FLAG_CPU_ACCESS	(AMDGPU_GEM_CREATE_MAX << 1)
Better put this into amdgpu_drm.h to keep all the flags together. We can 
reject kernel internal flags when we create the BO.

But in general I would still say we should stop clearing the flag when 
"bo->tbo.type != ttm_bo_type_device".

This way we are always on the save side and don't modify flags for 
kernel BOs.

Regards,
Christian.

> +
>   struct amdgpu_bo {
>   	/* Protected by tbo.reserved */
>   	u32				prefered_domains;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 8ee6965..fa8aeca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -128,7 +128,8 @@ static void amdgpu_ttm_placement_init(struct amdgpu_device *adev,
>   		places[c].flags = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
>   			TTM_PL_FLAG_VRAM;
>   
> -		if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
> +		if (flags & (AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
> +			     AMDGPU_BO_FLAG_CPU_ACCESS))
>   			places[c].lpfn = visible_pfn;
>   		else
>   			places[c].flags |= TTM_PL_FLAG_TOPDOWN;
> @@ -361,6 +362,12 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
>   	if (!kernel && bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
>   		bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
>   
> +	if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
> +		flags |= AMDGPU_BO_FLAG_CPU_ACCESS;
> +		/* Treat CPU_ACCESS_REQUIRED only as a hint if given by UMD */
> +		if (!kernel)
> +			flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> +	}
>   	bo->flags = flags;
>   
>   #ifdef CONFIG_X86_32
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index d9aa4a3..473076f 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -87,6 +87,7 @@ extern "C" {
>   #define AMDGPU_GEM_CREATE_SHADOW		(1 << 4)
>   /* Flag that allocating the BO should use linear VRAM */
>   #define AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS	(1 << 5)
> +#define AMDGPU_GEM_CREATE_MAX			(1 << 5)
>   
>   struct drm_amdgpu_gem_create_in  {
>   	/** the requested memory size */