On Sun 16 Jun 06:29 PDT 2019, Brian Masney wrote: > The files a3xx_gpu.c and a4xx_gpu.c have ifdefs for the OCMEM support > that was missing upstream. Add two new functions (adreno_gpu_ocmem_init > and adreno_gpu_ocmem_cleanup) that removes some duplicated code. We also > need to change the ifdef check for CONFIG_MSM_OCMEM to CONFIG_QCOM_OCMEM > now that OCMEM support is upstream. > > Signed-off-by: Brian Masney <masneyb@xxxxxxxxxxxxx> > --- > drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 33 +++++++------------- > drivers/gpu/drm/msm/adreno/a3xx_gpu.h | 3 +- > drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 30 ++++++------------ > drivers/gpu/drm/msm/adreno/a4xx_gpu.h | 3 +- > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 41 +++++++++++++++++++++++++ > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 ++++++ > 6 files changed, 74 insertions(+), 46 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c > index c3b4bc6e4155..72720bb2aca1 100644 > --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c > @@ -17,10 +17,6 @@ > * this program. If not, see <http://www.gnu.org/licenses/>. > */ > > -#ifdef CONFIG_MSM_OCMEM > -# include <mach/ocmem.h> > -#endif > - > #include "a3xx_gpu.h" > > #define A3XX_INT0_MASK \ > @@ -206,9 +202,9 @@ static int a3xx_hw_init(struct msm_gpu *gpu) > gpu_write(gpu, REG_A3XX_RBBM_GPR0_CTL, 0x00000000); > > /* Set the OCMEM base address for A330, etc */ > - if (a3xx_gpu->ocmem_hdl) { > + if (a3xx_gpu->ocmem.hdl) { > gpu_write(gpu, REG_A3XX_RB_GMEM_BASE_ADDR, > - (unsigned int)(a3xx_gpu->ocmem_base >> 14)); > + (unsigned int)(a3xx_gpu->ocmem.base >> 14)); This blindly requires that the ocmem allocator will return entries allocated to 16kB. Please ensure that a future implementation of the actual ocmem allocator maintains this (comments? checks?). > } > > /* Turn on performance counters: */ > @@ -329,10 +325,7 @@ static void a3xx_destroy(struct msm_gpu *gpu) > > adreno_gpu_cleanup(adreno_gpu); > > -#ifdef CONFIG_MSM_OCMEM > - if (a3xx_gpu->ocmem_base) > - ocmem_free(OCMEM_GRAPHICS, a3xx_gpu->ocmem_hdl); > -#endif > + adreno_gpu_ocmem_cleanup(&a3xx_gpu->ocmem); > > kfree(a3xx_gpu); > } > @@ -507,17 +500,10 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev) > > /* if needed, allocate gmem: */ > if (adreno_is_a330(adreno_gpu)) { > -#ifdef CONFIG_MSM_OCMEM > - /* TODO this is different/missing upstream: */ > - struct ocmem_buf *ocmem_hdl = > - ocmem_allocate(OCMEM_GRAPHICS, adreno_gpu->gmem); > - > - a3xx_gpu->ocmem_hdl = ocmem_hdl; > - a3xx_gpu->ocmem_base = ocmem_hdl->addr; > - adreno_gpu->gmem = ocmem_hdl->len; > - DBG("using %dK of OCMEM at 0x%08x", adreno_gpu->gmem / 1024, > - a3xx_gpu->ocmem_base); > -#endif > + ret = adreno_gpu_ocmem_init(&adreno_gpu->base.pdev->dev, > + adreno_gpu, &a3xx_gpu->ocmem); > + if (ret) > + goto fail; > } > > if (!gpu->aspace) { > @@ -530,11 +516,14 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev) > */ > DRM_DEV_ERROR(dev->dev, "No memory protection without IOMMU\n"); > ret = -ENXIO; > - goto fail; > + goto fail_cleanup_ocmem; > } > > return gpu; > > +fail_cleanup_ocmem: > + adreno_gpu_ocmem_cleanup(&a3xx_gpu->ocmem); > + > fail: > if (a3xx_gpu) > a3xx_destroy(&a3xx_gpu->base.base); > diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.h b/drivers/gpu/drm/msm/adreno/a3xx_gpu.h > index ab60dc9e344e..727c34f38f9e 100644 > --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.h > +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.h > @@ -30,8 +30,7 @@ struct a3xx_gpu { > struct adreno_gpu base; > > /* if OCMEM is used for GMEM: */ > - uint32_t ocmem_base; > - void *ocmem_hdl; > + struct adreno_ocmem ocmem; > }; > #define to_a3xx_gpu(x) container_of(x, struct a3xx_gpu, base) > > diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c > index ab2b752566d8..b8f825107796 100644 > --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c > @@ -2,9 +2,6 @@ > /* Copyright (c) 2014 The Linux Foundation. All rights reserved. > */ > #include "a4xx_gpu.h" > -#ifdef CONFIG_MSM_OCMEM > -# include <soc/qcom/ocmem.h> > -#endif > > #define A4XX_INT0_MASK \ > (A4XX_INT0_RBBM_AHB_ERROR | \ > @@ -188,7 +185,7 @@ static int a4xx_hw_init(struct msm_gpu *gpu) > (1 << 30) | 0xFFFF); > > gpu_write(gpu, REG_A4XX_RB_GMEM_BASE_ADDR, > - (unsigned int)(a4xx_gpu->ocmem_base >> 14)); > + (unsigned int)(a4xx_gpu->ocmem.base >> 14)); > > /* Turn on performance counters: */ > gpu_write(gpu, REG_A4XX_RBBM_PERFCTR_CTL, 0x01); > @@ -318,10 +315,7 @@ static void a4xx_destroy(struct msm_gpu *gpu) > > adreno_gpu_cleanup(adreno_gpu); > > -#ifdef CONFIG_MSM_OCMEM > - if (a4xx_gpu->ocmem_base) > - ocmem_free(OCMEM_GRAPHICS, a4xx_gpu->ocmem_hdl); > -#endif > + adreno_gpu_ocmem_cleanup(&a4xx_gpu->ocmem); > > kfree(a4xx_gpu); > } > @@ -578,17 +572,10 @@ struct msm_gpu *a4xx_gpu_init(struct drm_device *dev) > > /* if needed, allocate gmem: */ > if (adreno_is_a4xx(adreno_gpu)) { > -#ifdef CONFIG_MSM_OCMEM > - /* TODO this is different/missing upstream: */ > - struct ocmem_buf *ocmem_hdl = > - ocmem_allocate(OCMEM_GRAPHICS, adreno_gpu->gmem); > - > - a4xx_gpu->ocmem_hdl = ocmem_hdl; > - a4xx_gpu->ocmem_base = ocmem_hdl->addr; > - adreno_gpu->gmem = ocmem_hdl->len; > - DBG("using %dK of OCMEM at 0x%08x", adreno_gpu->gmem / 1024, > - a4xx_gpu->ocmem_base); > -#endif > + ret = adreno_gpu_ocmem_init(dev->dev, adreno_gpu, > + &a4xx_gpu->ocmem); > + if (ret) > + goto fail; > } > > if (!gpu->aspace) { > @@ -601,11 +588,14 @@ struct msm_gpu *a4xx_gpu_init(struct drm_device *dev) > */ > DRM_DEV_ERROR(dev->dev, "No memory protection without IOMMU\n"); > ret = -ENXIO; > - goto fail; > + goto fail_cleanup_ocmem; > } > > return gpu; > > +fail_cleanup_ocmem: > + adreno_gpu_ocmem_cleanup(&a4xx_gpu->ocmem); > + > fail: > if (a4xx_gpu) > a4xx_destroy(&a4xx_gpu->base.base); > diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.h b/drivers/gpu/drm/msm/adreno/a4xx_gpu.h > index d506311ee240..a01448cba2ea 100644 > --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.h > +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.h > @@ -16,8 +16,7 @@ struct a4xx_gpu { > struct adreno_gpu base; > > /* if OCMEM is used for GMEM: */ > - uint32_t ocmem_base; > - void *ocmem_hdl; > + struct adreno_ocmem ocmem; > }; > #define to_a4xx_gpu(x) container_of(x, struct a4xx_gpu, base) > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > index 6f7f4114afcf..e0a9409c8a32 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > @@ -29,6 +29,10 @@ > #include "msm_gem.h" > #include "msm_mmu.h" > > +#ifdef CONFIG_QCOM_OCMEM > +# include <soc/qcom/ocmem.h> > +#endif This file exists (after the previous patch), so no need to make its inclusion conditional. > + > static bool zap_available = true; > > static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname, > @@ -897,6 +901,43 @@ static int adreno_get_pwrlevels(struct device *dev, > return 0; > } > > +int adreno_gpu_ocmem_init(struct device *dev, struct adreno_gpu *adreno_gpu, > + struct adreno_ocmem *adreno_ocmem) > +{ > +#ifdef CONFIG_QCOM_OCMEM No need to make this conditional. > + struct ocmem_buf *ocmem_hdl; > + struct ocmem *ocmem; > + > + ocmem = of_get_ocmem(dev); > + if (!ocmem) { > + /* This is an optional property so return success. */ > + return 0; > + } > + > + ocmem_hdl = ocmem_allocate(ocmem, OCMEM_GRAPHICS, adreno_gpu->gmem); > + if (IS_ERR(ocmem_hdl)) > + return PTR_ERR(ocmem_hdl); > + > + adreno_ocmem->ocmem = ocmem; > + adreno_ocmem->base = ocmem_hdl->addr; > + adreno_ocmem->hdl = ocmem_hdl; > + adreno_gpu->gmem = ocmem_hdl->len; > + DBG("using %dK of OCMEM at 0x%08x", adreno_gpu->gmem / 1024, > + adreno_ocmem->base); > +#endif > + > + return 0; > +} > + > +void adreno_gpu_ocmem_cleanup(struct adreno_ocmem *adreno_ocmem) > +{ > +#ifdef CONFIG_QCOM_OCMEM > + if (adreno_ocmem->base) It would be nice to have ocmem_free() accept NULL, similar to kfree() et al. > + ocmem_free(adreno_ocmem->ocmem, OCMEM_GRAPHICS, > + adreno_ocmem->hdl); > +#endif > +} > + > int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > struct adreno_gpu *adreno_gpu, > const struct adreno_gpu_funcs *funcs, int nr_rings) > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > index 0925606ec9b5..1cd11570323b 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > @@ -136,6 +136,12 @@ struct adreno_gpu { > }; > #define to_adreno_gpu(x) container_of(x, struct adreno_gpu, base) > > +struct adreno_ocmem { > + struct ocmem *ocmem; > + uint32_t base; By ocmem being physically fixed this is sufficient, but unsigned long is a nicer type for carrying memory addresses. Regards, Bjorn > + void *hdl; > +}; > + > /* platform config data (ie. from DT, or pdata) */ > struct adreno_platform_config { > struct adreno_rev rev; > @@ -241,6 +247,10 @@ void adreno_dump(struct msm_gpu *gpu); > void adreno_wait_ring(struct msm_ringbuffer *ring, uint32_t ndwords); > struct msm_ringbuffer *adreno_active_ring(struct msm_gpu *gpu); > > +int adreno_gpu_ocmem_init(struct device *dev, struct adreno_gpu *adreno_gpu, > + struct adreno_ocmem *ocmem); > +void adreno_gpu_ocmem_cleanup(struct adreno_ocmem *ocmem); > + > int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > struct adreno_gpu *gpu, const struct adreno_gpu_funcs *funcs, > int nr_rings); > -- > 2.20.1 > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
