Re: [PATCH v2 3/3] drm/etnaviv: implement softpin

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,
nOn Fri, Aug 09, 2019 at 02:05:14PM +0200, Lucas Stach wrote:
> With softpin we allow the userspace to take control over the GPU virtual
> address space. The new capability is relected by a bump of the minor DRM
> version. There are a few restrictions for userspace to take into
> account:
> 
> 1. The kernel reserves a bit of the address space to implement zero page
> faulting and mapping of the kernel internal ring buffer. Userspace can
> query the kernel for the first usable GPU VM address via
> ETNAVIV_PARAM_SOFTPIN_START_ADDR.
> 
> 2. We only allow softpin on GPUs, which implement proper process
> separation via PPAS. If softpin is not available the softpin start
> address will be set to ~0.
> 
> 3. Softpin is all or nothing. A submit using softpin must not use any
> address fixups via relocs.
> 
> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> Reviewed-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> ---
> v2: Extend 'presumed' field description to match its softpin use.
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c        |  2 +-
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h        |  2 ++
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c        |  5 ++--
>  drivers/gpu/drm/etnaviv/etnaviv_gem.h        |  1 +
>  drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 26 +++++++++++++++++++-
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c        |  9 +++++++
>  include/uapi/drm/etnaviv_drm.h               | 10 +++++++-
>  7 files changed, 50 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 80f1edcbbea0..45851d510f62 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -529,7 +529,7 @@ static struct drm_driver etnaviv_drm_driver = {
>  	.desc               = "etnaviv DRM",
>  	.date               = "20151214",
>  	.major              = 1,
> -	.minor              = 2,
> +	.minor              = 3,
>  };
>  
>  /*
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index a488cfdb6bbf..32cfa5a48d42 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -24,6 +24,8 @@ struct etnaviv_gem_object;
>  struct etnaviv_gem_submit;
>  struct etnaviv_iommu_global;
>  
> +#define ETNAVIV_SOFTPIN_START_ADDRESS	SZ_4M /* must be >=
> SUBALLOC_SIZE */

what about doing this as code like

#if ETNAVIV_SOFTPIN_START_ADDRESS < SUBALLOC_SIZE
# error "ETNAVIV_SOFTPIN_START_ADDRESS < SUBALLOC_SIZE"
#endif

in e.g. entaviv_cmdbuf.c ?

apart from that

Reviewed-by: Guido Günther <agx@xxxxxxxxxxx> 

