[PATCH 0/4] drm/etnaviv: add total hi bandwidth perf counters

[PATCH 0/4] drm/etnaviv: add total hi bandwidth perf counters

[Christian Gmeiner]

This little patch set adds support for the total bandwidth used by HI. The
basic hi bandwidth read-out is quite simple but I needed to add some little
clean-ups to make it nice looking.

Christian Gmeiner (4):
  drm/etnaviv: rename pipe_reg_read(..)
  drm/etnaviv: call perf_reg_read(..)
  drm/etnaviv: add total hi bandwidth perfcounter
  drm/etnaviv: add pipe_select(..) helper

 drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 78 ++++++++++++++++-------
 1 file changed, 55 insertions(+), 23 deletions(-)

-- 
2.26.2

[PATCH 1/4] drm/etnaviv: rename pipe_reg_read(..)

[Christian Gmeiner]

pipe_reg_read(..) iterates over all pixel pipes, selects a perf counter
register and sums the actual perf counter value. Rename the function
to reflect more what it is actual doing.

Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 30 +++++++++++------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index 75f9db8f7bec..1f0402f7a7de 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -46,7 +46,7 @@ static u32 perf_reg_read(struct etnaviv_gpu *gpu,
 	return gpu_read(gpu, domain->profile_read);
 }
 
-static u32 pipe_reg_read(struct etnaviv_gpu *gpu,
+static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu,
 	const struct etnaviv_pm_domain *domain,
 	const struct etnaviv_pm_signal *signal)
 {
@@ -141,22 +141,22 @@ static const struct etnaviv_pm_domain doms_3d[] = {
 			{
 				"PIXEL_COUNT_KILLED_BY_COLOR_PIPE",
 				VIVS_MC_PROFILE_CONFIG0_PE_PIXEL_COUNT_KILLED_BY_COLOR_PIPE,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			},
 			{
 				"PIXEL_COUNT_KILLED_BY_DEPTH_PIPE",
 				VIVS_MC_PROFILE_CONFIG0_PE_PIXEL_COUNT_KILLED_BY_DEPTH_PIPE,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			},
 			{
 				"PIXEL_COUNT_DRAWN_BY_COLOR_PIPE",
 				VIVS_MC_PROFILE_CONFIG0_PE_PIXEL_COUNT_DRAWN_BY_COLOR_PIPE,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			},
 			{
 				"PIXEL_COUNT_DRAWN_BY_DEPTH_PIPE",
 				VIVS_MC_PROFILE_CONFIG0_PE_PIXEL_COUNT_DRAWN_BY_DEPTH_PIPE,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			}
 		}
 	},
@@ -184,32 +184,32 @@ static const struct etnaviv_pm_domain doms_3d[] = {
 			{
 				"VS_INST_COUNTER",
 				VIVS_MC_PROFILE_CONFIG0_SH_VS_INST_COUNTER,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			},
 			{
 				"RENDERED_VERTICE_COUNTER",
 				VIVS_MC_PROFILE_CONFIG0_SH_RENDERED_VERTICE_COUNTER,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			},
 			{
 				"VTX_BRANCH_INST_COUNTER",
 				VIVS_MC_PROFILE_CONFIG0_SH_VTX_BRANCH_INST_COUNTER,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			},
 			{
 				"VTX_TEXLD_INST_COUNTER",
 				VIVS_MC_PROFILE_CONFIG0_SH_VTX_TEXLD_INST_COUNTER,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			},
 			{
 				"PXL_BRANCH_INST_COUNTER",
 				VIVS_MC_PROFILE_CONFIG0_SH_PXL_BRANCH_INST_COUNTER,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			},
 			{
 				"PXL_TEXLD_INST_COUNTER",
 				VIVS_MC_PROFILE_CONFIG0_SH_PXL_TEXLD_INST_COUNTER,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			}
 		}
 	},
@@ -237,17 +237,17 @@ static const struct etnaviv_pm_domain doms_3d[] = {
 			{
 				"DEPTH_CLIPPED_COUNTER",
 				VIVS_MC_PROFILE_CONFIG1_PA_DEPTH_CLIPPED_COUNTER,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			},
 			{
 				"TRIVIAL_REJECTED_COUNTER",
 				VIVS_MC_PROFILE_CONFIG1_PA_TRIVIAL_REJECTED_COUNTER,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			},
 			{
 				"CULLED_COUNTER",
 				VIVS_MC_PROFILE_CONFIG1_PA_CULLED_COUNTER,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			}
 		}
 	},
