Re: [PATCH v6 23/57] drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.
On Wed, Sep 7, 2022 at 12:13 AM Daniel Vetter wrote:
>
> On Sun, Sep 04, 2022 at 03:41:00PM -0600, Jim Cromie wrote:
> > Use DECLARE_DYNDBG_CLASSMAP across DRM:
> >
> > - in .c files, since macro defines/initializes a record
> >
> > - in drivers, $mod_{drv,drm,param}.c
> >ie where param setup is done, since a classmap is param related
> >
> > - in drm/drm_print.c
> >since existing __drm_debug param is defined there,
> >and we ifdef it, and provide an elaborated alternative.
> >
> > - in drm_*_helper modules:
> >dp/drm_dp - 1st item in makefile target
> >drivers/gpu/drm/drm_crtc_helper.c - random pick iirc.
> >
> > Since these modules all use identical CLASSMAP declarations (ie: names
> > and .class_id's) they will all respond together to "class DRM_UT_*"
> > query-commands:
> >
> > :#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
> >
> > NOTES:
> >
> > This changes __drm_debug from int to ulong, so BIT() is usable on it.
> >
> > DRM's enum drm_debug_category values need to sync with the index of
> > their respective class-names here. Then .class_id == category, and
> > dyndbg's class FOO mechanisms will enable drm_dbg(DRM_UT_KMS, ...).
> >
> > Though DRM needs consistent categories across all modules, thats not
> > generally needed; modules X and Y could define FOO differently (ie a
> > different NAME => class_id mapping), changes are made according to
> > each module's private class-map.
> >
> > No callsites are actually selected by this patch, since none are
> > class'd yet.
> >
> > Signed-off-by: Jim Cromie
>
> So maybe I should just try, but what happens if a drm module doesn't have
> these classbits declared? You simply have to use the raw number instead?
without the classnames declared via macro,
dyndbg has no names by which to validate the query.
raw class numbers are not usable into >control.
This is what privatizes the module's class-id space.
If the macro is missing, the drm_dbg()s ( after conversion to reside
atop dyndbg)
will do this in `cat control`
seq_printf(m, " class unknown, _id:%d", dp->class_id);
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 +
> > drivers/gpu/drm/display/drm_dp_helper.c | 13
> > drivers/gpu/drm/drm_crtc_helper.c | 13
> > drivers/gpu/drm/drm_print.c | 27 +++--
> > drivers/gpu/drm/i915/i915_params.c | 12 +++
> > drivers/gpu/drm/nouveau/nouveau_drm.c | 13
> > include/drm/drm_print.h | 3 ++-
> > 7 files changed, 92 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index de7144b06e93..97e184f44a52 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -38,6 +38,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> >
> > #include "amdgpu.h"
> > #include "amdgpu_irq.h"
> > @@ -185,6 +187,18 @@ int amdgpu_vcnfw_log;
> >
> > static void amdgpu_drv_delayed_reset_work_handler(struct work_struct
> > *work);
> >
> > +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
>
> Iirc we've talked about maybe some kbuild trickery so that any module
> under drivers/gpu/drm gets these by default. I don't think we need to have
> this for the first cut, but a macro to avoid the copypaste mistakes would
> be really good here.
It *may be* that theres a perfect place to declare it once, for everyone.
For me thats exploratory, error prone.
Proving that the sub-optimal worked seemed a good place to stop.
that said, theres a macro in test-dynamic-debug that is a candidate
for wider availability - it needs a better name
#define DD_SYS_WRAP(_model, _flags) \
static unsigned long bits_##_model; \
static struct ddebug_class_param _flags##_model = { \
.bits = _##_model, \
.flags = #_flags, \
.map = _##_model, \
}; \
module_param_cb(_flags##_##_model, _ops_dyndbg_classes,
&_flags##_model, 0600)
Re: [PATCH v6 23/57] drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.
On Sun, Sep 04, 2022 at 03:41:00PM -0600, Jim Cromie wrote:
> Use DECLARE_DYNDBG_CLASSMAP across DRM:
>
> - in .c files, since macro defines/initializes a record
>
> - in drivers, $mod_{drv,drm,param}.c
>ie where param setup is done, since a classmap is param related
>
> - in drm/drm_print.c
>since existing __drm_debug param is defined there,
>and we ifdef it, and provide an elaborated alternative.
>
> - in drm_*_helper modules:
>dp/drm_dp - 1st item in makefile target
>drivers/gpu/drm/drm_crtc_helper.c - random pick iirc.
>
> Since these modules all use identical CLASSMAP declarations (ie: names
> and .class_id's) they will all respond together to "class DRM_UT_*"
> query-commands:
>
> :#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
>
> NOTES:
>
> This changes __drm_debug from int to ulong, so BIT() is usable on it.
>
> DRM's enum drm_debug_category values need to sync with the index of
> their respective class-names here. Then .class_id == category, and
> dyndbg's class FOO mechanisms will enable drm_dbg(DRM_UT_KMS, ...).
>
> Though DRM needs consistent categories across all modules, thats not
> generally needed; modules X and Y could define FOO differently (ie a
> different NAME => class_id mapping), changes are made according to
> each module's private class-map.
>
> No callsites are actually selected by this patch, since none are
> class'd yet.
>
> Signed-off-by: Jim Cromie
So maybe I should just try, but what happens if a drm module doesn't have
these classbits declared? You simply have to use the raw number instead?
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 +
> drivers/gpu/drm/display/drm_dp_helper.c | 13
> drivers/gpu/drm/drm_crtc_helper.c | 13
> drivers/gpu/drm/drm_print.c | 27 +++--
> drivers/gpu/drm/i915/i915_params.c | 12 +++
> drivers/gpu/drm/nouveau/nouveau_drm.c | 13
> include/drm/drm_print.h | 3 ++-
> 7 files changed, 92 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index de7144b06e93..97e184f44a52 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -38,6 +38,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include "amdgpu.h"
> #include "amdgpu_irq.h"
> @@ -185,6 +187,18 @@ int amdgpu_vcnfw_log;
>
> static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
>
> +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
Iirc we've talked about maybe some kbuild trickery so that any module
under drivers/gpu/drm gets these by default. I don't think we need to have
this for the first cut, but a macro to avoid the copypaste mistakes would
be really good here.
> + "DRM_UT_CORE",
> + "DRM_UT_DRIVER",
> + "DRM_UT_KMS",
> + "DRM_UT_PRIME",
> + "DRM_UT_ATOMIC",
> + "DRM_UT_VBL",
> + "DRM_UT_STATE",
> + "DRM_UT_LEASE",
> + "DRM_UT_DP",
> + "DRM_UT_DRMRES");
> +
> struct amdgpu_mgpu_info mgpu_info = {
> .mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
> .delayed_reset_work = __DELAYED_WORK_INITIALIZER(
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 92990a3d577a..cbb9c4d6d8f2 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -40,6 +41,18 @@
>
> #include "drm_dp_helper_internal.h"
>
> +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
> + "DRM_UT_CORE",
> + "DRM_UT_DRIVER",
> + "DRM_UT_KMS",
> + "DRM_UT_PRIME",
> + "DRM_UT_ATOMIC",
> + "DRM_UT_VBL",
> + "DRM_UT_STATE",
> + "DRM_UT_LEASE",
> + "DRM_UT_DP",
> + "DRM_UT_DRMRES");
> +
> struct dp_aux_backlight {
> struct backlight_device *base;
> struct drm_dp_aux *aux;
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> b/drivers/gpu/drm/drm_crtc_helper.c
> index 457448cc60f7..7d86020b5244 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -32,6 +32,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -50,6 +51,18 @@
>
> #include "drm_crtc_helper_internal.h"
>
> +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
> + "DRM_UT_CORE",
> +
[PATCH v6 23/57] drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.
Use DECLARE_DYNDBG_CLASSMAP across DRM:
- in .c files, since macro defines/initializes a record
- in drivers, $mod_{drv,drm,param}.c
ie where param setup is done, since a classmap is param related
- in drm/drm_print.c
since existing __drm_debug param is defined there,
and we ifdef it, and provide an elaborated alternative.
- in drm_*_helper modules:
dp/drm_dp - 1st item in makefile target
drivers/gpu/drm/drm_crtc_helper.c - random pick iirc.
Since these modules all use identical CLASSMAP declarations (ie: names
and .class_id's) they will all respond together to "class DRM_UT_*"
query-commands:
:#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
NOTES:
This changes __drm_debug from int to ulong, so BIT() is usable on it.
DRM's enum drm_debug_category values need to sync with the index of
their respective class-names here. Then .class_id == category, and
dyndbg's class FOO mechanisms will enable drm_dbg(DRM_UT_KMS, ...).
Though DRM needs consistent categories across all modules, thats not
generally needed; modules X and Y could define FOO differently (ie a
different NAME => class_id mapping), changes are made according to
each module's private class-map.
No callsites are actually selected by this patch, since none are
class'd yet.
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 +
drivers/gpu/drm/display/drm_dp_helper.c | 13
drivers/gpu/drm/drm_crtc_helper.c | 13
drivers/gpu/drm/drm_print.c | 27 +++--
drivers/gpu/drm/i915/i915_params.c | 12 +++
drivers/gpu/drm/nouveau/nouveau_drm.c | 13
include/drm/drm_print.h | 3 ++-
7 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index de7144b06e93..97e184f44a52 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -38,6 +38,8 @@
#include
#include
#include
+#include
+#include
#include "amdgpu.h"
#include "amdgpu_irq.h"
@@ -185,6 +187,18 @@ int amdgpu_vcnfw_log;
static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
struct amdgpu_mgpu_info mgpu_info = {
.mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
.delayed_reset_work = __DELAYED_WORK_INITIALIZER(
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
b/drivers/gpu/drm/display/drm_dp_helper.c
index 92990a3d577a..cbb9c4d6d8f2 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
@@ -40,6 +41,18 @@
#include "drm_dp_helper_internal.h"
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
struct dp_aux_backlight {
struct backlight_device *base;
struct drm_dp_aux *aux;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index 457448cc60f7..7d86020b5244 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include
#include
@@ -50,6 +51,18 @@
#include "drm_crtc_helper_internal.h"
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
/**
* DOC: overview
*
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index f783d4963d4b..ec32df35a3e3 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -40,7 +40,7 @@
* __drm_debug: Enable debug output.
* Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
*/
