diff mbox series

drm/v3d: Fix performance counter source settings on V3D 7.x

Message ID 20241106121736.5707-1-mcanal@igalia.com (mailing list archive)
State New, archived
Headers show
Series drm/v3d: Fix performance counter source settings on V3D 7.x | expand

Commit Message

Maíra Canal Nov. 6, 2024, 12:16 p.m. UTC
When the new register addresses were introduced for V3D 7.x, we added
new masks for performance counter sources on V3D 7.x.  Nevertheless,
we never apply these new masks when setting the sources.

Fix the performance counter source settings on V3D 7.x by introducing
a new macro, `V3D_SET_FIELD_VER`, which allows fields setting to vary
by version. Using this macro, we can provide different values for
source mask based on the V3D version, ensuring that sources are
correctly configure on V3D 7.x.

Fixes: 0ad5bc1ce463 ("drm/v3d: fix up register addresses for V3D 7.x")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_debugfs.c |  4 ++--
 drivers/gpu/drm/v3d/v3d_perfmon.c | 15 ++++++++-------
 drivers/gpu/drm/v3d/v3d_regs.h    | 29 +++++++++++++++++------------
 3 files changed, 27 insertions(+), 21 deletions(-)

Comments

Iago Toral Nov. 7, 2024, 6 a.m. UTC | #1
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>

El mié, 06-11-2024 a las 09:16 -0300, Maíra Canal escribió:
> When the new register addresses were introduced for V3D 7.x, we added
> new masks for performance counter sources on V3D 7.x.  Nevertheless,
> we never apply these new masks when setting the sources.
> 
> Fix the performance counter source settings on V3D 7.x by introducing
> a new macro, `V3D_SET_FIELD_VER`, which allows fields setting to vary
> by version. Using this macro, we can provide different values for
> source mask based on the V3D version, ensuring that sources are
> correctly configure on V3D 7.x.
> 
> Fixes: 0ad5bc1ce463 ("drm/v3d: fix up register addresses for V3D
> 7.x")
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  drivers/gpu/drm/v3d/v3d_debugfs.c |  4 ++--
>  drivers/gpu/drm/v3d/v3d_perfmon.c | 15 ++++++++-------
>  drivers/gpu/drm/v3d/v3d_regs.h    | 29 +++++++++++++++++------------
>  3 files changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c
> b/drivers/gpu/drm/v3d/v3d_debugfs.c
> index 19e3ee7ac897..76816f2551c1 100644
> --- a/drivers/gpu/drm/v3d/v3d_debugfs.c
> +++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
> @@ -237,8 +237,8 @@ static int v3d_measure_clock(struct seq_file *m,
> void *unused)
>  	if (v3d->ver >= 40) {
>  		int cycle_count_reg = V3D_PCTR_CYCLE_COUNT(v3d-
> >ver);
>  		V3D_CORE_WRITE(core, V3D_V4_PCTR_0_SRC_0_3,
> -			       V3D_SET_FIELD(cycle_count_reg,
> -					     V3D_PCTR_S0));
> +			       V3D_SET_FIELD_VER(cycle_count_reg,
> +						 V3D_PCTR_S0, v3d-
> >ver));
>  		V3D_CORE_WRITE(core, V3D_V4_PCTR_0_CLR, 1);
>  		V3D_CORE_WRITE(core, V3D_V4_PCTR_0_EN, 1);
>  	} else {
> diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c
> b/drivers/gpu/drm/v3d/v3d_perfmon.c
> index 156be13ab2ef..7df6dd933c63 100644
> --- a/drivers/gpu/drm/v3d/v3d_perfmon.c
> +++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
> @@ -240,17 +240,18 @@ void v3d_perfmon_start(struct v3d_dev *v3d,
> struct v3d_perfmon *perfmon)
>  
>  	for (i = 0; i < ncounters; i++) {
>  		u32 source = i / 4;
> -		u32 channel = V3D_SET_FIELD(perfmon->counters[i],
> V3D_PCTR_S0);
> +		u32 channel = V3D_SET_FIELD_VER(perfmon-
> >counters[i], V3D_PCTR_S0,
> +						v3d->ver);
>  
>  		i++;
> -		channel |= V3D_SET_FIELD(i < ncounters ? perfmon-
> >counters[i] : 0,
> -					 V3D_PCTR_S1);
> +		channel |= V3D_SET_FIELD_VER(i < ncounters ?
> perfmon->counters[i] : 0,
> +					     V3D_PCTR_S1, v3d->ver);
>  		i++;
> -		channel |= V3D_SET_FIELD(i < ncounters ? perfmon-
> >counters[i] : 0,
> -					 V3D_PCTR_S2);
> +		channel |= V3D_SET_FIELD_VER(i < ncounters ?
> perfmon->counters[i] : 0,
> +					     V3D_PCTR_S2, v3d->ver);
>  		i++;
> -		channel |= V3D_SET_FIELD(i < ncounters ? perfmon-
> >counters[i] : 0,
> -					 V3D_PCTR_S3);
> +		channel |= V3D_SET_FIELD_VER(i < ncounters ?
> perfmon->counters[i] : 0,
> +					     V3D_PCTR_S3, v3d->ver);
>  		V3D_CORE_WRITE(0, V3D_V4_PCTR_0_SRC_X(source),
> channel);
>  	}
>  
> diff --git a/drivers/gpu/drm/v3d/v3d_regs.h
> b/drivers/gpu/drm/v3d/v3d_regs.h
> index 1b1a62ad9585..6da3c69082bd 100644
> --- a/drivers/gpu/drm/v3d/v3d_regs.h
> +++ b/drivers/gpu/drm/v3d/v3d_regs.h
> @@ -15,6 +15,14 @@
>  		fieldval &
> field##_MASK;				\
>  	 })
>  
> +#define V3D_SET_FIELD_VER(value, field,
> ver)				\
> +	({							
> 	\
> +		typeof(ver) _ver =
> (ver);				\
> +		u32 fieldval = (value) <<
> field##_SHIFT(_ver);		\
> +		WARN_ON((fieldval & ~field##_MASK(_ver)) !=
> 0);		\
> +		fieldval &
> field##_MASK(_ver);				\
> +	 })
> +
>  #define V3D_GET_FIELD(word, field) (((word) & field##_MASK)
> >>		\
>  				    field##_SHIFT)
>  
> @@ -354,18 +362,15 @@
>  #define V3D_V4_PCTR_0_SRC_28_31                        0x0067c
>  #define V3D_V4_PCTR_0_SRC_X(x)                        
> (V3D_V4_PCTR_0_SRC_0_3 + \
>  							4 * (x))
> -# define V3D_PCTR_S0_MASK                              V3D_MASK(6,
> 0)
> -# define V3D_V7_PCTR_S0_MASK                           V3D_MASK(7,
> 0)
> -# define V3D_PCTR_S0_SHIFT                             0
> -# define V3D_PCTR_S1_MASK                              V3D_MASK(14,
> 8)
> -# define V3D_V7_PCTR_S1_MASK                           V3D_MASK(15,
> 8)
> -# define V3D_PCTR_S1_SHIFT                             8
> -# define V3D_PCTR_S2_MASK                              V3D_MASK(22,
> 16)
> -# define V3D_V7_PCTR_S2_MASK                           V3D_MASK(23,
> 16)
> -# define V3D_PCTR_S2_SHIFT                             16
> -# define V3D_PCTR_S3_MASK                              V3D_MASK(30,
> 24)
> -# define V3D_V7_PCTR_S3_MASK                           V3D_MASK(31,
> 24)
> -# define V3D_PCTR_S3_SHIFT                             24
> +# define V3D_PCTR_S0_MASK(ver) (((ver) >= 71) ? V3D_MASK(7, 0) :
> V3D_MASK(6, 0))
> +# define V3D_PCTR_S0_SHIFT(ver)                        0
> +# define V3D_PCTR_S1_MASK(ver) (((ver) >= 71) ? V3D_MASK(15, 8) :
> V3D_MASK(14, 8))
> +# define V3D_PCTR_S1_SHIFT(ver)                        8
> +# define V3D_PCTR_S2_MASK(ver) (((ver) >= 71) ? V3D_MASK(23, 16) :
> V3D_MASK(22, 16))
> +# define V3D_PCTR_S2_SHIFT(ver)                        16
> +# define V3D_PCTR_S3_MASK(ver) (((ver) >= 71) ? V3D_MASK(31, 24) :
> V3D_MASK(30, 24))
> +# define V3D_PCTR_S3_SHIFT(ver)                        24
> +
>  #define V3D_PCTR_CYCLE_COUNT(ver) ((ver >= 71) ? 0 : 32)
>  
>  /* Output values of the counters. */
Christian Gmeiner Nov. 7, 2024, 8:29 a.m. UTC | #2
> When the new register addresses were introduced for V3D 7.x, we added
> new masks for performance counter sources on V3D 7.x.  Nevertheless,
> we never apply these new masks when setting the sources.
> 
> Fix the performance counter source settings on V3D 7.x by introducing
> a new macro, `V3D_SET_FIELD_VER`, which allows fields setting to vary
> by version. Using this macro, we can provide different values for
> source mask based on the V3D version, ensuring that sources are
> correctly configure on V3D 7.x.
> 
> Fixes: 0ad5bc1ce463 ("drm/v3d: fix up register addresses for V3D 7.x")
> Signed-off-by: Maíra Canal <mcanal@igalia.com>


Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>

> ---
>   drivers/gpu/drm/v3d/v3d_debugfs.c |  4 ++--
>   drivers/gpu/drm/v3d/v3d_perfmon.c | 15 ++++++++-------
>   drivers/gpu/drm/v3d/v3d_regs.h    | 29 +++++++++++++++++------------
>   3 files changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c
> index 19e3ee7ac897..76816f2551c1 100644
> --- a/drivers/gpu/drm/v3d/v3d_debugfs.c
> +++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
> @@ -237,8 +237,8 @@ static int v3d_measure_clock(struct seq_file *m, void *unused)
>   	if (v3d->ver >= 40) {
>   		int cycle_count_reg = V3D_PCTR_CYCLE_COUNT(v3d->ver);
>   		V3D_CORE_WRITE(core, V3D_V4_PCTR_0_SRC_0_3,
> -			       V3D_SET_FIELD(cycle_count_reg,
> -					     V3D_PCTR_S0));
> +			       V3D_SET_FIELD_VER(cycle_count_reg,
> +						 V3D_PCTR_S0, v3d->ver));
>   		V3D_CORE_WRITE(core, V3D_V4_PCTR_0_CLR, 1);
>   		V3D_CORE_WRITE(core, V3D_V4_PCTR_0_EN, 1);
>   	} else {
> diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
> index 156be13ab2ef..7df6dd933c63 100644
> --- a/drivers/gpu/drm/v3d/v3d_perfmon.c
> +++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
> @@ -240,17 +240,18 @@ void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
>   
>   	for (i = 0; i < ncounters; i++) {
>   		u32 source = i / 4;
> -		u32 channel = V3D_SET_FIELD(perfmon->counters[i], V3D_PCTR_S0);
> +		u32 channel = V3D_SET_FIELD_VER(perfmon->counters[i], V3D_PCTR_S0,
> +						v3d->ver);
>   
>   		i++;
> -		channel |= V3D_SET_FIELD(i < ncounters ? perfmon->counters[i] : 0,
> -					 V3D_PCTR_S1);
> +		channel |= V3D_SET_FIELD_VER(i < ncounters ? perfmon->counters[i] : 0,
> +					     V3D_PCTR_S1, v3d->ver);
>   		i++;
> -		channel |= V3D_SET_FIELD(i < ncounters ? perfmon->counters[i] : 0,
> -					 V3D_PCTR_S2);
> +		channel |= V3D_SET_FIELD_VER(i < ncounters ? perfmon->counters[i] : 0,
> +					     V3D_PCTR_S2, v3d->ver);
>   		i++;
> -		channel |= V3D_SET_FIELD(i < ncounters ? perfmon->counters[i] : 0,
> -					 V3D_PCTR_S3);
> +		channel |= V3D_SET_FIELD_VER(i < ncounters ? perfmon->counters[i] : 0,
> +					     V3D_PCTR_S3, v3d->ver);
>   		V3D_CORE_WRITE(0, V3D_V4_PCTR_0_SRC_X(source), channel);
>   	}
>   
> diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h
> index 1b1a62ad9585..6da3c69082bd 100644
> --- a/drivers/gpu/drm/v3d/v3d_regs.h
> +++ b/drivers/gpu/drm/v3d/v3d_regs.h
> @@ -15,6 +15,14 @@
>   		fieldval & field##_MASK;				\
>   	 })
>   
> +#define V3D_SET_FIELD_VER(value, field, ver)				\
> +	({								\
> +		typeof(ver) _ver = (ver);				\
> +		u32 fieldval = (value) << field##_SHIFT(_ver);		\
> +		WARN_ON((fieldval & ~field##_MASK(_ver)) != 0);		\
> +		fieldval & field##_MASK(_ver);				\
> +	 })
> +
>   #define V3D_GET_FIELD(word, field) (((word) & field##_MASK) >>		\
>   				    field##_SHIFT)
>   
> @@ -354,18 +362,15 @@
>   #define V3D_V4_PCTR_0_SRC_28_31                        0x0067c
>   #define V3D_V4_PCTR_0_SRC_X(x)                         (V3D_V4_PCTR_0_SRC_0_3 + \
>   							4 * (x))
> -# define V3D_PCTR_S0_MASK                              V3D_MASK(6, 0)
> -# define V3D_V7_PCTR_S0_MASK                           V3D_MASK(7, 0)
> -# define V3D_PCTR_S0_SHIFT                             0
> -# define V3D_PCTR_S1_MASK                              V3D_MASK(14, 8)
> -# define V3D_V7_PCTR_S1_MASK                           V3D_MASK(15, 8)
> -# define V3D_PCTR_S1_SHIFT                             8
> -# define V3D_PCTR_S2_MASK                              V3D_MASK(22, 16)
> -# define V3D_V7_PCTR_S2_MASK                           V3D_MASK(23, 16)
> -# define V3D_PCTR_S2_SHIFT                             16
> -# define V3D_PCTR_S3_MASK                              V3D_MASK(30, 24)
> -# define V3D_V7_PCTR_S3_MASK                           V3D_MASK(31, 24)
> -# define V3D_PCTR_S3_SHIFT                             24
> +# define V3D_PCTR_S0_MASK(ver) (((ver) >= 71) ? V3D_MASK(7, 0) : V3D_MASK(6, 0))
> +# define V3D_PCTR_S0_SHIFT(ver)                        0
> +# define V3D_PCTR_S1_MASK(ver) (((ver) >= 71) ? V3D_MASK(15, 8) : V3D_MASK(14, 8))
> +# define V3D_PCTR_S1_SHIFT(ver)                        8
> +# define V3D_PCTR_S2_MASK(ver) (((ver) >= 71) ? V3D_MASK(23, 16) : V3D_MASK(22, 16))
> +# define V3D_PCTR_S2_SHIFT(ver)                        16
> +# define V3D_PCTR_S3_MASK(ver) (((ver) >= 71) ? V3D_MASK(31, 24) : V3D_MASK(30, 24))
> +# define V3D_PCTR_S3_SHIFT(ver)                        24
> +
>   #define V3D_PCTR_CYCLE_COUNT(ver) ((ver >= 71) ? 0 : 32)
>   
>   /* Output values of the counters. */
Maíra Canal Nov. 11, 2024, 12:23 p.m. UTC | #3
On 06/11/24 09:16, Maíra Canal wrote:
> When the new register addresses were introduced for V3D 7.x, we added
> new masks for performance counter sources on V3D 7.x.  Nevertheless,
> we never apply these new masks when setting the sources.
> 
> Fix the performance counter source settings on V3D 7.x by introducing
> a new macro, `V3D_SET_FIELD_VER`, which allows fields setting to vary
> by version. Using this macro, we can provide different values for
> source mask based on the V3D version, ensuring that sources are
> correctly configure on V3D 7.x.
> 
> Fixes: 0ad5bc1ce463 ("drm/v3d: fix up register addresses for V3D 7.x")
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>   drivers/gpu/drm/v3d/v3d_debugfs.c |  4 ++--
>   drivers/gpu/drm/v3d/v3d_perfmon.c | 15 ++++++++-------
>   drivers/gpu/drm/v3d/v3d_regs.h    | 29 +++++++++++++++++------------
>   3 files changed, 27 insertions(+), 21 deletions(-)
> 

Applied to misc/kernel.git (drm-misc-next).

Best Regards,
- Maíra
diff mbox series

Patch

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c
index 19e3ee7ac897..76816f2551c1 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -237,8 +237,8 @@  static int v3d_measure_clock(struct seq_file *m, void *unused)
 	if (v3d->ver >= 40) {
 		int cycle_count_reg = V3D_PCTR_CYCLE_COUNT(v3d->ver);
 		V3D_CORE_WRITE(core, V3D_V4_PCTR_0_SRC_0_3,
-			       V3D_SET_FIELD(cycle_count_reg,
-					     V3D_PCTR_S0));
+			       V3D_SET_FIELD_VER(cycle_count_reg,
+						 V3D_PCTR_S0, v3d->ver));
 		V3D_CORE_WRITE(core, V3D_V4_PCTR_0_CLR, 1);
 		V3D_CORE_WRITE(core, V3D_V4_PCTR_0_EN, 1);
 	} else {
diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
index 156be13ab2ef..7df6dd933c63 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -240,17 +240,18 @@  void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
 
 	for (i = 0; i < ncounters; i++) {
 		u32 source = i / 4;
-		u32 channel = V3D_SET_FIELD(perfmon->counters[i], V3D_PCTR_S0);
+		u32 channel = V3D_SET_FIELD_VER(perfmon->counters[i], V3D_PCTR_S0,
+						v3d->ver);
 
 		i++;
-		channel |= V3D_SET_FIELD(i < ncounters ? perfmon->counters[i] : 0,
-					 V3D_PCTR_S1);
+		channel |= V3D_SET_FIELD_VER(i < ncounters ? perfmon->counters[i] : 0,
+					     V3D_PCTR_S1, v3d->ver);
 		i++;
-		channel |= V3D_SET_FIELD(i < ncounters ? perfmon->counters[i] : 0,
-					 V3D_PCTR_S2);
+		channel |= V3D_SET_FIELD_VER(i < ncounters ? perfmon->counters[i] : 0,
+					     V3D_PCTR_S2, v3d->ver);
 		i++;
-		channel |= V3D_SET_FIELD(i < ncounters ? perfmon->counters[i] : 0,
-					 V3D_PCTR_S3);
+		channel |= V3D_SET_FIELD_VER(i < ncounters ? perfmon->counters[i] : 0,
+					     V3D_PCTR_S3, v3d->ver);
 		V3D_CORE_WRITE(0, V3D_V4_PCTR_0_SRC_X(source), channel);
 	}
 
diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h
index 1b1a62ad9585..6da3c69082bd 100644
--- a/drivers/gpu/drm/v3d/v3d_regs.h
+++ b/drivers/gpu/drm/v3d/v3d_regs.h
@@ -15,6 +15,14 @@ 
 		fieldval & field##_MASK;				\
 	 })
 
+#define V3D_SET_FIELD_VER(value, field, ver)				\
+	({								\
+		typeof(ver) _ver = (ver);				\
+		u32 fieldval = (value) << field##_SHIFT(_ver);		\
+		WARN_ON((fieldval & ~field##_MASK(_ver)) != 0);		\
+		fieldval & field##_MASK(_ver);				\
+	 })
+
 #define V3D_GET_FIELD(word, field) (((word) & field##_MASK) >>		\
 				    field##_SHIFT)
 
@@ -354,18 +362,15 @@ 
 #define V3D_V4_PCTR_0_SRC_28_31                        0x0067c
 #define V3D_V4_PCTR_0_SRC_X(x)                         (V3D_V4_PCTR_0_SRC_0_3 + \
 							4 * (x))
-# define V3D_PCTR_S0_MASK                              V3D_MASK(6, 0)
-# define V3D_V7_PCTR_S0_MASK                           V3D_MASK(7, 0)
-# define V3D_PCTR_S0_SHIFT                             0
-# define V3D_PCTR_S1_MASK                              V3D_MASK(14, 8)
-# define V3D_V7_PCTR_S1_MASK                           V3D_MASK(15, 8)
-# define V3D_PCTR_S1_SHIFT                             8
-# define V3D_PCTR_S2_MASK                              V3D_MASK(22, 16)
-# define V3D_V7_PCTR_S2_MASK                           V3D_MASK(23, 16)
-# define V3D_PCTR_S2_SHIFT                             16
-# define V3D_PCTR_S3_MASK                              V3D_MASK(30, 24)
-# define V3D_V7_PCTR_S3_MASK                           V3D_MASK(31, 24)
-# define V3D_PCTR_S3_SHIFT                             24
+# define V3D_PCTR_S0_MASK(ver) (((ver) >= 71) ? V3D_MASK(7, 0) : V3D_MASK(6, 0))
+# define V3D_PCTR_S0_SHIFT(ver)                        0
+# define V3D_PCTR_S1_MASK(ver) (((ver) >= 71) ? V3D_MASK(15, 8) : V3D_MASK(14, 8))
+# define V3D_PCTR_S1_SHIFT(ver)                        8
+# define V3D_PCTR_S2_MASK(ver) (((ver) >= 71) ? V3D_MASK(23, 16) : V3D_MASK(22, 16))
+# define V3D_PCTR_S2_SHIFT(ver)                        16
+# define V3D_PCTR_S3_MASK(ver) (((ver) >= 71) ? V3D_MASK(31, 24) : V3D_MASK(30, 24))
+# define V3D_PCTR_S3_SHIFT(ver)                        24
+
 #define V3D_PCTR_CYCLE_COUNT(ver) ((ver >= 71) ? 0 : 32)
 
 /* Output values of the counters. */