@@ -400,7 +400,7 @@ static const struct etnaviv_pm_domain doms_2d[] = {
 			{
 				"PIXELS_RENDERED_2D",
 				VIVS_MC_PROFILE_CONFIG0_PE_PIXELS_RENDERED_2D,
-				&pipe_reg_read
+				&pipe_perf_reg_read
 			}
 		}
 	}
-- 
2.26.2

[PATCH 2/4] drm/etnaviv: call perf_reg_read(..)

[Christian Gmeiner]

Replace the open coded access pattern with a function call.

Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index 1f0402f7a7de..782732e6ce72 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -58,8 +58,7 @@ static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu,
 		clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
 		clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(i);
 		gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
-		gpu_write(gpu, domain->profile_config, signal->data);
-		value += gpu_read(gpu, domain->profile_read);
+		value += perf_reg_read(gpu, domain, signal);
 	}
 
 	/* switch back to pixel pipe 0 to prevent GPU hang */
-- 
2.26.2

[PATCH 3/4] drm/etnaviv: add total hi bandwidth perfcounter

[Christian Gmeiner]

These two perf counters represent the total read and write
GPU bandwidth in terms of 64bits.

The used sequence was taken from Vivante kernel driver.

Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 35 ++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index 782732e6ce72..b37459f022d7 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -69,6 +69,29 @@ static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu,
 	return value;
 }
 
+static u32 pipe_reg_read(struct etnaviv_gpu *gpu,
+	const struct etnaviv_pm_domain *domain,
+	const struct etnaviv_pm_signal *signal)
+{
+	u32 clock = gpu_read(gpu, VIVS_HI_CLOCK_CONTROL);
+	u32 value = 0;
+	unsigned i;
+
+	for (i = 0; i < gpu->identity.pixel_pipes; i++) {
+		clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
+		clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(i);
+		gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+		value += gpu_read(gpu, signal->data);
+	}
+
+	/* switch back to pixel pipe 0 to prevent GPU hang */
+	clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
+	clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(0);
+	gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+
+	return value;
+}
+
 static u32 hi_total_cycle_read(struct etnaviv_gpu *gpu,
 	const struct etnaviv_pm_domain *domain,
 	const struct etnaviv_pm_signal *signal)
