On 07/20, Christian König wrote:
Am 20.07.21 um 02:49 schrieb Rodrigo Siqueira:
We don't have any mechanism for tracing FPU operations inside the display core, making the debug work a little bit tricky. This commit introduces a trace mechanism inside our DC_FP_START/END macros for trying to alleviate this problem.
Changes since V2:
- Make sure that we compile FPU operation only in the context that DCN is enabled.
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 Signed-off-by: Rodrigo Siqueira Rodrigo.Siqueira@amd.com
.../gpu/drm/amd/display/amdgpu_dm/Makefile | 4 ++ .../amd/display/amdgpu_dm/amdgpu_dm_trace.h | 21 ++++++ .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 64 +++++++++++++++++++ .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.h | 33 ++++++++++ drivers/gpu/drm/amd/display/dc/dc_trace.h | 3 + drivers/gpu/drm/amd/display/dc/os_types.h | 6 +- 6 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.h
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile index 91fb72c96545..718e123a3230 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile @@ -27,6 +27,10 @@ AMDGPUDM = amdgpu_dm.o amdgpu_dm_irq.o amdgpu_dm_mst_types.o amdgpu_dm_color.o +ifdef CONFIG_DRM_AMD_DC_DCN +AMDGPUDM += dc_fpu.o +endif
- ifneq ($(CONFIG_DRM_AMD_DC),) AMDGPUDM += amdgpu_dm_services.o amdgpu_dm_helpers.o amdgpu_dm_pp_smu.o amdgpu_dm_psr.o endif
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 46a33f64cf8e..230bb12c405e 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 @@ -637,6 +637,27 @@ TRACE_EVENT(amdgpu_refresh_rate_track, __entry->refresh_rate_ns) ); +TRACE_EVENT(dcn_fpu,
TP_PROTO(bool begin, const char *function, const int line),TP_ARGS(begin, function, line),TP_STRUCT__entry(__field(bool, begin)__field(const char *, function)__field(int, line)),TP_fast_assign(__entry->begin = begin;__entry->function = function;__entry->line = line;),TP_printk("%s()+%d: %s",__entry->function,__entry->line,__entry->begin ? "begin" : "end")+);
- #endif /* _AMDGPU_DM_TRACE_H_ */ #undef TRACE_INCLUDE_PATH
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c new file mode 100644 index 000000000000..d5d156a4517e --- /dev/null +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: MIT +/*
- Copyright 2021 Advanced Micro Devices, Inc.
- Permission is hereby granted, free of charge, to any person obtaining a
- copy of this software and associated documentation files (the "Software"),
- to deal in the Software without restriction, including without limitation
- the rights to use, copy, modify, merge, publish, distribute, sublicense,
- and/or sell copies of the Software, and to permit persons to whom the
- Software is furnished to do so, subject to the following conditions:
- The above copyright notice and this permission notice shall be included in
- all copies or substantial portions of the Software.
- THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
- OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- OTHER DEALINGS IN THE SOFTWARE.
- Authors: AMD
- */
+#include "dc_trace.h"
+#include <asm/fpu/api.h>
+/**
- dc_fpu_begin - Enables FPU protection
- @function_name: A string containing the function name for debug purposes
- (usually __func__)
- @line: A line number where DC_FP_START was invoked for debug purpose
- (usually __LINE__)
- This function is responsible for managing the use of kernel_fpu_begin() with
- the advantage of providing an event trace for debugging.
- Note: Do not call this function directly; always use DC_FP_START().
- */
+void dc_fpu_begin(const char *function_name, const int line) +{
- TRACE_DCN_FPU(true, function_name, line);
- kernel_fpu_begin();
+}
+/**
- dc_fpu_end - Disable FPU protection
- @function_name: A string containing the function name for debug purposes
- @line: A-line number where DC_FP_END was invoked for debug purpose
- This function is responsible for managing the use of kernel_fpu_end() with
- the advantage of providing an event trace for debugging.
- Note: Do not call this function directly; always use DC_FP_END().
- */
+void dc_fpu_end(const char *function_name, const int line) +{
- TRACE_DCN_FPU(false, function_name, line);
- kernel_fpu_end();
+} diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.h b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.h new file mode 100644 index 000000000000..fb54983c5c60 --- /dev/null +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.h @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: MIT */ +/*
- Copyright 2021 Advanced Micro Devices, Inc.
- Permission is hereby granted, free of charge, to any person obtaining a
- copy of this software and associated documentation files (the "Software"),
- to deal in the Software without restriction, including without limitation
- the rights to use, copy, modify, merge, publish, distribute, sublicense,
- and/or sell copies of the Software, and to permit persons to whom the
- Software is furnished to do so, subject to the following conditions:
- The above copyright notice and this permission notice shall be included in
- all copies or substantial portions of the Software.
- THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
- OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- OTHER DEALINGS IN THE SOFTWARE.
- Authors: AMD
- */
+#ifndef __DC_FPU_H__ +#define __DC_FPU_H__
+void dc_fpu_begin(const char *function_name, const int line); +void dc_fpu_end(const char *function_name, const int line);
+#endif /* __DC_FPU_H__ */ diff --git a/drivers/gpu/drm/amd/display/dc/dc_trace.h b/drivers/gpu/drm/amd/display/dc/dc_trace.h index d2615357269b..d598ba697e45 100644 --- a/drivers/gpu/drm/amd/display/dc/dc_trace.h +++ b/drivers/gpu/drm/amd/display/dc/dc_trace.h @@ -37,3 +37,6 @@ #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)
diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h b/drivers/gpu/drm/amd/display/dc/os_types.h index 126c2f3a4dd3..2ba49aef370d 100644 --- a/drivers/gpu/drm/amd/display/dc/os_types.h +++ b/drivers/gpu/drm/amd/display/dc/os_types.h @@ -52,9 +52,9 @@ #if defined(CONFIG_DRM_AMD_DC_DCN) #if defined(CONFIG_X86) -#include <asm/fpu/api.h> -#define DC_FP_START() kernel_fpu_begin() -#define DC_FP_END() kernel_fpu_end() +#include "amdgpu_dm/dc_fpu.h" +#define DC_FP_START() dc_fpu_begin(__func__, __LINE__) +#define DC_FP_END() dc_fpu_end(__func__, __LINE__)
This still has the issue that you don't correctly handle other architectures than x86.
I'm pretty sure the kernel bots will complain about that.
Hi Christian,
Kernel bots will not break because we are using CONFIG_DRM_AMD_DC_DCN to isolate DCN architecture. Currently, we support DCN on x86 and PPC (idk the details for ppc); in this series, I'm trying to enable us to start to isolate FPU code to a single place. When we start to move the FPU functions to a single place, we can take the opportunity to refactor the FPU code. After we isolate the FPU source, we can safely start to enable other architectures such as ARM.
This is why I did not try to enable it for other architecture yet. I think this is the second step.
Thanks Siqueira
Regards, Christian.
#elif defined(CONFIG_PPC64) #include <asm/switch_to.h> #include <asm/cputable.h>