From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Brian Masney <masneyb@onstation.org>
Cc: agross@kernel.org, david.brown@linaro.org, robdclark@gmail.com,
sean@poorly.run, robh+dt@kernel.org, airlied@linux.ie,
daniel@ffwll.ch, mark.rutland@arm.com, jonathan@marek.ca,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
devicetree@vger.kernel.org
Subject: Re: [PATCH 6/6] drm/msm/gpu: add ocmem init/cleanup functions
Date: Sun, 16 Jun 2019 11:06:33 -0700 [thread overview]
Message-ID: <20190616180633.GS22737@tuxbook-pro> (raw)
In-Reply-To: <20190616132930.6942-7-masneyb@onstation.org>
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
>
WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: Brian Masney <masneyb-1iNe0GrtECGEi8DpZVb4nw@public.gmane.org>
Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
jonathan-eSc4qw6YbEQ@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
airlied-cv59FeDIM0c@public.gmane.org,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
david.brown-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
agross-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
daniel-/w4YWyX8dFk@public.gmane.org,
sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org
Subject: Re: [PATCH 6/6] drm/msm/gpu: add ocmem init/cleanup functions
Date: Sun, 16 Jun 2019 11:06:33 -0700 [thread overview]
Message-ID: <20190616180633.GS22737@tuxbook-pro> (raw)
In-Reply-To: <20190616132930.6942-7-masneyb-1iNe0GrtECGEi8DpZVb4nw@public.gmane.org>
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
>
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
next prev parent reply other threads:[~2019-06-16 18:05 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-16 13:29 [PATCH 0/6] qcom: add OCMEM support Brian Masney
2019-06-16 13:29 ` [PATCH 1/6] dt-bindings: soc: qcom: add On Chip MEMory (OCMEM) bindings Brian Masney
2019-06-16 17:43 ` Bjorn Andersson
2019-06-17 14:29 ` Rob Herring
2019-06-17 14:29 ` Rob Herring
2019-06-16 13:29 ` [PATCH 2/6] dt-bindings: display: msm: gmu: add optional ocmem property Brian Masney
2019-06-16 13:29 ` Brian Masney
2019-06-19 18:06 ` [Freedreno] " Jordan Crouse
2019-06-19 18:06 ` Jordan Crouse
2019-06-19 20:16 ` Rob Herring
2019-06-19 20:16 ` Rob Herring
2019-06-19 20:21 ` Rob Clark
2019-06-21 2:14 ` Brian Masney
2019-06-21 2:14 ` Brian Masney
2019-06-22 23:28 ` Rob Clark
2019-06-22 23:28 ` Rob Clark
2019-06-16 13:29 ` [PATCH 3/6] firmware: qcom: scm: add support to restore secure config Brian Masney
2019-06-16 13:29 ` Brian Masney
2019-06-16 17:53 ` Bjorn Andersson
2019-06-16 17:53 ` Bjorn Andersson
2019-06-16 13:29 ` [PATCH 4/6] firmware: qcom: scm: add OCMEM lock/unlock interface Brian Masney
2019-06-16 13:29 ` Brian Masney
2019-06-16 17:54 ` Bjorn Andersson
2019-06-16 13:29 ` [PATCH 5/6] soc: qcom: add OCMEM driver Brian Masney
2019-06-16 13:29 ` Brian Masney
2019-06-16 17:41 ` Bjorn Andersson
2019-06-16 17:41 ` Bjorn Andersson
2019-06-18 2:02 ` Brian Masney
2019-06-18 2:02 ` Brian Masney
2019-06-18 2:29 ` Rob Clark
2019-06-18 2:29 ` Rob Clark
2019-06-16 13:29 ` [PATCH 6/6] drm/msm/gpu: add ocmem init/cleanup functions Brian Masney
2019-06-16 13:29 ` Brian Masney
2019-06-16 18:06 ` Bjorn Andersson [this message]
2019-06-16 18:06 ` Bjorn Andersson
2019-06-17 0:18 ` Brian Masney
2019-06-17 0:18 ` Brian Masney
2019-06-17 3:43 ` Bjorn Andersson
2019-06-17 3:43 ` Bjorn Andersson
2019-06-19 18:15 ` [Freedreno] " Jordan Crouse
2019-06-19 18:15 ` Jordan Crouse
2019-06-19 18:21 ` [Freedreno] " Jordan Crouse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190616180633.GS22737@tuxbook-pro \
--to=bjorn.andersson@linaro.org \
--cc=agross@kernel.org \
--cc=airlied@linux.ie \
--cc=daniel@ffwll.ch \
--cc=david.brown@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=jonathan@marek.ca \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=masneyb@onstation.org \
--cc=robdclark@gmail.com \
--cc=robh+dt@kernel.org \
--cc=sean@poorly.run \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.