diff mbox series

[v4,3/4] drm/amd/display: Add control mechanism for FPU utilization

Message ID 20210727005248.1827411-4-Rodrigo.Siqueira@amd.com (mailing list archive)
State New, archived
Headers show
Series drm/amd/display: Introduce FPU directory inside DC | expand

Commit Message

Rodrigo Siqueira Jordao July 27, 2021, 12:52 a.m. UTC
DC invokes DC_FPU_START/END in multiple parts of the code; this can
create a situation where we invoke this FPU operation in a nested way or
exit too early. For avoiding this situation, this commit adds a
mechanism where dc_fpu_begin/end manages the access to
kernel_fpu_begin/end.

Change since V3:
- Rebase

Change since V2:
- Christian: Do not use this_cpu_* between get/put_cpu_ptr().

Change since V1:
- Use a better variable names
- Use get_cpu_ptr and put_cpu_ptr to better balance preemption enable
and disable

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Anson Jacob <Anson.Jacob@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Hersen Wu <hersenxs.wu@amd.com>
Cc: Aric Cyr <aric.cyr@amd.com>
Cc: Jun Lei <jun.lei@amd.com>
Cc: Dmytro Laktyushkin <dmytro.laktyushkin@amd.com>
Cc: Qingqing Zhuo <qingqing.zhuo@amd.com>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 13 ++--
 .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.c    | 77 +++++++++++++------
 drivers/gpu/drm/amd/display/dc/dc_trace.h     |  4 +-
 3 files changed, 63 insertions(+), 31 deletions(-)

Comments

Christian König July 27, 2021, 7 a.m. UTC | #1
Am 27.07.21 um 02:52 schrieb Rodrigo Siqueira:
> DC invokes DC_FPU_START/END in multiple parts of the code; this can
> create a situation where we invoke this FPU operation in a nested way or
> exit too early. For avoiding this situation, this commit adds a
> mechanism where dc_fpu_begin/end manages the access to
> kernel_fpu_begin/end.
>
> Change since V3:
> - Rebase
>
> Change since V2:
> - Christian: Do not use this_cpu_* between get/put_cpu_ptr().
>
> Change since V1:
> - Use a better variable names
> - Use get_cpu_ptr and put_cpu_ptr to better balance preemption enable
> and disable
>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Anson Jacob <Anson.Jacob@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Hersen Wu <hersenxs.wu@amd.com>
> Cc: Aric Cyr <aric.cyr@amd.com>
> Cc: Jun Lei <jun.lei@amd.com>
> Cc: Dmytro Laktyushkin <dmytro.laktyushkin@amd.com>
> Cc: Qingqing Zhuo <qingqing.zhuo@amd.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 13 ++--
>   .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.c    | 77 +++++++++++++------
>   drivers/gpu/drm/amd/display/dc/dc_trace.h     |  4 +-
>   3 files changed, 63 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> index 230bb12c405e..fdcaea22b456 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> @@ -638,23 +638,26 @@ TRACE_EVENT(amdgpu_refresh_rate_track,
>   );
>   
>   TRACE_EVENT(dcn_fpu,
> -	    TP_PROTO(bool begin, const char *function, const int line),
> -	    TP_ARGS(begin, function, line),
> +	    TP_PROTO(bool begin, const char *function, const int line, const int recursion_depth),
> +	    TP_ARGS(begin, function, line, recursion_depth),
>   
>   	    TP_STRUCT__entry(
>   			     __field(bool, begin)
>   			     __field(const char *, function)
>   			     __field(int, line)
> +			     __field(int, recursion_depth)
>   	    ),
>   	    TP_fast_assign(
>   			   __entry->begin = begin;
>   			   __entry->function = function;
>   			   __entry->line = line;
> +			   __entry->recursion_depth = recursion_depth;
>   	    ),
> -	    TP_printk("%s()+%d: %s",
> +	    TP_printk("%s: recursion_depth: %d: %s()+%d:",
> +		      __entry->begin ? "begin" : "end",
> +		      __entry->recursion_depth,
>   		      __entry->function,
> -		      __entry->line,
> -		      __entry->begin ? "begin" : "end"
> +		      __entry->line
>   	    )
>   );
>   
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
> index d4ab27bf8d04..33807d746e76 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
> @@ -33,6 +33,19 @@
>   #include <asm/cputable.h>
>   #endif
>   
> +/**
> + * DOC: DC FPU manipulation overview
> + *
> + * DC core uses FPU operations in multiple parts of the code, which requires a
> + * more specialized way to manage these areas' entrance. To fulfill this
> + * requirement, we created some wrapper functions that encapsulate
> + * kernel_fpu_begin/end to better fit our need in the display component. In
> + * summary, in this file, you can find functions related to FPU operation
> + * management.
> + */
> +
> +static DEFINE_PER_CPU(int, fpu_recursion_depth);
> +
>   /**
>    * dc_fpu_begin - Enables FPU protection
>    * @function_name: A string containing the function name for debug purposes
> @@ -48,22 +61,30 @@
>    */
>   void dc_fpu_begin(const char *function_name, const int line)
>   {
> -	TRACE_DCN_FPU(true, function_name, line);
> +	int *pcpu;
> +
> +	pcpu = get_cpu_ptr(&fpu_recursion_depth);
> +	*pcpu += 1;
>   
> +	if (*pcpu == 1) {
>   #if defined(CONFIG_X86)
> -	kernel_fpu_begin();
> +		kernel_fpu_begin();
>   #elif defined(CONFIG_PPC64)
> -	if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
> -		preempt_disable();
> -		enable_kernel_vsx();
> -	} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
> -		preempt_disable();
> -		enable_kernel_altivec();
> -	} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
> -		preempt_disable();
> -		enable_kernel_fp();
> -	}
> +		if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
> +			preempt_disable();
> +			enable_kernel_vsx();
> +		} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
> +			preempt_disable();
> +			enable_kernel_altivec();
> +		} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
> +			preempt_disable();
> +			enable_kernel_fp();
> +		}
>   #endif
> +	}
> +
> +	TRACE_DCN_FPU(true, function_name, line, *pcpu);
> +	put_cpu_ptr(&fpu_recursion_depth);
>   }
>   
>   /**
> @@ -78,19 +99,27 @@ void dc_fpu_begin(const char *function_name, const int line)
>    */
>   void dc_fpu_end(const char *function_name, const int line)
>   {
> -	TRACE_DCN_FPU(false, function_name, line);
> +	int *pcpu;
> +
> +	pcpu = get_cpu_ptr(&fpu_recursion_depth);
> +	*pcpu -= 1;
> +	if (*pcpu <= 0) {
>   #if defined(CONFIG_X86)
> -	kernel_fpu_end();
> +		kernel_fpu_end();
>   #elif defined(CONFIG_PPC64)
> -	if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
> -		disable_kernel_vsx();
> -		preempt_enable();
> -	} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
> -		disable_kernel_altivec();
> -		preempt_enable();
> -	} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
> -		disable_kernel_fp();
> -		preempt_enable();
> -	}
> +		if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
> +			disable_kernel_vsx();
> +			preempt_enable();
> +		} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
> +			disable_kernel_altivec();
> +			preempt_enable();
> +		} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
> +			disable_kernel_fp();
> +			preempt_enable();
> +		}
>   #endif
> +	}
> +
> +	TRACE_DCN_FPU(false, function_name, line, *pcpu);
> +	put_cpu_ptr(&fpu_recursion_depth);
>   }
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_trace.h b/drivers/gpu/drm/amd/display/dc/dc_trace.h
> index d598ba697e45..c711797e5c9e 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_trace.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_trace.h
> @@ -38,5 +38,5 @@
>   #define TRACE_DCN_CLOCK_STATE(dcn_clocks) \
>   	trace_amdgpu_dm_dc_clocks_state(dcn_clocks)
>   
> -#define TRACE_DCN_FPU(begin, function, line) \
> -	trace_dcn_fpu(begin, function, line)
> +#define TRACE_DCN_FPU(begin, function, line, ref_count) \
> +	trace_dcn_fpu(begin, function, line, ref_count)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
index 230bb12c405e..fdcaea22b456 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
@@ -638,23 +638,26 @@  TRACE_EVENT(amdgpu_refresh_rate_track,
 );
 
 TRACE_EVENT(dcn_fpu,
-	    TP_PROTO(bool begin, const char *function, const int line),
-	    TP_ARGS(begin, function, line),
+	    TP_PROTO(bool begin, const char *function, const int line, const int recursion_depth),
+	    TP_ARGS(begin, function, line, recursion_depth),
 
 	    TP_STRUCT__entry(
 			     __field(bool, begin)
 			     __field(const char *, function)
 			     __field(int, line)
+			     __field(int, recursion_depth)
 	    ),
 	    TP_fast_assign(
 			   __entry->begin = begin;
 			   __entry->function = function;
 			   __entry->line = line;
+			   __entry->recursion_depth = recursion_depth;
 	    ),
-	    TP_printk("%s()+%d: %s",
+	    TP_printk("%s: recursion_depth: %d: %s()+%d:",
+		      __entry->begin ? "begin" : "end",
+		      __entry->recursion_depth,
 		      __entry->function,
-		      __entry->line,
-		      __entry->begin ? "begin" : "end"
+		      __entry->line
 	    )
 );
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
index d4ab27bf8d04..33807d746e76 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
@@ -33,6 +33,19 @@ 
 #include <asm/cputable.h>
 #endif
 
+/**
+ * DOC: DC FPU manipulation overview
+ *
+ * DC core uses FPU operations in multiple parts of the code, which requires a
+ * more specialized way to manage these areas' entrance. To fulfill this
+ * requirement, we created some wrapper functions that encapsulate
+ * kernel_fpu_begin/end to better fit our need in the display component. In
+ * summary, in this file, you can find functions related to FPU operation
+ * management.
+ */
+
+static DEFINE_PER_CPU(int, fpu_recursion_depth);
+
 /**
  * dc_fpu_begin - Enables FPU protection
  * @function_name: A string containing the function name for debug purposes
@@ -48,22 +61,30 @@ 
  */
 void dc_fpu_begin(const char *function_name, const int line)
 {
-	TRACE_DCN_FPU(true, function_name, line);
+	int *pcpu;
+
+	pcpu = get_cpu_ptr(&fpu_recursion_depth);
+	*pcpu += 1;
 
+	if (*pcpu == 1) {
 #if defined(CONFIG_X86)
-	kernel_fpu_begin();
+		kernel_fpu_begin();
 #elif defined(CONFIG_PPC64)
-	if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
-		preempt_disable();
-		enable_kernel_vsx();
-	} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
-		preempt_disable();
-		enable_kernel_altivec();
-	} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
-		preempt_disable();
-		enable_kernel_fp();
-	}
+		if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
+			preempt_disable();
+			enable_kernel_vsx();
+		} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
+			preempt_disable();
+			enable_kernel_altivec();
+		} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
+			preempt_disable();
+			enable_kernel_fp();
+		}
 #endif
+	}
+
+	TRACE_DCN_FPU(true, function_name, line, *pcpu);
+	put_cpu_ptr(&fpu_recursion_depth);
 }
 
 /**
@@ -78,19 +99,27 @@  void dc_fpu_begin(const char *function_name, const int line)
  */
 void dc_fpu_end(const char *function_name, const int line)
 {
-	TRACE_DCN_FPU(false, function_name, line);
+	int *pcpu;
+
+	pcpu = get_cpu_ptr(&fpu_recursion_depth);
+	*pcpu -= 1;
+	if (*pcpu <= 0) {
 #if defined(CONFIG_X86)
-	kernel_fpu_end();
+		kernel_fpu_end();
 #elif defined(CONFIG_PPC64)
-	if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
-		disable_kernel_vsx();
-		preempt_enable();
-	} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
-		disable_kernel_altivec();
-		preempt_enable();
-	} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
-		disable_kernel_fp();
-		preempt_enable();
-	}
+		if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
+			disable_kernel_vsx();
+			preempt_enable();
+		} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
+			disable_kernel_altivec();
+			preempt_enable();
+		} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
+			disable_kernel_fp();
+			preempt_enable();
+		}
 #endif
+	}
+
+	TRACE_DCN_FPU(false, function_name, line, *pcpu);
+	put_cpu_ptr(&fpu_recursion_depth);
 }
diff --git a/drivers/gpu/drm/amd/display/dc/dc_trace.h b/drivers/gpu/drm/amd/display/dc/dc_trace.h
index d598ba697e45..c711797e5c9e 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_trace.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_trace.h
@@ -38,5 +38,5 @@ 
 #define TRACE_DCN_CLOCK_STATE(dcn_clocks) \
 	trace_amdgpu_dm_dc_clocks_state(dcn_clocks)
 
-#define TRACE_DCN_FPU(begin, function, line) \
-	trace_dcn_fpu(begin, function, line)
+#define TRACE_DCN_FPU(begin, function, line, ref_count) \
+	trace_dcn_fpu(begin, function, line, ref_count)