| Message ID | 20190616132930.6942-7-masneyb@onstation.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | qcom: add OCMEM support | expand |
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@onstation.org> > --- > 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 >
Hi Bjorn, On Sun, Jun 16, 2019 at 11:06:33AM -0700, Bjorn Andersson wrote: > > 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. I have these #ifdefs for the a5xx and a6xx GPUs that don't have ocmem in the SoC. Without the #ifdefs, those systems would need to have the ocmem driver in their kernel. Thanks for the quick review on the patch set! Brian
On Sun 16 Jun 17:18 PDT 2019, Brian Masney wrote: > Hi Bjorn, > > On Sun, Jun 16, 2019 at 11:06:33AM -0700, Bjorn Andersson wrote: > > > 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. > > I have these #ifdefs for the a5xx and a6xx GPUs that don't have ocmem > in the SoC. Without the #ifdefs, those systems would need to have the > ocmem driver in their kernel. > In order to provide the means for compiling a kernel for a[56]xx without having to compile ocmem you should move these #ifdef to the ocmem header file and provide static inline dummies for the case when it's not. (and use #if IS_ENABLED(CONFIG_FOO)) Don't forget to add depends on QCOM_OCMEM || QCOM_OCMEM=n to the DRM_MSM config option, to allow the driver pair to be selected in all possible ways. > Thanks for the quick review on the patch set! > Regards, Bjorn
On Sun, Jun 16, 2019 at 09:29:30AM -0400, 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@onstation.org> > --- > 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)); > } > > /* 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); a3xx_destroy is going to have to be able to cleanup the ocmem anyway, so you might as well stick it in there instead of having a second goto label. > 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); And same. > 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 You won't need a ifdef here if the rest of the support is merged too. > +# include <soc/qcom/ocmem.h> > +#endif > + > 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 Same. It should be assumed that the generic ocmem functions will be properly stubbed. I suppose you have an argument for wanting to not compile in this extra code if it isn't needed, but if so you'll want to #ifdef the entire function(s) and use stubs in the include file instead. > + 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) > + 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; > + 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
On Wed, Jun 19, 2019 at 12:15:26PM -0600, Jordan Crouse wrote: > On Sun, Jun 16, 2019 at 09:29:30AM -0400, 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. Sorry for reviewing v1 when there was a v2 in flight. That will teach me to not keep up on my email. I think you caught most of this, but a few things I still saw. <snip> Jordan
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)); } /* 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 + 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 + 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) + 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; + 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);
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@onstation.org> --- 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(-)