[PATCH] drm/amd/display: Make BREAK_TO_DEBUGGER() a debug print

[PATCH] drm/amd/display: Make BREAK_TO_DEBUGGER() a debug print

[Nicholas Kazlauskas]

[Why]
Warnings in the kernel are generally treated as errors.

The BREAK_TO_DEBUGGER macro is not a critical error or warning, but
rather intended for developer use to help investigate behavior and
sequences for other issues.

We do still make use of DC_ERROR/ASSERT(0) in various places in the
code for things that are genuine issues.

Since most developers don't actually KGDB while debugging the kernel
these essentially would have no value on their own since the KGDB
breakpoint wouldn't trigger - ASSERT(0) was used as a shortcut to get
a stacktrace.

[How]
Turn it into a DRM_DEBUG_DRIVER print instead. We unfortunately lose
the stacktrace, but we still do retain some of the useful debug
information this offers by having at least the function and line
number loggable.

If KGDB is supported in the kernel this will still trigger a real
breakpoint as well.

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 drivers/gpu/drm/amd/display/dc/os_types.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h b/drivers/gpu/drm/amd/display/dc/os_types.h
index 6d7bca562eec..604ceb6c0375 100644
--- a/drivers/gpu/drm/amd/display/dc/os_types.h
+++ b/drivers/gpu/drm/amd/display/dc/os_types.h
@@ -111,7 +111,15 @@
 #define ASSERT(expr) WARN_ON_ONCE(!(expr))
 #endif
 
-#define BREAK_TO_DEBUGGER() ASSERT(0)
+#if defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB)
+#define BREAK_TO_DEBUGGER() \
+	do { \
+		DRM_DEBUG_DRIVER("%s():%d\n", __func__, __LINE__); \
+		kgdb_breakpoint(); \
+	} while (0)
+#else
+#define BREAK_TO_DEBUGGER() DRM_DEBUG_DRIVER("%s():%d\n", __func__, __LINE__)
+#endif
 
 #define DC_ERR(...)  do { \
 	dm_error(__VA_ARGS__); \
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: Make BREAK_TO_DEBUGGER() a debug print

[Harry Wentland]

On 2020-05-22 2:03 p.m., Nicholas Kazlauskas wrote:
> [Why]
> Warnings in the kernel are generally treated as errors.
> 
> The BREAK_TO_DEBUGGER macro is not a critical error or warning, but
> rather intended for developer use to help investigate behavior and
> sequences for other issues.
> 
> We do still make use of DC_ERROR/ASSERT(0) in various places in the
> code for things that are genuine issues.
> 
> Since most developers don't actually KGDB while debugging the kernel
> these essentially would have no value on their own since the KGDB
> breakpoint wouldn't trigger - ASSERT(0) was used as a shortcut to get
> a stacktrace.
> 
> [How]
> Turn it into a DRM_DEBUG_DRIVER print instead. We unfortunately lose
> the stacktrace, but we still do retain some of the useful debug
> information this offers by having at least the function and line
> number loggable.
> 
> If KGDB is supported in the kernel this will still trigger a real
> breakpoint as well.
> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  drivers/gpu/drm/amd/display/dc/os_types.h | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h b/drivers/gpu/drm/amd/display/dc/os_types.h
> index 6d7bca562eec..604ceb6c0375 100644
> --- a/drivers/gpu/drm/amd/display/dc/os_types.h
> +++ b/drivers/gpu/drm/amd/display/dc/os_types.h
> @@ -111,7 +111,15 @@
>  #define ASSERT(expr) WARN_ON_ONCE(!(expr))
>  #endif
>  
> -#define BREAK_TO_DEBUGGER() ASSERT(0)
> +#if defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB)
> +#define BREAK_TO_DEBUGGER() \
> +	do { \
> +		DRM_DEBUG_DRIVER("%s():%d\n", __func__, __LINE__); \
> +		kgdb_breakpoint(); \
> +	} while (0)
> +#else
> +#define BREAK_TO_DEBUGGER() DRM_DEBUG_DRIVER("%s():%d\n", __func__, __LINE__)
> +#endif
>  
>  #define DC_ERR(...)  do { \
>  	dm_error(__VA_ARGS__); \
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: Make BREAK_TO_DEBUGGER() a debug print

[Michel Dänzer]

On 2020-05-22 8:03 p.m., Nicholas Kazlauskas wrote:
> [Why]
> Warnings in the kernel are generally treated as errors.
> 
> The BREAK_TO_DEBUGGER macro is not a critical error or warning, but
> rather intended for developer use to help investigate behavior and
> sequences for other issues.
> 
> We do still make use of DC_ERROR/ASSERT(0) in various places in the
> code for things that are genuine issues.
> 
> Since most developers don't actually KGDB while debugging the kernel
> these essentially would have no value on their own since the KGDB
> breakpoint wouldn't trigger - ASSERT(0) was used as a shortcut to get
> a stacktrace.
> 
> [How]
> Turn it into a DRM_DEBUG_DRIVER print instead. We unfortunately lose
> the stacktrace, but we still do retain some of the useful debug
> information this offers by having at least the function and line
> number loggable.
> 
> If KGDB is supported in the kernel this will still trigger a real
> breakpoint as well.

Not sure this makes sense now that WARN_ON_ONCE is used for ASSERT().
The name "BREAK_TO_DEBUGGER" implies that something unexpected happened
which needs to be investigated, so having a backtrace seems important to me.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx