[PATCH] drm/etnaviv: Set up initial PULSE_EATER register

[PATCH] drm/etnaviv: Set up initial PULSE_EATER register

[Wladimir J. van der Laan]

Set up the PULSE_EATER register (0x0010C) in etnaviv_gpu_hw_init. This
ports three mostly undocumented model/revision-specific register
overrides from the Vivante kernel driver.

This is relevant as at least the "disable internal DFS" for revisions >
0x5420 has shown to have a huge impact on shader performance (sped up
memory read performance by 7.5x and write performance by 1.5x) on an
affected GPU.
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 1e0ec81..e1a9d64 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -548,6 +548,9 @@ void etnaviv_gpu_start_fe(struct etnaviv_gpu *gpu, u32 address, u16 prefetch)
 		  VIVS_FE_COMMAND_CONTROL_PREFETCH(prefetch));
 }
 
+/* base value for VIVS_PM_PULSE_EATER register on models where it cannot be read */
+#define PULSE_EATER_BASE_VALUE (0x01590880)
+
 static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
 {
 	u16 prefetch;
@@ -588,6 +591,33 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
 		gpu_write(gpu, VIVS_MC_BUS_CONFIG, bus_config);
 	}
 
+	/* set up the pulse eater */
+	if (etnaviv_is_model_rev(gpu, GC4000, 0x5208) ||
+	    etnaviv_is_model_rev(gpu, GC4000, 0x5222)) {
+		/* undocumented workaround from vivante kernel driver */
+		u32 pulse_eater = PULSE_EATER_BASE_VALUE;
+		pulse_eater |= BIT(23);
+		gpu_write(gpu, VIVS_PM_PULSE_EATER, pulse_eater);
+	}
+
+	if (etnaviv_is_model_rev(gpu, GC1000, 0x5039) ||
+	    etnaviv_is_model_rev(gpu, GC1000, 0x5040)) {
+		/* undocumented workaround from vivante kernel driver */
+		u32 pulse_eater = PULSE_EATER_BASE_VALUE;
+		pulse_eater &= ~BIT(16);
+		pulse_eater |= BIT(17);
+		gpu_write(gpu, VIVS_PM_PULSE_EATER, pulse_eater);
+	}
+
+	if ((gpu->identity.revision > 0x5420) &&
+            (gpu->identity.features & chipFeatures_PIPE_3D))
+	{
+		/* disable internal DFS: this is extremely important to performance */
+		u32 pulse_eater = gpu_read(gpu, VIVS_PM_PULSE_EATER);
+		pulse_eater |= BIT(18);
+		gpu_write(gpu, VIVS_PM_PULSE_EATER, pulse_eater);
+	}
+
 	/* setup the MMU */
 	etnaviv_iommu_restore(gpu);
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/etnaviv: Set up initial PULSE_EATER register

[Lucas Stach]

Am Mittwoch, den 14.12.2016, 09:39 +0100 schrieb Wladimir J. van der
Laan:
> Set up the PULSE_EATER register (0x0010C) in etnaviv_gpu_hw_init. This
> ports three mostly undocumented model/revision-specific register
> overrides from the Vivante kernel driver.
> 
> This is relevant as at least the "disable internal DFS" for revisions >
> 0x5420 has shown to have a huge impact on shader performance (sped up
> memory read performance by 7.5x and write performance by 1.5x) on an
> affected GPU.

Missing Singed-off-by

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 1e0ec81..e1a9d64 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -548,6 +548,9 @@ void etnaviv_gpu_start_fe(struct etnaviv_gpu *gpu, u32 address, u16 prefetch)
>  		  VIVS_FE_COMMAND_CONTROL_PREFETCH(prefetch));
>  }
>  
> +/* base value for VIVS_PM_PULSE_EATER register on models where it cannot be read */
> +#define PULSE_EATER_BASE_VALUE (0x01590880)
> +

Please don't introduce freestanding defines.

>  static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
>  {
>  	u16 prefetch;
> @@ -588,6 +591,33 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
>  		gpu_write(gpu, VIVS_MC_BUS_CONFIG, bus_config);
>  	}

Please split this out into a separate function like the mlgc setup.

>  
> +	/* set up the pulse eater */
> +	if (etnaviv_is_model_rev(gpu, GC4000, 0x5208) ||
> +	    etnaviv_is_model_rev(gpu, GC4000, 0x5222)) {
> +		/* undocumented workaround from vivante kernel driver */
> +		u32 pulse_eater = PULSE_EATER_BASE_VALUE;

If this is in a separate function, the definition of this variable can
move out of the individual blocks. Just have a single
u32 pulse_eater = 0x01590880;
at the top of the function with a comment where this value comes from.

> +		pulse_eater |= BIT(23);
> +		gpu_write(gpu, VIVS_PM_PULSE_EATER, pulse_eater);
> +	}
> +
> +	if (etnaviv_is_model_rev(gpu, GC1000, 0x5039) ||
> +	    etnaviv_is_model_rev(gpu, GC1000, 0x5040)) {
> +		/* undocumented workaround from vivante kernel driver */

I'm not sure how much value this comment holds. Most of the PM register
setup consists of undocumented workarounds extracted from the Vivante
driver. But that's a matter of taste, so feel free to remove or keep it.

> +		u32 pulse_eater = PULSE_EATER_BASE_VALUE;
> +		pulse_eater &= ~BIT(16);
> +		pulse_eater |= BIT(17);
> +		gpu_write(gpu, VIVS_PM_PULSE_EATER, pulse_eater);
> +	}
> +
> +	if ((gpu->identity.revision > 0x5420) &&
> +            (gpu->identity.features & chipFeatures_PIPE_3D))
> +	{
> +		/* disable internal DFS: this is extremely important to performance */
> +		u32 pulse_eater = gpu_read(gpu, VIVS_PM_PULSE_EATER);
> +		pulse_eater |= BIT(18);
> +		gpu_write(gpu, VIVS_PM_PULSE_EATER, pulse_eater);
> +	}
> +
>  	/* setup the MMU */
>  	etnaviv_iommu_restore(gpu);
>  

Regards,
Lucas

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel