Message ID | 20190809120424.32679-3-l.stach@pengutronix.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v3,1/8] drm/etnaviv: simplify unbind checks | expand |
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@pengutronix.de> > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> Reviewed-by: Guido Günther <agx@sigxcpu.org> > --- > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/etnaviv
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;