diff mbox series

[05/12] drm/msm/adreno: Use quirk to identify cached-coherent support

Message ID 20230706211045.204925-6-robdclark@gmail.com (mailing list archive)
State Superseded
Headers show
Series drm/msm/adreno: Move away from legacy revision matching | expand

Commit Message

Rob Clark July 6, 2023, 9:10 p.m. UTC
From: Rob Clark <robdclark@chromium.org>

It is better to explicitly list it.  With the move to opaque chip-id's
for future devices, we should avoid trying to infer things like
generation from the numerical value.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 23 +++++++++++++++-------
 drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  1 +
 2 files changed, 17 insertions(+), 7 deletions(-)

Comments

Konrad Dybcio July 6, 2023, 11:29 p.m. UTC | #1
On 6.07.2023 23:10, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> It is better to explicitly list it.  With the move to opaque chip-id's
> for future devices, we should avoid trying to infer things like
> generation from the numerical value.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  drivers/gpu/drm/msm/adreno/adreno_device.c | 23 +++++++++++++++-------
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  1 +
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index f469f951a907..3c531da417b9 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -256,6 +256,7 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_512K,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
>  		.init = a6xx_gpu_init,
>  	}, {
>  		.rev = ADRENO_REV(6, 1, 9, ANY_ID),
> @@ -266,6 +267,7 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_512K,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a615_zap.mdt",
>  		.hwcg = a615_hwcg,
> @@ -278,6 +280,7 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_1M,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a630_zap.mdt",
>  		.hwcg = a630_hwcg,
> @@ -290,6 +293,7 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_1M,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a640_zap.mdt",
>  		.hwcg = a640_hwcg,
> @@ -302,7 +306,8 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_1M + SZ_128K,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> -		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> +			ADRENO_QUIRK_HAS_HW_APRIV,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a650_zap.mdt",
>  		.hwcg = a650_hwcg,
> @@ -316,7 +321,8 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_1M + SZ_512K,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> -		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> +			ADRENO_QUIRK_HAS_HW_APRIV,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a660_zap.mdt",
>  		.hwcg = a660_hwcg,
> @@ -329,7 +335,8 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_512K,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> -		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> +			ADRENO_QUIRK_HAS_HW_APRIV,
>  		.init = a6xx_gpu_init,
>  		.hwcg = a660_hwcg,
>  		.address_space_size = SZ_16G,
> @@ -342,6 +349,7 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_2M,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a640_zap.mdt",
>  		.hwcg = a640_hwcg,
> @@ -353,7 +361,8 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_4M,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> -		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> +			ADRENO_QUIRK_HAS_HW_APRIV,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a690_zap.mdt",
>  		.hwcg = a690_hwcg,
> @@ -565,9 +574,9 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
>  	if (ret)
>  		return ret;
>  
> -	if (config.rev.core >= 6)
> -		if (!adreno_has_gmu_wrapper(to_adreno_gpu(gpu)))
> -			priv->has_cached_coherent = true;
> +	priv->has_cached_coherent =
> +		!!(info->quirks & ADRENO_QUIRK_HAS_CACHED_COHERENT) &&
> +		!adreno_has_gmu_wrapper(to_adreno_gpu(gpu));
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index a7c4a2c536e3..e08d41337169 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -33,6 +33,7 @@ enum {
>  #define ADRENO_QUIRK_FAULT_DETECT_MASK		BIT(1)
>  #define ADRENO_QUIRK_LMLOADKILL_DISABLE		BIT(2)
>  #define ADRENO_QUIRK_HAS_HW_APRIV		BIT(3)
> +#define ADRENO_QUIRK_HAS_CACHED_COHERENT	BIT(4)
>  
>  struct adreno_rev {
>  	uint8_t  core;
Dmitry Baryshkov July 7, 2023, 2:29 a.m. UTC | #2
On 07/07/2023 00:10, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> It is better to explicitly list it.  With the move to opaque chip-id's
> for future devices, we should avoid trying to infer things like
> generation from the numerical value.

Would it be better to push this to DT? I mean, we already have a 
'dma-cache-coherent' property for it.

> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>   drivers/gpu/drm/msm/adreno/adreno_device.c | 23 +++++++++++++++-------
>   drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  1 +
>   2 files changed, 17 insertions(+), 7 deletions(-)
>
Rob Clark July 7, 2023, 3:53 p.m. UTC | #3
On Thu, Jul 6, 2023 at 7:29 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 07/07/2023 00:10, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > It is better to explicitly list it.  With the move to opaque chip-id's
> > for future devices, we should avoid trying to infer things like
> > generation from the numerical value.
>
> Would it be better to push this to DT? I mean, we already have a
> 'dma-cache-coherent' property for it.

I suppose that would also handle the case where some a6xy are coherent
but others aren't..  OTOH it isn't the case that dma operations are
coherent, just that they can be.  It depends on smmu pte bits.  Maybe
that bit of pedanticism doesn't matter since we mostly bypass the dma
api, but we still do need to (ab)use dma_map_sgtable/dma_unmap_sgtable
for cache ops

BR,
-R

> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >   drivers/gpu/drm/msm/adreno/adreno_device.c | 23 +++++++++++++++-------
> >   drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  1 +
> >   2 files changed, 17 insertions(+), 7 deletions(-)
> >
>
> --
> With best wishes
> Dmitry
>
Akhil P Oommen July 13, 2023, 8:05 p.m. UTC | #4
On Thu, Jul 06, 2023 at 02:10:38PM -0700, Rob Clark wrote:
> 
> From: Rob Clark <robdclark@chromium.org>
> 
> It is better to explicitly list it.  With the move to opaque chip-id's
> for future devices, we should avoid trying to infer things like
> generation from the numerical value.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_device.c | 23 +++++++++++++++-------
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  1 +
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index f469f951a907..3c531da417b9 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -256,6 +256,7 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_512K,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
>  		.init = a6xx_gpu_init,
>  	}, {
>  		.rev = ADRENO_REV(6, 1, 9, ANY_ID),
> @@ -266,6 +267,7 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_512K,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a615_zap.mdt",
>  		.hwcg = a615_hwcg,
> @@ -278,6 +280,7 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_1M,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a630_zap.mdt",
>  		.hwcg = a630_hwcg,
> @@ -290,6 +293,7 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_1M,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a640_zap.mdt",
>  		.hwcg = a640_hwcg,
> @@ -302,7 +306,8 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_1M + SZ_128K,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> -		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> +			ADRENO_QUIRK_HAS_HW_APRIV,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a650_zap.mdt",
>  		.hwcg = a650_hwcg,
> @@ -316,7 +321,8 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_1M + SZ_512K,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> -		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> +			ADRENO_QUIRK_HAS_HW_APRIV,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a660_zap.mdt",
>  		.hwcg = a660_hwcg,
> @@ -329,7 +335,8 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_512K,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> -		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> +			ADRENO_QUIRK_HAS_HW_APRIV,
>  		.init = a6xx_gpu_init,
>  		.hwcg = a660_hwcg,
>  		.address_space_size = SZ_16G,
> @@ -342,6 +349,7 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_2M,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a640_zap.mdt",
>  		.hwcg = a640_hwcg,
> @@ -353,7 +361,8 @@ static const struct adreno_info gpulist[] = {
>  		},
>  		.gmem = SZ_4M,
>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> -		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> +			ADRENO_QUIRK_HAS_HW_APRIV,
>  		.init = a6xx_gpu_init,
>  		.zapfw = "a690_zap.mdt",
>  		.hwcg = a690_hwcg,
> @@ -565,9 +574,9 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
>  	if (ret)
>  		return ret;
>  
> -	if (config.rev.core >= 6)
> -		if (!adreno_has_gmu_wrapper(to_adreno_gpu(gpu)))
> -			priv->has_cached_coherent = true;
> +	priv->has_cached_coherent =
> +		!!(info->quirks & ADRENO_QUIRK_HAS_CACHED_COHERENT) &&
> +		!adreno_has_gmu_wrapper(to_adreno_gpu(gpu));
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index a7c4a2c536e3..e08d41337169 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -33,6 +33,7 @@ enum {
>  #define ADRENO_QUIRK_FAULT_DETECT_MASK		BIT(1)
>  #define ADRENO_QUIRK_LMLOADKILL_DISABLE		BIT(2)
>  #define ADRENO_QUIRK_HAS_HW_APRIV		BIT(3)
> +#define ADRENO_QUIRK_HAS_CACHED_COHERENT	BIT(4)
a shorter ADRENO_IS_IOCOHERENT?

-Akhil

>  
>  struct adreno_rev {
>  	uint8_t  core;
> -- 
> 2.41.0
>
Rob Clark July 13, 2023, 10:25 p.m. UTC | #5
On Thu, Jul 13, 2023 at 1:06 PM Akhil P Oommen <quic_akhilpo@quicinc.com> wrote:
>
> On Thu, Jul 06, 2023 at 02:10:38PM -0700, Rob Clark wrote:
> >
> > From: Rob Clark <robdclark@chromium.org>
> >
> > It is better to explicitly list it.  With the move to opaque chip-id's
> > for future devices, we should avoid trying to infer things like
> > generation from the numerical value.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >  drivers/gpu/drm/msm/adreno/adreno_device.c | 23 +++++++++++++++-------
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  1 +
> >  2 files changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > index f469f951a907..3c531da417b9 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > @@ -256,6 +256,7 @@ static const struct adreno_info gpulist[] = {
> >               },
> >               .gmem = SZ_512K,
> >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> >               .init = a6xx_gpu_init,
> >       }, {
> >               .rev = ADRENO_REV(6, 1, 9, ANY_ID),
> > @@ -266,6 +267,7 @@ static const struct adreno_info gpulist[] = {
> >               },
> >               .gmem = SZ_512K,
> >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> >               .init = a6xx_gpu_init,
> >               .zapfw = "a615_zap.mdt",
> >               .hwcg = a615_hwcg,
> > @@ -278,6 +280,7 @@ static const struct adreno_info gpulist[] = {
> >               },
> >               .gmem = SZ_1M,
> >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> >               .init = a6xx_gpu_init,
> >               .zapfw = "a630_zap.mdt",
> >               .hwcg = a630_hwcg,
> > @@ -290,6 +293,7 @@ static const struct adreno_info gpulist[] = {
> >               },
> >               .gmem = SZ_1M,
> >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> >               .init = a6xx_gpu_init,
> >               .zapfw = "a640_zap.mdt",
> >               .hwcg = a640_hwcg,
> > @@ -302,7 +306,8 @@ static const struct adreno_info gpulist[] = {
> >               },
> >               .gmem = SZ_1M + SZ_128K,
> >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > -             .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > +                     ADRENO_QUIRK_HAS_HW_APRIV,
> >               .init = a6xx_gpu_init,
> >               .zapfw = "a650_zap.mdt",
> >               .hwcg = a650_hwcg,
> > @@ -316,7 +321,8 @@ static const struct adreno_info gpulist[] = {
> >               },
> >               .gmem = SZ_1M + SZ_512K,
> >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > -             .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > +                     ADRENO_QUIRK_HAS_HW_APRIV,
> >               .init = a6xx_gpu_init,
> >               .zapfw = "a660_zap.mdt",
> >               .hwcg = a660_hwcg,
> > @@ -329,7 +335,8 @@ static const struct adreno_info gpulist[] = {
> >               },
> >               .gmem = SZ_512K,
> >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > -             .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > +                     ADRENO_QUIRK_HAS_HW_APRIV,
> >               .init = a6xx_gpu_init,
> >               .hwcg = a660_hwcg,
> >               .address_space_size = SZ_16G,
> > @@ -342,6 +349,7 @@ static const struct adreno_info gpulist[] = {
> >               },
> >               .gmem = SZ_2M,
> >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> >               .init = a6xx_gpu_init,
> >               .zapfw = "a640_zap.mdt",
> >               .hwcg = a640_hwcg,
> > @@ -353,7 +361,8 @@ static const struct adreno_info gpulist[] = {
> >               },
> >               .gmem = SZ_4M,
> >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > -             .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > +                     ADRENO_QUIRK_HAS_HW_APRIV,
> >               .init = a6xx_gpu_init,
> >               .zapfw = "a690_zap.mdt",
> >               .hwcg = a690_hwcg,
> > @@ -565,9 +574,9 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
> >       if (ret)
> >               return ret;
> >
> > -     if (config.rev.core >= 6)
> > -             if (!adreno_has_gmu_wrapper(to_adreno_gpu(gpu)))
> > -                     priv->has_cached_coherent = true;
> > +     priv->has_cached_coherent =
> > +             !!(info->quirks & ADRENO_QUIRK_HAS_CACHED_COHERENT) &&
> > +             !adreno_has_gmu_wrapper(to_adreno_gpu(gpu));
> >
> >       return 0;
> >  }
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > index a7c4a2c536e3..e08d41337169 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > @@ -33,6 +33,7 @@ enum {
> >  #define ADRENO_QUIRK_FAULT_DETECT_MASK               BIT(1)
> >  #define ADRENO_QUIRK_LMLOADKILL_DISABLE              BIT(2)
> >  #define ADRENO_QUIRK_HAS_HW_APRIV            BIT(3)
> > +#define ADRENO_QUIRK_HAS_CACHED_COHERENT     BIT(4)
> a shorter ADRENO_IS_IOCOHERENT?

I prefer "HAS" to "IS".. maybe it is just me but "IS" sounds to me
like all dma is coherent, while in fact gpu mappings can be either
coherent or not.

I suppose it could be "HAS_IOCOHERENT" but we do use "CACHED_COHERENT"
elsewhere.

BR,
-R

>
> -Akhil
>
> >
> >  struct adreno_rev {
> >       uint8_t  core;
> > --
> > 2.41.0
> >
Akhil P Oommen July 17, 2023, 10 p.m. UTC | #6
On Thu, Jul 13, 2023 at 03:25:33PM -0700, Rob Clark wrote:
> 
> On Thu, Jul 13, 2023 at 1:06 PM Akhil P Oommen <quic_akhilpo@quicinc.com> wrote:
> >
> > On Thu, Jul 06, 2023 at 02:10:38PM -0700, Rob Clark wrote:
> > >
> > > From: Rob Clark <robdclark@chromium.org>
> > >
> > > It is better to explicitly list it.  With the move to opaque chip-id's
> > > for future devices, we should avoid trying to infer things like
> > > generation from the numerical value.
> > >
> > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > ---
> > >  drivers/gpu/drm/msm/adreno/adreno_device.c | 23 +++++++++++++++-------
> > >  drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  1 +
> > >  2 files changed, 17 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > index f469f951a907..3c531da417b9 100644
> > > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > @@ -256,6 +256,7 @@ static const struct adreno_info gpulist[] = {
> > >               },
> > >               .gmem = SZ_512K,
> > >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> > >               .init = a6xx_gpu_init,
> > >       }, {
> > >               .rev = ADRENO_REV(6, 1, 9, ANY_ID),
> > > @@ -266,6 +267,7 @@ static const struct adreno_info gpulist[] = {
> > >               },
> > >               .gmem = SZ_512K,
> > >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> > >               .init = a6xx_gpu_init,
> > >               .zapfw = "a615_zap.mdt",
> > >               .hwcg = a615_hwcg,
> > > @@ -278,6 +280,7 @@ static const struct adreno_info gpulist[] = {
> > >               },
> > >               .gmem = SZ_1M,
> > >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> > >               .init = a6xx_gpu_init,
> > >               .zapfw = "a630_zap.mdt",
> > >               .hwcg = a630_hwcg,
> > > @@ -290,6 +293,7 @@ static const struct adreno_info gpulist[] = {
> > >               },
> > >               .gmem = SZ_1M,
> > >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> > >               .init = a6xx_gpu_init,
> > >               .zapfw = "a640_zap.mdt",
> > >               .hwcg = a640_hwcg,
> > > @@ -302,7 +306,8 @@ static const struct adreno_info gpulist[] = {
> > >               },
> > >               .gmem = SZ_1M + SZ_128K,
> > >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > -             .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > > +                     ADRENO_QUIRK_HAS_HW_APRIV,
> > >               .init = a6xx_gpu_init,
> > >               .zapfw = "a650_zap.mdt",
> > >               .hwcg = a650_hwcg,
> > > @@ -316,7 +321,8 @@ static const struct adreno_info gpulist[] = {
> > >               },
> > >               .gmem = SZ_1M + SZ_512K,
> > >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > -             .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > > +                     ADRENO_QUIRK_HAS_HW_APRIV,
> > >               .init = a6xx_gpu_init,
> > >               .zapfw = "a660_zap.mdt",
> > >               .hwcg = a660_hwcg,
> > > @@ -329,7 +335,8 @@ static const struct adreno_info gpulist[] = {
> > >               },
> > >               .gmem = SZ_512K,
> > >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > -             .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > > +                     ADRENO_QUIRK_HAS_HW_APRIV,
> > >               .init = a6xx_gpu_init,
> > >               .hwcg = a660_hwcg,
> > >               .address_space_size = SZ_16G,
> > > @@ -342,6 +349,7 @@ static const struct adreno_info gpulist[] = {
> > >               },
> > >               .gmem = SZ_2M,
> > >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> > >               .init = a6xx_gpu_init,
> > >               .zapfw = "a640_zap.mdt",
> > >               .hwcg = a640_hwcg,
> > > @@ -353,7 +361,8 @@ static const struct adreno_info gpulist[] = {
> > >               },
> > >               .gmem = SZ_4M,
> > >               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > -             .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > > +             .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > > +                     ADRENO_QUIRK_HAS_HW_APRIV,
> > >               .init = a6xx_gpu_init,
> > >               .zapfw = "a690_zap.mdt",
> > >               .hwcg = a690_hwcg,
> > > @@ -565,9 +574,9 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
> > >       if (ret)
> > >               return ret;
> > >
> > > -     if (config.rev.core >= 6)
> > > -             if (!adreno_has_gmu_wrapper(to_adreno_gpu(gpu)))
> > > -                     priv->has_cached_coherent = true;
> > > +     priv->has_cached_coherent =
> > > +             !!(info->quirks & ADRENO_QUIRK_HAS_CACHED_COHERENT) &&
> > > +             !adreno_has_gmu_wrapper(to_adreno_gpu(gpu));
> > >
> > >       return 0;
> > >  }
> > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > > index a7c4a2c536e3..e08d41337169 100644
> > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > > @@ -33,6 +33,7 @@ enum {
> > >  #define ADRENO_QUIRK_FAULT_DETECT_MASK               BIT(1)
> > >  #define ADRENO_QUIRK_LMLOADKILL_DISABLE              BIT(2)
> > >  #define ADRENO_QUIRK_HAS_HW_APRIV            BIT(3)
> > > +#define ADRENO_QUIRK_HAS_CACHED_COHERENT     BIT(4)
> > a shorter ADRENO_IS_IOCOHERENT?
> 
> I prefer "HAS" to "IS".. maybe it is just me but "IS" sounds to me
> like all dma is coherent, while in fact gpu mappings can be either
> coherent or not.

Okay. Sounds good. 

-Akhil.
> 
> I suppose it could be "HAS_IOCOHERENT" but we do use "CACHED_COHERENT"
> elsewhere.
> 
> BR,
> -R
> 
> >
> > -Akhil
> >
> > >
> > >  struct adreno_rev {
> > >       uint8_t  core;
> > > --
> > > 2.41.0
> > >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index f469f951a907..3c531da417b9 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -256,6 +256,7 @@  static const struct adreno_info gpulist[] = {
 		},
 		.gmem = SZ_512K,
 		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
 		.init = a6xx_gpu_init,
 	}, {
 		.rev = ADRENO_REV(6, 1, 9, ANY_ID),
@@ -266,6 +267,7 @@  static const struct adreno_info gpulist[] = {
 		},
 		.gmem = SZ_512K,
 		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
 		.init = a6xx_gpu_init,
 		.zapfw = "a615_zap.mdt",
 		.hwcg = a615_hwcg,
@@ -278,6 +280,7 @@  static const struct adreno_info gpulist[] = {
 		},
 		.gmem = SZ_1M,
 		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
 		.init = a6xx_gpu_init,
 		.zapfw = "a630_zap.mdt",
 		.hwcg = a630_hwcg,
@@ -290,6 +293,7 @@  static const struct adreno_info gpulist[] = {
 		},
 		.gmem = SZ_1M,
 		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
 		.init = a6xx_gpu_init,
 		.zapfw = "a640_zap.mdt",
 		.hwcg = a640_hwcg,
@@ -302,7 +306,8 @@  static const struct adreno_info gpulist[] = {
 		},
 		.gmem = SZ_1M + SZ_128K,
 		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
-		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
+			ADRENO_QUIRK_HAS_HW_APRIV,
 		.init = a6xx_gpu_init,
 		.zapfw = "a650_zap.mdt",
 		.hwcg = a650_hwcg,
@@ -316,7 +321,8 @@  static const struct adreno_info gpulist[] = {
 		},
 		.gmem = SZ_1M + SZ_512K,
 		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
-		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
+			ADRENO_QUIRK_HAS_HW_APRIV,
 		.init = a6xx_gpu_init,
 		.zapfw = "a660_zap.mdt",
 		.hwcg = a660_hwcg,
@@ -329,7 +335,8 @@  static const struct adreno_info gpulist[] = {
 		},
 		.gmem = SZ_512K,
 		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
-		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
+			ADRENO_QUIRK_HAS_HW_APRIV,
 		.init = a6xx_gpu_init,
 		.hwcg = a660_hwcg,
 		.address_space_size = SZ_16G,
@@ -342,6 +349,7 @@  static const struct adreno_info gpulist[] = {
 		},
 		.gmem = SZ_2M,
 		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
 		.init = a6xx_gpu_init,
 		.zapfw = "a640_zap.mdt",
 		.hwcg = a640_hwcg,
@@ -353,7 +361,8 @@  static const struct adreno_info gpulist[] = {
 		},
 		.gmem = SZ_4M,
 		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
-		.quirks = ADRENO_QUIRK_HAS_HW_APRIV,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
+			ADRENO_QUIRK_HAS_HW_APRIV,
 		.init = a6xx_gpu_init,
 		.zapfw = "a690_zap.mdt",
 		.hwcg = a690_hwcg,
@@ -565,9 +574,9 @@  static int adreno_bind(struct device *dev, struct device *master, void *data)
 	if (ret)
 		return ret;
 
-	if (config.rev.core >= 6)
-		if (!adreno_has_gmu_wrapper(to_adreno_gpu(gpu)))
-			priv->has_cached_coherent = true;
+	priv->has_cached_coherent =
+		!!(info->quirks & ADRENO_QUIRK_HAS_CACHED_COHERENT) &&
+		!adreno_has_gmu_wrapper(to_adreno_gpu(gpu));
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index a7c4a2c536e3..e08d41337169 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -33,6 +33,7 @@  enum {
 #define ADRENO_QUIRK_FAULT_DETECT_MASK		BIT(1)
 #define ADRENO_QUIRK_LMLOADKILL_DISABLE		BIT(2)
 #define ADRENO_QUIRK_HAS_HW_APRIV		BIT(3)
+#define ADRENO_QUIRK_HAS_CACHED_COHERENT	BIT(4)
 
 struct adreno_rev {
 	uint8_t  core;