@@ -102,8 +125,18 @@ static const struct etnaviv_pm_domain doms_3d[] = {
 		.name = "HI",
 		.profile_read = VIVS_MC_PROFILE_HI_READ,
 		.profile_config = VIVS_MC_PROFILE_CONFIG2,
-		.nr_signals = 5,
+		.nr_signals = 7,
 		.signal = (const struct etnaviv_pm_signal[]) {
+			{
+				"TOTAL_READ_BYTES8",
+				VIVS_HI_PROFILE_READ_BYTES8,
+				&pipe_reg_read,
+			},
+			{
+				"TOTAL_WRITE_BYTES8",
+				VIVS_HI_PROFILE_WRITE_BYTES8,
+				&pipe_reg_read,
+			},
 			{
 				"TOTAL_CYCLES",
 				0,
-- 
2.26.2

Re: [PATCH 3/4] drm/etnaviv: add total hi bandwidth perfcounter

[Lucas Stach]

On Fr, 2020-08-14 at 11:05 +0200, Christian Gmeiner wrote:
> These two perf counters represent the total read and write
> GPU bandwidth in terms of 64bits.
> 
> The used sequence was taken from Vivante kernel driver.
> 
> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 35 ++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> index 782732e6ce72..b37459f022d7 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> @@ -69,6 +69,29 @@ static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu,
>  	return value;
>  }
>  
> +static u32 pipe_reg_read(struct etnaviv_gpu *gpu,
> +	const struct etnaviv_pm_domain *domain,
> +	const struct etnaviv_pm_signal *signal)
> +{
> +	u32 clock = gpu_read(gpu, VIVS_HI_CLOCK_CONTROL);
> +	u32 value = 0;
> +	unsigned i;
> +
> +	for (i = 0; i < gpu->identity.pixel_pipes; i++) {
> +		clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
> +		clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(i);
> +		gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
> +		value += gpu_read(gpu, signal->data);
> +	}
> +
> +	/* switch back to pixel pipe 0 to prevent GPU hang */
> +	clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
> +	clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(0);
> +	gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
> +
> +	return value;
> +}
> +
>  static u32 hi_total_cycle_read(struct etnaviv_gpu *gpu,
>  	const struct etnaviv_pm_domain *domain,
>  	const struct etnaviv_pm_signal *signal)
> @@ -102,8 +125,18 @@ static const struct etnaviv_pm_domain doms_3d[] = {
>  		.name = "HI",
>  		.profile_read = VIVS_MC_PROFILE_HI_READ,
>  		.profile_config = VIVS_MC_PROFILE_CONFIG2,
> -		.nr_signals = 5,
> +		.nr_signals = 7,

I've tripped across this part. It's something I don't particularly
like, as this value has a risk of getting inconsistent with the actual
array. Maybe we could split out signal array from this initialization,
so we could then use the ARRAY_SIZE macro to initialize this value?

But that's not really related to this patch and can be done in a
follow-up cleanup.

Regards,
Lucas

>  		.signal = (const struct etnaviv_pm_signal[]) {
> +			{
> +				"TOTAL_READ_BYTES8",
> +				VIVS_HI_PROFILE_READ_BYTES8,
> +				&pipe_reg_read,
> +			},
> +			{
> +				"TOTAL_WRITE_BYTES8",
> +				VIVS_HI_PROFILE_WRITE_BYTES8,
> +				&pipe_reg_read,
> +			},
>  			{
>  				"TOTAL_CYCLES",
>  				0,

[PATCH 4/4] drm/etnaviv: add pipe_select(..) helper

[Christian Gmeiner]

Replace the open coded pixel pipe selection pattern with a function.

Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 24 +++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index b37459f022d7..bafdfe49c1d8 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -46,6 +46,14 @@ static u32 perf_reg_read(struct etnaviv_gpu *gpu,
 	return gpu_read(gpu, domain->profile_read);
 }
 
+static inline void pipe_select(struct etnaviv_gpu *gpu, u32 clock, unsigned pipe)
+{
+	clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
+	clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(pipe);
+
+	gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+}
+
 static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu,
 	const struct etnaviv_pm_domain *domain,
 	const struct etnaviv_pm_signal *signal)
@@ -55,16 +63,12 @@ static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu,
 	unsigned i;
 
 	for (i = 0; i < gpu->identity.pixel_pipes; i++) {
-		clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
-		clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(i);
-		gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+		pipe_select(gpu, clock, i);
 		value += perf_reg_read(gpu, domain, signal);
 	}
 
 	/* switch back to pixel pipe 0 to prevent GPU hang */
-	clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
-	clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(0);
-	gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+	pipe_select(gpu, clock, 0);
 
 	return value;
 }
@@ -78,16 +82,12 @@ static u32 pipe_reg_read(struct etnaviv_gpu *gpu,
 	unsigned i;
 
 	for (i = 0; i < gpu->identity.pixel_pipes; i++) {
-		clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
-		clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(i);
-		gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+		pipe_select(gpu, clock, i);
 		value += gpu_read(gpu, signal->data);
 	}
 
 	/* switch back to pixel pipe 0 to prevent GPU hang */
-	clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
-	clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(0);
-	gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+	pipe_select(gpu, clock, 0);
 
 	return value;
 }
-- 
2.26.2

Re: [PATCH 0/4] drm/etnaviv: add total hi bandwidth perf counters

[Christian Gmeiner]

Hi Lucas,

almost one month has passed since I sent this little patch series - is
there any update on this?


Am Fr., 14. Aug. 2020 um 11:05 Uhr schrieb Christian Gmeiner
<christian.gmeiner@gmail.com>:
>
> This little patch set adds support for the total bandwidth used by HI. The
> basic hi bandwidth read-out is quite simple but I needed to add some little
> clean-ups to make it nice looking.
>
> Christian Gmeiner (4):
>   drm/etnaviv: rename pipe_reg_read(..)
>   drm/etnaviv: call perf_reg_read(..)
>   drm/etnaviv: add total hi bandwidth perfcounter
>   drm/etnaviv: add pipe_select(..) helper
>
>  drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 78 ++++++++++++++++-------
>  1 file changed, 55 insertions(+), 23 deletions(-)
>
> --
> 2.26.2
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

Re: [PATCH 0/4] drm/etnaviv: add total hi bandwidth perf counters

[Lucas Stach]

On Fr, 2020-08-14 at 11:05 +0200, Christian Gmeiner wrote:
> This little patch set adds support for the total bandwidth used by HI. The
> basic hi bandwidth read-out is quite simple but I needed to add some little
> clean-ups to make it nice looking.
> 
> Christian Gmeiner (4):
>   drm/etnaviv: rename pipe_reg_read(..)
>   drm/etnaviv: call perf_reg_read(..)
>   drm/etnaviv: add total hi bandwidth perfcounter
>   drm/etnaviv: add pipe_select(..) helper
> 
>  drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 78 ++++++++++++++++-------
>  1 file changed, 55 insertions(+), 23 deletions(-)

Thanks,

I've applied the whole series to my etnaviv/next branch.

regards,
Lucas