Re: [PATCH v3 3/8] drm/etnaviv: share a single cmdbuf suballoc region across all GPUs

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

 



Hi,
On Fri, Aug 09, 2019 at 02:04:19PM +0200, Lucas Stach wrote:
> There is no need for each GPU to have it's own cmdbuf suballocation
> region. Only allocate a single one for the the etnaviv virtual device
> and share it across all GPUs.
> 
> As the suballoc space is now potentially shared by more hardware jobs
> running in parallel, double its size to 512KB to avoid contention.
> 
> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> Reviewed-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>

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

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c     | 14 +++++++-------
>  drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h     |  4 ++--
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c        | 19 +++++++++++++++----
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h        |  2 ++
>  drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  2 +-
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c        | 17 ++++-------------
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.h        |  1 -
>  7 files changed, 31 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> index e842d988edd4..b0babc0f7230 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> @@ -13,13 +13,13 @@
>  #include "etnaviv_mmu.h"
>  #include "etnaviv_perfmon.h"
>  
> -#define SUBALLOC_SIZE		SZ_256K
> +#define SUBALLOC_SIZE		SZ_512K
>  #define SUBALLOC_GRANULE	SZ_4K
>  #define SUBALLOC_GRANULES	(SUBALLOC_SIZE / SUBALLOC_GRANULE)
>  
>  struct etnaviv_cmdbuf_suballoc {
>  	/* suballocated dma buffer properties */
> -	struct etnaviv_gpu *gpu;
> +	struct device *dev;
>  	void *vaddr;
>  	dma_addr_t paddr;
>  
> @@ -31,7 +31,7 @@ struct etnaviv_cmdbuf_suballoc {
>  };
>  
>  struct etnaviv_cmdbuf_suballoc *
> -etnaviv_cmdbuf_suballoc_new(struct etnaviv_gpu * gpu)
> +etnaviv_cmdbuf_suballoc_new(struct device *dev)
>  {
>  	struct etnaviv_cmdbuf_suballoc *suballoc;
>  	int ret;
> @@ -40,11 +40,11 @@ etnaviv_cmdbuf_suballoc_new(struct etnaviv_gpu * gpu)
>  	if (!suballoc)
>  		return ERR_PTR(-ENOMEM);
>  
> -	suballoc->gpu = gpu;
> +	suballoc->dev = dev;
>  	mutex_init(&suballoc->lock);
>  	init_waitqueue_head(&suballoc->free_event);
>  
> -	suballoc->vaddr = dma_alloc_wc(gpu->dev, SUBALLOC_SIZE,
> +	suballoc->vaddr = dma_alloc_wc(dev, SUBALLOC_SIZE,
>  				       &suballoc->paddr, GFP_KERNEL);
>  	if (!suballoc->vaddr) {
>  		ret = -ENOMEM;
> @@ -76,7 +76,7 @@ void etnaviv_cmdbuf_suballoc_unmap(struct etnaviv_iommu *mmu,
>  
>  void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc)
>  {
> -	dma_free_wc(suballoc->gpu->dev, SUBALLOC_SIZE, suballoc->vaddr,
> +	dma_free_wc(suballoc->dev, SUBALLOC_SIZE, suballoc->vaddr,
>  		    suballoc->paddr);
>  	kfree(suballoc);
>  }
> @@ -101,7 +101,7 @@ int etnaviv_cmdbuf_init(struct etnaviv_cmdbuf_suballoc *suballoc,
>  						       suballoc->free_space,
>  						       msecs_to_jiffies(10 * 1000));
>  		if (!ret) {
> -			dev_err(suballoc->gpu->dev,
> +			dev_err(suballoc->dev,
>  				"Timeout waiting for cmdbuf space\n");
>  			return -ETIMEDOUT;
>  		}
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h
> index 583f56bf011c..a28668e46e26 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h
> @@ -8,7 +8,7 @@
>  
>  #include <linux/types.h>
>  
> -struct etnaviv_gpu;
> +struct device;
>  struct etnaviv_iommu;
>  struct etnaviv_vram_mapping;
>  struct etnaviv_cmdbuf_suballoc;
> @@ -25,7 +25,7 @@ struct etnaviv_cmdbuf {
>  };
>  
>  struct etnaviv_cmdbuf_suballoc *
> -etnaviv_cmdbuf_suballoc_new(struct etnaviv_gpu * gpu);
> +etnaviv_cmdbuf_suballoc_new(struct device *dev);
>  void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc);
>  int etnaviv_cmdbuf_suballoc_map(struct etnaviv_cmdbuf_suballoc *suballoc,
>  				struct etnaviv_iommu *mmu,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 9d4404723489..5fa3aa7bdbc5 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -530,23 +530,32 @@ static int etnaviv_bind(struct device *dev)
>  	INIT_LIST_HEAD(&priv->gem_list);
>  	priv->num_gpus = 0;
>  
> +	priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(drm->dev);
> +	if (IS_ERR(priv->cmdbuf_suballoc)) {
> +		dev_err(drm->dev, "Failed to create cmdbuf suballocator\n");
> +		ret = PTR_ERR(priv->cmdbuf_suballoc);
> +		goto out_free_priv;
> +	}
> +
>  	dev_set_drvdata(dev, drm);
>  
>  	ret = component_bind_all(dev, drm);
>  	if (ret < 0)
> -		goto out_bind;
> +		goto out_destroy_suballoc;
>  
>  	load_gpu(drm);
>  
>  	ret = drm_dev_register(drm, 0);
>  	if (ret)
> -		goto out_register;
> +		goto out_unbind;
>  
>  	return 0;
>  
> -out_register:
> +out_unbind:
>  	component_unbind_all(dev, drm);
> -out_bind:
> +out_destroy_suballoc:
> +	etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
> +out_free_priv:
>  	kfree(priv);
>  out_put:
>  	drm_dev_put(drm);
> @@ -565,6 +574,8 @@ static void etnaviv_unbind(struct device *dev)
>  
>  	dev->dma_parms = NULL;
>  
> +	etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
> +
>  	drm->dev_private = NULL;
>  	kfree(priv);
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index eabe394c4e25..e052d7db66ae 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -36,6 +36,8 @@ struct etnaviv_drm_private {
>  	struct device_dma_parameters dma_parms;
>  	struct etnaviv_gpu *gpu[ETNA_MAX_PIPES];
>  
> +	struct etnaviv_cmdbuf_suballoc *cmdbuf_suballoc;
> +
>  	/* list of GEM objects: */
>  	struct mutex gem_lock;
>  	struct list_head gem_list;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index f535a627f297..3f4f6ab388de 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -496,7 +496,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
>  		goto err_submit_ww_acquire;
>  	}
>  
> -	ret = etnaviv_cmdbuf_init(gpu->cmdbuf_suballoc, &submit->cmdbuf,
> +	ret = etnaviv_cmdbuf_init(priv->cmdbuf_suballoc, &submit->cmdbuf,
>  				  ALIGN(args->stream_size, 8) + 8);
>  	if (ret)
>  		goto err_submit_objects;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index f00547b88a13..179bc6c544ca 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -693,6 +693,7 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
>  
>  int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
>  {
> +	struct etnaviv_drm_private *priv = gpu->drm->dev_private;
>  	int ret, i;
>  
>  	ret = pm_runtime_get_sync(gpu->dev);
> @@ -760,23 +761,16 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
>  		goto fail;
>  	}
>  
> -	gpu->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(gpu);
> -	if (IS_ERR(gpu->cmdbuf_suballoc)) {
> -		dev_err(gpu->dev, "Failed to create cmdbuf suballocator\n");
> -		ret = PTR_ERR(gpu->cmdbuf_suballoc);
> -		goto destroy_iommu;
> -	}
> -
> -	ret = etnaviv_cmdbuf_suballoc_map(gpu->cmdbuf_suballoc, gpu->mmu,
> +	ret = etnaviv_cmdbuf_suballoc_map(priv->cmdbuf_suballoc, gpu->mmu,
>  					  &gpu->cmdbuf_mapping,
>  					  gpu->memory_base);
>  	if (ret) {
>  		dev_err(gpu->dev, "failed to map cmdbuf suballoc\n");
> -		goto destroy_suballoc;
> +		goto destroy_iommu;
>  	}
>  
>  	/* Create buffer: */
> -	ret = etnaviv_cmdbuf_init(gpu->cmdbuf_suballoc, &gpu->buffer,
> +	ret = etnaviv_cmdbuf_init(priv->cmdbuf_suballoc, &gpu->buffer,
>  				  PAGE_SIZE);
>  	if (ret) {
>  		dev_err(gpu->dev, "could not create command buffer\n");
> @@ -815,8 +809,6 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
>  	etnaviv_cmdbuf_free(&gpu->buffer);
>  unmap_suballoc:
>  	etnaviv_cmdbuf_suballoc_unmap(gpu->mmu, &gpu->cmdbuf_mapping);
> -destroy_suballoc:
> -	etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc);
>  destroy_iommu:
>  	etnaviv_iommu_destroy(gpu->mmu);
>  fail:
> @@ -1692,7 +1684,6 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master,
>  	if (gpu->initialized) {
>  		etnaviv_cmdbuf_free(&gpu->buffer);
>  		etnaviv_cmdbuf_suballoc_unmap(gpu->mmu, &gpu->cmdbuf_mapping);
> -		etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc);
>  		etnaviv_iommu_destroy(gpu->mmu);
>  		gpu->initialized = false;
>  	}
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 6a6add350d2d..933c8d016f11 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -137,7 +137,6 @@ struct etnaviv_gpu {
>  	int irq;
>  
>  	struct etnaviv_iommu *mmu;
> -	struct etnaviv_cmdbuf_suballoc *cmdbuf_suballoc;
>  
>  	/* Power Control: */
>  	struct clk *clk_bus;
> -- 
> 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