> +
>  struct etnaviv_file_private {
>  	struct etnaviv_iommu_context	*mmu;
>  	struct drm_sched_entity		sched_entity[ETNA_MAX_PIPES];
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 1a64fad55740..be62f1368272 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -248,7 +248,8 @@ void etnaviv_gem_mapping_unreference(struct etnaviv_vram_mapping *mapping)
>  }
>  
>  struct etnaviv_vram_mapping *etnaviv_gem_mapping_get(
> -	struct drm_gem_object *obj, struct etnaviv_iommu_context *mmu_context)
> +	struct drm_gem_object *obj, struct etnaviv_iommu_context *mmu_context,
> +	u64 va)
>  {
>  	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>  	struct etnaviv_vram_mapping *mapping;
> @@ -309,7 +310,7 @@ struct etnaviv_vram_mapping *etnaviv_gem_mapping_get(
>  
>  	ret = etnaviv_iommu_map_gem(mmu_context, etnaviv_obj,
>  				    mmu_context->global->memory_base,
> -				    mapping, 0);
> +				    mapping, va);
>  	if (ret < 0)
>  		kfree(mapping);
>  	else
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index 2d01ee1f65c1..f1164934ecdf 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -77,6 +77,7 @@ static inline bool is_active(struct etnaviv_gem_object *etnaviv_obj)
>  
>  struct etnaviv_gem_submit_bo {
>  	u32 flags;
> +	u64 va;
>  	struct etnaviv_gem_object *obj;
>  	struct etnaviv_vram_mapping *mapping;
>  	struct dma_fence *excl;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 63a1206492d2..75afce42921b 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -72,6 +72,14 @@ static int submit_lookup_objects(struct etnaviv_gem_submit *submit,
>  		}
>  
>  		submit->bos[i].flags = bo->flags;
> +		if (submit->flags & ETNA_SUBMIT_SOFTPIN) {
> +			if (bo->presumed < ETNAVIV_SOFTPIN_START_ADDRESS) {
> +				DRM_ERROR("invalid softpin address\n");
> +				ret = -EINVAL;
> +				goto out_unlock;
> +			}
> +			submit->bos[i].va = bo->presumed;
> +		}
>  
>  		/* normally use drm_gem_object_lookup(), but for bulk lookup
>  		 * all under single table_lock just hit object_idr directly:
> @@ -224,11 +232,17 @@ static int submit_pin_objects(struct etnaviv_gem_submit *submit)
>  		struct etnaviv_vram_mapping *mapping;
>  
>  		mapping = etnaviv_gem_mapping_get(&etnaviv_obj->base,
> -						  submit->mmu_context);
> +						  submit->mmu_context,
> +						  submit->bos[i].va);
>  		if (IS_ERR(mapping)) {
>  			ret = PTR_ERR(mapping);
>  			break;
>  		}
> +
> +		if ((submit->flags & ETNA_SUBMIT_SOFTPIN) &&
> +		     submit->bos[i].va != mapping->iova)
> +			return -EINVAL;
> +
>  		atomic_inc(&etnaviv_obj->gpu_active);
>  
>  		submit->bos[i].flags |= BO_PINNED;
> @@ -261,6 +275,10 @@ static int submit_reloc(struct etnaviv_gem_submit *submit, void *stream,
>  	u32 *ptr = stream;
>  	int ret;
>  
> +	/* Submits using softpin don't blend with relocs */
> +	if ((submit->flags & ETNA_SUBMIT_SOFTPIN) && nr_relocs != 0)
> +		return -EINVAL;
> +
>  	for (i = 0; i < nr_relocs; i++) {
>  		const struct drm_etnaviv_gem_submit_reloc *r = relocs + i;
>  		struct etnaviv_gem_submit_bo *bo;
> @@ -445,6 +463,12 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
>  		return -EINVAL;
>  	}
>  
> +	if ((args->flags & ETNA_SUBMIT_SOFTPIN) &&
> +	    priv->mmu_global->version != ETNAVIV_IOMMU_V2) {
> +		DRM_ERROR("softpin requested on incompatible MMU\n");
> +		return -EINVAL;
> +	}
> +
>  	/*
>  	 * Copy the command submission and bo array to kernel space in
>  	 * one go, and do this outside of any locks.
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index d8a83ebfce47..d47d1a8e0219 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -42,6 +42,8 @@ static const struct platform_device_id gpu_ids[] = {
>  
>  int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
>  {
> +	struct etnaviv_drm_private *priv = gpu->drm->dev_private;
> +
>  	switch (param) {
>  	case ETNAVIV_PARAM_GPU_MODEL:
>  		*value = gpu->identity.model;
> @@ -147,6 +149,13 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
>  		*value = gpu->identity.varyings_count;
>  		break;
>  
> +	case ETNAVIV_PARAM_SOFTPIN_START_ADDR:
> +		if (priv->mmu_global->version == ETNAVIV_IOMMU_V2)
> +			*value = ETNAVIV_SOFTPIN_START_ADDRESS;
> +		else
> +			*value = ~0ULL;
> +		break;
> +
>  	default:
>  		DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
>  		return -EINVAL;
> diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
> index 0d5c49dc478c..09d0df8b71c5 100644
> --- a/include/uapi/drm/etnaviv_drm.h
> +++ b/include/uapi/drm/etnaviv_drm.h
> @@ -73,6 +73,7 @@ struct drm_etnaviv_timespec {
>  #define ETNAVIV_PARAM_GPU_INSTRUCTION_COUNT         0x18
>  #define ETNAVIV_PARAM_GPU_NUM_CONSTANTS             0x19
>  #define ETNAVIV_PARAM_GPU_NUM_VARYINGS              0x1a
> +#define ETNAVIV_PARAM_SOFTPIN_START_ADDR            0x1b
>  
>  #define ETNA_MAX_PIPES 4
>  
> @@ -148,6 +149,11 @@ struct drm_etnaviv_gem_submit_reloc {
>   * then patching the cmdstream for this entry is skipped.  This can
>   * avoid kernel needing to map/access the cmdstream bo in the common
>   * case.
> + * If the submit is a softpin submit (ETNA_SUBMIT_SOFTPIN) the 'presumed'
> + * field is interpreted as the fixed location to map the bo into the gpu
> + * virtual address space. If the kernel is unable to map the buffer at
> + * this location the submit will fail. This means userspace is responsible
> + * for the whole gpu virtual address management.
>   */
>  #define ETNA_SUBMIT_BO_READ             0x0001
>  #define ETNA_SUBMIT_BO_WRITE            0x0002
> @@ -177,9 +183,11 @@ struct drm_etnaviv_gem_submit_pmr {
>  #define ETNA_SUBMIT_NO_IMPLICIT         0x0001
>  #define ETNA_SUBMIT_FENCE_FD_IN         0x0002
>  #define ETNA_SUBMIT_FENCE_FD_OUT        0x0004
> +#define ETNA_SUBMIT_SOFTPIN             0x0008
>  #define ETNA_SUBMIT_FLAGS		(ETNA_SUBMIT_NO_IMPLICIT | \
>  					 ETNA_SUBMIT_FENCE_FD_IN | \
> -					 ETNA_SUBMIT_FENCE_FD_OUT)
> +					 ETNA_SUBMIT_FENCE_FD_OUT| \
> +					 ETNA_SUBMIT_SOFTPIN)
>  #define ETNA_PIPE_3D      0x00
>  #define ETNA_PIPE_2D      0x01
>  #define ETNA_PIPE_VG      0x02
> -- 
> 2.20.1
> 
> _______________________________________________
> etnaviv mailing list
> etnaviv@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux