[PATCH 3/4] drm/amd/display: Add ref count control for FPU utilization

[Christian König]

Am 31.03.21 um 14:25 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.
> For avoiding this situation, this commit adds a mechanism where
> dc_fpu_begin/end manages the access to kernel_fpu_begin/end.

In general please name this "recursion" control instead of "ref count". 
Let's call the child by it's name :)

>
> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> Acked-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 13 ++++---
>   .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.c    | 34 ++++++++++++++++---
>   drivers/gpu/drm/amd/display/dc/dc_trace.h     |  4 +--
>   3 files changed, 40 insertions(+), 11 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..cd4f0d3f37fb 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 ref_count),
> +	    TP_ARGS(begin, function, line, ref_count),
>   
>   	    TP_STRUCT__entry(
>   			     __field(bool, begin)
>   			     __field(const char *, function)
>   			     __field(int, line)
> +			     __field(int, ref_count)
>   	    ),
>   	    TP_fast_assign(
>   			   __entry->begin = begin;
>   			   __entry->function = function;
>   			   __entry->line = line;
> +			   __entry->ref_count = ref_count;
>   	    ),
> -	    TP_printk("%s()+%d: %s",
> +	    TP_printk("%s: ref_count: %d: %s()+%d:",
> +		      __entry->begin ? "begin" : "end",
> +		      __entry->ref_count,
>   		      __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 ff34007509de..5dcefe193523 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
> @@ -28,6 +28,19 @@
>   
>   #include <asm/fpu/api.h>
>   
> +/**
> + * DOC: 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(atomic_t, fpu_ref);
> +
>   /**
>    * dc_fpu_begin - Enables FPU protection
>    * @function_name: A string containing the function name for debug purposes
> @@ -40,8 +53,14 @@
>    */
>   void dc_fpu_begin(const char *function_name, const int line)
>   {
> -	TRACE_DCN_FPU(true, function_name, line);
> -	kernel_fpu_begin();
> +	int ret;
> +	atomic_t *local_fpu_ref = this_cpu_ptr(&fpu_ref);
> +
> +	ret = atomic_inc_return(local_fpu_ref);

You need to disable preemption for this or otherwise it can be that you 
schedule to another process.

Regards,
Christian.

> +	TRACE_DCN_FPU(true, function_name, line, ret);
> +
> +	if (ret == 1)
> +		kernel_fpu_begin();
>   }
>   
>   /**
> @@ -56,6 +75,13 @@ 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);
> -	kernel_fpu_end();
> +
> +	int ret;
> +	atomic_t *local_fpu_ref = this_cpu_ptr(&fpu_ref);
> +
> +	ret = atomic_dec_return(local_fpu_ref);
> +	TRACE_DCN_FPU(false, function_name, line, ret);
> +
> +	if (!ret)
> +		kernel_fpu_end();
>   }
> 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)