[Intel-gfx] ✗ Fi.CI.BUILD: failure for DYNDBG: opt-in class'd debug for modules, use in drm. (rev3)

2022-08-05 Thread Patchwork
== Series Details ==

Series: DYNDBG: opt-in class'd debug for modules, use in drm. (rev3)
URL   : https://patchwork.freedesktop.org/series/106427/
State : failure

== Summary ==

Error: patch 
https://patchwork.freedesktop.org/api/1.0/series/106427/revisions/3/mbox/ not 
applied
Applying: dyndbg: fix static_branch manipulation
Applying: dyndbg: fix module.dyndbg handling
Applying: dyndbg: show both old and new in change-info
Applying: dyndbg: reverse module walk in cat control
Applying: dyndbg: reverse module.callsite walk in cat control
Applying: dyndbg: use ESCAPE_SPACE for cat control
Applying: dyndbg: let query-modname override actual module name
Applying: dyndbg: add test_dynamic_debug module
Applying: dyndbg: drop EXPORTed dynamic_debug_exec_queries
Applying: dyndbg: cleanup local vars in ddebug_init
Applying: dyndbg: create and use struct _ddebug_info
Applying: dyndbg: add class_id to pr_debug callsites
Applying: dyndbg: add __pr_debug_cls for testing
Applying: dyndbg: add DECLARE_DYNDBG_CLASSMAP macro
Applying: kernel/module: add __dyndbg_classes section
Applying: dyndbg: add ddebug_attach_module_classes
Applying: dyndbg: validate class FOO by checking with module
Applying: doc-dyndbg: describe "class CLASS_NAME" query support
Applying: doc-dyndbg: edit dynamic-debug-howto for brevity, audience
Applying: dyndbg: add drm.debug style (drm/parameters/debug) bitmap support
Applying: dyndbg: test DECLARE_DYNDBG_CLASSMAP, sysfs nodes
Applying: drm_print: condense enum drm_debug_category
Applying: drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.
Applying: drm_print: interpose drm_*dbg with forwarding macros
Applying: drm_print: wrap drm_*_dbg in dyndbg descriptor factory macro
Using index info to reconstruct a base tree...
M   drivers/gpu/drm/Kconfig
M   drivers/gpu/drm/Makefile
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/Makefile
Auto-merging drivers/gpu/drm/Kconfig
CONFLICT (content): Merge conflict in drivers/gpu/drm/Kconfig
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0025 drm_print: wrap drm_*_dbg in dyndbg descriptor factory 
macro
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".




[Intel-gfx] [PATCH v5 28/33] drm_print: refine drm_debug_enabled for jump-label

2022-08-05 Thread Jim Cromie
In order to use dynamic-debug's jump-label optimization in drm-debug,
its clarifying to refine drm_debug_enabled into 3 uses:

1.   drm_debug_enabled - legacy, public
2. __drm_debug_enabled - optimized for dyndbg jump-label enablement.
3.  _drm_debug_enabled - pr_debug instrumented, observable

1. The legacy version always checks the bits.

2. is privileged, for use by __drm_dbg(), __drm_dev_dbg(), which do an
early return unless the category is enabled.  For dyndbg builds, debug
callsites are selectively "pre-enabled", so __drm_debug_enabled()
short-circuits to true there.  Remaining callers of 1 may be able to
use 2, case by case.

3. is 1st wrapped in a macro, with a pr_debug, which reports each
usage in /proc/dynamic_debug/control, making it observable in the
logs.  The macro lets the pr_debug see the real caller, not an inline
function.

When plugged into 1, 3 identified ~10 remaining callers of the
function, leading to the follow-on cleanup patch, and would allow
activating the pr_debugs, estimating the callrate, and the potential
savings by using the wrapper macro.  It is unused ATM, but it fills
out the picture.

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/drm_print.c |  4 ++--
 include/drm/drm_print.h | 28 
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 29a29949ad0b..cb203d63b286 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -285,7 +285,7 @@ void __drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
struct va_format vaf;
va_list args;
 
-   if (!drm_debug_enabled(category))
+   if (!__drm_debug_enabled(category))
return;
 
va_start(args, format);
@@ -308,7 +308,7 @@ void ___drm_dbg(enum drm_debug_category category, const 
char *format, ...)
struct va_format vaf;
va_list args;
 
-   if (!drm_debug_enabled(category))
+   if (!__drm_debug_enabled(category))
return;
 
va_start(args, format);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index dfdd81c3287c..7631b5fb669e 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -321,11 +321,39 @@ enum drm_debug_category {
DRM_UT_DRMRES
 };
 
+/*
+ * 3 name flavors of drm_debug_enabled:
+ *   drm_debug_enabled - public/legacy, always checks bits
+ *  _drm_debug_enabled - instrumented to observe call-rates, est overheads.
+ * __drm_debug_enabled - privileged - knows jump-label state, can short-circuit
+ */
 static inline bool drm_debug_enabled(enum drm_debug_category category)
 {
return unlikely(__drm_debug & BIT(category));
 }
 
+/*
+ * Wrap fn in macro, so that the pr_debug sees the actual caller, not
+ * the inline fn.  Using this name creates a callsite entry / control
+ * point in /proc/dynamic_debug/control.
+ */
+#define _drm_debug_enabled(category)   \
+   ({  \
+   pr_debug("todo: maybe avoid via dyndbg\n"); \
+   drm_debug_enabled(category);\
+   })
+
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+/*
+ * dyndbg is wrapping the drm.debug API, so as to avoid the runtime
+ * bit-test overheads of drm_debug_enabled() in those api calls.
+ * In this case, executed callsites are known enabled, so true.
+ */
+#define __drm_debug_enabled(category)  true
+#else
+#define __drm_debug_enabled(category)  drm_debug_enabled(category)
+#endif
+
 /*
  * struct device based logging
  *
-- 
2.37.1



[Intel-gfx] [PATCH v5 26/33] drm-print.h: include dyndbg header

2022-08-05 Thread Jim Cromie
lkp robot told me:

  >> drivers/gpu/drm/drm_ioc32.c:989:2:
  error: call to undeclared function '_dynamic_func_call_cls';
  ISO C99 and later do not support implicit function declarations
  [-Wimplicit-function-declaration]

   DRM_DEBUG("comm=\"%s\", pid=%d, dev=0x%lx, auth=%d, %s\n",

Since that macro is defined in drm_print.h, and under DRM_USE_DYN*=y
configs, invokes dyndbg-factory macros, include dynamic_debug.h from
there too, so that those configs have the definitions of all the
macros in the callchain.

This is done as a separate patch mostly to see how lkp sorts it.

Signed-off-by: Jim Cromie 
---
 include/drm/drm_print.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 2d2cef76b5c1..f8bb3e7158c6 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
-- 
2.37.1



[Intel-gfx] [PATCH v5 18/33] doc-dyndbg: describe "class CLASS_NAME" query support

2022-08-05 Thread Jim Cromie
Add an explanation of the new "class CLASS_NAME" syntax and meaning,
noting that the module determines if CLASS_NAME applies to it.

Signed-off-by: Jim Cromie 
---
 Documentation/admin-guide/dynamic-debug-howto.rst | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst 
b/Documentation/admin-guide/dynamic-debug-howto.rst
index a89cfa083155..d8954ab05c7b 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -35,6 +35,7 @@ Dynamic debug has even more useful features:
- line number (including ranges of line numbers)
- module name
- format string
+   - class name (as known/declared by each module)
 
  * Provides a debugfs control file: ``/dynamic_debug/control``
which can be read to display the complete list of known debug
@@ -142,6 +143,7 @@ against.  Possible keywords are:::
 'file' string |
 'module' string |
 'format' string |
+'class' string |
 'line' line-range
 
   line-range ::= lineno |
@@ -203,6 +205,15 @@ format
format "nfsd: SETATTR"  // a neater way to match a format with 
whitespace
format 'nfsd: SETATTR'  // yet another way to match a format with 
whitespace
 
+class
+The given class_name is validated against each module, which may
+have declared a list of known class_names.  If the class_name is
+found for a module, callsite & class matching and adjustment
+proceeds.  Examples::
+
+   class DRM_UT_KMS# a DRM.debug category
+   class JUNK  # silent non-match
+
 line
 The given line number or range of line numbers is compared
 against the line number of each ``pr_debug()`` callsite.  A single
-- 
2.37.1



[Intel-gfx] [PATCH v5 09/33] dyndbg: drop EXPORTed dynamic_debug_exec_queries

2022-08-05 Thread Jim Cromie
This exported fn is unused, and will not be needed. Lets dump it.

The export was added to let drm control pr_debugs, as part of using
them to avoid drm_debug_enabled overheads.  But its better to just
implement the drm.debug bitmap interface, then its available for
everyone.

Fixes: a2d375eda771 ("dyndbg: refine export, rename to 
dynamic_debug_exec_queries()")
Fixes: 4c0d77828d4f ("dyndbg: export ddebug_exec_queries")
Signed-off-by: Jim Cromie 
Acked-by: Jason Baron 
---
 include/linux/dynamic_debug.h |  9 -
 lib/dynamic_debug.c   | 29 -
 2 files changed, 38 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index f30b01aa9fa4..8d9eec5f6d8b 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -55,9 +55,6 @@ struct _ddebug {
 
 #if defined(CONFIG_DYNAMIC_DEBUG_CORE)
 
-/* exported for module authors to exercise >control */
-int dynamic_debug_exec_queries(const char *query, const char *modname);
-
 int ddebug_add_module(struct _ddebug *tab, unsigned int n,
const char *modname);
 extern int ddebug_remove_module(const char *mod_name);
@@ -221,12 +218,6 @@ static inline int ddebug_dyndbg_module_param_cb(char 
*param, char *val,
rowsize, groupsize, buf, len, ascii);   \
} while (0)
 
-static inline int dynamic_debug_exec_queries(const char *query, const char 
*modname)
-{
-   pr_warn("kernel not built with CONFIG_DYNAMIC_DEBUG_CORE\n");
-   return 0;
-}
-
 #endif /* !CONFIG_DYNAMIC_DEBUG_CORE */
 
 #endif
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 5a849716220a..e96dc216463b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -558,35 +558,6 @@ static int ddebug_exec_queries(char *query, const char 
*modname)
return nfound;
 }
 
-/**
- * dynamic_debug_exec_queries - select and change dynamic-debug prints
- * @query: query-string described in admin-guide/dynamic-debug-howto
- * @modname: string containing module name, usually _name
- *
- * This uses the >/proc/dynamic_debug/control reader, allowing module
- * authors to modify their dynamic-debug callsites. The modname is
- * canonically struct module.mod_name, but can also be null or a
- * module-wildcard, for example: "drm*".
- */
-int dynamic_debug_exec_queries(const char *query, const char *modname)
-{
-   int rc;
-   char *qry; /* writable copy of query */
-
-   if (!query) {
-   pr_err("non-null query/command string expected\n");
-   return -EINVAL;
-   }
-   qry = kstrndup(query, PAGE_SIZE, GFP_KERNEL);
-   if (!qry)
-   return -ENOMEM;
-
-   rc = ddebug_exec_queries(qry, modname);
-   kfree(qry);
-   return rc;
-}
-EXPORT_SYMBOL_GPL(dynamic_debug_exec_queries);
-
 #define PREFIX_SIZE 64
 
 static int remaining(int wrote)
-- 
2.37.1



[Intel-gfx] [PATCH v5 30/33] drm_print: add _ddebug descriptor to drm_*dbg prototypes

2022-08-05 Thread Jim Cromie
upgrade the callchain to drm_dbg() and drm_dev_dbg(); add a struct
_ddebug ptr parameter to them, and supply that additional param by
replacing the '_no_desc' flavor of dyndbg Factory macro currently used
with the flavor that supplies the descriptor.

NOTES:

The descriptor gives these fns access to the decorator flags, but does
none of the dynamic-prefixing done by __dynamic_emit_prefix().

DRM already has conventions for logging/messaging; just tossing
optional decorations on top may not help.  Instead, existing flags (or
new ones) can be used to make current conventions optional.

For CONFIG_DRM_USE_DYNAMIC_DEBUG=N, just pass null.

Note: desc->class_id is redundant with category parameter, but its
availability is dependent on desc.

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/drm_print.c |  8 +---
 include/drm/drm_print.h | 23 ---
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index ec477c44a784..5b93c11895bb 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -278,8 +279,8 @@ void drm_dev_printk(const struct device *dev, const char 
*level,
 }
 EXPORT_SYMBOL(drm_dev_printk);
 
-void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
-  const char *format, ...)
+void __drm_dev_dbg(struct _ddebug *desc, const struct device *dev,
+  enum drm_debug_category category, const char *format, ...)
 {
struct va_format vaf;
va_list args;
@@ -287,6 +288,7 @@ void __drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
if (!__drm_debug_enabled(category))
return;
 
+   /* we know we are printing for either syslog, tracefs, or both */
va_start(args, format);
vaf.fmt = format;
vaf.va = 
@@ -302,7 +304,7 @@ void __drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
 }
 EXPORT_SYMBOL(__drm_dev_dbg);
 
-void ___drm_dbg(enum drm_debug_category category, const char *format, ...)
+void ___drm_dbg(struct _ddebug *desc, enum drm_debug_category category, const 
char *format, ...)
 {
struct va_format vaf;
va_list args;
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 7631b5fb669e..46f14cfb401e 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -363,9 +363,10 @@ static inline bool drm_debug_enabled(enum 
drm_debug_category category)
 __printf(3, 4)
 void drm_dev_printk(const struct device *dev, const char *level,
const char *format, ...);
-__printf(3, 4)
-void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
-const char *format, ...);
+struct _ddebug;
+__printf(4, 5)
+void __drm_dev_dbg(struct _ddebug *desc, const struct device *dev,
+  enum drm_debug_category category, const char *format, ...);
 
 /**
  * DRM_DEV_ERROR() - Error output.
@@ -415,11 +416,11 @@ void __drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
 
 #if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
 #define drm_dev_dbg(dev, cat, fmt, ...)\
-   __drm_dev_dbg(dev, cat, fmt, ##__VA_ARGS__)
+   __drm_dev_dbg(NULL, dev, cat, fmt, ##__VA_ARGS__)
 #else
 #define drm_dev_dbg(dev, cat, fmt, ...)\
-   _dynamic_func_call_no_desc(fmt, __drm_dev_dbg,  \
-  dev, cat, fmt, ##__VA_ARGS__)
+   _dynamic_func_call_cls(cat, fmt, __drm_dev_dbg, \
+  dev, cat, fmt, ##__VA_ARGS__)
 #endif
 
 /**
@@ -523,17 +524,17 @@ void __drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
  * Prefer drm_device based logging over device or prink based logging.
  */
 
-__printf(2, 3)
-void ___drm_dbg(enum drm_debug_category category, const char *format, ...);
+__printf(3, 4)
+void ___drm_dbg(struct _ddebug *desc, enum drm_debug_category category, const 
char *format, ...);
 __printf(1, 2)
 void __drm_err(const char *format, ...);
 
 #if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-#define __drm_dbg(fmt, ...)___drm_dbg(fmt, ##__VA_ARGS__)
+#define __drm_dbg(fmt, ...)___drm_dbg(NULL, fmt, ##__VA_ARGS__)
 #else
 #define __drm_dbg(cat, fmt, ...)   \
-   _dynamic_func_call_no_desc(fmt, ___drm_dbg, \
-  cat, fmt, ##__VA_ARGS__)
+   _dynamic_func_call_cls(cat, fmt, ___drm_dbg,\
+  cat, fmt, ##__VA_ARGS__)
 #endif
 
 /* Macros to make printk easier */
-- 
2.37.1



[Intel-gfx] [PATCH v5 23/33] drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.

2022-08-05 Thread Jim Cromie
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 8890300766a5..183d604559fe 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"
@@ -183,6 +185,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 e7c22c2ca90c..c8a4076aa4ab 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -38,6 +39,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 b632825654a9..73e16fc0659f 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.
  */

[Intel-gfx] [PATCH v5 33/33] nouveau: WIP add 2 LEVEL_NUM classmaps for CLI, SUBDEV

2022-08-05 Thread Jim Cromie
clone the nvkm_printk,_,__ macro ladder into nvkm_drmdbg,_,__.
And alter the debug, trace, spam macros to use the renamed ladder.

This *sets-up* (not done yet) to remove the _subdev->debug >= (l)
condition, once the bitmap-param is wired up correctly, and figured
into dyndbg's jump-label enablement.  WIP.

Then undo the 1-line change that reduced count of prdbgs from 632 to 119.

ie: s/NV_SUBDEV_DBG_##l/NV_DBG_##l/

So heres what happened: new symbol is 15 (or 10), and fails this macro
test, so gets compiled out, and the dev_dbg is excluded.

if (CONFIG_NOUVEAU_DEBUG >= (l) && _subdev->debug >= (l))   \
dev_dbg(_subdev->device->dev, "%s: "f, _subdev->name, ##a); \

I could hack this, by doing (l + base), but base is pretty distant.

OTOH, the whole CONFIG_NOUVEAU_DEBUG check could be reworked; given
that trace is minumum recommended, theres not that many callsites
elided (SPAM only iirc) at compile-time, and dyndbg means keeping them
has "zero" run-cost (and 56 bytes per callsite).  So this config item
doesnt do much when DRM_USE_DYNAMIC_DEBUG=y.

So this is a useful place to stop and look around, try to guess which
trail to take..

nouveau has additional debug variables to consider:

drivers/gpu/drm/nouveau/include/nvkm/core/device.h
131:if (_device->debug >= (l)) \

drivers/gpu/drm/nouveau/include/nvkm/core/client.h
39: if (_client->debug >= NV_DBG_##l)  \

drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h
54: if (CONFIG_NOUVEAU_DEBUG >= (l) && _subdev->debug >= (l))  \

This is another baby-step, that seems not to break, so lets get a
snapshot.

whats done here:

In nouveau_drm.c, do DECLARE_DYNDBG_CLASSMAP(LEVEL_NUM type) 2x more,
for cli and subdev, right after the drm DECLARE.  Adjust base for
each, to share the 0..30 classid-space.

These declare class-names: NV_CLI_DBG_* NV_SUBDEV_DBG_* accordingly.
Note: class-name-space is flat and wide, so super-generic names like
INFO should be prefixed; who could predict what a generic V3 does
across all modules.

s should be included

adjusting the base to avoid each other, and the 0-10
already mapped to drm-debug-categorys (just above this addition).

In nvkm/core/debug.h, add enums to match the names, with initial
values to match the bases.

In nvkm/core/subdev.h, alter (recently added) nvkm_drmdbg_() to use
NV_SUBDEV_DBG_* instead of NV_DBG_*.

NB: in both classmaps, Ive left FATAL..WARN out, they're not really
optional the way INFO..SPAM are; dyndbg shouldn't be able to turn them off.

bash-5.1# modprobe nouveau
[  966.107833] dyndbg:   3 debug prints in module wmi
[  966.342188] dyndbg: class[0]: module:nouveau base:15 len:5 ty:1
[  966.342873] dyndbg:  15: 0 NV_SUBDEV_DBG_OFF
[  966.343352] dyndbg:  16: 1 NV_SUBDEV_DBG_INFO
[  966.343912] dyndbg:  17: 2 NV_SUBDEV_DBG_DEBUG
[  966.33] dyndbg:  18: 3 NV_SUBDEV_DBG_TRACE
[  966.344938] dyndbg:  19: 4 NV_SUBDEV_DBG_SPAM
[  966.345402] dyndbg: class[1]: module:nouveau base:10 len:5 ty:1
[  966.346011] dyndbg:  10: 0 NV_CLI_DBG_OFF
[  966.346477] dyndbg:  11: 1 NV_CLI_DBG_INFO
[  966.346989] dyndbg:  12: 2 NV_CLI_DBG_DEBUG
[  966.347442] dyndbg:  13: 3 NV_CLI_DBG_TRACE
[  966.347875] dyndbg:  14: 4 NV_CLI_DBG_SPAM
[  966.348284] dyndbg: class[2]: module:nouveau base:0 len:10 ty:0
[  966.34] dyndbg:  0: 0 DRM_UT_CORE
[  966.349310] dyndbg:  1: 1 DRM_UT_DRIVER
[  966.349694] dyndbg:  2: 2 DRM_UT_KMS
[  966.350083] dyndbg:  3: 3 DRM_UT_PRIME
[  966.350482] dyndbg:  4: 4 DRM_UT_ATOMIC
[  966.351016] dyndbg:  5: 5 DRM_UT_VBL
[  966.351475] dyndbg:  6: 6 DRM_UT_STATE
[  966.351899] dyndbg:  7: 7 DRM_UT_LEASE
[  966.352309] dyndbg:  8: 8 DRM_UT_DP
[  966.352678] dyndbg:  9: 9 DRM_UT_DRMRES
[  966.353104] dyndbg: module:nouveau attached 3 classes
[  966.353759] dyndbg: 119 debug prints in module nouveau

NOTE: it was 632 with previous commit, switching NV_DEBUG to use
NV_SUBDEV_DBG_DEBUG instead of NV_DBG_DEBUG may be the cause.

Signed-off-by: Jim Cromie 
---
 .../gpu/drm/nouveau/include/nvkm/core/debug.h | 16 +
 .../drm/nouveau/include/nvkm/core/subdev.h| 17 ++
 drivers/gpu/drm/nouveau/nouveau_drm.c |  7 ++
 drivers/gpu/drm/nouveau/nvkm/core/subdev.c| 23 +++
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/debug.h 
b/drivers/gpu/drm/nouveau/include/nvkm/core/debug.h
index b4a9c7d991ca..6a155a23a3d1 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/core/debug.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/core/debug.h
@@ -9,4 +9,20 @@
 #define NV_DBG_TRACE5
 #define NV_DBG_PARANOIA 6
 #define NV_DBG_SPAM 7
+
+enum nv_cli_dbg_verbose {
+   NV_CLI_DBG_OFF = 10,
+   NV_CLI_DBG_INFO,
+   NV_CLI_DBG_DEBUG,
+   NV_CLI_DBG_TRACE,
+   NV_CLI_DBG_SPAM
+};
+enum nv_subdev_dbg_verbose {
+   NV_SUBDEV_DBG_OFF = 15,
+   NV_SUBDEV_DBG_INFO,

[Intel-gfx] [PATCH v5 20/33] dyndbg: add drm.debug style (drm/parameters/debug) bitmap support

2022-08-05 Thread Jim Cromie
Add kernel_param_ops and callbacks to use a class-map to validate and
apply input to a sysfs-node, which allows users to control classes
defined in that class-map.  This supports uses like:

  echo 0x3 > /sys/module/drm/parameters/debug

IE add these:

 - int param_set_dyndbg_classes()
 - int param_get_dyndbg_classes()
 - struct kernel_param_ops param_ops_dyndbg_classes

Following the model of kernel/params.c STANDARD_PARAM_DEFS, these are
non-static and exported.  This might be unnecessary here.

get/set use an augmented kernel_param; the arg refs a new struct
ddebug_class_param, which contains:

A union of  bits, level; which points to user module's ulong
storing the module's debug-state.  By ref'g the client's bit-state
_var, code coordinates with existing code (like drm_debug_enabled)
which uses it, so existing works unchanged.  Changing to a ulong
allows use of BIT() etc.

FLAGS: dyndbg.flags toggled by changes to bitmap. Usually just "p".

MAP: a pointer to struct ddebug_classes_map, which maps those
class-names to .class_ids 0..N that the module is using.  This
class-map is declared & initialized by DECLARE_DYNDBG_CLASSMAP.

map-type: 4 enums DD_CLASS_TYPE_* select 2 input forms and 2 meanings.

numeric input:
  DD_CLASS_TYPE_DISJOINT_BITS   integer input, independent bits. ie: drm.debug
  DD_CLASS_TYPE_LEVEL_NUM   integer input, 0..N levels

classnames-list (comma separated) input:
  DD_CLASS_TYPE_DISJOINT_NAMES  each name affects a bit, others preserved
  DD_CLASS_TYPE_LEVEL_NAMES names have level meanings, like kern_levels.h

_NAMES- comma-separated classnames (with optional +-)
_NUM  - numeric input, 0-N expected
_BITS - numeric input, 0x1F bitmap form expected

_DISJOINT - bits are independent
_LEVEL- (x /sys/module/drm/parameters/debug_catnames

A standard _LEVEL_NAMES use, with one class, that sets all in the
class-map according to (x /sys/module/test_dynamic_debug/parameters/p_level_names
  : problem solved
  echo -L1 > /sys/module/test_dynamic_debug/parameters/p_level_names

Note this artifact:

  : this is same as prev cmd (due to +/-)
  echo L0 > /sys/module/test_dynamic_debug/parameters/p_level_names

  : this is "even-more" off, but same wo __pr_debug_class(L0, "..").
  echo -L0 > /sys/module/test_dynamic_debug/parameters/p_level_names

A stress-test/make-work usage (kid toggling a light switch):

  echo +L7,L0,L7,L0,L7,L0,L7,L0,L7,L0,L7,L0,L7 \
   > /sys/module/test_dynamic_debug/parameters/p_level_names

ddebug_apply_class_bitmap(): inside-fn, works on bitmaps, receives
new-bits, finds diffs vs client-bitvector holding "current" state,
and issues exec_query to commit the adjustment.

param_set_dyndbg_classes(): interface fn, sends _NAMES to
param_set_dyndbg_classnames() and returns, falls thru to handle _BITS,
_NUM internally, and calls ddebug_apply_class_bitmap().  Finishes by
updating state.

param_set_dyndbg_classnames(): handles classnames-list in loop, calls
ddebug_apply_class_bitmap for each, then updates state.

NOTES:

_LEVEL_ is overlay on _DISJOINT_; inputs are converted to a bitmask,
and is done by callbacks.  IOW this is possible, and possibly confusing:

  echo class V3 +p > control
  echo class V1 -p > control

IMO thats ok, relative verbosity is an interface property.

_LEVEL_NUM maps still need class-names, even though the names are not
usable at the sysfs interface (unlike with _NAMES style).  The names
are the only way to >control the classes.

 - It must have a "V0" name,
   something below "V1" to turn "V1" off.
   __pr_debug_cls(V0,..) is printk, don't do that.

 - "class names" is required at the >control interface.
 - relative levels are not enforced at >control

_LEVEL_NAMES bear +/- signs, which alters the on-bit-pos by 1.  IOW,
+L2 means L0,L1,L2, and -L2 means just L0,L1.  This kinda spoils the
readback fidelity, since the L0 bit gets turned on by any use of any
L*, except "-L0".

All the interface uncertainty here pertains to the _NAMES features.
Nobody has actually asked for this, so its practical (if a little
tedious) to split it out.

Signed-off-by: Jim Cromie 
---
. drop kp->mod->name as unneeded (build-dependent) 
. param_set_dyndbg_classnames, not _class_strings
. DD_CLASS_TYPE_* name changes, per Jason
. callbacks:
  extend comments on DD_CLASS_TYPE_* handling, flow notes
  varname changes
. kdoc tweaks
. add stub macro: #define KP_NAME(kp)   kp->name
  later, add "$module." prefix when config doesn't do it
. s/struct ddebug_classes_bitmap_param/struct ddebug_class_param/
. fix levels state handling

v4 had trouble where level-type's state was fouled by conversion to
bitmap form given to apply-bitmap.  fix by:

   (simplify/clarify)
   add old-bits, new-bits to sysfs-iface
   use CLASSMAP_BITMASK more
   in param_set_dyndbg_class{es,names}():
  move state-saving inside switches, and tailor it to type.
  this preserves lvl-state, vs -v4 which didnt.

I could "hack" in an offset, but the problem feels deeper.

The 

[Intel-gfx] [PATCH v5 25/33] drm_print: wrap drm_*_dbg in dyndbg descriptor factory macro

2022-08-05 Thread Jim Cromie
For CONFIG_DRM_USE_DYNAMIC_DEBUG=y, wrap __drm_dbg() & __drm_dev_dbg()
in one of dyndbg's Factory macros: _dynamic_func_call_no_desc().

This adds the callsite descriptor into the code, and an entry for each
into /proc/dynamic_debug/control.

  #> echo class DRM_UT_ATOMIC +p > /proc/dynamic_debug/control

CONFIG_DRM_USE_DYNAMIC_DEBUG=y/n is configurable because of the .data
footprint cost of per-callsite control; 56 bytes/site * ~2k for i915,
~4k callsites for amdgpu.  This is large enough that a kernel builder
might not want it.

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/Kconfig  | 12 
 drivers/gpu/drm/Makefile |  2 ++
 include/drm/drm_print.h  | 12 
 3 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f65656df3619..aad2ac5641c1 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -50,6 +50,18 @@ config DRM_DEBUG_MM
 
  If in doubt, say "N".
 
+config DRM_USE_DYNAMIC_DEBUG
+   bool "use dynamic debug to implement drm.debug"
+   default y
+   depends on DRM
+   depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
+   depends on JUMP_LABEL
+   help
+ Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
+ Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
+ bytes per callsite, the .data costs can be substantial, and
+ are therefore configurable.
+
 config DRM_DEBUG_SELFTEST
tristate "kselftests for DRM"
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 15fe3163f822..272de137d207 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -3,6 +3,8 @@
 # Makefile for the drm device driver.  This driver provides support for the
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
+CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
+
 drm-y   := drm_aperture.o drm_auth.o drm_cache.o \
drm_file.o drm_gem.o drm_ioctl.o \
drm_drv.o \
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index c429c258c957..2d2cef76b5c1 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -384,8 +384,14 @@ void __drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
}   \
 })
 
+#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
 #define drm_dev_dbg(dev, cat, fmt, ...)\
__drm_dev_dbg(dev, cat, fmt, ##__VA_ARGS__)
+#else
+#define drm_dev_dbg(dev, cat, fmt, ...)\
+   _dynamic_func_call_no_desc(fmt, __drm_dev_dbg,  \
+  dev, cat, fmt, ##__VA_ARGS__)
+#endif
 
 /**
  * DRM_DEV_DEBUG() - Debug output for generic drm code
@@ -492,7 +498,13 @@ void ___drm_dbg(enum drm_debug_category category, const 
char *format, ...);
 __printf(1, 2)
 void __drm_err(const char *format, ...);
 
+#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
 #define __drm_dbg(fmt, ...)___drm_dbg(fmt, ##__VA_ARGS__)
+#else
+#define __drm_dbg(cat, fmt, ...)   \
+   _dynamic_func_call_no_desc(fmt, ___drm_dbg, \
+  cat, fmt, ##__VA_ARGS__)
+#endif
 
 /* Macros to make printk easier */
 
-- 
2.37.1



[Intel-gfx] [PATCH v5 19/33] doc-dyndbg: edit dynamic-debug-howto for brevity, audience

2022-08-05 Thread Jim Cromie
Rework/modernize docs:

 - use /proc/dynamic_debug/control in examples
   its *always* there (when dyndbg is config'd), even when  is not.
   drop  talk, its a distraction here.

 - alias ddcmd='echo $* > /proc/dynamic_debug/control
   focus on args: declutter, hide boilerplate, make pwd independent.

 - swap sections: Viewing before Controlling. control file as Catalog.

 - focus on use by a system administrator
   add an alias to make examples more readable
   drop grep-101 lessons, admins know this.

 - use init/main.c as 1st example, thread it thru doc where useful.
   everybodys kernel boots, runs these.

 - add *prdbg* api section
   to the bottom of the file, its for developers more than admins.
   move list of api functions there.

 - simplify - drop extra words, phrases, sentences.

 - add "decorator" flags line to unify "prefix", trim fmlt descriptions

CC: linux-...@vger.kernel.org
Signed-off-by: Jim Cromie 

---
fixup-doc: trailing colons for block headers, trim fedora numbers. Bagas
---
 .../admin-guide/dynamic-debug-howto.rst   | 235 +-
 1 file changed, 117 insertions(+), 118 deletions(-)

diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst 
b/Documentation/admin-guide/dynamic-debug-howto.rst
index d8954ab05c7b..faa22f77847a 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -5,30 +5,19 @@ Dynamic debug
 Introduction
 
 
-This document describes how to use the dynamic debug (dyndbg) feature.
+Dynamic debug allows you to dynamically enable/disable kernel
+debug-print code to obtain additional kernel information.
 
-Dynamic debug is designed to allow you to dynamically enable/disable
-kernel code to obtain additional kernel information.  Currently, if
-``CONFIG_DYNAMIC_DEBUG`` is set, then all ``pr_debug()``/``dev_dbg()`` and
-``print_hex_dump_debug()``/``print_hex_dump_bytes()`` calls can be dynamically
-enabled per-callsite.
+If ``/proc/dynamic_debug/control`` exists, your kernel has dynamic
+debug.  You'll need root access (sudo su) to use this.
 
-If you do not want to enable dynamic debug globally (i.e. in some embedded
-system), you may set ``CONFIG_DYNAMIC_DEBUG_CORE`` as basic support of dynamic
-debug and add ``ccflags := -DDYNAMIC_DEBUG_MODULE`` into the Makefile of any
-modules which you'd like to dynamically debug later.
+Dynamic debug provides:
 
-If ``CONFIG_DYNAMIC_DEBUG`` is not set, ``print_hex_dump_debug()`` is just
-shortcut for ``print_hex_dump(KERN_DEBUG)``.
+ * a Catalog of all *prdbgs* in your kernel.
+   ``cat /proc/dynamic_debug/control`` to see them.
 
-For ``print_hex_dump_debug()``/``print_hex_dump_bytes()``, format string is
-its ``prefix_str`` argument, if it is constant string; or ``hexdump``
-in case ``prefix_str`` is built dynamically.
-
-Dynamic debug has even more useful features:
-
- * Simple query language allows turning on and off debugging
-   statements by matching any combination of 0 or 1 of:
+ * a Simple query/command language to alter *prdbgs* by selecting on
+   any combination of 0 or 1 of:
 
- source filename
- function name
@@ -37,107 +26,88 @@ Dynamic debug has even more useful features:
- format string
- class name (as known/declared by each module)
 
- * Provides a debugfs control file: ``/dynamic_debug/control``
-   which can be read to display the complete list of known debug
-   statements, to help guide you
-
-Controlling dynamic debug Behaviour
-===
-
-The behaviour of ``pr_debug()``/``dev_dbg()`` are controlled via writing to a
-control file in the 'debugfs' filesystem. Thus, you must first mount
-the debugfs filesystem, in order to make use of this feature.
-Subsequently, we refer to the control file as:
-``/dynamic_debug/control``. For example, if you want to enable
-printing from source file ``svcsock.c``, line 1603 you simply do::
-
-  nullarbor:~ # echo 'file svcsock.c line 1603 +p' >
-   /dynamic_debug/control
-
-If you make a mistake with the syntax, the write will fail thus::
-
-  nullarbor:~ # echo 'file svcsock.c wtf 1 +p' >
-   /dynamic_debug/control
-  -bash: echo: write error: Invalid argument
-
-Note, for systems without 'debugfs' enabled, the control file can be
-found in ``/proc/dynamic_debug/control``.
-
 Viewing Dynamic Debug Behaviour
 ===
 
-You can view the currently configured behaviour of all the debug
-statements via::
+You can view the currently configured behaviour in the *prdbg* catalog::
 
-  nullarbor:~ # cat /dynamic_debug/control
+  :#> head -n7 /proc/dynamic_debug/control
   # filename:lineno [module]function flags format
-  net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup =_ "SVCRDMA Module 
Removed, deregister RPC RDMA transport\012"
-  net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init =_ "\011max_inline 
  : %d\012"
-  net/sunrpc/svc_rdma.c:340 

[Intel-gfx] [PATCH v5 22/33] drm_print: condense enum drm_debug_category

2022-08-05 Thread Jim Cromie
enum drm_debug_category has 10 categories, but is initialized with
bitmasks which require 10 bits of underlying storage.  By using
natural enumeration, and moving the BIT(cat) into drm_debug_enabled(),
the enum fits in 4 bits, allowing the category to be represented
directly in pr_debug callsites, via the ddebug.class_id field.

While this slightly pessimizes the bit-test in drm_debug_enabled(),
using dyndbg with JUMP_LABEL will avoid the function entirely.

NOTE: this change forecloses the possibility of doing:

  drm_dbg(DRM_UT_CORE|DRM_UT_KMS, "weird 2-cat experiment")

but thats already strongly implied by the use of the enum itself; its
not a normal enum if it can be 2 values simultaneously.

Signed-off-by: Jim Cromie 
---
 include/drm/drm_print.h | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 22fabdeed297..b3b470440e46 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -279,49 +279,49 @@ enum drm_debug_category {
 * @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
 * drm_memory.c, ...
 */
-   DRM_UT_CORE = 0x01,
+   DRM_UT_CORE,
/**
 * @DRM_UT_DRIVER: Used in the vendor specific part of the driver: i915,
 * radeon, ... macro.
 */
-   DRM_UT_DRIVER   = 0x02,
+   DRM_UT_DRIVER,
/**
 * @DRM_UT_KMS: Used in the modesetting code.
 */
-   DRM_UT_KMS  = 0x04,
+   DRM_UT_KMS,
/**
 * @DRM_UT_PRIME: Used in the prime code.
 */
-   DRM_UT_PRIME= 0x08,
+   DRM_UT_PRIME,
/**
 * @DRM_UT_ATOMIC: Used in the atomic code.
 */
-   DRM_UT_ATOMIC   = 0x10,
+   DRM_UT_ATOMIC,
/**
 * @DRM_UT_VBL: Used for verbose debug message in the vblank code.
 */
-   DRM_UT_VBL  = 0x20,
+   DRM_UT_VBL,
/**
 * @DRM_UT_STATE: Used for verbose atomic state debugging.
 */
-   DRM_UT_STATE= 0x40,
+   DRM_UT_STATE,
/**
 * @DRM_UT_LEASE: Used in the lease code.
 */
-   DRM_UT_LEASE= 0x80,
+   DRM_UT_LEASE,
/**
 * @DRM_UT_DP: Used in the DP code.
 */
-   DRM_UT_DP   = 0x100,
+   DRM_UT_DP,
/**
 * @DRM_UT_DRMRES: Used in the drm managed resources code.
 */
-   DRM_UT_DRMRES   = 0x200,
+   DRM_UT_DRMRES
 };
 
 static inline bool drm_debug_enabled(enum drm_debug_category category)
 {
-   return unlikely(__drm_debug & category);
+   return unlikely(__drm_debug & BIT(category));
 }
 
 /*
-- 
2.37.1



[Intel-gfx] [PATCH v5 32/33] nouveau: adapt NV_DEBUG, NV_ATOMIC to use DRM.debug

2022-08-05 Thread Jim Cromie
These 2 macros used drm_debug_enabled() on DRM_UT_{DRIVER,ATOMIC}
respectively, replace those with drm_dbg_##cat invocations.

this results in new class'd prdbg callsites:

:#> grep nouveau /proc/dynamic_debug/control | grep class | wc
1161130   15584
:#> grep nouveau /proc/dynamic_debug/control | grep class | grep DRIVER | wc
 74 7049709
:#> grep nouveau /proc/dynamic_debug/control | grep class | grep ATOMIC | wc
 31 3074237
:#> grep nouveau /proc/dynamic_debug/control | grep class | grep KMS | wc
 11 1191638

the KMS entries are due to existing uses of drm_dbg_kms().

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/nouveau/nouveau_drv.h | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h 
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index b2a970aa9bf4..f266cd6b0405 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -39,6 +39,7 @@
  */
 
 #include 
+#include 
 
 #include 
 #include 
@@ -264,13 +265,16 @@ void nouveau_drm_device_remove(struct drm_device *dev);
 #define NV_WARN(drm,f,a...) NV_PRINTK(warn, &(drm)->client, f, ##a)
 #define NV_INFO(drm,f,a...) NV_PRINTK(info, &(drm)->client, f, ##a)
 
-#define NV_DEBUG(drm,f,a...) do {  
\
-   if (drm_debug_enabled(DRM_UT_DRIVER))  \
-   NV_PRINTK(info, &(drm)->client, f, ##a);   \
+#define NV_DRMDBG(cat,c,f,a...) do {   \
+   struct nouveau_cli *_cli = (c); \
+   drm_dbg_##cat(_cli->drm->dev, "%s: "f, _cli->name, ##a); \
 } while(0)
-#define NV_ATOMIC(drm,f,a...) do { 
\
-   if (drm_debug_enabled(DRM_UT_ATOMIC))  \
-   NV_PRINTK(info, &(drm)->client, f, ##a);   \
+
+#define NV_DEBUG(drm,f,a...) do {  \
+   NV_DRMDBG(driver, &(drm)->client, f, ##a);  \
+} while(0)
+#define NV_ATOMIC(drm,f,a...) do { \
+   NV_DRMDBG(atomic, &(drm)->client, f, ##a);  \
 } while(0)
 
 #define NV_PRINTK_ONCE(l,c,f,a...) NV_PRINTK(l##_once,c,f, ##a)
-- 
2.37.1



[Intel-gfx] [PATCH v5 27/33] drm-print: add drm_dbg_driver to improve namespace symmetry

2022-08-05 Thread Jim Cromie
drm_print defines all of these:
drm_dbg_{core,kms,prime,atomic,vbl,lease,_dp,_drmres}

but not drm_dbg_driver itself, since it was the original drm_dbg.

To improve namespace symmetry, change the drm_dbg defn to
drm_dbg_driver, and redef grandfathered name to symmetric one.

This will help with nouveau, which uses its own stack of macros to
construct calls to dev_info, dev_dbg, etc, for which adaptation means
drm_dbg_##driver constructs.

Signed-off-by: Jim Cromie 
---
 include/drm/drm_print.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index f8bb3e7158c6..dfdd81c3287c 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -468,7 +468,7 @@ void __drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
 
 #define drm_dbg_core(drm, fmt, ...)\
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_CORE, fmt, ##__VA_ARGS__)
-#define drm_dbg(drm, fmt, ...) \
+#define drm_dbg_driver(drm, fmt, ...)  
\
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DRIVER, fmt, 
##__VA_ARGS__)
 #define drm_dbg_kms(drm, fmt, ...) \
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_KMS, fmt, ##__VA_ARGS__)
@@ -487,6 +487,7 @@ void __drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
 #define drm_dbg_drmres(drm, fmt, ...)  \
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DRMRES, fmt, 
##__VA_ARGS__)
 
+#define drm_dbg(drm, fmt, ...) drm_dbg_driver(drm, fmt, ##__VA_ARGS__)
 
 /*
  * printk based logging
-- 
2.37.1



[Intel-gfx] [PATCH v5 13/33] dyndbg: add __pr_debug_cls for testing

2022-08-05 Thread Jim Cromie
For selftest purposes, add __pr_debug_cls(class, fmt, ...)

I didn't think we'd need to define this, since DRM effectively has it
already in drm_dbg, drm_devdbg.  But test_dynamic_debug needs it in
order to demonstrate all the moving parts.

Note the __ prefix; its not intended for general use, at least until a
need emerges.  ISTM the drm.debug model (macro wrappers inserting enum
const 1st arg) is the baseline approach.

That said, nouveau might want it for easy use in its debug macros. TBD.

NB: it does require a builtin-constant class, __pr_debug_cls(i++, ...)
is disallowed by compiler.

Signed-off-by: Jim Cromie 
---
 include/linux/dynamic_debug.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index b6803f7e818f..cb4696c91901 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -221,6 +221,13 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
   KERN_DEBUG, prefix_str, prefix_type, \
   rowsize, groupsize, buf, len, ascii)
 
+/* for test only, generally expect drm.debug style macro wrappers */
+#define __pr_debug_cls(cls, fmt, ...) do { \
+   BUILD_BUG_ON_MSG(!__builtin_constant_p(cls),\
+"expecting constant class int/enum");  \
+   dynamic_pr_debug_cls(cls, fmt, ##__VA_ARGS__);  \
+   } while (0)
+
 #else /* !CONFIG_DYNAMIC_DEBUG_CORE */
 
 #include 
-- 
2.37.1



[Intel-gfx] [PATCH v5 14/33] dyndbg: add DECLARE_DYNDBG_CLASSMAP macro

2022-08-05 Thread Jim Cromie
With DECLARE_DYNDBG_CLASSMAP modules can declare up to 31 classnames.
By doing so, they authorize dyndbg to manipulate class'd prdbgs (ie:
__pr_debug_cls, and soon drm_*dbg)

   :#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control

The macro declares and initializes a static struct ddebug_class_map:

 - maps approved class-names to class_ids used in module,
   by array order. forex: DRM_UT_*
 - class-name vals allow validation of "class FOO" queries
   using macro is opt-in
 - enum class_map_type - determines interface, behavior

Each module has its own .class_id space, and only known class-names
will be authorized for a manipulation.  Only DRM stuff should know this:

  :#> echo class DRM_UT_CORE +p > control   # across all modules

pr_debugs (with default class_id) are still controllable as before.

DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, classes...) is::

 _var: name of the static struct var. user passes to module_param_cb()
   if they want a sysfs node.

 _maptype: this is hard-coded to DD_CLASS_TYPE_DISJOINT_BITS for now.

 _base: usually 0, it allows splitting 31 classes into subranges, so
that multiple classes / sysfs-nodes can share the module's
class-id space.

 classes: list of class_name strings, these are mapped to class-ids
  starting at _base.  This class-names list must have a
  corresponding ENUM, with SYMBOLS that match the literals,
  and 1st enum val = _base.

enum class_map_type has 4 values, on 2 factors::

 - classes are disjoint/independent vs relative/xcontrol interface
doesn't enforce the LEVELS relationship, so you could confusingly have
V3 enabled, but V1 disabled.  OTOH, the control iface already allows
infinite tweaking of the underlying callsites; sysfs node readback can
only tell the user what they previously wrote.

2. All dyndbg >control reduces to a query/command, includes +/-, which
is at-root a kernel patching operation with +/- semantics.  And the
_NAMES handling exposes it to the user, making it API-adjacent.

And its not just >control where +/- gets used (which is settled), the
new place is with sysfs-nodes exposing _*_NAMES classes, and here its
subtly different.

_DISJOINT_NAMES: is simple, independent
_LEVEL_NAMES: masks-on bits 0 .. N-1, N..max off

  # turn on L3,L2,L1 others off
  echo +L3 > /sys/module/test_dynamic_debug/parameters/p_level_names

  # turn on L2,L1 others off
  echo -L3 > /sys/module/test_dynamic_debug/parameters/p_level_names

IOW, the - changes the threshold-on bitpos by 1.

Alternatively, we could treat the +/- as half-duplex, where -L3 turns
off L>2 (and ignores L1), and +L2 would turn on L<=2 (and ignore others).

Signed-off-by: Jim Cromie 
---
. revised DD_CLASS_TYPE_{DISJOINT,LEVEL}_* names
. reorder-enum by necessity - _NAMES feature is extra.
---
 include/linux/dynamic_debug.h | 56 +++
 1 file changed, 56 insertions(+)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index cb4696c91901..71cdc8612257 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -56,6 +56,62 @@ struct _ddebug {
 #endif
 } __attribute__((aligned(8)));
 
+enum class_map_type {
+   DD_CLASS_TYPE_DISJOINT_BITS,
+   /**
+* DD_CLASS_TYPE_DISJOINT_BITS: classes are independent, one per bit.
+* expecting hex input. Built for drm.debug, basis for other types.
+*/
+   DD_CLASS_TYPE_LEVEL_NUM,
+   /**
+* DD_CLASS_TYPE_LEVEL_NUM: input is numeric level, 0-N.
+* N turns on just bits N-1 .. 0, so N=0 turns all bits off.
+*/
+   DD_CLASS_TYPE_DISJOINT_NAMES,
+   /**
+* DD_CLASS_TYPE_DISJOINT_NAMES: input is a CSV of [+-]CLASS_NAMES,
+* classes are independent, like _DISJOINT_BITS.  Model user
+* would be /sys/module/drm/parameters/debug_categories.
+*/
+   DD_CLASS_TYPE_LEVEL_NAMES,
+   /**
+* DD_CLASS_TYPE_LEVEL_NAMES: input is a CSV of [+-]CLASS_NAMES,
+* intended for names like: INFO,DEBUG,TRACE, with a module prefix
+* avoid EMERG,ALERT,CRIT,ERR,WARNING: they're not debug
+*/
+};
+
+struct ddebug_class_map {
+   struct list_head link;
+   struct module *mod;
+   const char *mod_name;   /* needed for builtins */
+   const char **class_names;
+   const int length;
+   const int base; /* index of 1st .class_id, allows split/shared 
space */
+   enum class_map_type map_type;
+};
+
+/**
+ * DECLARE_DYNDBG_CLASSMAP - declare classnames known by a module
+ * @_var:   a struct ddebug_class_map, passed to module_param_cb
+ * @_type:  enum class_map_type, chooses bits/verbose, numeric/symbolic
+ * @_base:  offset of 1st class-name. splits .class_id space
+ * @classes: class-names used to control class'd prdbgs
+ */
+#define DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, ...)\
+   static const char *_var##_classnames[] = { __VA_ARGS__ };   \

[Intel-gfx] [PATCH v5 29/33] drm_print: prefer bare printk KERN_DEBUG on generic fn

2022-08-05 Thread Jim Cromie
drm_print.c calls pr_debug() just once, from __drm_printfn_debug(),
which is a generic/service fn.  The callsite is compile-time enabled
by DEBUG in both DYNAMIC_DEBUG=y/n builds.

For dyndbg builds, reverting this callsite back to bare printk is
correcting a few anti-features:

1- callsite is generic, serves multiple drm users.
   it is soft-wired on currently by #define DEBUG
   could accidentally: #> echo -p > /proc/dynamic_debug/control

2- optional "decorations" by dyndbg are unhelpful/misleading here,
   they describe only the generic site, not end users

IOW, 1,2 are unhelpful at best, and possibly confusing.

reverting yields a nominal data and text shrink:

   textdata bss dec hex filename
 462583   36604   54592 553779   87333 /kernel/drivers/gpu/drm/drm.ko
 462515   36532   54592 553639   872a7 -dirty/kernel/drivers/gpu/drm/drm.ko

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/drm_print.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index cb203d63b286..ec477c44a784 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -23,8 +23,6 @@
  * Rob Clark 
  */
 
-#define DEBUG /* for pr_debug() */
-
 #include 
 
 #include 
@@ -185,7 +183,8 @@ EXPORT_SYMBOL(__drm_printfn_info);
 
 void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf)
 {
-   pr_debug("%s %pV", p->prefix, vaf);
+   /* pr_debug callsite decorations are unhelpful here */
+   printk(KERN_DEBUG "%s %pV", p->prefix, vaf);
 }
 EXPORT_SYMBOL(__drm_printfn_debug);
 
-- 
2.37.1



[Intel-gfx] [PATCH v5 17/33] dyndbg: validate class FOO by checking with module

2022-08-05 Thread Jim Cromie
Add module-to-class validation:

  #> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control

If a query has "class FOO", then ddebug_find_valid_class(), called
from ddebug_change(), requires that FOO is known to module X,
otherwize the query is skipped entirely for X.  This protects each
module's class-space, other than the default:31.

The authors' choice of FOO is highly selective, giving isolation
and/or coordinated sharing of FOOs.  For example, only DRM modules
should know and respond to DRM_UT_KMS.

So this, combined with module's opt-in declaration of known classes,
effectively privatizes the .class_id space for each module (or
coordinated set of modules).

Notes:

For all "class FOO" queries, ddebug_find_valid_class() is called, it
returns the map matching the query, and sets valid_class via an
*outvar).

If no "class FOO" is supplied, valid_class = _CLASS_DFLT.  This
insures that legacy queries do not trample on new class'd callsites,
as they get added.

Also add a new column to control-file output, displaying non-default
class-name (when found) or the "unknown _id:", if it has not been
(correctly) declared with one of the declarator macros.

Signed-off-by: Jim Cromie 
---
 lib/dynamic_debug.c | 76 -
 1 file changed, 68 insertions(+), 8 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a3ace5866f1b..eb410b412f0d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -56,6 +56,7 @@ struct ddebug_query {
const char *module;
const char *function;
const char *format;
+   const char *class_string;
unsigned int first_lineno, last_lineno;
 };
 
@@ -136,15 +137,33 @@ static void vpr_info_dq(const struct ddebug_query *query, 
const char *msg)
fmtlen--;
}
 
-   v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\" 
lineno=%u-%u\n",
-msg,
-query->function ?: "",
-query->filename ?: "",
-query->module ?: "",
-fmtlen, query->format ?: "",
-query->first_lineno, query->last_lineno);
+   v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\" 
lineno=%u-%u class=%s\n",
+ msg,
+ query->function ?: "",
+ query->filename ?: "",
+ query->module ?: "",
+ fmtlen, query->format ?: "",
+ query->first_lineno, query->last_lineno, query->class_string);
 }
 
+static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table 
const *dt,
+ const char 
*class_string, int *class_id)
+{
+   struct ddebug_class_map *map;
+   int idx;
+
+   list_for_each_entry(map, >maps, link) {
+   idx = match_string(map->class_names, map->length, class_string);
+   if (idx >= 0) {
+   *class_id = idx + map->base;
+   return map;
+   }
+   }
+   *class_id = -ENOENT;
+   return NULL;
+}
+
+#define __outvar /* filled by callee */
 /*
  * Search the tables for _ddebug's which match the given `query' and
  * apply the `flags' and `mask' to them.  Returns number of matching
@@ -159,6 +178,8 @@ static int ddebug_change(const struct ddebug_query *query,
unsigned int newflags;
unsigned int nfound = 0;
struct flagsbuf fbuf, nbuf;
+   struct ddebug_class_map *map = NULL;
+   int __outvar valid_class;
 
/* search for matching ddebugs */
mutex_lock(_lock);
@@ -169,9 +190,22 @@ static int ddebug_change(const struct ddebug_query *query,
!match_wildcard(query->module, dt->mod_name))
continue;
 
+   if (query->class_string) {
+   map = ddebug_find_valid_class(dt, query->class_string, 
_class);
+   if (!map)
+   continue;
+   } else {
+   /* constrain query, do not touch class'd callsites */
+   valid_class = _DPRINTK_CLASS_DFLT;
+   }
+
for (i = 0; i < dt->num_ddebugs; i++) {
struct _ddebug *dp = >ddebugs[i];
 
+   /* match site against query-class */
+   if (dp->class_id != valid_class)
+   continue;
+
/* match against the source filename */
if (query->filename &&
!match_wildcard(query->filename, dp->filename) &&
@@ -420,6 +454,8 @@ static int ddebug_parse_query(char *words[], int nwords,
} else if (!strcmp(keyword, "line")) {
if (parse_linerange(query, arg))
return -EINVAL;
+   } else if (!strcmp(keyword, "class")) {
+   

[Intel-gfx] [PATCH v5 31/33] nouveau: change nvkm_debug/trace to use dev_dbg POC

2022-08-05 Thread Jim Cromie
These 2 macros formerly used dev_info, and they still check
subdev->debug to gate the printing.  So dyndbg control is redundant
ATM (and possibly confusing, since its off by default).

prdbg count is up from 3, or from 65 (with VMM_DEBUG here)

[7.765379] dyndbg: 516 debug prints in module nouveau

Its possible to control error, warn, info callsites too, but they're
usually on, and the .data overheads on ~450 more callsites (56 bytes
each) would just be wasted.

$ for l in fatal error warn info debug trace spam; do
  echo $l; ack nvkm_$l drivers/gpu |wc; done
fatal
  3  19 335
error
2891956   30651
warn
 84 5138860
info
 14  881502
debug
3872339   40844
trace
 31 2193368
spam
  1   7 123

bash-5.1# echo $(( 516-65-387-31-1 ))
32

Thats approximate; not accounting #defines and doc/comment mentions.

NOTE: this patch changes the log-level of the macro-issued messages
from KERN_INFO to KERN_DEBUG.  Adding a .kern_lvl field to struct
_ddebug could fix that.

RFC: dyndbg & subdev->debug

Separate class-maps for each subdev are possible; except for the
coordinated use of _base, each is independent, including choice of
DISJOINT or LEVELS, as long as class-names don't conflict.
So theres some flexibility.

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h 
b/drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h
index 96113c8bee8c..065d07ccea87 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h
@@ -59,8 +59,8 @@ void nvkm_subdev_intr(struct nvkm_subdev *);
 #define nvkm_error(s,f,a...) nvkm_printk((s), ERROR,err, f, ##a)
 #define nvkm_warn(s,f,a...)  nvkm_printk((s),  WARN, notice, f, ##a)
 #define nvkm_info(s,f,a...)  nvkm_printk((s),  INFO,   info, f, ##a)
-#define nvkm_debug(s,f,a...) nvkm_printk((s), DEBUG,   info, f, ##a)
-#define nvkm_trace(s,f,a...) nvkm_printk((s), TRACE,   info, f, ##a)
+#define nvkm_debug(s,f,a...) nvkm_printk((s), DEBUG,dbg, f, ##a)
+#define nvkm_trace(s,f,a...) nvkm_printk((s), TRACE,dbg, f, ##a)
 #define nvkm_spam(s,f,a...)  nvkm_printk((s),  SPAM,dbg, f, ##a)
 
 #define nvkm_error_ratelimited(s,f,a...) nvkm_printk((s), ERROR, 
err_ratelimited, f, ##a)
-- 
2.37.1



[Intel-gfx] [PATCH v5 10/33] dyndbg: cleanup local vars in ddebug_init

2022-08-05 Thread Jim Cromie
rework var-names for clarity, regularity
rename variables
  - n to mod_sites - it counts sites-per-module
  - entries to i - display only
  - iter_start to iter_mod_start - marks start of each module's subrange
  - modct to mod_ct - stylistic

new iterator var:
  - site - cursor parallel to iter
this allows 'demotion' of iter->site (for removal later)

treat vars as iters:
  - drop init at top
init just above for-loop, in a textual block

Signed-off-by: Jim Cromie 
---
 lib/dynamic_debug.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e96dc216463b..2e8ebef3bd0d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1059,11 +1059,10 @@ static int __init dynamic_debug_init_control(void)
 
 static int __init dynamic_debug_init(void)
 {
-   struct _ddebug *iter, *iter_start;
-   const char *modname = NULL;
+   struct _ddebug *iter, *iter_mod_start;
+   int ret, i, mod_sites, mod_ct;
+   const char *modname;
char *cmdline;
-   int ret = 0;
-   int n = 0, entries = 0, modct = 0;
 
if (&__start___dyndbg == &__stop___dyndbg) {
if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
@@ -1074,30 +1073,32 @@ static int __init dynamic_debug_init(void)
ddebug_init_success = 1;
return 0;
}
-   iter = __start___dyndbg;
+
+   iter = iter_mod_start = __start___dyndbg;
modname = iter->modname;
-   iter_start = iter;
-   for (; iter < __stop___dyndbg; iter++) {
-   entries++;
+   i = mod_sites = mod_ct = 0;
+
+   for (; iter < __stop___dyndbg; iter++, i++, mod_sites++) {
+
if (strcmp(modname, iter->modname)) {
-   modct++;
-   ret = ddebug_add_module(iter_start, n, modname);
+   mod_ct++;
+   ret = ddebug_add_module(iter_mod_start, mod_sites, 
modname);
if (ret)
goto out_err;
-   n = 0;
+
+   mod_sites = 0;
modname = iter->modname;
-   iter_start = iter;
+   iter_mod_start = iter;
}
-   n++;
}
-   ret = ddebug_add_module(iter_start, n, modname);
+   ret = ddebug_add_module(iter_mod_start, mod_sites, modname);
if (ret)
goto out_err;
 
ddebug_init_success = 1;
vpr_info("%d prdebugs in %d modules, %d KiB in ddebug tables, %d kiB in 
__dyndbg section\n",
-entries, modct, (int)((modct * sizeof(struct ddebug_table)) >> 
10),
-(int)((entries * sizeof(struct _ddebug)) >> 10));
+i, mod_ct, (int)((mod_ct * sizeof(struct ddebug_table)) >> 10),
+(int)((i * sizeof(struct _ddebug)) >> 10));
 
/* now that ddebug tables are loaded, process all boot args
 * again to find and activate queries given in dyndbg params.
-- 
2.37.1



[Intel-gfx] [PATCH v5 16/33] dyndbg: add ddebug_attach_module_classes

2022-08-05 Thread Jim Cromie
Add ddebug_attach_module_classes(), call it from ddebug_add_module().
It scans the classes/section its given, finds records where the
module-name matches the module being added, and adds them to the
module's maps list.  No locking here, since the record
isn't yet linked into the ddebug_tables list.

It is called indirectly from 2 sources:

 - from load_module(), where it scans the module's __dyndbg_classes
   section, which contains DYNAMIC_DEBUG_CLASSES definitions from just
   the module.

 - from dynamic_debug_init(), where all DYNAMIC_DEBUG_CLASSES
   definitions of each builtin module have been packed together.
   This is why ddebug_attach_module_classes() checks module-name.

NOTES

Its (highly) likely that builtin classes will be ordered by module
name (just like prdbg descriptors are in the __dyndbg section).  So
the list can be replaced by a vector (ptr + length), which will work
for loaded modules too.  This would imitate whats currently done for
the _ddebug descriptors.

That said, converting to vector,len is close to pointless; a small
minority of modules will ever define a class-map, and almost all of
them will have only 1 or 2 class-maps, so theres only a couple dozen
pointers to save.

Signed-off-by: Jim Cromie 
---
 lib/dynamic_debug.c | 34 +-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0d6cb6b258bd..a3ace5866f1b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -45,7 +45,7 @@ extern struct ddebug_class_map __start___dyndbg_classes[];
 extern struct ddebug_class_map __stop___dyndbg_classes[];
 
 struct ddebug_table {
-   struct list_head link;
+   struct list_head link, maps;
const char *mod_name;
unsigned int num_ddebugs;
struct _ddebug *ddebugs;
@@ -921,6 +921,32 @@ static const struct proc_ops proc_fops = {
.proc_write = ddebug_proc_write
 };
 
+static void ddebug_attach_module_classes(struct ddebug_table *dt,
+struct ddebug_class_map *classes,
+int num_classes)
+{
+   struct ddebug_class_map *cm;
+   int i, j, ct = 0;
+
+   for (cm = classes, i = 0; i < num_classes; i++, cm++) {
+
+   if (!strcmp(cm->mod_name, dt->mod_name)) {
+
+   v2pr_info("class[%d]: module:%s base:%d len:%d 
ty:%d\n", i,
+ cm->mod_name, cm->base, cm->length, 
cm->map_type);
+
+   for (j = 0; j < cm->length; j++)
+   v3pr_info(" %d: %d %s\n", j + cm->base, j,
+ cm->class_names[j]);
+
+   list_add(>link, >maps);
+   ct++;
+   }
+   }
+   if (ct)
+   vpr_info("module:%s attached %d classes\n", dt->mod_name, ct);
+}
+
 /*
  * Allocate a new ddebug_table for the given module
  * and add it to the global list.
@@ -953,6 +979,12 @@ static int __ddebug_add_module(struct _ddebug_info *di, 
unsigned int base,
 
INIT_LIST_HEAD(>link);
 
+   INIT_LIST_HEAD(>link);
+   INIT_LIST_HEAD(>maps);
+
+   if (classes && num_classes)
+   ddebug_attach_module_classes(dt, classes, num_classes);
+
mutex_lock(_lock);
list_add_tail(>link, _tables);
mutex_unlock(_lock);
-- 
2.37.1



[Intel-gfx] [PATCH v5 24/33] drm_print: interpose drm_*dbg with forwarding macros

2022-08-05 Thread Jim Cromie
change drm_dev_dbg & drm_dbg to macros, which forward to the renamed
functions (with __ prefix added).

Those functions sit below the categorized layer of macros implementing
the DRM debug.category API, and implement most of it.  These are good
places to insert dynamic-debug jump-label mechanics, which will allow
DRM to avoid the runtime cost of drm_debug_enabled().

no functional changes.

memory cost baseline: (unchanged)
bash-5.1# drms_load
[9.220389] dyndbg:   1 debug prints in module drm
[9.224426] ACPI: bus type drm_connector registered
[9.302192] dyndbg:   2 debug prints in module ttm
[9.305033] dyndbg:   8 debug prints in module video
[9.627563] dyndbg: 127 debug prints in module i915
[9.721505] AMD-Vi: AMD IOMMUv2 functionality not available on this system - 
This is not a bug.
[   10.091345] dyndbg: 2196 debug prints in module amdgpu
[   10.106589] [drm] amdgpu kernel modesetting enabled.
[   10.107270] amdgpu: CRAT table not found
[   10.107926] amdgpu: Virtual CRAT table created for CPU
[   10.108398] amdgpu: Topology: Add CPU node
[   10.168507] dyndbg:   3 debug prints in module wmi
[   10.329587] dyndbg:   3 debug prints in module nouveau

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/drm_print.c | 10 +-
 include/drm/drm_print.h |  9 +++--
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index ec32df35a3e3..29a29949ad0b 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -279,8 +279,8 @@ void drm_dev_printk(const struct device *dev, const char 
*level,
 }
 EXPORT_SYMBOL(drm_dev_printk);
 
-void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
-const char *format, ...)
+void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
+  const char *format, ...)
 {
struct va_format vaf;
va_list args;
@@ -301,9 +301,9 @@ void drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
 
va_end(args);
 }
-EXPORT_SYMBOL(drm_dev_dbg);
+EXPORT_SYMBOL(__drm_dev_dbg);
 
-void __drm_dbg(enum drm_debug_category category, const char *format, ...)
+void ___drm_dbg(enum drm_debug_category category, const char *format, ...)
 {
struct va_format vaf;
va_list args;
@@ -320,7 +320,7 @@ void __drm_dbg(enum drm_debug_category category, const char 
*format, ...)
 
va_end(args);
 }
-EXPORT_SYMBOL(__drm_dbg);
+EXPORT_SYMBOL(___drm_dbg);
 
 void __drm_err(const char *format, ...)
 {
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 668273e36c2c..c429c258c957 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -335,7 +335,7 @@ __printf(3, 4)
 void drm_dev_printk(const struct device *dev, const char *level,
const char *format, ...);
 __printf(3, 4)
-void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
+void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
 const char *format, ...);
 
 /**
@@ -384,6 +384,9 @@ void drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
}   \
 })
 
+#define drm_dev_dbg(dev, cat, fmt, ...)\
+   __drm_dev_dbg(dev, cat, fmt, ##__VA_ARGS__)
+
 /**
  * DRM_DEV_DEBUG() - Debug output for generic drm code
  *
@@ -485,10 +488,12 @@ void drm_dev_dbg(const struct device *dev, enum 
drm_debug_category category,
  */
 
 __printf(2, 3)
-void __drm_dbg(enum drm_debug_category category, const char *format, ...);
+void ___drm_dbg(enum drm_debug_category category, const char *format, ...);
 __printf(1, 2)
 void __drm_err(const char *format, ...);
 
+#define __drm_dbg(fmt, ...)___drm_dbg(fmt, ##__VA_ARGS__)
+
 /* Macros to make printk easier */
 
 #define _DRM_PRINTK(once, level, fmt, ...) \
-- 
2.37.1



[Intel-gfx] [PATCH v5 15/33] kernel/module: add __dyndbg_classes section

2022-08-05 Thread Jim Cromie
Add __dyndbg_classes section, using __dyndbg as a model. Use it:

vmlinux.lds.h:

KEEP the new section, which also silences orphan section warning on
loadable modules.  Add (__start_/__stop_)__dyndbg_classes linker
symbols for the c externs (below).

kernel/module/main.c:
- fill new fields in find_module_sections(), using section_objs()
- extend callchain prototypes
  to pass classes, length
  load_module(): pass new info to dynamic_debug_setup()
  dynamic_debug_setup(): new params, pass through to ddebug_add_module()

dynamic_debug.c:
- add externs to the linker symbols.

ddebug_add_module():
- It currently builds a debug_table, and *will* find and attach classes.

dynamic_debug_init():
- add class fields to the _ddebug_info cursor var: di.

Signed-off-by: Jim Cromie 
---
 include/asm-generic/vmlinux.lds.h | 3 +++
 include/linux/dynamic_debug.h | 2 ++
 kernel/module/main.c  | 2 ++
 lib/dynamic_debug.c   | 7 +++
 4 files changed, 14 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index 7515a465ec03..9b8bd5504ad9 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -345,6 +345,9 @@
*(__tracepoints)\
/* implement dynamic printk debug */\
. = ALIGN(8);   \
+   __start___dyndbg_classes = .;   \
+   KEEP(*(__dyndbg_classes))   \
+   __stop___dyndbg_classes = .;\
__start___dyndbg = .;   \
KEEP(*(__dyndbg))   \
__stop___dyndbg = .;\
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 71cdc8612257..46ed10682e87 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -115,7 +115,9 @@ struct ddebug_class_map {
 /* encapsulate linker provided built-in (or module) dyndbg data */
 struct _ddebug_info {
struct _ddebug *descs;
+   struct ddebug_class_map *classes;
unsigned int num_descs;
+   unsigned int num_classes;
 };
 
 #if defined(CONFIG_DYNAMIC_DEBUG_CORE)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index cfe10356793d..f68f331c867d 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2095,6 +2095,8 @@ static int find_module_sections(struct module *mod, 
struct load_info *info)
 
info->dyndbg.descs = section_objs(info, "__dyndbg",
sizeof(*info->dyndbg.descs), 
>dyndbg.num_descs);
+   info->dyndbg.classes = section_objs(info, "__dyndbg_classes",
+   sizeof(*info->dyndbg.classes), 
>dyndbg.num_classes);
 
return 0;
 }
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 457ce936191a..0d6cb6b258bd 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -41,6 +41,8 @@
 
 extern struct _ddebug __start___dyndbg[];
 extern struct _ddebug __stop___dyndbg[];
+extern struct ddebug_class_map __start___dyndbg_classes[];
+extern struct ddebug_class_map __stop___dyndbg_classes[];
 
 struct ddebug_table {
struct list_head link;
@@ -1079,7 +1081,9 @@ static int __init dynamic_debug_init(void)
 
struct _ddebug_info di = {
.descs = __start___dyndbg,
+   .classes = __start___dyndbg_classes,
.num_descs = __stop___dyndbg - __start___dyndbg,
+   .num_classes = __stop___dyndbg_classes - 
__start___dyndbg_classes,
};
 
if (&__start___dyndbg == &__stop___dyndbg) {
@@ -1122,6 +1126,9 @@ static int __init dynamic_debug_init(void)
 i, mod_ct, (int)((mod_ct * sizeof(struct ddebug_table)) >> 10),
 (int)((i * sizeof(struct _ddebug)) >> 10));
 
+   if (di.num_classes)
+   v2pr_info("  %d builtin ddebug class-maps\n", di.num_classes);
+
/* now that ddebug tables are loaded, process all boot args
 * again to find and activate queries given in dyndbg params.
 * While this has already been done for known boot params, it
-- 
2.37.1



[Intel-gfx] [PATCH v5 11/33] dyndbg: create and use struct _ddebug_info

2022-08-05 Thread Jim Cromie
this new struct gathers the linker provided vectors/sections:

  descs - the vector of descriptors in __dyndbg section.
  num_descs - length of the data/section

Use it as follows:

In lib/dynamic_debug.c:

Alter ddebug_add_module() params-list, replacing descriptor-table and
its length with a single _ddebug_info object containing them.  This
future-proofs the function against the looming addition of class-map
info, for either the builtin module set, or the loaded module.

In dynamic_debug_init(): add & initialize an auto struct _ddebug_info
var, and use it like a cursor / multi-part iterator.  Re-initialize
the var's component values before each call to ddebug_add_module().
This gives us the inch-worm walk thru the modules in the built-in
__dyndbgs data section.

In kernel/module/{main.c,internal.h}:

Embed a struct _ddebug_info into struct load_info, replacing the 2 fields it
contains.  Populate its members in find_module_sections.

Also adjust dynamic_debug_setup/remove() to match the change to
ddebug_add_module().

Note: this adds an include dynamic_debug, and might be prone to
include loops, since its also smuggled in by printk.h.  Nothing has
puked in robot-land.

cc: Luis Chamberlain 
Signed-off-by: Jim Cromie 
---
 include/linux/dynamic_debug.h | 13 +++-
 kernel/module/internal.h  |  4 ++--
 kernel/module/main.c  | 18 
 lib/dynamic_debug.c   | 40 +++
 4 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 8d9eec5f6d8b..6a2001250da1 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -51,12 +51,16 @@ struct _ddebug {
 #endif
 } __attribute__((aligned(8)));
 
-
+/* encapsulate linker provided built-in (or module) dyndbg data */
+struct _ddebug_info {
+   struct _ddebug *descs;
+   unsigned int num_descs;
+};
 
 #if defined(CONFIG_DYNAMIC_DEBUG_CORE)
 
-int ddebug_add_module(struct _ddebug *tab, unsigned int n,
-   const char *modname);
+int ddebug_add_module(struct _ddebug_info *dyndbg, const char *modname);
+
 extern int ddebug_remove_module(const char *mod_name);
 extern __printf(2, 3)
 void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...);
@@ -184,8 +188,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 #include 
 #include 
 
-static inline int ddebug_add_module(struct _ddebug *tab, unsigned int n,
-   const char *modname)
+static inline int ddebug_add_module(struct _ddebug_info *dinfo, const char 
*modname)
 {
return 0;
 }
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index ec104c2950c3..ce42b5b8b4da 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -53,6 +53,7 @@ extern const struct kernel_symbol __stop___ksymtab_gpl[];
 extern const s32 __start___kcrctab[];
 extern const s32 __start___kcrctab_gpl[];
 
+#include 
 struct load_info {
const char *name;
/* pointer to module in temporary copy, freed at end of load_module() */
@@ -62,8 +63,7 @@ struct load_info {
Elf_Shdr *sechdrs;
char *secstrings, *strtab;
unsigned long symoffs, stroffs, init_typeoffs, core_typeoffs;
-   struct _ddebug *debug;
-   unsigned int num_debug;
+   struct _ddebug_info dyndbg;
bool sig_ok;
 #ifdef CONFIG_KALLSYMS
unsigned long mod_kallsyms_init_off;
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 0548151dd933..cfe10356793d 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1593,16 +1593,16 @@ static void free_modinfo(struct module *mod)
}
 }
 
-static void dynamic_debug_setup(struct module *mod, struct _ddebug *debug, 
unsigned int num)
+static void dynamic_debug_setup(struct module *mod, struct _ddebug_info 
*dyndbg)
 {
-   if (!debug)
+   if (!dyndbg->num_descs)
return;
-   ddebug_add_module(debug, num, mod->name);
+   ddebug_add_module(dyndbg, mod->name);
 }
 
-static void dynamic_debug_remove(struct module *mod, struct _ddebug *debug)
+static void dynamic_debug_remove(struct module *mod, struct _ddebug_info 
*dyndbg)
 {
-   if (debug)
+   if (dyndbg->num_descs)
ddebug_remove_module(mod->name);
 }
 
@@ -2093,8 +2093,8 @@ static int find_module_sections(struct module *mod, 
struct load_info *info)
if (section_addr(info, "__obsparm"))
pr_warn("%s: Ignoring obsolete parameters\n", mod->name);
 
-   info->debug = section_objs(info, "__dyndbg",
-  sizeof(*info->debug), >num_debug);
+   info->dyndbg.descs = section_objs(info, "__dyndbg",
+   sizeof(*info->dyndbg.descs), 
>dyndbg.num_descs);
 
return 0;
 }
@@ -2783,7 +2783,7 @@ static int load_module(struct load_info *info, const char 
__user *uargs,
}
 
init_build_id(mod, 

[Intel-gfx] [PATCH v5 12/33] dyndbg: add class_id to pr_debug callsites

2022-08-05 Thread Jim Cromie
DRM issues ~10 exclusive categories of debug messages; to represent
this directly in dyndbg, add a new field: struct _ddebug.class_id:5.

This gives us 32 classes, which is a practical usability limit
with a bitmap interface:

  #> echo 0x012345678 > /sys/module/drm/parameters/debug

All existing callsites are initialized with _DPRINTK_CLASS_DFLT, which
is 2^5-1.  This reserves 0-30 for use in new categorized/class'd
pr_debugs, which fits perfectly with natural enums (ints: 0..N).

Then extend the init macro: DEFINE_DYNAMIC_DEBUG_METADATA() with
_CLS(cls, ...), and redef old name using extended name.

And extend the factory macro callchain with _cls() versions to provide
the callsite.class_id, with old-names passing _DPRINTK_CLASS_DFLT.

This sets us up to create class'd prdebug callsites (class'd callsites
are those with .class_id != _DPRINTK_CLASS_DFLT).

No behavior change.

cc: Rasmus Villemoes 
Signed-off-by: Jim Cromie 
---
 include/linux/dynamic_debug.h | 71 +++
 1 file changed, 55 insertions(+), 16 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6a2001250da1..b6803f7e818f 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -6,6 +6,8 @@
 #include 
 #endif
 
+#include 
+
 /*
  * An instance of this structure is created in a special
  * ELF section at every dynamic debug callsite.  At runtime,
@@ -21,6 +23,9 @@ struct _ddebug {
const char *filename;
const char *format;
unsigned int lineno:18;
+#define CLS_BITS 5
+   unsigned int class_id:CLS_BITS;
+#define _DPRINTK_CLASS_DFLT((1 << CLS_BITS) - 1)
/*
 * The flags field controls the behaviour at the callsite.
 * The bits here are changed dynamically when the user
@@ -88,7 +93,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 const struct ib_device *ibdev,
 const char *fmt, ...);
 
-#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)   \
+#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt)  \
static struct _ddebug  __aligned(8) \
__section("__dyndbg") name = {  \
.modname = KBUILD_MODNAME,  \
@@ -97,8 +102,14 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
.format = (fmt),\
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT,\
+   .class_id = cls,\
_DPRINTK_KEY_INIT   \
-   }
+   };  \
+   BUILD_BUG_ON_MSG(cls > _DPRINTK_CLASS_DFLT, \
+"classid value overflow")
+
+#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)   \
+   DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_CLASS_DFLT, fmt)
 
 #ifdef CONFIG_JUMP_LABEL
 
@@ -129,17 +140,34 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 
 #endif /* CONFIG_JUMP_LABEL */
 
-#define __dynamic_func_call(id, fmt, func, ...) do {   \
-   DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \
-   if (DYNAMIC_DEBUG_BRANCH(id))   \
-   func(, ##__VA_ARGS__);   \
-} while (0)
-
-#define __dynamic_func_call_no_desc(id, fmt, func, ...) do {   \
-   DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \
+/*
+ * Factory macros: ($prefix)dynamic_func_call($suffix)
+ *
+ * Lower layer (with __ prefix) gets the callsite metadata, and wraps
+ * the func inside a debug-branch/static-key construct.  Upper layer
+ * (with _ prefix) does the UNIQUE_ID once, so that lower can ref the
+ * name/label multiple times, and tie the elements together.
+ * Multiple flavors:
+ * (|_cls):adds in _DPRINT_CLASS_DFLT as needed
+ * (|_no_desc):former gets callsite descriptor as 1st arg (for prdbgs)
+ */
+#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do {  \
+   DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt);\
if (DYNAMIC_DEBUG_BRANCH(id))   \
-   func(__VA_ARGS__);  \
+   func(, ##__VA_ARGS__);   \
 } while (0)
+#define __dynamic_func_call(id, fmt, func, ...)
\
+   __dynamic_func_call_cls(id, _DPRINTK_CLASS_DFLT, fmt,   \
+   func, ##__VA_ARGS__)
+
+#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) do {  \
+   DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt);\
+   if (DYNAMIC_DEBUG_BRANCH(id))   \
+   func(__VA_ARGS__);  \
+} while (0)
+#define __dynamic_func_call_no_desc(id, fmt, func, ...) 

[Intel-gfx] [PATCH v5 06/33] dyndbg: use ESCAPE_SPACE for cat control

2022-08-05 Thread Jim Cromie
`cat control` currently does octal escape, so '\n' becomes "\012".
Change this to display as "\n" instead, which reads much cleaner.

   :#> head -n7 /proc/dynamic_debug/control
   # filename:lineno [module]function flags format
   init/main.c:1179 [main]initcall_blacklist =_ "blacklisting initcall %s\n"
   init/main.c:1218 [main]initcall_blacklisted =_ "initcall %s blacklisted\n"
   init/main.c:1424 [main]run_init_process =_ "  with arguments:\n"
   init/main.c:1426 [main]run_init_process =_ "%s\n"
   init/main.c:1427 [main]run_init_process =_ "  with environment:\n"
   init/main.c:1429 [main]run_init_process =_ "%s\n"

Signed-off-by: Jim Cromie 
Acked-by: Jason Baron 
---
 lib/dynamic_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8ff11977b8bd..e5cbe603000c 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -900,7 +900,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
   trim_prefix(dp->filename), dp->lineno,
   iter->table->mod_name, dp->function,
   ddebug_describe_flags(dp->flags, ));
-   seq_escape(m, dp->format, "\t\r\n\"");
+   seq_escape_str(m, dp->format, ESCAPE_SPACE, "\t\r\n\"");
seq_puts(m, "\"\n");
 
return 0;
-- 
2.37.1



[Intel-gfx] [PATCH v5 21/33] dyndbg: test DECLARE_DYNDBG_CLASSMAP, sysfs nodes

2022-08-05 Thread Jim Cromie
Demonstrate use of DECLARE_DYNDBG_CLASSMAP macro, and expose them as
sysfs-nodes for testing.

For each of the 4 class-map-types:

  - declare a class-map of that type,
  - declare the enum corresponding to those class-names
  - share _base across 0..30 range
  - add a __pr_debug_cls() call for each class-name
  - declare 2 sysnodes for each class-map
for 'p' flag, and future 'T' flag

These declarations create the following sysfs parameter interface:

  :#> pwd
  /sys/module/test_dynamic_debug/parameters
  :#> ls
  T_disjoint_bits  T_disjoint_names  T_level_names  T_level_num  do_prints
  p_disjoint_bits  p_disjoint_names  p_level_names  p_level_num

NOTES:

The local wrapper macro is an api candidate, but there are already too
many parameters.  OTOH, maybe related enum should be in there too,
since it has _base inter-dependencies.

The T_* params control the (future) T flag on the same class'd
pr_debug callsites as their p* counterparts.  Using them will fail,
until the dyndbg-trace patches are added in.

:#> echo 1 > T_disjoint
[   28.792489] dyndbg: disjoint: 0x1 > test_dynamic_debug.T_D2
[   28.793848] dyndbg: query 0: "class D2_CORE +T" mod:*
[   28.795086] dyndbg: split into words: "class" "D2_CORE" "+T"
[   28.796467] dyndbg: op='+'
[   28.797148] dyndbg: unknown flag 'T'
[   28.798021] dyndbg: flags parse failed
[   28.798947] dyndbg: processed 1 queries, with 0 matches, 1 errs
[   28.800378] dyndbg: bit_0: -22 matches on class: D2_CORE -> 0x1
[   28.801959] dyndbg: test_dynamic_debug.T_D2: updated 0x0 -> 0x1
[   28.803974] dyndbg: total matches: -22

Signed-off-by: Jim Cromie 
---
 lib/test_dynamic_debug.c | 125 ++-
 1 file changed, 110 insertions(+), 15 deletions(-)

diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index ba3882ca3e48..8dd250ad022b 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -10,57 +10,152 @@
 
 #include 
 
-static void do_prints(void); /* device under test */
-
-/* run tests by reading or writing sysfs node */
+/* run tests by reading or writing sysfs node: do_prints */
 
+static void do_prints(void); /* device under test */
 static int param_set_do_prints(const char *instr, const struct kernel_param 
*kp)
 {
do_prints();
return 0;
 }
-
 static int param_get_do_prints(char *buffer, const struct kernel_param *kp)
 {
do_prints();
return scnprintf(buffer, PAGE_SIZE, "did do_prints\n");
 }
-
 static const struct kernel_param_ops param_ops_do_prints = {
.set = param_set_do_prints,
.get = param_get_do_prints,
 };
-
 module_param_cb(do_prints, _ops_do_prints, NULL, 0600);
 
-static void do_alpha(void)
+/*
+ * Using the CLASSMAP api:
+ * - classmaps must have corresponding enum
+ * - enum symbols must match/correlate with class-name strings in the map.
+ * - base must equal enum's 1st value
+ * - multiple maps must set their base to share the 0-30 class_id space !!
+ *   (build-bug-on tips welcome)
+ * Additionally, here:
+ * - tie together sysname, mapname, bitsname, flagsname
+ */
+#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)
+
+/* numeric input, independent bits */
+enum cat_disjoint_bits {
+   D2_CORE = 0,
+   D2_DRIVER,
+   D2_KMS,
+   D2_PRIME,
+   D2_ATOMIC,
+   D2_VBL,
+   D2_STATE,
+   D2_LEASE,
+   D2_DP,
+   D2_DRMRES };
+DECLARE_DYNDBG_CLASSMAP(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+   "D2_CORE",
+   "D2_DRIVER",
+   "D2_KMS",
+   "D2_PRIME",
+   "D2_ATOMIC",
+   "D2_VBL",
+   "D2_STATE",
+   "D2_LEASE",
+   "D2_DP",
+   "D2_DRMRES");
+DD_SYS_WRAP(disjoint_bits, p);
+DD_SYS_WRAP(disjoint_bits, T);
+
+/* symbolic input, independent bits */
+enum cat_disjoint_names { LOW = 11, MID, HI };
+DECLARE_DYNDBG_CLASSMAP(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
+   "LOW", "MID", "HI");
+DD_SYS_WRAP(disjoint_names, p);
+DD_SYS_WRAP(disjoint_names, T);
+
+/* numeric verbosity, V2 > V1 related */
+enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
+DECLARE_DYNDBG_CLASSMAP(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
+  "V0", "V1", "V2", "V3", "V4", "V5", "V6", "V7");
+DD_SYS_WRAP(level_num, p);

[Intel-gfx] [PATCH v5 08/33] dyndbg: add test_dynamic_debug module

2022-08-05 Thread Jim Cromie
Provide a simple module to allow testing DYNAMIC_DEBUG behavior.  It
calls do_prints() from module-init, and with a sysfs-node.

  dmesg -C
  dmesg -w &
  modprobe test_dynamic_debug dyndbg=+p
  echo 1 > /sys/module/dynamic_debug/parameters/verbose

  cat /sys/module/test_dynamic_debug/parameters/do_prints
  echo module test_dynamic_debug +mftl > /proc/dynamic_debug/control
  echo junk > /sys/module/test_dynamic_debug/parameters/do_prints

Signed-off-by: Jim Cromie 
Acked-by: Jason Baron 
---
 MAINTAINERS  |  2 ++
 lib/Kconfig.debug| 10 ++
 lib/Makefile |  1 +
 lib/test_dynamic_debug.c | 70 
 4 files changed, 83 insertions(+)
 create mode 100644 lib/test_dynamic_debug.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 64379c699903..a14fc4b6a10b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7093,6 +7093,8 @@ M:Jason Baron 
 S: Maintained
 F: include/linux/dynamic_debug.h
 F: lib/dynamic_debug.c
+M: Jim Cromie 
+F: lib/test_dynamic_debug.c
 
 DYNAMIC INTERRUPT MODERATION
 M: Tal Gilboa 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2e24db4bff19..ca5978e1d18a 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2529,6 +2529,16 @@ config TEST_STATIC_KEYS
 
  If unsure, say N.
 
+config TEST_DYNAMIC_DEBUG
+   tristate "Test DYNAMIC_DEBUG"
+   depends on DYNAMIC_DEBUG
+   help
+ This module registers a tracer callback to count enabled
+ pr_debugs in a 'do_debugging' function, then alters their
+ enablements, calls the function, and compares counts.
+
+ If unsure, say N.
+
 config TEST_KMOD
tristate "kmod stress tester"
depends on m
diff --git a/lib/Makefile b/lib/Makefile
index f99bf61f8bbc..9c316df868de 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_TEST_SORT) += test_sort.o
 obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
+obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
 obj-$(CONFIG_TEST_PRINTF) += test_printf.o
 obj-$(CONFIG_TEST_SCANF) += test_scanf.o
 obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
new file mode 100644
index ..ba3882ca3e48
--- /dev/null
+++ b/lib/test_dynamic_debug.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Kernel module for testing dynamic_debug
+ *
+ * Authors:
+ *  Jim Cromie 
+ */
+
+#define pr_fmt(fmt) "test_dd: " fmt
+
+#include 
+
+static void do_prints(void); /* device under test */
+
+/* run tests by reading or writing sysfs node */
+
+static int param_set_do_prints(const char *instr, const struct kernel_param 
*kp)
+{
+   do_prints();
+   return 0;
+}
+
+static int param_get_do_prints(char *buffer, const struct kernel_param *kp)
+{
+   do_prints();
+   return scnprintf(buffer, PAGE_SIZE, "did do_prints\n");
+}
+
+static const struct kernel_param_ops param_ops_do_prints = {
+   .set = param_set_do_prints,
+   .get = param_get_do_prints,
+};
+
+module_param_cb(do_prints, _ops_do_prints, NULL, 0600);
+
+static void do_alpha(void)
+{
+   pr_debug("do alpha\n");
+}
+static void do_beta(void)
+{
+   pr_debug("do beta\n");
+}
+
+static void do_prints(void)
+{
+   do_alpha();
+   do_beta();
+}
+
+static int __init test_dynamic_debug_init(void)
+{
+   pr_debug("init start\n");
+
+   do_prints();
+
+   pr_debug("init done\n");
+   return 0;
+}
+
+static void __exit test_dynamic_debug_exit(void)
+{
+   pr_debug("exiting\n");
+}
+
+module_init(test_dynamic_debug_init);
+module_exit(test_dynamic_debug_exit);
+
+MODULE_AUTHOR("Jim Cromie ");
+MODULE_LICENSE("GPL");
-- 
2.37.1



[Intel-gfx] [PATCH v5 05/33] dyndbg: reverse module.callsite walk in cat control

2022-08-05 Thread Jim Cromie
Walk the module's vector of callsites backwards; ie N..0.  This
"corrects" the backwards appearance of a module's prdbg vector when
walked 0..N.  I think this is due to linker mechanics, which I'm
inclined to treat as immutable, and the order is fixable in display.

No functional changes.

Combined with previous commit, which reversed tables-list, we get:

  :#> head -n7 /proc/dynamic_debug/control
  # filename:lineno [module]function flags format
  init/main.c:1179 [main]initcall_blacklist =_ "blacklisting initcall %s\012"
  init/main.c:1218 [main]initcall_blacklisted =_ "initcall %s blacklisted\012"
  init/main.c:1424 [main]run_init_process =_ "  with arguments:\012"
  init/main.c:1426 [main]run_init_process =_ "%s\012"
  init/main.c:1427 [main]run_init_process =_ "  with environment:\012"
  init/main.c:1429 [main]run_init_process =_ "%s\012"

Signed-off-by: Jim Cromie 
Acked-by: Jason Baron 
---
 lib/dynamic_debug.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 7fb99492c16f..8ff11977b8bd 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -59,7 +59,7 @@ struct ddebug_query {
 
 struct ddebug_iter {
struct ddebug_table *table;
-   unsigned int idx;
+   int idx;
 };
 
 struct flag_settings {
@@ -805,13 +805,12 @@ static struct _ddebug *ddebug_iter_first(struct 
ddebug_iter *iter)
 {
if (list_empty(_tables)) {
iter->table = NULL;
-   iter->idx = 0;
return NULL;
}
iter->table = list_entry(ddebug_tables.next,
 struct ddebug_table, link);
-   iter->idx = 0;
-   return >table->ddebugs[iter->idx];
+   iter->idx = iter->table->num_ddebugs;
+   return >table->ddebugs[--iter->idx];
 }
 
 /*
@@ -824,15 +823,16 @@ static struct _ddebug *ddebug_iter_next(struct 
ddebug_iter *iter)
 {
if (iter->table == NULL)
return NULL;
-   if (++iter->idx == iter->table->num_ddebugs) {
+   if (--iter->idx < 0) {
/* iterate to next table */
-   iter->idx = 0;
if (list_is_last(>table->link, _tables)) {
iter->table = NULL;
return NULL;
}
iter->table = list_entry(iter->table->link.next,
 struct ddebug_table, link);
+   iter->idx = iter->table->num_ddebugs;
+   --iter->idx;
}
return >table->ddebugs[iter->idx];
 }
-- 
2.37.1



[Intel-gfx] [PATCH v5 07/33] dyndbg: let query-modname override actual module name

2022-08-05 Thread Jim Cromie
dyndbg's control-parser: ddebug_parse_query(), requires that search
terms: module, func, file, lineno, are used only once in a query; a
thing cannot be named both foo and bar.

The cited commit added an overriding module modname, taken from the
module loader, which is authoritative.  So it set query.module 1st,
which disallowed its use in the query-string.

But now, its useful to allow a module-load to enable classes across a
whole (or part of) a subsystem at once.

  # enable (dynamic-debug in) drm only
  modprobe drm dyndbg="class DRM_UT_CORE +p"

  # get drm_helper too
  modprobe drm dyndbg="class DRM_UT_CORE module drm* +p"

  # get everything that knows DRM_UT_CORE
  modprobe drm dyndbg="class DRM_UT_CORE module * +p"

  # also for boot-args:
  drm.dyndbg="class DRM_UT_CORE module * +p"

So convert the override into a default, by filling it only when/after
the query-string omitted the module.

NB: the query class FOO handling is forthcoming.

Fixes: 8e59b5cfb9a6 dynamic_debug: add modname arg to exec_query callchain
Signed-off-by: Jim Cromie 
Acked-by: Jason Baron 
---
 lib/dynamic_debug.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e5cbe603000c..5a849716220a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -385,10 +385,6 @@ static int ddebug_parse_query(char *words[], int nwords,
return -EINVAL;
}
 
-   if (modname)
-   /* support $modname.dyndbg= */
-   query->module = modname;
-
for (i = 0; i < nwords; i += 2) {
char *keyword = words[i];
char *arg = words[i+1];
@@ -429,6 +425,13 @@ static int ddebug_parse_query(char *words[], int nwords,
if (rc)
return rc;
}
+   if (!query->module && modname)
+   /*
+* support $modname.dyndbg=, when
+* not given in the query itself
+*/
+   query->module = modname;
+
vpr_info_dq(query, "parsed");
return 0;
 }
-- 
2.37.1



[Intel-gfx] [PATCH v5 04/33] dyndbg: reverse module walk in cat control

2022-08-05 Thread Jim Cromie
/proc/dynamic_debug/control walks the prdbg catalog in "reverse",
fix this by adding new ddebug_tables to tail of list.

This puts init/main.c entries 1st, which looks more than coincidental.

no functional changes.

Signed-off-by: Jim Cromie 
Acked-by: Jason Baron 
---
 lib/dynamic_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8faf584f2f4b..7fb99492c16f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -970,7 +970,7 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
dt->ddebugs = tab;
 
mutex_lock(_lock);
-   list_add(>link, _tables);
+   list_add_tail(>link, _tables);
mutex_unlock(_lock);
 
vpr_info("%3u debug prints in module %s\n", n, dt->mod_name);
-- 
2.37.1



[Intel-gfx] [PATCH v5 03/33] dyndbg: show both old and new in change-info

2022-08-05 Thread Jim Cromie
print "old => new" flag values to the info("change") message.

no functional change.

Signed-off-by: Jim Cromie 
Acked-by: Jason Baron 
---
 lib/dynamic_debug.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a56c1286ffa4..8faf584f2f4b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -156,7 +156,7 @@ static int ddebug_change(const struct ddebug_query *query,
struct ddebug_table *dt;
unsigned int newflags;
unsigned int nfound = 0;
-   struct flagsbuf fbuf;
+   struct flagsbuf fbuf, nbuf;
 
/* search for matching ddebugs */
mutex_lock(_lock);
@@ -217,11 +217,12 @@ static int ddebug_change(const struct ddebug_query *query,
static_branch_enable(>key.dd_key_true);
}
 #endif
+   v4pr_info("changed %s:%d [%s]%s %s => %s\n",
+ trim_prefix(dp->filename), dp->lineno,
+ dt->mod_name, dp->function,
+ ddebug_describe_flags(dp->flags, ),
+ ddebug_describe_flags(newflags, ));
dp->flags = newflags;
-   v4pr_info("changed %s:%d [%s]%s =%s\n",
-trim_prefix(dp->filename), dp->lineno,
-dt->mod_name, dp->function,
-ddebug_describe_flags(dp->flags, ));
}
}
mutex_unlock(_lock);
-- 
2.37.1



[Intel-gfx] [PATCH v5 02/33] dyndbg: fix module.dyndbg handling

2022-08-05 Thread Jim Cromie
For CONFIG_DYNAMIC_DEBUG=N, the ddebug_dyndbg_module_param_cb()
stub-fn is too permissive:

bash-5.1# modprobe drm JUNKdyndbg
bash-5.1# modprobe drm dyndbgJUNK
[   42.933220] dyndbg param is supported only in CONFIG_DYNAMIC_DEBUG builds
[   42.937484] ACPI: bus type drm_connector registered

This caused no ill effects, because unknown parameters are either
ignored by default with an "unknown parameter" warning, or ignored
because dyndbg allows its no-effect use on non-dyndbg builds.

But since the code has an explicit feedback message, it should be
issued accurately.  Fix with strcmp for exact param-name match.

Reported-by: Rasmus Villemoes 
Fixes: b48420c1d301 dynamic_debug: make dynamic-debug work for module 
initialization
Signed-off-by: Jim Cromie 
Acked-by: Jason Baron 
---
 include/linux/dynamic_debug.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index dce631e678dd..f30b01aa9fa4 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -201,7 +201,7 @@ static inline int ddebug_remove_module(const char *mod)
 static inline int ddebug_dyndbg_module_param_cb(char *param, char *val,
const char *modname)
 {
-   if (strstr(param, "dyndbg")) {
+   if (!strcmp(param, "dyndbg")) {
/* avoid pr_warn(), which wants pr_fmt() fully defined */
printk(KERN_WARNING "dyndbg param is supported only in "
"CONFIG_DYNAMIC_DEBUG builds\n");
-- 
2.37.1



[Intel-gfx] [PATCH v5 01/33] dyndbg: fix static_branch manipulation

2022-08-05 Thread Jim Cromie
In https://lore.kernel.org/lkml/20211209150910.ga23...@axis.com/

Vincent's patch commented on, and worked around, a bug toggling
static_branch's, when a 2nd PRINTK-ish flag was added.  The bug
results in a premature static_branch_disable when the 1st of 2 flags
was disabled.

The cited commit computed newflags, but then in the JUMP_LABEL block,
failed to use that result, instead using just one of the terms in it.
Using newflags instead made the code work properly.

This is Vincents test-case, reduced.  It needs the 2nd flag to
demonstrate the bug, but it's explanatory here.

pt_test() {
echo 5 > /sys/module/dynamic_debug/verbose

site="module tcp" # just one callsite
echo " $site =_ " > /proc/dynamic_debug/control # clear it

# A B ~A ~B
for flg in +T +p "-T #broke here" -p; do
echo " $site $flg " > /proc/dynamic_debug/control
done;

# A B ~B ~A
for flg in +T +p "-p #broke here" -T; do
echo " $site $flg " > /proc/dynamic_debug/control
done
}
pt_test

Fixes: 84da83a6ffc0 dyndbg: combine flags & mask into a struct, simplify with it
CC: vincent.whitchu...@axis.com
Signed-off-by: Jim Cromie 
Acked-by: Jason Baron 
---
.drop @stable, no exposed bug.
.add jbaron ack
---
 lib/dynamic_debug.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index dd7f56af9aed..a56c1286ffa4 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -211,10 +211,11 @@ static int ddebug_change(const struct ddebug_query *query,
continue;
 #ifdef CONFIG_JUMP_LABEL
if (dp->flags & _DPRINTK_FLAGS_PRINT) {
-   if (!(modifiers->flags & _DPRINTK_FLAGS_PRINT))
+   if (!(newflags & _DPRINTK_FLAGS_PRINT))

static_branch_disable(>key.dd_key_true);
-   } else if (modifiers->flags & _DPRINTK_FLAGS_PRINT)
+   } else if (newflags & _DPRINTK_FLAGS_PRINT) {
static_branch_enable(>key.dd_key_true);
+   }
 #endif
dp->flags = newflags;
v4pr_info("changed %s:%d [%s]%s =%s\n",
-- 
2.37.1



[Intel-gfx] [PATCH v5 00/33] DYNDBG: opt-in class'd debug for modules, use in drm.

2022-08-05 Thread Jim Cromie
Hi Jason, Greg, DRM-folk,

heres V5. Diffs vs V4:

  1st 9 have Jason's Ack
  10 is new: simple var cleanup
  11 new struct, to contain future addins. touches kernel/module/

  dyndbg-param callbacks moved to last (of dyndbg patches)
  they're where the uncertainty is,
  and the bulk of the API addition (class keyword is small)

  patches between the move are the guts of the "class foo" feature.
  exposure of feature is only via >control, until sysfs parts added
  
  DRM chunk:
  included here to see how patchwork's merge & test proceeds.
  categories -> classes re-work is unchanged - feels pretty solid.
  nouveau & LEVEL made some progress, still WIP.

  I left out tracefs bits for now.

UNCERTAINTIES / RFC:

1st is my "sharing" technique, across DRM's multi modules.  Its
literally duplicated decls, by each participating module, each adding
static data in its own special section.  I didn't try at all to shrink
or unify this any further.  There might be a perfect place to add a
single decl, but that quest will require a pony.

Sharing by class-names lacks the compiler support that varnames get;
"incomplete" enables will probably happen if one of the declarations
miss-spells one of the classnames.

That said, see the how-to below: stringifying va-args in the macro
would allow direct use of the enum-consts, and cc will insist that
those consts are defined, eliminating part of the un-support.

I could split the _BITS,_NUM handling out from _NAMES (ie split
drm.debug style patch); theres no user-demand for the _NAMES feature
ATM, and theres probably some legit API-bikeshedding to do.

PATCHSET DESCRIPTION

This patchset adds 'typed' "class FOO" support to dynamic-debug, where
'typed' means either DISJOINT_BITS, like drm_dbg(DRM_UT_CORE,..), or
LEVEL_NUM (like nouveau debug-levels).  Use it in DRM modules: core,
helpers, and in drivers i915, amdgpu, nouveau.

If a module is using class'd prdbgs (pr_debug_class, dev_dbg_class, or
adapted drm_dbg_) or similar in its code, it can "opt in" to
allow dyndbg to manipulate those class'd prdebugs, by declaring in a
c-file:

 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");
// how-to stringify __va_args inside the macro ?

By doing this, a module tells dyndbg that it:

   - is using class-ids [0..N] in prdbg callsites
 ( 0..N == DRM_UT_CORE..DRM_UT_DRMRES )
   - wants to refer to them by class-names [0..N]
   - is mapping those names to those class-ids
   - expects users to enable them via >control or >parameter/knob

Then, a user can enable the prdbgs by their class:

   :#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control

And with another 3-line bitmap param decl/init, wrapping the
drm_debug_classes var in a module-param-cb:

   :#> echo 0x1 > /sys/module/drm/parameters/debug

and optionally using classnames:

   :#> echo +DRM_UT_CORE,-DRM_UT_KMS \
> /sys/module/drm/parameters/debug_cats

DYNAMIC_DEBUG gets:

struct _ddebug gets a new .class_id:5 field, big enough to represent
drm_debug_category (after squeezing).  It defaults to 31 for all
existing prdbgs.  class_id also supports verbose uses.

classmaps (as declared by macro above) are in their own linker
section, are loaded by kernel/module, and handled by add_module,
which attaches classmaps to their module's ddebug table.
 
ddebug_change() handles a class FOO query by validating that FOO is
known by each module in the loop.  The query is skipped unless the
module knows FOO, so no changes are possible w/o a good classname.

Without class FOO in a query/command, only ids=31 can be changed by
that query.  This protects all class'd prdbgs from changes by legacy
class-less user queries.

With this support, the module opt-in approach means that:

   - modules declare classnames they like, meaningful names: DRM_UT_*
 these are numbered [0..N]
   - modules call pr_debug_class(N, "fmt..",...)
 or drm_dbg(CAT, "fmt..",...) - same form.
   - class-id space, while limited:0-30, is private to each module
   - "class FOO" is only way to select a class'd prdbg
   - unrelated modules use 0..N separately, for different purposes.
   - modules "share" classnames by separate decls (uses of macro)
 all drm modules reuse the above declaration.
 then they respond together to a >control

4 CLASS_TYPES are defined; they split behavior on 2 factors:

   1. independent bits vs related:(X sysknob ambiguity
   as was case when both were accepted on same knob
 - narrower interfaces
   uint is uint
 - can defer DISJOINT_NAMES handling, but keep 

[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/ttm: remove calc_ctrl_surf_instr_size

2022-08-05 Thread Patchwork
== Series Details ==

Series: series starting with [1/2] drm/i915/ttm: remove 
calc_ctrl_surf_instr_size
URL   : https://patchwork.freedesktop.org/series/107031/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11971_full -> Patchwork_107031v1_full


Summary
---

  **SUCCESS**

  No regressions found.

  

Participating hosts (12 -> 12)
--

  No changes in participating hosts

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_107031v1_full:

### IGT changes ###

 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@prime_self_import@reimport-vs-gem_close-race:
- {shard-rkl}:[PASS][1] -> [FAIL][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-rkl-2/igt@prime_self_import@reimport-vs-gem_close-race.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-rkl-5/igt@prime_self_import@reimport-vs-gem_close-race.html

  
Known issues


  Here are the changes found in Patchwork_107031v1_full that come from known 
issues:

### CI changes ###

 Issues hit 

  * boot:
- shard-glk:  ([PASS][3], [PASS][4], [PASS][5], [PASS][6], 
[PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], 
[PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], 
[PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], 
[PASS][25], [PASS][26], [PASS][27]) -> ([PASS][28], [PASS][29], [PASS][30], 
[FAIL][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], 
[PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], 
[PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], 
[PASS][49], [PASS][50], [PASS][51], [FAIL][52]) ([i915#4392])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk6/boot.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk6/boot.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk1/boot.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk1/boot.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk1/boot.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk2/boot.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk2/boot.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk2/boot.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk2/boot.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk3/boot.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk3/boot.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk3/boot.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk5/boot.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk5/boot.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk9/boot.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk9/boot.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk5/boot.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk9/boot.html
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk6/boot.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk8/boot.html
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk8/boot.html
   [24]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk8/boot.html
   [25]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk7/boot.html
   [26]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk7/boot.html
   [27]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/shard-glk7/boot.html
   [28]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-glk3/boot.html
   [29]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-glk3/boot.html
   [30]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-glk1/boot.html
   [31]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-glk1/boot.html
   [32]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-glk1/boot.html
   [33]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-glk2/boot.html
   [34]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-glk9/boot.html
   [35]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-glk2/boot.html
   [36]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-glk9/boot.html
   [37]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/shard-glk9/boot.html
   [38]: 

[Intel-gfx] [PULL] drm-intel-next (updated without the bad commit)

2022-08-05 Thread Rodrigo Vivi
Hi Dave and Daniel,

Please pick this one instead of drm-intel-next-fixes-2022-08-04.

Mauro has pointed out that the commit "drm/i915/gt: Batch TLB invalidations"
was introducing a worst regression in comparison to its intent.
The work is in progress to fix on our -next branches, but we shouldn't
backport it for now.

Sorry for the noise.

And a big thanks to Mauro who noticed and pointed it out before
the propagation.

Here goes drm-intel-next-2022-08-05:
- disable pci resize on 32-bit systems (Nirmoy)
- don't leak the ccs state (Matt)
- TLB invalidation fixes (Chris)
[Excluding the bad commit drm/i915/gt: Batch TLB invalidations \
present on drm-intel-next-fixes-2022-08-04]

Thanks,
Rodrigo.

The following changes since commit 5493ee1919eae4f49d62276cf5986b7f7c7aa8f6:

  Merge tag 'amd-drm-next-5.20-2022-07-29' of 
https://gitlab.freedesktop.org/agd5f/linux into drm-next (2022-08-03 14:00:19 
+1000)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-2022-08-05

for you to fetch changes up to fc30eea1542dd787c6aa46e970014e97e390c5b2:

  Merge drm/drm-next into drm-intel-next (2022-08-04 10:19:24 -0400)


- disable pci resize on 32-bit systems (Nirmoy)
- don't leak the ccs state (Matt)
- TLB invalidation fixes (Chris)
[Excluding the bad commit drm/i915/gt: Batch TLB invalidations \
present on drm-intel-next-fixes-2022-08-04]


Ankit Nautiyal (1):
  drm/i915/hdmi: Prune modes that require HDMI2.1 FRL

Anusha Srivatsa (3):
  drm/i915/display: Cleanup intel_phy_is_combo()
  drm/i915: Pass drm_i915_private struct instead of gt for 
gen11_gu_misc_irq_handler/ack()
  drm/i915/dg2: Add support for DC5 state

Imre Deak (3):
  drm/i915/tgl+: Fix HDMI transcoder clock vs. DDI BUF disabling order
  drm/i915/d12+: Disable DMC handlers during loading/disabling the firmware
  drm/i915/d13: Add Wa_16015201720 disabling clock gating for PIPEDMC-A/B

José Roberto de Souza (2):
  Revert "drm/i915/display: Ensure PSR gets disabled if no encoders in new 
state"
  drm/i915/psr: Disable PSR before disable pipe

Jouni Högander (1):
  drm/i915/display: Ensure PSR gets disabled if no encoders in new state

Madhumitha Tolakanahalli Pradeep (1):
  drm/i915/dmc: Update DG2 DMC firmware to v2.07

Matt Roper (1):
  drm/i915: Add Wa_14016291713

Rodrigo Vivi (1):
  Merge drm/drm-next into drm-intel-next

Stanislav Lisovskiy (1):
  drm/i915/dg2: Bump up CDCLK for DG2

Swati Sharma (1):
  drm/i915/display: Add debug print for scaler filter

 drivers/gpu/drm/i915/display/intel_cdclk.c |  4 +-
 .../gpu/drm/i915/display/intel_crtc_state_dump.c   |  9 ++--
 drivers/gpu/drm/i915/display/intel_ddi.c   |  6 ++-
 drivers/gpu/drm/i915/display/intel_display.c   | 14 +++--
 .../gpu/drm/i915/display/intel_display_debugfs.c   |  5 +-
 drivers/gpu/drm/i915/display/intel_display_power.c | 13 -
 drivers/gpu/drm/i915/display/intel_dmc.c   | 60 +++---
 drivers/gpu/drm/i915/display/intel_dmc.h   |  1 +
 drivers/gpu/drm/i915/display/intel_dmc_regs.h  |  2 +
 drivers/gpu/drm/i915/display/intel_fbc.c   |  6 +++
 drivers/gpu/drm/i915/display/intel_hdmi.c  |  9 
 drivers/gpu/drm/i915/display/intel_psr.c   | 14 ++---
 drivers/gpu/drm/i915/i915_irq.c| 16 +++---
 drivers/gpu/drm/i915/i915_reg.h|  7 +++
 14 files changed, 126 insertions(+), 40 deletions(-)


Re: [Intel-gfx] [PULL] drm-intel-next-fixes

2022-08-05 Thread Rodrigo Vivi
On Fri, Aug 05, 2022 at 05:25:43PM +0200, Mauro Carvalho Chehab wrote:
> On Fri, 5 Aug 2022 10:39:44 -0400
> Rodrigo Vivi  wrote:
> 
> > On Fri, Aug 05, 2022 at 10:46:57AM +0200, Mauro Carvalho Chehab wrote:
> > > Hi Rodrigo,
> > > 
> > > On Thu, 4 Aug 2022 13:33:06 -0400
> > > Rodrigo Vivi  wrote:
> > >   
> > > > Hi Dave and Daniel,
> > > > 
> > > > Here goes drm-intel-next-fixes-2022-08-04:
> > > > 
> > > > - disable pci resize on 32-bit systems (Nirmoy)
> > > > - don't leak the ccs state (Matt)
> > > > - TLB invalidation fixes (Chris)
> > > > 
> > > > Thanks,
> > > > Rodrigo.
> > > > 
> > > > The following changes since commit 
> > > > 2bc7ea71a73747a77e7f83bc085b0d2393235410:
> > > > 
> > > >   Merge tag 'topic/nouveau-misc-2022-07-27' of 
> > > > git://anongit.freedesktop.org/drm/drm into drm-next (2022-07-27 
> > > > 11:34:07 +1000)
> > > > 
> > > > are available in the Git repository at:
> > > > 
> > > >   git://anongit.freedesktop.org/drm/drm-intel 
> > > > tags/drm-intel-next-fixes-2022-08-04
> > > > 
> > > > for you to fetch changes up to e57b9369e0c6f60155027e120fafd4b57e385b71:
> > > > 
> > > >   drm/i915/gt: Batch TLB invalidations (2022-08-01 09:48:06 -0400)
> > > > 
> > > > 
> > > > - disable pci resize on 32-bit systems (Nirmoy)
> > > > - don't leak the ccs state (Matt)
> > > > - TLB invalidation fixes (Chris)
> > > > 
> > > > 
> > > > Chris Wilson (4):
> > > >   drm/i915/gt: Ignore TLB invalidations on idle engines
> > > >   drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
> > > >   drm/i915/gt: Skip TLB invalidations once wedged  
> > >   
> > > >   drm/i915/gt: Batch TLB invalidations  
> > > This patch actually adds a regression due to a silly mistake. 
> > > The fix is here:
> > > 
> > >   https://patchwork.freedesktop.org/patch/496249/?series=106805=4
> > >  
> > 
> > Thank you for the heads up.
> > 
> > Since that patch is not merged yet, what are your recommendations here?
> > Should I remove this from drm-intel-next-fixes now?
> 
> Those patches are OK to merge:
> 
>drm/i915/gt: Ignore TLB invalidations on idle engines
>drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
>drm/i915/gt: Skip TLB invalidations once wedged  
> 
> And helps reduce performance regressions due to TLB cache invalidation.
> So, I would keep them.
> 
> With regards to the 4th patch, please don't merge:
> 
>   drm/i915/gt: Batch TLB invalidations
> 
> or merge it together with:
> 
>   drm/i915: pass a pointer for tlb seqno at vma_invalidate_tlb()

Since this one has not landed yet I won't rush it in. I'm removing the
Batch TLB invalidations and sending a new pull request without that
patch.

Thank you for the info and great work there.

> 
> > Which regression is worst?
> 
> The regression caused by the batch patch is a lot worse, as it
> effectively disables TLB cache invalidation. The fix was meant to
> be merged at the patch, but somehow between several rebases and 3
> machines involved on tests, internal development and upstream, the
> branch actually sent upstream was not the right one. It got only
> noticed after reviewing a newer patch. Yeah, that sucks.
> 
> > Also dim has trouble with fixes for fixes in the same round.
> > Please ping me when you get that patch merged so I can pull that.
> 
> OK.
> 
> Regards,
> Mauro


[Intel-gfx] ✗ Fi.CI.BAT: failure for Sanitycheck PCI BARs (rev2)

2022-08-05 Thread Patchwork
== Series Details ==

Series: Sanitycheck PCI BARs (rev2)
URL   : https://patchwork.freedesktop.org/series/106927/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11971 -> Patchwork_106927v2


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_106927v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_106927v2, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/index.html

Participating hosts (43 -> 41)
--

  Missing(2): bat-dg2-8 fi-bdw-samus 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_106927v2:

### IGT changes ###

 Possible regressions 

  * igt@i915_selftest@live@guc_hang:
- fi-cfl-8109u:   [PASS][1] -> [DMESG-WARN][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/fi-cfl-8109u/igt@i915_selftest@live@guc_hang.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/fi-cfl-8109u/igt@i915_selftest@live@guc_hang.html

  
 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_module_load@load:
- {bat-dg2-10}:   [PASS][3] -> [DMESG-WARN][4]
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/bat-dg2-10/igt@i915_module_l...@load.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/bat-dg2-10/igt@i915_module_l...@load.html

  
Known issues


  Here are the changes found in Patchwork_106927v2 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@i915_pm_rpm@module-reload:
- fi-cfl-8109u:   [PASS][5] -> [DMESG-FAIL][6] ([i915#62])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/fi-cfl-8109u/igt@i915_pm_...@module-reload.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/fi-cfl-8109u/igt@i915_pm_...@module-reload.html

  * igt@i915_selftest@live@late_gt_pm:
- fi-cfl-8109u:   [PASS][7] -> [DMESG-WARN][8] ([i915#5904]) +29 
similar issues
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html

  * igt@i915_suspend@basic-s2idle-without-i915:
- fi-cfl-8109u:   [PASS][9] -> [DMESG-WARN][10] ([i915#5904] / 
[i915#62])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/fi-cfl-8109u/igt@i915_susp...@basic-s2idle-without-i915.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/fi-cfl-8109u/igt@i915_susp...@basic-s2idle-without-i915.html

  * igt@i915_suspend@basic-s3-without-i915:
- fi-bdw-5557u:   [PASS][11] -> [INCOMPLETE][12] ([i915#146])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/fi-bdw-5557u/igt@i915_susp...@basic-s3-without-i915.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/fi-bdw-5557u/igt@i915_susp...@basic-s3-without-i915.html

  * igt@kms_frontbuffer_tracking@basic:
- fi-cfl-8109u:   [PASS][13] -> [DMESG-WARN][14] ([i915#62]) +12 
similar issues
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/fi-cfl-8109u/igt@kms_frontbuffer_track...@basic.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/fi-cfl-8109u/igt@kms_frontbuffer_track...@basic.html

  
 Possible fixes 

  * igt@gem_exec_suspend@basic-s0@smem:
- {bat-rplp-1}:   [DMESG-WARN][15] ([i915#2867]) -> [PASS][16]
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/bat-rplp-1/igt@gem_exec_suspend@basic...@smem.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/bat-rplp-1/igt@gem_exec_suspend@basic...@smem.html

  * igt@i915_selftest@live@slpc:
- {bat-rpls-1}:   [DMESG-FAIL][17] ([i915#6367]) -> [PASS][18]
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/bat-rpls-1/igt@i915_selftest@l...@slpc.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/bat-rpls-1/igt@i915_selftest@l...@slpc.html

  
 Warnings 

  * igt@i915_selftest@live@hangcheck:
- bat-dg1-5:  [DMESG-FAIL][19] ([i915#4957]) -> [DMESG-FAIL][20] 
([i915#4494] / [i915#4957])
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/bat-dg1-5/igt@i915_selftest@l...@hangcheck.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106927v2/bat-dg1-5/igt@i915_selftest@l...@hangcheck.html
- bat-dg1-6:  [DMESG-FAIL][21] ([i915#4494] / [i915#4957]) -> 
[DMESG-FAIL][22] ([i915#4957])
   [21]: 

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Sanitycheck PCI BARs (rev2)

2022-08-05 Thread Patchwork
== Series Details ==

Series: Sanitycheck PCI BARs (rev2)
URL   : https://patchwork.freedesktop.org/series/106927/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gt/intel_gt.h:106:16: warning: trying to copy expression 
type 31
+drivers/gpu/drm/i915/gt/intel_reset.c:1410:5: warning: context imbalance in 
'intel_gt_reset_trylock' - different lock contexts for basic block




[Intel-gfx] [PATCH v3 2/2] drm/i915: Sanitycheck PCI BARs

2022-08-05 Thread Piorkowski, Piotr
From: Piotr Piórkowski 

For proper operation of i915 we need usable PCI GTTMMADDR BAR 0
(1 for GEN2). In most cases we also need usable PCI GFXMEM BAR 2.
Let's add functions to check if BARs are set, and that it have
a size greater than 0.
In case GTTMMADDR BAR, let's validate at the beginning of i915
initialization.
For other BARs, let's validate before first use.

Signed-off-by: Piotr Piórkowski 
Cc: Jani Nikula 
Cc: Lucas De Marchi 
Cc: Matt Roper 
Reviewed-by: Jani Nikula 
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c  |  4 
 drivers/gpu/drm/i915/gt/intel_ggtt.c|  7 ++
 drivers/gpu/drm/i915/gt/intel_region_lmem.c |  4 
 drivers/gpu/drm/i915/i915_pci.c | 25 +
 drivers/gpu/drm/i915/i915_pci.h |  6 +
 5 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index c369cfd185bc..4f4c9461a23b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -18,6 +18,7 @@
 #include "gt/intel_region_lmem.h"
 #include "i915_drv.h"
 #include "i915_gem_stolen.h"
+#include "i915_pci.h"
 #include "i915_reg.h"
 #include "i915_utils.h"
 #include "i915_vgpu.h"
@@ -828,6 +829,9 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
if (WARN_ON_ONCE(instance))
return ERR_PTR(-ENODEV);
 
+   if (!i915_pci_resource_valid(pdev, GEN12_LMEM_BAR))
+   return ERR_PTR(-ENXIO);
+
/* Use DSM base address instead for stolen memory */
dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE);
if (IS_DG1(uncore->i915)) {
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 8214e07a0f5b..30cf5c3369d9 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -18,6 +18,7 @@
 #include "intel_gt_regs.h"
 #include "intel_pci_config.h"
 #include "i915_drv.h"
+#include "i915_pci.h"
 #include "i915_scatterlist.h"
 #include "i915_utils.h"
 #include "i915_vgpu.h"
@@ -931,6 +932,9 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
u16 snb_gmch_ctl;
 
if (!HAS_LMEM(i915)) {
+   if (!i915_pci_resource_valid(pdev, GTT_APERTURE_BAR))
+   return -ENXIO;
+
ggtt->gmadr = pci_resource(pdev, GTT_APERTURE_BAR);
ggtt->mappable_end = resource_size(>gmadr);
}
@@ -1085,6 +1089,9 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
unsigned int size;
u16 snb_gmch_ctl;
 
+   if (!i915_pci_resource_valid(pdev, GTT_APERTURE_BAR))
+   return -ENXIO;
+
ggtt->gmadr = pci_resource(pdev, GTT_APERTURE_BAR);
ggtt->mappable_end = resource_size(>gmadr);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c 
b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index 1e79d3c8b126..f3ad93db0b21 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -4,6 +4,7 @@
  */
 
 #include "i915_drv.h"
+#include "i915_pci.h"
 #include "i915_reg.h"
 #include "intel_memory_region.h"
 #include "intel_pci_config.h"
@@ -201,6 +202,9 @@ static struct intel_memory_region *setup_lmem(struct 
intel_gt *gt)
if (!IS_DGFX(i915))
return ERR_PTR(-ENODEV);
 
+   if (!i915_pci_resource_valid(pdev, GEN12_LMEM_BAR))
+   return ERR_PTR(-ENXIO);
+
if (HAS_FLAT_CCS(i915)) {
resource_size_t lmem_range;
u64 tile_stolen, flat_ccs_base;
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index aacc10f2e73f..9fd788e147a3 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -30,6 +30,7 @@
 #include "i915_drv.h"
 #include "i915_pci.h"
 #include "i915_reg.h"
+#include "intel_pci_config.h"
 
 #define PLATFORM(x) .platform = (x)
 #define GEN(x) \
@@ -1262,6 +1263,27 @@ static bool force_probe(u16 device_id, const char 
*devices)
return ret;
 }
 
+bool i915_pci_resource_valid(struct pci_dev *pdev, int bar)
+{
+   if (!pci_resource_flags(pdev, bar))
+   return false;
+
+   if (pci_resource_flags(pdev, bar) & IORESOURCE_UNSET)
+   return false;
+
+   if (!pci_resource_len(pdev, bar))
+   return false;
+
+   return true;
+}
+
+static bool intel_mmio_bar_valid(struct pci_dev *pdev, struct 
intel_device_info *intel_info)
+{
+   int gttmmaddr_bar = intel_info->graphics.ver == 2 ? GEN2_GTTMMADR_BAR : 
GTTMMADR_BAR;
+
+   return i915_pci_resource_valid(pdev, gttmmaddr_bar);
+}
+
 static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id 
*ent)
 {
struct intel_device_info *intel_info =
@@ -1287,6 +1309,9 @@ static int i915_pci_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
if (PCI_FUNC(pdev->devfn))
return -ENODEV;
 
+  

[Intel-gfx] [PATCH v3 0/2] Sanitycheck PCI BARs

2022-08-05 Thread Piorkowski, Piotr
From: Piotr Piórkowski 

When initializing the i915, we want to be sure that the PCI
BARs have been properly initialized.
As part of this series, I have prepared two patches,
one that introduces BARs names to use in code instead of numbers,
and another that adds function to validate BARs before use.
This is an evolution of the concept I presented in the patch:
https://patchwork.freedesktop.org/patch/470184/?series=99094
The main difference between the original patch and what I have
prepared here is that previously I checked all BARs at the beginning
of i915 initialization, and now I only check BAR 0 at beginning.
This is due to the fact that I have noticed that it can happen that only
BAR 0 is available (I have observed this in the case of virtualization,
on some platforms).
Therefore, at the beginning, let's verify only BAR 0, and the others only
before the first use.

v2: Fix build
v3: replace header linux/pci.h to forward declaration for struct pci_dev

Signed-off-by: Piotr Piórkowski 
Cc: Jani Nikula 
Cc: Lucas De Marchi 
Cc: Matt Roper 


Piotr Piórkowski (2):
  drm/i915: Use of BARs names instead of numbers
  drm/i915: Sanitycheck PCI BARs

 .../gpu/drm/i915/display/intel_lpe_audio.c|  5 ++--
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 11 +---
 drivers/gpu/drm/i915/gt/intel_ggtt.c  | 16 +---
 drivers/gpu/drm/i915/gt/intel_gt.c|  3 ++-
 drivers/gpu/drm/i915/gt/intel_region_lmem.c   | 17 +++--
 drivers/gpu/drm/i915/gvt/cfg_space.c  |  5 ++--
 drivers/gpu/drm/i915/i915_pci.c   | 25 +++
 drivers/gpu/drm/i915/i915_pci.h   |  6 +
 drivers/gpu/drm/i915/intel_pci_config.h   |  7 ++
 9 files changed, 76 insertions(+), 19 deletions(-)

-- 
2.25.1



[Intel-gfx] [PATCH v3 1/2] drm/i915: Use of BARs names instead of numbers

2022-08-05 Thread Piorkowski, Piotr
From: Piotr Piórkowski 

At the moment, when we refer to some PCI BAR we use the number of
this BAR in the code. The meaning of BARs between different platforms
may be different. Therefore, in order to organize the code,
let's start using defined names instead of numbers.

v2: Add lost header in cfg_space.c

Signed-off-by: Piotr Piórkowski 
Cc: Jani Nikula 
Cc: Lucas De Marchi 
Cc: Matt Roper 
Reviewed-by: Jani Nikula 
---
 drivers/gpu/drm/i915/display/intel_lpe_audio.c |  5 +++--
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c |  7 ---
 drivers/gpu/drm/i915/gt/intel_ggtt.c   |  9 +
 drivers/gpu/drm/i915/gt/intel_gt.c |  3 ++-
 drivers/gpu/drm/i915/gt/intel_region_lmem.c| 13 ++---
 drivers/gpu/drm/i915/gvt/cfg_space.c   |  5 +++--
 drivers/gpu/drm/i915/intel_pci_config.h|  7 +++
 7 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_lpe_audio.c 
b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
index 4970bf146c4a..1e18696aaecf 100644
--- a/drivers/gpu/drm/i915/display/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
@@ -73,6 +73,7 @@
 #include "i915_drv.h"
 #include "intel_de.h"
 #include "intel_lpe_audio.h"
+#include "intel_pci_config.h"
 
 #define HAS_LPE_AUDIO(dev_priv) ((dev_priv)->audio.lpe.platdev != NULL)
 
@@ -100,9 +101,9 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
rsc[0].flags= IORESOURCE_IRQ;
rsc[0].name = "hdmi-lpe-audio-irq";
 
-   rsc[1].start= pci_resource_start(pdev, 0) +
+   rsc[1].start= pci_resource_start(pdev, GTTMMADR_BAR) +
I915_HDMI_LPE_AUDIO_BASE;
-   rsc[1].end  = pci_resource_start(pdev, 0) +
+   rsc[1].end  = pci_resource_start(pdev, GTTMMADR_BAR) +
I915_HDMI_LPE_AUDIO_BASE + I915_HDMI_LPE_AUDIO_SIZE - 1;
rsc[1].flags= IORESOURCE_MEM;
rsc[1].name = "hdmi-lpe-audio-mmio";
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 166d0a4b9e8c..c369cfd185bc 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -22,6 +22,7 @@
 #include "i915_utils.h"
 #include "i915_vgpu.h"
 #include "intel_mchbar_regs.h"
+#include "intel_pci_config.h"
 
 /*
  * The BIOS typically reserves some of the system's memory for the exclusive
@@ -830,7 +831,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
/* Use DSM base address instead for stolen memory */
dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE);
if (IS_DG1(uncore->i915)) {
-   lmem_size = pci_resource_len(pdev, 2);
+   lmem_size = pci_resource_len(pdev, GEN12_LMEM_BAR);
if (WARN_ON(lmem_size < dsm_base))
return ERR_PTR(-ENODEV);
} else {
@@ -842,11 +843,11 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
}
 
dsm_size = lmem_size - dsm_base;
-   if (pci_resource_len(pdev, 2) < lmem_size) {
+   if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
io_start = 0;
io_size = 0;
} else {
-   io_start = pci_resource_start(pdev, 2) + dsm_base;
+   io_start = pci_resource_start(pdev, GEN12_LMEM_BAR) + dsm_base;
io_size = dsm_size;
}
 
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 15a915bb4088..8214e07a0f5b 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -16,6 +16,7 @@
 #include "intel_ggtt_gmch.h"
 #include "intel_gt.h"
 #include "intel_gt_regs.h"
+#include "intel_pci_config.h"
 #include "i915_drv.h"
 #include "i915_scatterlist.h"
 #include "i915_utils.h"
@@ -869,8 +870,8 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 
size)
u32 pte_flags;
int ret;
 
-   GEM_WARN_ON(pci_resource_len(pdev, 0) != gen6_gttmmadr_size(i915));
-   phys_addr = pci_resource_start(pdev, 0) + gen6_gttadr_offset(i915);
+   GEM_WARN_ON(pci_resource_len(pdev, GTTMMADR_BAR) != 
gen6_gttmmadr_size(i915));
+   phys_addr = pci_resource_start(pdev, GTTMMADR_BAR) + 
gen6_gttadr_offset(i915);
 
/*
 * On BXT+/ICL+ writes larger than 64 bit to the GTT pagetable range
@@ -930,7 +931,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
u16 snb_gmch_ctl;
 
if (!HAS_LMEM(i915)) {
-   ggtt->gmadr = pci_resource(pdev, 2);
+   ggtt->gmadr = pci_resource(pdev, GTT_APERTURE_BAR);
ggtt->mappable_end = resource_size(>gmadr);
}
 
@@ -1084,7 +1085,7 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
unsigned int size;
u16 snb_gmch_ctl;
 
-   ggtt->gmadr = pci_resource(pdev, 2);
+   ggtt->gmadr = pci_resource(pdev, GTT_APERTURE_BAR);

Re: [Intel-gfx] [PULL] drm-intel-next-fixes

2022-08-05 Thread Mauro Carvalho Chehab
On Fri, 5 Aug 2022 10:39:44 -0400
Rodrigo Vivi  wrote:

> On Fri, Aug 05, 2022 at 10:46:57AM +0200, Mauro Carvalho Chehab wrote:
> > Hi Rodrigo,
> > 
> > On Thu, 4 Aug 2022 13:33:06 -0400
> > Rodrigo Vivi  wrote:
> >   
> > > Hi Dave and Daniel,
> > > 
> > > Here goes drm-intel-next-fixes-2022-08-04:
> > > 
> > > - disable pci resize on 32-bit systems (Nirmoy)
> > > - don't leak the ccs state (Matt)
> > > - TLB invalidation fixes (Chris)
> > > 
> > > Thanks,
> > > Rodrigo.
> > > 
> > > The following changes since commit 
> > > 2bc7ea71a73747a77e7f83bc085b0d2393235410:
> > > 
> > >   Merge tag 'topic/nouveau-misc-2022-07-27' of 
> > > git://anongit.freedesktop.org/drm/drm into drm-next (2022-07-27 11:34:07 
> > > +1000)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://anongit.freedesktop.org/drm/drm-intel 
> > > tags/drm-intel-next-fixes-2022-08-04
> > > 
> > > for you to fetch changes up to e57b9369e0c6f60155027e120fafd4b57e385b71:
> > > 
> > >   drm/i915/gt: Batch TLB invalidations (2022-08-01 09:48:06 -0400)
> > > 
> > > 
> > > - disable pci resize on 32-bit systems (Nirmoy)
> > > - don't leak the ccs state (Matt)
> > > - TLB invalidation fixes (Chris)
> > > 
> > > 
> > > Chris Wilson (4):
> > >   drm/i915/gt: Ignore TLB invalidations on idle engines
> > >   drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
> > >   drm/i915/gt: Skip TLB invalidations once wedged  
> >   
> > >   drm/i915/gt: Batch TLB invalidations  
> > This patch actually adds a regression due to a silly mistake. 
> > The fix is here:
> > 
> > https://patchwork.freedesktop.org/patch/496249/?series=106805=4
> >  
> 
> Thank you for the heads up.
> 
> Since that patch is not merged yet, what are your recommendations here?
> Should I remove this from drm-intel-next-fixes now?

Those patches are OK to merge:

   drm/i915/gt: Ignore TLB invalidations on idle engines
   drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
   drm/i915/gt: Skip TLB invalidations once wedged  

And helps reduce performance regressions due to TLB cache invalidation.
So, I would keep them.

With regards to the 4th patch, please don't merge:

drm/i915/gt: Batch TLB invalidations

or merge it together with:

drm/i915: pass a pointer for tlb seqno at vma_invalidate_tlb()

> Which regression is worst?

The regression caused by the batch patch is a lot worse, as it
effectively disables TLB cache invalidation. The fix was meant to
be merged at the patch, but somehow between several rebases and 3
machines involved on tests, internal development and upstream, the
branch actually sent upstream was not the right one. It got only
noticed after reviewing a newer patch. Yeah, that sucks.

> Also dim has trouble with fixes for fixes in the same round.
> Please ping me when you get that patch merged so I can pull that.

OK.

Regards,
Mauro


Re: [Intel-gfx] [PATCH] i915/pmu: Wire GuC backend to per-client busyness

2022-08-05 Thread Umesh Nerlige Ramappa

On Fri, Aug 05, 2022 at 10:45:30AM +0100, Tvrtko Ursulin wrote:


On 05/08/2022 00:21, Umesh Nerlige Ramappa wrote:

From: John Harrison 

GuC provides engine_id and last_switch_in ticks for an active context in
the pphwsp. The context image provides a 32 bit total ticks which is the
accumulated by the context (a.k.a. context[CTX_TIMESTAMP]). This
information is used to calculate the context busyness as follows:

If the engine_id is valid, then busyness is the sum of accumulated total
ticks and active ticks. Active ticks is calculated with current gt time
as reference.

If engine_id is invalid, busyness is equal to accumulated total ticks.

Since KMD (CPU) retrieves busyness data from 2 sources - GPU and GuC, a
potential race was highlighted in an earlier review that can lead to
double accounting of busyness. While the solution to this is a wip,
busyness is still usable for platforms running GuC submission.

Remaining work: Enable and test context busyness for
virtual_parent_context_ops and virtual_child_context_ops.


I meant track the IGT work in the jira internally. :)


Oh, I did do that and added this here as well. Note that I have not 
enabled the busyness in i915 for the parent/child context ops since I 
was not able to verify it yet.




Otherwise:

Acked-by: Tvrtko Ursulin 

Also, can someone else please do the full review? I'm afraid with the 
passage of time I forgot what little I knew about how GuC tracks this 
data. :(


I will ask around



Some nits and questions below.


v2: (Tvrtko)
- Use COPS_RUNTIME_ACTIVE_TOTAL
- Add code comment for the race
- Undo local variables initializations

v3:
- Add support for virtual engines based on
  https://patchwork.freedesktop.org/series/105227/

v4:
- Update commit message with remaining work.
- Rebase

Signed-off-by: John Harrison 
Signed-off-by: Umesh Nerlige Ramappa 
---
 drivers/gpu/drm/i915/gt/intel_context.c   | 12 +++-
 drivers/gpu/drm/i915/gt/intel_context.h   |  6 +-
 drivers/gpu/drm/i915/gt/intel_context_types.h |  6 ++
 drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h   |  5 ++
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 65 ++-
 drivers/gpu/drm/i915/i915_drm_client.c|  6 +-
 6 files changed, 89 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c 
b/drivers/gpu/drm/i915/gt/intel_context.c
index 654a092ed3d6..e2d70a9fdac0 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -576,16 +576,24 @@ void intel_context_bind_parent_child(struct intel_context 
*parent,
child->parallel.parent = parent;
 }
-u64 intel_context_get_total_runtime_ns(const struct intel_context *ce)
+u64 intel_context_get_total_runtime_ns(struct intel_context *ce)
 {
u64 total, active;
+   if (ce->ops->update_stats)
+   ce->ops->update_stats(ce);
+
total = ce->stats.runtime.total;
if (ce->ops->flags & COPS_RUNTIME_CYCLES)
total *= ce->engine->gt->clock_period_ns;
active = READ_ONCE(ce->stats.active);
-   if (active)
+   /*
+* When COPS_RUNTIME_ACTIVE_TOTAL is set for ce->cops, the backend
+* already provides the total active time of the context, so skip this
+* calculation when this flag is set.
+*/
+   if (active && !(ce->ops->flags & COPS_RUNTIME_ACTIVE_TOTAL))
active = intel_context_clock() - active;
return total + active;
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h 
b/drivers/gpu/drm/i915/gt/intel_context.h
index 8e2d70630c49..3d1d7436c1a4 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -58,7 +58,7 @@ static inline bool intel_context_is_parent(struct 
intel_context *ce)
return !!ce->parallel.number_children;
 }
-static inline bool intel_context_is_pinned(struct intel_context *ce);
+static inline bool intel_context_is_pinned(const struct intel_context *ce);
 static inline struct intel_context *
 intel_context_to_parent(struct intel_context *ce)
@@ -118,7 +118,7 @@ static inline int intel_context_lock_pinned(struct 
intel_context *ce)
  * Returns: true if the context is currently pinned for use by the GPU.
  */
 static inline bool
-intel_context_is_pinned(struct intel_context *ce)
+intel_context_is_pinned(const struct intel_context *ce)
 {
return atomic_read(>pin_count);
 }
@@ -362,7 +362,7 @@ intel_context_clear_nopreempt(struct intel_context *ce)
clear_bit(CONTEXT_NOPREEMPT, >flags);
 }
-u64 intel_context_get_total_runtime_ns(const struct intel_context *ce);
+u64 intel_context_get_total_runtime_ns(struct intel_context *ce);
 u64 intel_context_get_avg_runtime_ns(struct intel_context *ce);
 static inline u64 intel_context_clock(void)
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 04eacae1aca5..f7ff4c7d81c7 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ 

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/ttm: remove calc_ctrl_surf_instr_size

2022-08-05 Thread Patchwork
== Series Details ==

Series: series starting with [1/2] drm/i915/ttm: remove 
calc_ctrl_surf_instr_size
URL   : https://patchwork.freedesktop.org/series/107031/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11971 -> Patchwork_107031v1


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/index.html

Participating hosts (43 -> 41)
--

  Missing(2): fi-bdw-samus bat-jsl-3 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_107031v1:

### IGT changes ###

 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@core_hotunplug@unbind-rebind:
- {bat-dg2-10}:   [PASS][1] -> [DMESG-WARN][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/bat-dg2-10/igt@core_hotunp...@unbind-rebind.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/bat-dg2-10/igt@core_hotunp...@unbind-rebind.html

  
Known issues


  Here are the changes found in Patchwork_107031v1 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@gem_render_tiled_blits@basic:
- fi-apl-guc: [PASS][3] -> [INCOMPLETE][4] ([i915#6532])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/fi-apl-guc/igt@gem_render_tiled_bl...@basic.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/fi-apl-guc/igt@gem_render_tiled_bl...@basic.html

  * igt@i915_selftest@live@gem:
- fi-blb-e6850:   NOTRUN -> [DMESG-FAIL][5] ([i915#4528])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/fi-blb-e6850/igt@i915_selftest@l...@gem.html

  
 Possible fixes 

  * igt@i915_module_load@reload:
- {bat-rpls-2}:   [DMESG-WARN][6] ([i915#5950]) -> [PASS][7]
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/bat-rpls-2/igt@i915_module_l...@reload.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/bat-rpls-2/igt@i915_module_l...@reload.html

  * igt@i915_selftest@live@gem_contexts:
- {bat-dg2-8}:[INCOMPLETE][8] ([i915#6523]) -> [PASS][9]
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/bat-dg2-8/igt@i915_selftest@live@gem_contexts.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/bat-dg2-8/igt@i915_selftest@live@gem_contexts.html

  * igt@i915_selftest@live@hangcheck:
- bat-dg1-5:  [DMESG-FAIL][10] ([i915#4957]) -> [PASS][11]
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/bat-dg1-5/igt@i915_selftest@l...@hangcheck.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/bat-dg1-5/igt@i915_selftest@l...@hangcheck.html

  * igt@i915_selftest@live@requests:
- fi-blb-e6850:   [DMESG-FAIL][12] ([i915#4528]) -> [PASS][13]
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/fi-blb-e6850/igt@i915_selftest@l...@requests.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/fi-blb-e6850/igt@i915_selftest@l...@requests.html

  * igt@i915_selftest@live@slpc:
- {bat-rpls-1}:   [DMESG-FAIL][14] ([i915#6367]) -> [PASS][15]
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11971/bat-rpls-1/igt@i915_selftest@l...@slpc.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107031v1/bat-rpls-1/igt@i915_selftest@l...@slpc.html

  
  {name}: This element is suppressed. This means it is ignored when computing
  the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5950]: https://gitlab.freedesktop.org/drm/intel/issues/5950
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6434]: https://gitlab.freedesktop.org/drm/intel/issues/6434
  [i915#6523]: https://gitlab.freedesktop.org/drm/intel/issues/6523
  [i915#6531]: 

[Intel-gfx] [PATCH i-g-t 1/3] tests/gem_exec_fence: Fix wrong engine checked for store_dword capability

2022-08-05 Thread Janusz Krzysztofik
Commit ed7c8cfb67e3 ("tests/i915/gem_exec_fence: Add
__for_each_physical_engine to utilize all engines."), while replacing
depraciated for_each_physical_engine(e, fd) loop with
__for_each_physical_engine(fd, e2) and gem_can_store_dword() inside that
loop with gem_class_can_store_dword(), didn't switch to e2 consequently --
eb_ring(e) argument of gem_can_store_dword() was replaced with e->class
passed to gem_class_can_store_dword() instead of e2->class.  As a result,
a batch that stores dword is now submitted to all engines, also those that
don't support that operation.  Fix it.

Signed-off-by: Janusz Krzysztofik 
---
 tests/i915/gem_exec_fence.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index 7ff7614dad..048870053a 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -331,7 +331,7 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
 
i = 0;
for_each_ctx_engine(fd, ctx, e2) {
-   if (!gem_class_can_store_dword(fd, e->class))
+   if (!gem_class_can_store_dword(fd, e2->class))
continue;
 
if (flags & NONBLOCK) {
-- 
2.25.1



Re: [Intel-gfx] [PULL] drm-intel-next-fixes

2022-08-05 Thread Rodrigo Vivi
On Fri, Aug 05, 2022 at 10:46:57AM +0200, Mauro Carvalho Chehab wrote:
> Hi Rodrigo,
> 
> On Thu, 4 Aug 2022 13:33:06 -0400
> Rodrigo Vivi  wrote:
> 
> > Hi Dave and Daniel,
> > 
> > Here goes drm-intel-next-fixes-2022-08-04:
> > 
> > - disable pci resize on 32-bit systems (Nirmoy)
> > - don't leak the ccs state (Matt)
> > - TLB invalidation fixes (Chris)
> > 
> > Thanks,
> > Rodrigo.
> > 
> > The following changes since commit 2bc7ea71a73747a77e7f83bc085b0d2393235410:
> > 
> >   Merge tag 'topic/nouveau-misc-2022-07-27' of 
> > git://anongit.freedesktop.org/drm/drm into drm-next (2022-07-27 11:34:07 
> > +1000)
> > 
> > are available in the Git repository at:
> > 
> >   git://anongit.freedesktop.org/drm/drm-intel 
> > tags/drm-intel-next-fixes-2022-08-04
> > 
> > for you to fetch changes up to e57b9369e0c6f60155027e120fafd4b57e385b71:
> > 
> >   drm/i915/gt: Batch TLB invalidations (2022-08-01 09:48:06 -0400)
> > 
> > 
> > - disable pci resize on 32-bit systems (Nirmoy)
> > - don't leak the ccs state (Matt)
> > - TLB invalidation fixes (Chris)
> > 
> > 
> > Chris Wilson (4):
> >   drm/i915/gt: Ignore TLB invalidations on idle engines
> >   drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
> >   drm/i915/gt: Skip TLB invalidations once wedged
> 
> >   drm/i915/gt: Batch TLB invalidations
> This patch actually adds a regression due to a silly mistake. 
> The fix is here:
> 
>   https://patchwork.freedesktop.org/patch/496249/?series=106805=4
>

Thank you for the heads up.

Since that patch is not merged yet, what are your recommendations here?
Should I remove this from drm-intel-next-fixes now? Which regression is worst?

Also dim has trouble with fixes for fixes in the same round.
Please ping me when you get that patch merged so I can pull that.

Thanks,
Rodrigo.

> Regards,
> Mauro


[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/ttm: remove calc_ctrl_surf_instr_size

2022-08-05 Thread Patchwork
== Series Details ==

Series: series starting with [1/2] drm/i915/ttm: remove 
calc_ctrl_surf_instr_size
URL   : https://patchwork.freedesktop.org/series/107031/
State : warning

== Summary ==

Error: dim checkpatch failed
ad7f4d913058 drm/i915/ttm: remove calc_ctrl_surf_instr_size
179693b5b5ef drm/i915/ttm: fix CCS handling
-:19: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description 
(prefer a maximum 75 chars per line)
#19: 
  - We are using plain integer in various places, which we can easily overflow

total: 0 errors, 1 warnings, 0 checks, 78 lines checked




[Intel-gfx] [PATCH i-g-t v2 3/3] tests/gem_exec_fence: Restore pre-hang checks in *await-hang scenarios

2022-08-05 Thread Janusz Krzysztofik
Commit c8f6aaf32d83 "tests/gem_exec_fence: Check stored values only for
valid workloads" resolved an issue, observed in *await-hang scenarios,
where a fence exposed by an invalid spin batch was signaled asynchronously
to pending checks for depended test batches still waiting for that fence.
Those checks have been disabled, weakening those scenarios.

This change takes an alternative approach: it makes the invalid spin batch
dependent on another fence so the test has full control over the moment
when that batch starts, triggers a GPU hang and its fence is signaled.
With that in place, the test is able to check synchronously if execution
of dependent test batches is still blocked on the not yet signaled fence
of the not yet completed spin batch, as it does in counterpart non-hanging
scenarios.

v2: preserve checking the pipeline runs ahead of the hang (Chris)

Signed-off-by: Janusz Krzysztofik 
---
 tests/i915/gem_exec_fence.c | 41 ++---
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index 78d83460f7..34f6b89337 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -309,10 +309,10 @@ static void test_fence_await(int fd, const intel_ctx_t 
*ctx,
 {
const struct intel_execution_engine2 *e2;
uint32_t scratch = gem_create(fd, 4096);
-   igt_spin_t *spin;
+   igt_spin_t *spin, *invalid_spin;
uint32_t *out;
uint64_t scratch_offset, ahnd = get_reloc_ahnd(fd, ctx->id);
-   int i;
+   int out_fence, i;
 
scratch_offset = get_offset(ahnd, scratch, 4096, 0);
 
@@ -325,10 +325,25 @@ static void test_fence_await(int fd, const intel_ctx_t 
*ctx,
.ctx = ctx,
.engine = e->flags,
.flags = IGT_SPIN_FENCE_OUT |
-IGT_SPIN_POLL_RUN |
-spin_hang(flags));
+IGT_SPIN_POLL_RUN);
igt_assert(spin->out_fence != -1);
 
+   if (flags & HANG) {
+   invalid_spin = igt_spin_new(fd,
+   .ahnd = ahnd,
+   .ctx = ctx,
+   .engine = e->flags,
+   .fence = spin->out_fence,
+   .flags = IGT_SPIN_FENCE_IN |
+IGT_SPIN_FENCE_OUT |
+IGT_SPIN_POLL_RUN |
+spin_hang(flags));
+   igt_assert(invalid_spin->out_fence != -1);
+   out_fence = invalid_spin->out_fence;
+   } else {
+   out_fence = spin->out_fence;
+   }
+
i = 0;
for_each_ctx_engine(fd, ctx, e2) {
if (!gem_class_can_store_dword(fd, e2->class))
@@ -337,12 +352,12 @@ static void test_fence_await(int fd, const intel_ctx_t 
*ctx,
i++;
 
if (flags & NONBLOCK) {
-   igt_store_word(fd, ahnd, ctx, e2, spin->out_fence,
+   igt_store_word(fd, ahnd, ctx, e2, out_fence,
   scratch, scratch_offset, i, i);
} else {
igt_fork(child, 1) {
ahnd = get_reloc_ahnd(fd, ctx->id);
-   igt_store_word(fd, ahnd, ctx, e2, 
spin->out_fence,
+   igt_store_word(fd, ahnd, ctx, e2, out_fence,
   scratch, scratch_offset, i, i);
put_ahnd(ahnd);
}
@@ -350,6 +365,16 @@ static void test_fence_await(int fd, const intel_ctx_t 
*ctx,
}
 
igt_spin_busywait_until_started(spin);
+   if (flags & HANG) {
+   igt_assert(fence_busy(spin->out_fence));
+   igt_fail_on(igt_spin_has_started(invalid_spin));
+   igt_assert(fence_busy(out_fence));
+   for (int n = 0; n <= i; n++)
+   igt_assert_eq_u32(out[n], 0);
+
+   igt_spin_end(spin);
+   igt_spin_busywait_until_started(invalid_spin);
+   }
/* Long, but not too long to anger preemption disable checks */
usleep(50 * 1000); /* 50 ms, typical preempt reset is 150+ms */
 
@@ -365,7 +390,7 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
igt_waitchildren();
 
gem_set_domain(fd, scratch, I915_GEM_DOMAIN_GTT, 0);
-   igt_assert(!fence_busy(spin->out_fence));
+   igt_assert(!fence_busy(out_fence));
if ((flags & HANG) == 0) {
do
igt_assert_eq_u32(out[i], i);
@@ -373,6 +398,8 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,

[Intel-gfx] [PATCH i-g-t v2 2/3] tests/gem_exec_fence: Exclude 0 from use in store batches

2022-08-05 Thread Janusz Krzysztofik
When i = 0, condition out[i] == i will be true even if a batch supposed
to store i in out[i] fails.  Don't use i = 0 in batches that store values
in out[i].

v2: still check for out[0] == 0 (Chris)

Signed-off-by: Janusz Krzysztofik 
---
 tests/i915/gem_exec_fence.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index 048870053a..78d83460f7 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -334,6 +334,8 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
if (!gem_class_can_store_dword(fd, e2->class))
continue;
 
+   i++;
+
if (flags & NONBLOCK) {
igt_store_word(fd, ahnd, ctx, e2, spin->out_fence,
   scratch, scratch_offset, i, i);
@@ -345,8 +347,6 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
put_ahnd(ahnd);
}
}
-
-   i++;
}
 
igt_spin_busywait_until_started(spin);
@@ -356,7 +356,7 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
if ((flags & HANG) == 0) {
/* Check for invalidly completing the task early */
igt_assert(fence_busy(spin->out_fence));
-   for (int n = 0; n < i; n++)
+   for (int n = 0; n <= i; n++)
igt_assert_eq_u32(out[n], 0);
 
igt_spin_end(spin);
@@ -366,8 +366,11 @@ static void test_fence_await(int fd, const intel_ctx_t 
*ctx,
 
gem_set_domain(fd, scratch, I915_GEM_DOMAIN_GTT, 0);
igt_assert(!fence_busy(spin->out_fence));
-   while ((flags & HANG) == 0 && i--)
-   igt_assert_eq_u32(out[i], i);
+   if ((flags & HANG) == 0) {
+   do
+   igt_assert_eq_u32(out[i], i);
+   while (i--);
+   }
munmap(out, 4096);
 
igt_spin_free(fd, spin);
-- 
2.25.1



Re: [Intel-gfx] How to convert drivers/gpu/drm/i915/ to use local workqueue?

2022-08-05 Thread Tetsuo Handa
On 2022/06/30 22:09, Tvrtko Ursulin wrote:
>>> On the i915 specifics, the caller in 
>>> drivers/gpu/drm/i915/gt/selftest_execlists.c
>>> I am pretty sure can be removed. It is synchronized with the error capture 
>>> side of
>>> things which is not required for the test to work.
>>>
>>> I can send a patch for that or you can, as you prefer?
>>
>> OK. Please send a patch for that, for that patch will go linux-next.git tree 
>> via
>> a tree for gpu/drm/i915 driver.
> 
> Patch sent. If I am right the easiest solution was just to remove the flush.
> If I was wrong though I'll need to create a dedicated wq so we will see what
> our automated CI will say.

How was the result?



[Intel-gfx] [PATCH 1/2] drm/i915/ttm: remove calc_ctrl_surf_instr_size

2022-08-05 Thread Matthew Auld
We only ever need to emit one ccs block copy command.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Ramalingam C 
---
 drivers/gpu/drm/i915/gt/intel_migrate.c | 35 +++--
 1 file changed, 3 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c 
b/drivers/gpu/drm/i915/gt/intel_migrate.c
index 9a0814422ba4..1bbed7aa436a 100644
--- a/drivers/gpu/drm/i915/gt/intel_migrate.c
+++ b/drivers/gpu/drm/i915/gt/intel_migrate.c
@@ -511,44 +511,16 @@ static inline u32 *i915_flush_dw(u32 *cmd, u32 flags)
return cmd;
 }
 
-static u32 calc_ctrl_surf_instr_size(struct drm_i915_private *i915, int size)
-{
-   u32 num_cmds, num_blks, total_size;
-
-   if (!GET_CCS_BYTES(i915, size))
-   return 0;
-
-   /*
-* XY_CTRL_SURF_COPY_BLT transfers CCS in 256 byte
-* blocks. one XY_CTRL_SURF_COPY_BLT command can
-* transfer upto 1024 blocks.
-*/
-   num_blks = DIV_ROUND_UP(GET_CCS_BYTES(i915, size),
-   NUM_CCS_BYTES_PER_BLOCK);
-   num_cmds = DIV_ROUND_UP(num_blks, NUM_CCS_BLKS_PER_XFER);
-   total_size = XY_CTRL_SURF_INSTR_SIZE * num_cmds;
-
-   /*
-* Adding a flush before and after XY_CTRL_SURF_COPY_BLT
-*/
-   total_size += 2 * MI_FLUSH_DW_SIZE;
-
-   return total_size;
-}
-
 static int emit_copy_ccs(struct i915_request *rq,
 u32 dst_offset, u8 dst_access,
 u32 src_offset, u8 src_access, int size)
 {
struct drm_i915_private *i915 = rq->engine->i915;
int mocs = rq->engine->gt->mocs.uc_index << 1;
-   u32 num_ccs_blks, ccs_ring_size;
+   u32 num_ccs_blks;
u32 *cs;
 
-   ccs_ring_size = calc_ctrl_surf_instr_size(i915, size);
-   WARN_ON(!ccs_ring_size);
-
-   cs = intel_ring_begin(rq, round_up(ccs_ring_size, 2));
+   cs = intel_ring_begin(rq, 12);
if (IS_ERR(cs))
return PTR_ERR(cs);
 
@@ -583,8 +555,7 @@ static int emit_copy_ccs(struct i915_request *rq,
FIELD_PREP(XY_CTRL_SURF_MOCS_MASK, mocs);
 
cs = i915_flush_dw(cs, MI_FLUSH_DW_LLC | MI_FLUSH_DW_CCS);
-   if (ccs_ring_size & 1)
-   *cs++ = MI_NOOP;
+   *cs++ = MI_NOOP;
 
intel_ring_advance(rq, cs);
 
-- 
2.37.1



[Intel-gfx] [PATCH 2/2] drm/i915/ttm: fix CCS handling

2022-08-05 Thread Matthew Auld
Crucible + recent Mesa seems to sometimes hit:

GEM_BUG_ON(num_ccs_blks > NUM_CCS_BLKS_PER_XFER)

And it looks like we can also trigger this with gem_lmem_swapping, if we
modify the test to use slightly larger object sizes.

Looking closer it looks like we have the following issues in
migrate_copy():

  - We are using plain integer in various places, which we can easily overflow
with a large object.

  - We pass the entire object size (when the src is lmem) into
emit_pte() and then try to copy it, which doesn't work, since we
only have a few fixed sized windows in which to map the pages and
perform the copy. With an object > 8M we therefore aren't properly
copying the pages. And then with an object > 64M we trigger the
GEM_BUG_ON(num_ccs_blks > NUM_CCS_BLKS_PER_XFER).

So it looks like our copy handling for any object > 8M (which is our
CHUNK_SZ) is currently broken on DG2.

Fixes: da0595ae91da ("drm/i915/migrate: Evict and restore the flatccs capable 
lmem obj")
Testcase: igt@gem_lmem_swapping@basic-big
Testcase: igt@gem_lmem_swapping@verify-ccs-big
Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Ramalingam C 
---
 drivers/gpu/drm/i915/gt/intel_migrate.c | 44 -
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c 
b/drivers/gpu/drm/i915/gt/intel_migrate.c
index 1bbed7aa436a..aaaf1906026c 100644
--- a/drivers/gpu/drm/i915/gt/intel_migrate.c
+++ b/drivers/gpu/drm/i915/gt/intel_migrate.c
@@ -609,9 +609,9 @@ static int emit_copy(struct i915_request *rq,
return 0;
 }
 
-static int scatter_list_length(struct scatterlist *sg)
+static u64 scatter_list_length(struct scatterlist *sg)
 {
-   int len = 0;
+   u64 len = 0;
 
while (sg && sg_dma_len(sg)) {
len += sg_dma_len(sg);
@@ -621,28 +621,26 @@ static int scatter_list_length(struct scatterlist *sg)
return len;
 }
 
-static void
+static int
 calculate_chunk_sz(struct drm_i915_private *i915, bool src_is_lmem,
-  int *src_sz, u32 bytes_to_cpy, u32 ccs_bytes_to_cpy)
+  u64 bytes_to_cpy, u64 ccs_bytes_to_cpy)
 {
-   if (ccs_bytes_to_cpy) {
-   if (!src_is_lmem)
-   /*
-* When CHUNK_SZ is passed all the pages upto CHUNK_SZ
-* will be taken for the blt. in Flat-ccs supported
-* platform Smem obj will have more pages than required
-* for main meory hence limit it to the required size
-* for main memory
-*/
-   *src_sz = min_t(int, bytes_to_cpy, CHUNK_SZ);
-   } else { /* ccs handling is not required */
-   *src_sz = CHUNK_SZ;
-   }
+   if (ccs_bytes_to_cpy && !src_is_lmem)
+   /*
+* When CHUNK_SZ is passed all the pages upto CHUNK_SZ
+* will be taken for the blt. in Flat-ccs supported
+* platform Smem obj will have more pages than required
+* for main meory hence limit it to the required size
+* for main memory
+*/
+   return min_t(u64, bytes_to_cpy, CHUNK_SZ);
+   else
+   return CHUNK_SZ;
 }
 
-static void get_ccs_sg_sgt(struct sgt_dma *it, u32 bytes_to_cpy)
+static void get_ccs_sg_sgt(struct sgt_dma *it, u64 bytes_to_cpy)
 {
-   u32 len;
+   u64 len;
 
do {
GEM_BUG_ON(!it->sg || !sg_dma_len(it->sg));
@@ -673,12 +671,12 @@ intel_context_migrate_copy(struct intel_context *ce,
 {
struct sgt_dma it_src = sg_sgt(src), it_dst = sg_sgt(dst), it_ccs;
struct drm_i915_private *i915 = ce->engine->i915;
-   u32 ccs_bytes_to_cpy = 0, bytes_to_cpy;
+   u64 ccs_bytes_to_cpy = 0, bytes_to_cpy;
enum i915_cache_level ccs_cache_level;
u32 src_offset, dst_offset;
u8 src_access, dst_access;
struct i915_request *rq;
-   int src_sz, dst_sz;
+   u64 src_sz, dst_sz;
bool ccs_is_src, overwrite_ccs;
int err;
 
@@ -761,8 +759,8 @@ intel_context_migrate_copy(struct intel_context *ce,
if (err)
goto out_rq;
 
-   calculate_chunk_sz(i915, src_is_lmem, _sz,
-  bytes_to_cpy, ccs_bytes_to_cpy);
+   src_sz = calculate_chunk_sz(i915, src_is_lmem,
+   bytes_to_cpy, ccs_bytes_to_cpy);
 
len = emit_pte(rq, _src, src_cache_level, src_is_lmem,
   src_offset, src_sz);
-- 
2.37.1



[Intel-gfx] [PATCH i-g-t] tests/gem_lmem_swapping: add some coverage for big objects

2022-08-05 Thread Matthew Auld
Everything we are testing here unfortunately fits within one packet (8M)
which means we have zero coverage when we need to split the copy over
multiple packets (including the aux CCS state).

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Ramalingam C 
---
 tests/i915/gem_lmem_swapping.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
index 1a4f4ca5..67e95cea 100644
--- a/tests/i915/gem_lmem_swapping.c
+++ b/tests/i915/gem_lmem_swapping.c
@@ -63,6 +63,7 @@ struct params {
 #define TEST_ENGINES   (1 << 4)
 #define TEST_MULTI (1 << 5)
 #define TEST_CCS   (1 << 6)
+#define TEST_BIG   (1 << 7)
unsigned int flags;
unsigned int seed;
bool oom_test;
@@ -477,8 +478,8 @@ static void fill_params(int i915, struct params *params,
 {
const int swap_mb = /* For lmem, swap is total of smem + swap. */
igt_get_total_ram_mb() + igt_get_total_swap_mb();
-   const unsigned int size = 1 << 20;
const int max_swap_pct = 75;
+   unsigned int size;
/*
 * In random mode, add 85% hard limit to use system memory.
 * noticed that 88.8% can trigger OOM on some system.
@@ -487,6 +488,17 @@ static void fill_params(int i915, struct params *params,
int spill_mb;
uint32_t handle;
 
+   size = 1 << 20;
+   if (flags & TEST_BIG) {
+   /*
+* The kernel only copies in small chunks, so make sure we
+* have some coverage where multiple packets are needed to copy
+* the entire object.
+*/
+   size = 1u << 26; /* 64M */
+   size += 1u << 16; /* So we also exceed NUM_CCS_BLKS_PER_XFER */
+   }
+
if (flags & TEST_RANDOM) {
params->size.min = 4096;
handle = create_bo(i915, >size.min, >region,
@@ -733,6 +745,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
unsigned int flags;
} *test, tests[] = {
{ "basic", 0 },
+   { "basic-big", TEST_BIG },
{ "random", TEST_RANDOM },
{ "random-engines", TEST_RANDOM | TEST_ENGINES },
{ "heavy-random", TEST_RANDOM | TEST_HEAVY },
@@ -746,6 +759,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
{ "parallel-random-verify", TEST_PARALLEL | TEST_RANDOM | 
TEST_VERIFY },
{ "parallel-multi", TEST_PARALLEL | TEST_RANDOM | TEST_VERIFY | 
TEST_ENGINES | TEST_MULTI },
{ "verify-ccs", TEST_CCS },
+   { "verify-ccs-big", TEST_CCS | TEST_BIG },
{ "verify-random-ccs", TEST_CCS | TEST_RANDOM },
{ "heavy-verify-random-ccs", TEST_CCS | TEST_RANDOM | 
TEST_HEAVY },
{ "heavy-verify-multi-ccs", TEST_CCS | TEST_RANDOM | TEST_HEAVY 
| TEST_ENGINES | TEST_MULTI },
-- 
2.37.1



[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [CI,1/2] drm/i915/edid: convert DP, HDMI and LVDS to drm_edid

2022-08-05 Thread Patchwork
== Series Details ==

Series: series starting with [CI,1/2] drm/i915/edid: convert DP, HDMI and LVDS 
to drm_edid
URL   : https://patchwork.freedesktop.org/series/107020/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11970_full -> Patchwork_107020v1_full


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_107020v1_full absolutely need 
to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_107020v1_full, please notify your bug team to allow 
them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (12 -> 12)
--

  No changes in participating hosts

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_107020v1_full:

### IGT changes ###

 Possible regressions 

  * igt@i915_pm_rpm@gem-execbuf-stress@extra-wait-smem0:
- shard-apl:  [PASS][1] -> [DMESG-WARN][2] +1 similar issue
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-apl2/igt@i915_pm_rpm@gem-execbuf-str...@extra-wait-smem0.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-apl2/igt@i915_pm_rpm@gem-execbuf-str...@extra-wait-smem0.html
- shard-kbl:  [PASS][3] -> [DMESG-WARN][4]
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-kbl4/igt@i915_pm_rpm@gem-execbuf-str...@extra-wait-smem0.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-kbl4/igt@i915_pm_rpm@gem-execbuf-str...@extra-wait-smem0.html

  * igt@i915_pm_rpm@system-suspend-execbuf:
- shard-glk:  [PASS][5] -> [FAIL][6] +1 similar issue
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk7/igt@i915_pm_...@system-suspend-execbuf.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-glk9/igt@i915_pm_...@system-suspend-execbuf.html

  * igt@kms_3d:
- shard-kbl:  [PASS][7] -> [DMESG-FAIL][8]
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-kbl7/igt@kms_3d.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-kbl7/igt@kms_3d.html
- shard-snb:  [PASS][9] -> [DMESG-FAIL][10]
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-snb5/igt@kms_3d.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-snb6/igt@kms_3d.html
- shard-skl:  [PASS][11] -> [DMESG-FAIL][12]
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl9/igt@kms_3d.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-skl10/igt@kms_3d.html

  * igt@kms_hdmi_inject@inject-4k:
- shard-kbl:  [PASS][13] -> [INCOMPLETE][14]
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-kbl1/igt@kms_hdmi_inj...@inject-4k.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-kbl1/igt@kms_hdmi_inj...@inject-4k.html
- shard-skl:  NOTRUN -> [INCOMPLETE][15] +1 similar issue
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-skl10/igt@kms_hdmi_inj...@inject-4k.html

  * igt@kms_hdmi_inject@inject-audio:
- shard-snb:  [PASS][16] -> [INCOMPLETE][17]
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-snb2/igt@kms_hdmi_inj...@inject-audio.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-snb5/igt@kms_hdmi_inj...@inject-audio.html
- shard-skl:  [PASS][18] -> [INCOMPLETE][19]
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl6/igt@kms_hdmi_inj...@inject-audio.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-skl7/igt@kms_hdmi_inj...@inject-audio.html

  
 Warnings 

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc:
- shard-apl:  [SKIP][20] ([fdo#109271]) -> [TIMEOUT][21]
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-apl7/igt@kms_frontbuffer_track...@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc.html
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-apl1/igt@kms_frontbuffer_track...@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
- shard-kbl:  [FAIL][22] ([fdo#108145] / [i915#265]) -> 
[INCOMPLETE][23]
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-kbl1/igt@kms_plane_alpha_bl...@pipe-a-constant-alpha-max.html
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/shard-kbl1/igt@kms_plane_alpha_bl...@pipe-a-constant-alpha-max.html

  
 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * 

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [CI,1/2] drm/i915/edid: convert DP, HDMI and LVDS to drm_edid

2022-08-05 Thread Patchwork
== Series Details ==

Series: series starting with [CI,1/2] drm/i915/edid: convert DP, HDMI and LVDS 
to drm_edid
URL   : https://patchwork.freedesktop.org/series/107020/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11970 -> Patchwork_107020v1


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/index.html

Participating hosts (43 -> 42)
--

  Additional (1): bat-dg2-10 
  Missing(2): fi-skl-guc fi-bdw-samus 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_107020v1:

### IGT changes ###

 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_huc_copy@huc-copy:
- {bat-dg2-10}:   NOTRUN -> [SKIP][1]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/bat-dg2-10/igt@gem_huc_c...@huc-copy.html

  * igt@i915_module_load@reload:
- {bat-dg2-10}:   NOTRUN -> [DMESG-WARN][2]
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/bat-dg2-10/igt@i915_module_l...@reload.html

  
Known issues


  Here are the changes found in Patchwork_107020v1 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@kms_chamelium@common-hpd-after-suspend:
- fi-bdw-5557u:   NOTRUN -> [SKIP][3] ([fdo#109271] / [fdo#111827])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/fi-bdw-5557u/igt@kms_chamel...@common-hpd-after-suspend.html

  * igt@kms_frontbuffer_tracking@basic:
- fi-cfl-8109u:   [PASS][4] -> [DMESG-FAIL][5] ([i915#62])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/fi-cfl-8109u/igt@kms_frontbuffer_track...@basic.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/fi-cfl-8109u/igt@kms_frontbuffer_track...@basic.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1:
- fi-cfl-8109u:   [PASS][6] -> [DMESG-WARN][7] ([i915#62]) +11 similar 
issues
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/fi-cfl-8109u/igt@kms_pipe_crc_basic@nonblocking-crc-frame-seque...@pipe-c-dp-1.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/fi-cfl-8109u/igt@kms_pipe_crc_basic@nonblocking-crc-frame-seque...@pipe-c-dp-1.html

  
 Possible fixes 

  * igt@i915_selftest@live@gt_heartbeat:
- fi-skl-6700k2:  [DMESG-FAIL][8] ([i915#5334]) -> [PASS][9]
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/fi-skl-6700k2/igt@i915_selftest@live@gt_heartbeat.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/fi-skl-6700k2/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@hangcheck:
- {fi-ehl-2}: [INCOMPLETE][10] ([i915#5153] / [i915#6106]) -> 
[PASS][11]
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/fi-ehl-2/igt@i915_selftest@l...@hangcheck.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/fi-ehl-2/igt@i915_selftest@l...@hangcheck.html

  * igt@i915_selftest@live@requests:
- {bat-rpls-1}:   [INCOMPLETE][12] ([i915#4983] / [i915#6380]) -> 
[PASS][13]
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/bat-rpls-1/igt@i915_selftest@l...@requests.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/bat-rpls-1/igt@i915_selftest@l...@requests.html

  * igt@i915_suspend@basic-s3-without-i915:
- fi-bdw-5557u:   [INCOMPLETE][14] ([i915#146]) -> [PASS][15]
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/fi-bdw-5557u/igt@i915_susp...@basic-s3-without-i915.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/fi-bdw-5557u/igt@i915_susp...@basic-s3-without-i915.html

  
 Warnings 

  * igt@i915_selftest@live@hangcheck:
- bat-dg1-5:  [DMESG-FAIL][16] ([i915#4494] / [i915#4957]) -> 
[DMESG-FAIL][17] ([i915#4957])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/bat-dg1-5/igt@i915_selftest@l...@hangcheck.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/bat-dg1-5/igt@i915_selftest@l...@hangcheck.html
- bat-dg1-6:  [DMESG-FAIL][18] ([i915#4494] / [i915#4957]) -> 
[DMESG-FAIL][19] ([i915#4957])
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/bat-dg1-6/igt@i915_selftest@l...@hangcheck.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107020v1/bat-dg1-6/igt@i915_selftest@l...@hangcheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
  the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#111827]: 

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/hdcp: register cleanup (rev3)

2022-08-05 Thread Patchwork
== Series Details ==

Series: drm/i915/hdcp: register cleanup (rev3)
URL   : https://patchwork.freedesktop.org/series/106964/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11970_full -> Patchwork_106964v3_full


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_106964v3_full absolutely need 
to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_106964v3_full, please notify your bug team to allow 
them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (12 -> 12)
--

  No changes in participating hosts

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_106964v3_full:

### IGT changes ###

 Possible regressions 

  * igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions:
- shard-skl:  NOTRUN -> [INCOMPLETE][1]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/shard-skl6/igt@kms_cursor_legacy@flip-vs-cur...@atomic-transitions.html

  
 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@kms_rotation_crc@primary-y-tiled-reflect-x-90:
- {shard-rkl}:[SKIP][2] ([i915#1845] / [i915#4098]) -> 
[INCOMPLETE][3]
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-rkl-5/igt@kms_rotation_...@primary-y-tiled-reflect-x-90.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/shard-rkl-6/igt@kms_rotation_...@primary-y-tiled-reflect-x-90.html

  
Known issues


  Here are the changes found in Patchwork_106964v3_full that come from known 
issues:

### CI changes ###

 Issues hit 

  * boot:
- shard-glk:  ([PASS][4], [PASS][5], [PASS][6], [PASS][7], 
[PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], 
[PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], 
[PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25], 
[PASS][26], [PASS][27], [PASS][28]) -> ([PASS][29], [FAIL][30], [PASS][31], 
[PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], 
[PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], 
[PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], 
[PASS][50], [PASS][51], [PASS][52], [PASS][53]) ([i915#4392])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk9/boot.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk9/boot.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk9/boot.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk9/boot.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk8/boot.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk8/boot.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk8/boot.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk7/boot.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk7/boot.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk7/boot.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk6/boot.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk6/boot.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk6/boot.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk5/boot.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk5/boot.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk5/boot.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk3/boot.html
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk3/boot.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk3/boot.html
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk2/boot.html
   [24]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk2/boot.html
   [25]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk2/boot.html
   [26]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk1/boot.html
   [27]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk1/boot.html
   [28]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-glk1/boot.html
   [29]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/shard-glk2/boot.html
   [30]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/shard-glk2/boot.html
   [31]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/shard-glk2/boot.html
   [32]: 

Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gt: document TLB cache invalidation functions

2022-08-05 Thread Mauro Carvalho Chehab
On Fri, 5 Aug 2022 10:24:25 +0100
Tvrtko Ursulin  wrote:

> On 05/08/2022 10:08, Andi Shyti wrote:
> > Hi Randy,
> >   
> >>> +/**
> >>> + * intel_gt_invalidate_tlb_full - do full TLB cache invalidation
> >>> + * @gt: GT structure  
> >>
> >> In multiple places (here and below) it would be nice to know what a
> >> GT structure is. I looked thru multiple C and header files yesterday
> >> and didn't find any comments about it.
> >>
> >> Just saying that @gt is a GT structure isn't very helpful, other
> >> than making kernel-doc shut up.  
> > 
> > the 'gt' belongs to the drivers/gpu/drm/i915/gt/ subsystem and
> > it's widely used a throughout i915.
> > 
> > I think it's inappropriate to describe it just here. On the other
> > hand I agree that a better documentation is required for the GT
> > itself where other parts can point to.  

GT is actually a well-understood term for GPU developers. It is an alias
for:

https://en.wikipedia.org/wiki/Intel_Graphics_Technology

It is basically the "core" of the GPU, where the engine units sit.

I agree with Andi: terms like this should likely be defined on a glossary
at i915.rst file.

> Yeah agreed there is no point of copy pasting some explanation all over 
> the place. Could we just do s/GT structure/struct intel_gt/, or "pointer 
> to struct intel_gt to operate on", would that be good enough?

IMO, it won't make any difference. kerneldoc already says that the
parameter has struct intel_gt type on its output:

.. c:function:: void intel_gt_fini_tlb (struct intel_gt *gt)

   free TLB-specific vars

**Parameters**

``struct intel_gt *gt``
  GT structure

**Description**

Frees any resources needed by TLB cache invalidation logic.

This struct somewhat is similar to struct device. This is a container
struct that has the common data needed to control the GT hardware.

Almost all functions that work with GT needs it. There's not much sense
to describe it everywhere. What makes sense is to have struct intel_gt
documented at intel_gt_types.h, letting the build system to generate 
hiperlinks to it.

This is easier said than done...

Regards,
Mauro


[Intel-gfx] [CI 1/2] drm/i915/edid: convert DP, HDMI and LVDS to drm_edid

2022-08-05 Thread Jani Nikula
Convert all the connectors that use cached connector edid and
detect_edid to drm_edid.

Since drm_get_edid() calls drm_connector_update_edid_property() while
drm_edid_read*() do not, we need to call drm_edid_connector_update()
separately, in part due to the EDID caching behaviour in HDMI and
DP. Especially DP depends on the details parsed from EDID. (The big
behavioural change conflating EDID reading with parsing and property
update was done in commit 5186421cbfe2 ("drm: Introduce epoch counter to
drm_connector"))

v4: Call drm_edid_connector_update() after reading HDMI/DP EDID

v3: Don't leak vga switcheroo EDID in LVDS init (Ville)

v2: Don't leak opregion fallback EDID (Ville)

Reviewed-by: Ville Syrjälä 
Signed-off-by: Jani Nikula 
---
 .../gpu/drm/i915/display/intel_connector.c|  4 +-
 .../drm/i915/display/intel_display_types.h|  4 +-
 drivers/gpu/drm/i915/display/intel_dp.c   | 80 +++
 drivers/gpu/drm/i915/display/intel_hdmi.c | 28 ---
 drivers/gpu/drm/i915/display/intel_lvds.c | 37 +
 5 files changed, 87 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_connector.c 
b/drivers/gpu/drm/i915/display/intel_connector.c
index 1dcc268927a2..d83b2a64f618 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -95,12 +95,12 @@ void intel_connector_destroy(struct drm_connector 
*connector)
 {
struct intel_connector *intel_connector = to_intel_connector(connector);
 
-   kfree(intel_connector->detect_edid);
+   drm_edid_free(intel_connector->detect_edid);
 
intel_hdcp_cleanup(intel_connector);
 
if (!IS_ERR_OR_NULL(intel_connector->edid))
-   kfree(intel_connector->edid);
+   drm_edid_free(intel_connector->edid);
 
intel_panel_fini(intel_connector);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 0da9b208d56e..d476df0ac9df 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -592,8 +592,8 @@ struct intel_connector {
struct intel_panel panel;
 
/* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
-   struct edid *edid;
-   struct edid *detect_edid;
+   const struct drm_edid *edid;
+   const struct drm_edid *detect_edid;
 
/* Number of times hotplug detection was tried after an HPD interrupt */
int hotplug_retries;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 32292c0be2bd..8a3b2dbebe04 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3577,12 +3577,11 @@ static u8 intel_dp_autotest_edid(struct intel_dp 
*intel_dp)
intel_dp->aux.i2c_defer_count);
intel_dp->compliance.test_data.edid = 
INTEL_DP_RESOLUTION_FAILSAFE;
} else {
-   struct edid *block = intel_connector->detect_edid;
+   /* FIXME: Get rid of drm_edid_raw() */
+   const struct edid *block = 
drm_edid_raw(intel_connector->detect_edid);
 
-   /* We have to write the checksum
-* of the last block read
-*/
-   block += intel_connector->detect_edid->extensions;
+   /* We have to write the checksum of the last block read */
+   block += block->extensions;
 
if (drm_dp_dpcd_writeb(_dp->aux, DP_TEST_EDID_CHECKSUM,
   block->checksum) <= 0)
@@ -4461,7 +4460,7 @@ bool intel_digital_port_connected(struct intel_encoder 
*encoder)
return is_connected;
 }
 
-static struct edid *
+static const struct drm_edid *
 intel_dp_get_edid(struct intel_dp *intel_dp)
 {
struct intel_connector *intel_connector = intel_dp->attached_connector;
@@ -4472,18 +4471,22 @@ intel_dp_get_edid(struct intel_dp *intel_dp)
if (IS_ERR(intel_connector->edid))
return NULL;
 
-   return drm_edid_duplicate(intel_connector->edid);
+   return drm_edid_dup(intel_connector->edid);
} else
-   return drm_get_edid(_connector->base,
-   _dp->aux.ddc);
+   return drm_edid_read_ddc(_connector->base,
+_dp->aux.ddc);
 }
 
 static void
 intel_dp_update_dfp(struct intel_dp *intel_dp,
-   const struct edid *edid)
+   const struct drm_edid *drm_edid)
 {
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
struct intel_connector *connector = intel_dp->attached_connector;
+   const struct edid *edid;
+
+   /* FIXME: Get rid of drm_edid_raw() */
+   edid = drm_edid_raw(drm_edid);
 
intel_dp->dfp.max_bpc =

[Intel-gfx] [CI 2/2] drm/i915/bios: convert intel_bios_init_panel() to drm_edid

2022-08-05 Thread Jani Nikula
Try to use struct drm_edid where possible, even if having to fall back
to looking into struct edid down low via drm_edid_raw().

v2: Rebase

Signed-off-by: Jani Nikula 
Reviewed-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_bios.c | 19 ++-
 drivers/gpu/drm/i915/display/intel_bios.h |  4 ++--
 drivers/gpu/drm/i915/display/intel_dp.c   |  2 +-
 drivers/gpu/drm/i915/display/intel_lvds.c |  2 +-
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
b/drivers/gpu/drm/i915/display/intel_bios.c
index 51dde5bfd956..2fa296d8e69d 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -606,14 +606,14 @@ get_lfp_data_tail(const struct bdb_lvds_lfp_data *data,
 
 static int opregion_get_panel_type(struct drm_i915_private *i915,
   const struct intel_bios_encoder_data 
*devdata,
-  const struct edid *edid)
+  const struct drm_edid *drm_edid)
 {
return intel_opregion_get_panel_type(i915);
 }
 
 static int vbt_get_panel_type(struct drm_i915_private *i915,
  const struct intel_bios_encoder_data *devdata,
- const struct edid *edid)
+ const struct drm_edid *drm_edid)
 {
const struct bdb_lvds_options *lvds_options;
 
@@ -638,12 +638,13 @@ static int vbt_get_panel_type(struct drm_i915_private 
*i915,
 
 static int pnpid_get_panel_type(struct drm_i915_private *i915,
const struct intel_bios_encoder_data *devdata,
-   const struct edid *edid)
+   const struct drm_edid *drm_edid)
 {
const struct bdb_lvds_lfp_data *data;
const struct bdb_lvds_lfp_data_ptrs *ptrs;
const struct lvds_pnp_id *edid_id;
struct lvds_pnp_id edid_id_nodate;
+   const struct edid *edid = drm_edid_raw(drm_edid); /* FIXME */
int i, best = -1;
 
if (!edid)
@@ -685,7 +686,7 @@ static int pnpid_get_panel_type(struct drm_i915_private 
*i915,
 
 static int fallback_get_panel_type(struct drm_i915_private *i915,
   const struct intel_bios_encoder_data 
*devdata,
-  const struct edid *edid)
+  const struct drm_edid *drm_edid)
 {
return 0;
 }
@@ -699,13 +700,13 @@ enum panel_type {
 
 static int get_panel_type(struct drm_i915_private *i915,
  const struct intel_bios_encoder_data *devdata,
- const struct edid *edid)
+ const struct drm_edid *drm_edid)
 {
struct {
const char *name;
int (*get_panel_type)(struct drm_i915_private *i915,
  const struct intel_bios_encoder_data 
*devdata,
- const struct edid *edid);
+ const struct drm_edid *drm_edid);
int panel_type;
} panel_types[] = {
[PANEL_TYPE_OPREGION] = {
@@ -728,7 +729,7 @@ static int get_panel_type(struct drm_i915_private *i915,
int i;
 
for (i = 0; i < ARRAY_SIZE(panel_types); i++) {
-   panel_types[i].panel_type = panel_types[i].get_panel_type(i915, 
devdata, edid);
+   panel_types[i].panel_type = panel_types[i].get_panel_type(i915, 
devdata, drm_edid);
 
drm_WARN_ON(>drm, panel_types[i].panel_type > 0xf &&
panel_types[i].panel_type != 0xff);
@@ -3144,11 +3145,11 @@ void intel_bios_init(struct drm_i915_private *i915)
 void intel_bios_init_panel(struct drm_i915_private *i915,
   struct intel_panel *panel,
   const struct intel_bios_encoder_data *devdata,
-  const struct edid *edid)
+  const struct drm_edid *drm_edid)
 {
init_vbt_panel_defaults(panel);
 
-   panel->vbt.panel_type = get_panel_type(i915, devdata, edid);
+   panel->vbt.panel_type = get_panel_type(i915, devdata, drm_edid);
 
parse_panel_options(i915, panel);
parse_generic_dtd(i915, panel);
diff --git a/drivers/gpu/drm/i915/display/intel_bios.h 
b/drivers/gpu/drm/i915/display/intel_bios.h
index e47582b0de0a..defea578a768 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.h
+++ b/drivers/gpu/drm/i915/display/intel_bios.h
@@ -32,8 +32,8 @@
 
 #include 
 
+struct drm_edid;
 struct drm_i915_private;
-struct edid;
 struct intel_bios_encoder_data;
 struct intel_crtc_state;
 struct intel_encoder;
@@ -235,7 +235,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv);
 void intel_bios_init_panel(struct drm_i915_private *dev_priv,
   struct intel_panel *panel,
   const struct 

Re: [Intel-gfx] [PATCH] i915/pmu: Wire GuC backend to per-client busyness

2022-08-05 Thread Tvrtko Ursulin



On 05/08/2022 00:21, Umesh Nerlige Ramappa wrote:

From: John Harrison 

GuC provides engine_id and last_switch_in ticks for an active context in
the pphwsp. The context image provides a 32 bit total ticks which is the
accumulated by the context (a.k.a. context[CTX_TIMESTAMP]). This
information is used to calculate the context busyness as follows:

If the engine_id is valid, then busyness is the sum of accumulated total
ticks and active ticks. Active ticks is calculated with current gt time
as reference.

If engine_id is invalid, busyness is equal to accumulated total ticks.

Since KMD (CPU) retrieves busyness data from 2 sources - GPU and GuC, a
potential race was highlighted in an earlier review that can lead to
double accounting of busyness. While the solution to this is a wip,
busyness is still usable for platforms running GuC submission.

Remaining work: Enable and test context busyness for
virtual_parent_context_ops and virtual_child_context_ops.


I meant track the IGT work in the jira internally. :)

Otherwise:

Acked-by: Tvrtko Ursulin 

Also, can someone else please do the full review? I'm afraid with the 
passage of time I forgot what little I knew about how GuC tracks this 
data. :(


Some nits and questions below.


v2: (Tvrtko)
- Use COPS_RUNTIME_ACTIVE_TOTAL
- Add code comment for the race
- Undo local variables initializations

v3:
- Add support for virtual engines based on
   https://patchwork.freedesktop.org/series/105227/

v4:
- Update commit message with remaining work.
- Rebase

Signed-off-by: John Harrison 
Signed-off-by: Umesh Nerlige Ramappa 
---
  drivers/gpu/drm/i915/gt/intel_context.c   | 12 +++-
  drivers/gpu/drm/i915/gt/intel_context.h   |  6 +-
  drivers/gpu/drm/i915/gt/intel_context_types.h |  6 ++
  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h   |  5 ++
  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 65 ++-
  drivers/gpu/drm/i915/i915_drm_client.c|  6 +-
  6 files changed, 89 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c 
b/drivers/gpu/drm/i915/gt/intel_context.c
index 654a092ed3d6..e2d70a9fdac0 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -576,16 +576,24 @@ void intel_context_bind_parent_child(struct intel_context 
*parent,
child->parallel.parent = parent;
  }
  
-u64 intel_context_get_total_runtime_ns(const struct intel_context *ce)

+u64 intel_context_get_total_runtime_ns(struct intel_context *ce)
  {
u64 total, active;
  
+	if (ce->ops->update_stats)

+   ce->ops->update_stats(ce);
+
total = ce->stats.runtime.total;
if (ce->ops->flags & COPS_RUNTIME_CYCLES)
total *= ce->engine->gt->clock_period_ns;
  
  	active = READ_ONCE(ce->stats.active);

-   if (active)
+   /*
+* When COPS_RUNTIME_ACTIVE_TOTAL is set for ce->cops, the backend
+* already provides the total active time of the context, so skip this
+* calculation when this flag is set.
+*/
+   if (active && !(ce->ops->flags & COPS_RUNTIME_ACTIVE_TOTAL))
active = intel_context_clock() - active;
  
  	return total + active;

diff --git a/drivers/gpu/drm/i915/gt/intel_context.h 
b/drivers/gpu/drm/i915/gt/intel_context.h
index 8e2d70630c49..3d1d7436c1a4 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -58,7 +58,7 @@ static inline bool intel_context_is_parent(struct 
intel_context *ce)
return !!ce->parallel.number_children;
  }
  
-static inline bool intel_context_is_pinned(struct intel_context *ce);

+static inline bool intel_context_is_pinned(const struct intel_context *ce);
  
  static inline struct intel_context *

  intel_context_to_parent(struct intel_context *ce)
@@ -118,7 +118,7 @@ static inline int intel_context_lock_pinned(struct 
intel_context *ce)
   * Returns: true if the context is currently pinned for use by the GPU.
   */
  static inline bool
-intel_context_is_pinned(struct intel_context *ce)
+intel_context_is_pinned(const struct intel_context *ce)
  {
return atomic_read(>pin_count);
  }
@@ -362,7 +362,7 @@ intel_context_clear_nopreempt(struct intel_context *ce)
clear_bit(CONTEXT_NOPREEMPT, >flags);
  }
  
-u64 intel_context_get_total_runtime_ns(const struct intel_context *ce);

+u64 intel_context_get_total_runtime_ns(struct intel_context *ce);
  u64 intel_context_get_avg_runtime_ns(struct intel_context *ce);
  
  static inline u64 intel_context_clock(void)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 04eacae1aca5..f7ff4c7d81c7 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -38,6 +38,9 @@ struct intel_context_ops {
  #define COPS_RUNTIME_CYCLES_BIT 1
  #define COPS_RUNTIME_CYCLES BIT(COPS_RUNTIME_CYCLES_BIT)
  
+#define COPS_RUNTIME_ACTIVE_TOTAL_BIT 

Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gt: document TLB cache invalidation functions

2022-08-05 Thread Tvrtko Ursulin



On 05/08/2022 10:08, Andi Shyti wrote:

Hi Randy,


+/**
+ * intel_gt_invalidate_tlb_full - do full TLB cache invalidation
+ * @gt: GT structure


In multiple places (here and below) it would be nice to know what a
GT structure is. I looked thru multiple C and header files yesterday
and didn't find any comments about it.

Just saying that @gt is a GT structure isn't very helpful, other
than making kernel-doc shut up.


the 'gt' belongs to the drivers/gpu/drm/i915/gt/ subsystem and
it's widely used a throughout i915.

I think it's inappropriate to describe it just here. On the other
hand I agree that a better documentation is required for the GT
itself where other parts can point to.


Yeah agreed there is no point of copy pasting some explanation all over 
the place. Could we just do s/GT structure/struct intel_gt/, or "pointer 
to struct intel_gt to operate on", would that be good enough?


Regards,

Tvrtko


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix NPD in PMU during driver teardown

2022-08-05 Thread Tvrtko Ursulin



On 04/08/2022 19:56, Summers, Stuart wrote:

On Thu, 2022-08-04 at 09:42 +0100, Tvrtko Ursulin wrote:

On 04/08/2022 00:03, Stuart Summers wrote:

In the driver teardown, we are unregistering the gt prior
to unregistering the PMU. This means there is a small window
of time in which the application can request metrics from the
PMU, some of which are calling into the uapi engines list,
while the engines are not available. In this case we can
see null pointer dereferences.

Fix this ordering in both the driver load and unload sequences.

v1: Actually address the driver load/unload ordering issue
v2: Remove the NULL checks since they shouldn't be necessary
  now with the proper ordering

Fixes: 42014f69bb235 ("drm/i915: Hook up GT power management")


What happened in this commit to break it?


Hm.. well this was the patch that added the abstraction ordering issue,
i.e. gt_register/unregister. There isn't anything specifically broken
here since we don't actually access the gt structure underneath. My
assertion is that this patch should have taken the expansion of the gt
structure into consideration and added the correct ordering with
respect to the pmu.

Are you asking for the specific patch where things stopped working
functionally?


No, I was looking for the info on what is actually broken with the 
ordering, as is since I couldn't spot it myself yesterday. In other 
words, with patch 2/2 applied - does 1/2 fix anything further for the 
PMU use cases?


Regards,

Tvrtko


Signed-off-by: Stuart Summers 
---
   drivers/gpu/drm/i915/i915_driver.c | 11 ++-
   1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_driver.c
b/drivers/gpu/drm/i915/i915_driver.c
index deb8a8b76965a..ee4dcb85d2060 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -717,7 +717,6 @@ static void i915_driver_register(struct
drm_i915_private *dev_priv)
struct drm_device *dev = _priv->drm;
   
   	i915_gem_driver_register(dev_priv);

-   i915_pmu_register(dev_priv);
   
   	intel_vgpu_register(dev_priv);
   
@@ -731,11 +730,12 @@ static void i915_driver_register(struct

drm_i915_private *dev_priv)
i915_debugfs_register(dev_priv);
i915_setup_sysfs(dev_priv);
   
+	intel_gt_driver_register(to_gt(dev_priv));

+
/* Depends on sysfs having been initialized */
+   i915_pmu_register(dev_priv);
i915_perf_register(dev_priv);
   
-	intel_gt_driver_register(to_gt(dev_priv));

-


There was a bit of a time distance since we originally discussed this
so
things kind of evaporated from my head. Or at least it feels like
that.
   Today when I look at the code I don't understand why is this
shuffle
relevant.

The sysfs comment does not really apply to pmu, but also if I look
into
intel_gt_driver_(un)register I did not spot what is the relevant
part
which interacts with the PMU.

On register it is engine list first then PMU.

On unregister it is PMU first, then engine list:

i915_driver_remove
  i915_driver_unregister
i915_pmu_unregister
  i915_gem_driver_remove
clears uabi engines list

Help please? :)

Regards,

Tvrtko


intel_display_driver_register(dev_priv);
   
   	intel_power_domains_enable(dev_priv);

@@ -762,11 +762,12 @@ static void i915_driver_unregister(struct
drm_i915_private *dev_priv)
   
   	intel_display_driver_unregister(dev_priv);
   
-	intel_gt_driver_unregister(to_gt(dev_priv));

-
i915_perf_unregister(dev_priv);
+   /* GT should be available until PMU is gone */
i915_pmu_unregister(dev_priv);
   
+	intel_gt_driver_unregister(to_gt(dev_priv));

+
i915_teardown_sysfs(dev_priv);
drm_dev_unplug(_priv->drm);
   


[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/hdcp: register cleanup (rev3)

2022-08-05 Thread Patchwork
== Series Details ==

Series: drm/i915/hdcp: register cleanup (rev3)
URL   : https://patchwork.freedesktop.org/series/106964/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11970 -> Patchwork_106964v3


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/index.html

Participating hosts (43 -> 29)
--

  Missing(14): fi-bdw-samus bat-dg1-6 bat-dg1-5 bat-dg2-8 fi-skl-guc 
bat-adlm-1 bat-dg2-9 bat-adlp-6 bat-adlp-4 bat-jsl-3 bat-rplp-1 bat-rpls-1 
bat-rpls-2 bat-jsl-1 

Known issues


  Here are the changes found in Patchwork_106964v3 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@kms_chamelium@common-hpd-after-suspend:
- fi-bdw-5557u:   NOTRUN -> [SKIP][1] ([fdo#109271] / [fdo#111827])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/fi-bdw-5557u/igt@kms_chamel...@common-hpd-after-suspend.html

  
 Possible fixes 

  * igt@i915_selftest@live@gt_heartbeat:
- fi-skl-6700k2:  [DMESG-FAIL][2] ([i915#5334]) -> [PASS][3]
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/fi-skl-6700k2/igt@i915_selftest@live@gt_heartbeat.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/fi-skl-6700k2/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@hangcheck:
- {fi-ehl-2}: [INCOMPLETE][4] ([i915#5153] / [i915#6106]) -> 
[PASS][5]
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/fi-ehl-2/igt@i915_selftest@l...@hangcheck.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/fi-ehl-2/igt@i915_selftest@l...@hangcheck.html

  * igt@i915_suspend@basic-s3-without-i915:
- fi-bdw-5557u:   [INCOMPLETE][6] ([i915#146]) -> [PASS][7]
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/fi-bdw-5557u/igt@i915_susp...@basic-s3-without-i915.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/fi-bdw-5557u/igt@i915_susp...@basic-s3-without-i915.html

  
  {name}: This element is suppressed. This means it is ignored when computing
  the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#5153]: https://gitlab.freedesktop.org/drm/intel/issues/5153
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#6106]: https://gitlab.freedesktop.org/drm/intel/issues/6106


Build changes
-

  * Linux: CI_DRM_11970 -> Patchwork_106964v3

  CI-20190529: 20190529
  CI_DRM_11970: 1d7aa8092dbbaef7c6a81903e0432f5b90da4d63 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6613: 209230467200f2fa63a6f71fe626470dd813 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_106964v3: 1d7aa8092dbbaef7c6a81903e0432f5b90da4d63 @ 
git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

d3892ea96767 drm/i915/hdcp: replace BIT() with REG_BIT() in register definitions
ab5112d18446 drm/i915/hdcp: split out hdcp registers to a separate file

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106964v3/index.html


Re: [Intel-gfx] [PATCH v4 4/6] drm/i915: Implement intersect/compatible functions

2022-08-05 Thread Matthew Auld

On 04/08/2022 09:59, Arunpravin Paneer Selvam wrote:

Implemented a new intersect and compatible callback function
fetching start offset from drm buddy allocator.

v3: move the bits that are specific to buddy_man (Matthew)
v4: consider the block size /range (Matthew)

Signed-off-by: Christian König 
Signed-off-by: Arunpravin Paneer Selvam 
---
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c   | 41 +--
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 73 +++
  2 files changed, 74 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c 
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 70e2ed4e99df..bf5fd6886ca0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -379,7 +379,6 @@ static bool i915_ttm_eviction_valuable(struct 
ttm_buffer_object *bo,
   const struct ttm_place *place)
  {
struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
-   struct ttm_resource *res = bo->resource;
  
  	if (!obj)

return false;
@@ -396,45 +395,7 @@ static bool i915_ttm_eviction_valuable(struct 
ttm_buffer_object *bo,
if (!i915_gem_object_evictable(obj))
return false;
  
-	switch (res->mem_type) {

-   case I915_PL_LMEM0: {
-   struct ttm_resource_manager *man =
-   ttm_manager_type(bo->bdev, res->mem_type);
-   struct i915_ttm_buddy_resource *bman_res =
-   to_ttm_buddy_resource(res);
-   struct drm_buddy *mm = bman_res->mm;
-   struct drm_buddy_block *block;
-
-   if (!place->fpfn && !place->lpfn)
-   return true;
-
-   GEM_BUG_ON(!place->lpfn);
-
-   /*
-* If we just want something mappable then we can quickly check
-* if the current victim resource is using any of the CPU
-* visible portion.
-*/
-   if (!place->fpfn &&
-   place->lpfn == i915_ttm_buddy_man_visible_size(man))
-   return bman_res->used_visible_size > 0;
-
-   /* Real range allocation */
-   list_for_each_entry(block, _res->blocks, link) {
-   unsigned long fpfn =
-   drm_buddy_block_offset(block) >> PAGE_SHIFT;
-   unsigned long lpfn = fpfn +
-   (drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
-
-   if (place->fpfn < lpfn && place->lpfn > fpfn)
-   return true;
-   }
-   return false;
-   } default:
-   break;
-   }
-
-   return true;
+   return ttm_bo_eviction_valuable(bo, place);
  }
  
  static void i915_ttm_evict_flags(struct ttm_buffer_object *bo,

diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c 
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index a5109548abc0..9def01d5f368 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -178,6 +178,77 @@ static void i915_ttm_buddy_man_free(struct 
ttm_resource_manager *man,
kfree(bman_res);
  }
  
+static bool i915_ttm_buddy_man_intersects(struct ttm_resource_manager *man,

+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+   struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res);
+   struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
+   struct drm_buddy *mm = >mm;
+   struct drm_buddy_block *block;
+
+   if (!place->fpfn && !place->lpfn)
+   return true;
+
+   GEM_BUG_ON(!place->lpfn);
+
+   /*
+* If we just want something mappable then we can quickly check
+* if the current victim resource is using any of the CP


Nit: s/CP/CPU/

Reviewed-by: Matthew Auld 


+* visible portion.
+*/
+   if (!place->fpfn &&
+   place->lpfn == i915_ttm_buddy_man_visible_size(man))
+   return bman_res->used_visible_size > 0;
+
+   /* Check each drm buddy block individually */
+   list_for_each_entry(block, _res->blocks, link) {
+   unsigned long fpfn =
+   drm_buddy_block_offset(block) >> PAGE_SHIFT;
+   unsigned long lpfn = fpfn +
+   (drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
+
+   if (place->fpfn < lpfn && place->lpfn > fpfn)
+   return true;
+   }
+
+   return false;
+}
+
+static bool i915_ttm_buddy_man_compatible(struct ttm_resource_manager *man,
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+   

Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gt: document TLB cache invalidation functions

2022-08-05 Thread Andi Shyti
Hi Randy,

> > +/**
> > + * intel_gt_invalidate_tlb_full - do full TLB cache invalidation
> > + * @gt: GT structure
> 
> In multiple places (here and below) it would be nice to know what a
> GT structure is. I looked thru multiple C and header files yesterday
> and didn't find any comments about it.
> 
> Just saying that @gt is a GT structure isn't very helpful, other
> than making kernel-doc shut up.

the 'gt' belongs to the drivers/gpu/drm/i915/gt/ subsystem and
it's widely used a throughout i915.

I think it's inappropriate to describe it just here. On the other
hand I agree that a better documentation is required for the GT
itself where other parts can point to.

Andi


[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/hdcp: register cleanup (rev3)

2022-08-05 Thread Patchwork
== Series Details ==

Series: drm/i915/hdcp: register cleanup (rev3)
URL   : https://patchwork.freedesktop.org/series/106964/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.




[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/hdcp: register cleanup (rev3)

2022-08-05 Thread Patchwork
== Series Details ==

Series: drm/i915/hdcp: register cleanup (rev3)
URL   : https://patchwork.freedesktop.org/series/106964/
State : warning

== Summary ==

Error: dim checkpatch failed
d17261c0cd1c drm/i915/hdcp: split out hdcp registers to a separate file
Traceback (most recent call last):
  File "scripts/spdxcheck.py", line 11, in 
import git
ModuleNotFoundError: No module named 'git'
-:36: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does 
MAINTAINERS need updating?
#36: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 556 lines checked
83599206d592 drm/i915/hdcp: replace BIT() with REG_BIT() in register definitions




Re: [Intel-gfx] [PULL] drm-intel-next-fixes

2022-08-05 Thread Mauro Carvalho Chehab
Hi Rodrigo,

On Thu, 4 Aug 2022 13:33:06 -0400
Rodrigo Vivi  wrote:

> Hi Dave and Daniel,
> 
> Here goes drm-intel-next-fixes-2022-08-04:
> 
> - disable pci resize on 32-bit systems (Nirmoy)
> - don't leak the ccs state (Matt)
> - TLB invalidation fixes (Chris)
> 
> Thanks,
> Rodrigo.
> 
> The following changes since commit 2bc7ea71a73747a77e7f83bc085b0d2393235410:
> 
>   Merge tag 'topic/nouveau-misc-2022-07-27' of 
> git://anongit.freedesktop.org/drm/drm into drm-next (2022-07-27 11:34:07 
> +1000)
> 
> are available in the Git repository at:
> 
>   git://anongit.freedesktop.org/drm/drm-intel 
> tags/drm-intel-next-fixes-2022-08-04
> 
> for you to fetch changes up to e57b9369e0c6f60155027e120fafd4b57e385b71:
> 
>   drm/i915/gt: Batch TLB invalidations (2022-08-01 09:48:06 -0400)
> 
> 
> - disable pci resize on 32-bit systems (Nirmoy)
> - don't leak the ccs state (Matt)
> - TLB invalidation fixes (Chris)
> 
> 
> Chris Wilson (4):
>   drm/i915/gt: Ignore TLB invalidations on idle engines
>   drm/i915/gt: Invalidate TLB of the OA unit at TLB invalidations
>   drm/i915/gt: Skip TLB invalidations once wedged

>   drm/i915/gt: Batch TLB invalidations
This patch actually adds a regression due to a silly mistake. 
The fix is here:

https://patchwork.freedesktop.org/patch/496249/?series=106805=4

Regards,
Mauro


[Intel-gfx] [PATCH i-g-t 1/3] tests/gem_exec_fence: Fix wrong engine checked for store_dword capability

2022-08-05 Thread Janusz Krzysztofik
Commit ed7c8cfb67e3 ("tests/i915/gem_exec_fence: Add
__for_each_physical_engine to utilize all engines."), while replacing
depraciated for_each_physical_engine(e, fd) loop with
__for_each_physical_engine(fd, e2) and gem_can_store_dword() inside that
loop with gem_class_can_store_dword(), didn't switch to e2 consequently --
eb_ring(e) argument of gem_can_store_dword() was replaced with e->class
passed to gem_class_can_store_dword() instead of e2->class.  As a result,
a batch that stores dword is now submitted to all engines, also those that
don't support that operation.  Fix it.

Signed-off-by: Janusz Krzysztofik 
---
 tests/i915/gem_exec_fence.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index 7ff7614dad..048870053a 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -331,7 +331,7 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
 
i = 0;
for_each_ctx_engine(fd, ctx, e2) {
-   if (!gem_class_can_store_dword(fd, e->class))
+   if (!gem_class_can_store_dword(fd, e2->class))
continue;
 
if (flags & NONBLOCK) {
-- 
2.25.1



Re: [Intel-gfx] [PATCH] drm/i915/pps: added get_pps_idx() hook as part of pps_get_register() cleanup

2022-08-05 Thread Manna, Animesh



> -Original Message-
> From: Nikula, Jani 
> Sent: Wednesday, August 3, 2022 1:44 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: ville.syrj...@linux.intel.com; Shankar, Uma ;
> Manna, Animesh 
> Subject: Re: [PATCH] drm/i915/pps: added get_pps_idx() hook as part of
> pps_get_register() cleanup
> 
> On Wed, 03 Aug 2022, Animesh Manna  wrote:
> > To support dual LFP two instances of pps added from display gen12 onwards.
> > Few older platform like VLV also has dual pps support but handling is
> > different. So added separate hook get_pps_idx() to formulate which pps
> > instance to used for a soecific LFP on a specific platform.
> >
> > Simplified pps_get_register() which use get_pps_idx() hook to derive
> > the pps instance and get_pps_idx() will be initialized at pps_init().
> >
> > Signed-off-by: Animesh Manna 
> > ---
> >  drivers/gpu/drm/i915/display/intel_bios.c |  5 
> >  drivers/gpu/drm/i915/display/intel_bios.h |  1 +
> >  .../drm/i915/display/intel_display_types.h|  2 ++
> >  drivers/gpu/drm/i915/display/intel_pps.c  | 25 ++-
> >  4 files changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
> > b/drivers/gpu/drm/i915/display/intel_bios.c
> > index 51dde5bfd956..42315615a728 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> > @@ -611,6 +611,11 @@ static int opregion_get_panel_type(struct
> drm_i915_private *i915,
> > return intel_opregion_get_panel_type(i915);
> >  }
> >
> > +bool intel_bios_is_lfp2(struct intel_encoder *encoder) {
> > +   return encoder->devdata && encoder->devdata->child.handle ==
> > +DEVICE_HANDLE_LFP2; }
> 
> AFAICS the encoder never really needs to know whether it's "lfp1" or "lfp2". 
> It
> needs to know the pps controller number.
> 
> > +
> >  static int vbt_get_panel_type(struct drm_i915_private *i915,
> >   const struct intel_bios_encoder_data *devdata,
> >   const struct edid *edid)
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.h
> > b/drivers/gpu/drm/i915/display/intel_bios.h
> > index e47582b0de0a..aea72a87ea2c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.h
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> > @@ -251,6 +251,7 @@ bool intel_bios_is_lspcon_present(const struct
> drm_i915_private *i915,
> >   enum port port);
> >  bool intel_bios_is_lane_reversal_needed(const struct drm_i915_private 
> > *i915,
> > enum port port);
> > +bool intel_bios_is_lfp2(struct intel_encoder *encoder);
> >  enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv,
> > enum port port);  bool intel_bios_get_dsc_params(struct intel_encoder
> *encoder,
> >struct intel_crtc_state *crtc_state, diff --git
> > a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 0da9b208d56e..95f71a572b07 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1723,6 +1723,8 @@ struct intel_dp {
> >
> > /* When we last wrote the OUI for eDP */
> > unsigned long last_oui_write;
> > +
> > +   int (*get_pps_idx)(struct intel_dp *intel_dp);
> 
> Making this a function pointer should be a separate step. Please don't try to 
> do
> too many things at once. Rule of thumb, one change per patch.
> 
> Also, this should be placed near the other function pointer members in struct
> intel_dp.
> 
> >  };
> >
> >  enum lspcon_vendor {
> > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c
> > b/drivers/gpu/drm/i915/display/intel_pps.c
> > index 1b21a341962f..c9cdb302d318 100644
> > --- a/drivers/gpu/drm/i915/display/intel_pps.c
> > +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> > @@ -231,6 +231,17 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp)
> > return backlight_controller;
> >  }
> >
> > +static int
> > +gen12_power_sequencer_idx(struct intel_dp *intel_dp) {
> > +   struct intel_encoder *encoder = _to_dig_port(intel_dp)->base;
> > +
> > +   if (intel_bios_is_lfp2(encoder))
> > +   return 1;
> 
> This is actually not how this works. The bxt_power_sequencer_idx() matches
> bspec 20149: "PWM and PPS are assumed to be aligned to be from same block
> and not mismatch" i.e. the backlight controller id and the pps id are the 
> same.
> There are no requirements to map lfp# and pps controller like this, even if it
> might be true in the common case.
> 
> The problem is we need the information *before* we call
> intel_bios_init_panel().
> 
> It's a catch-22. We need the pps id to read the panel EDID, and we need the
> panel EDID to choose the correct panel type in VBT, which we need to get the
> pps id from the VBT.
> 
> This is highlighted in [1]. The 2nd eDP (which is not even physically 
> 

[Intel-gfx] [PATCH i-g-t 2/3] tests/gem_exec_fence: Exclude 0 from use in store batches

2022-08-05 Thread Janusz Krzysztofik
When i == 0, condition out[i] == i will be true even if a batch supposed
to store i in out[i] fails.  Don't use 0.

Signed-off-by: Janusz Krzysztofik 
---
 tests/i915/gem_exec_fence.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index 048870053a..11d99781b0 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -334,6 +334,8 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
if (!gem_class_can_store_dword(fd, e2->class))
continue;
 
+   i++;
+
if (flags & NONBLOCK) {
igt_store_word(fd, ahnd, ctx, e2, spin->out_fence,
   scratch, scratch_offset, i, i);
@@ -345,8 +347,6 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
put_ahnd(ahnd);
}
}
-
-   i++;
}
 
igt_spin_busywait_until_started(spin);
@@ -356,7 +356,7 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
if ((flags & HANG) == 0) {
/* Check for invalidly completing the task early */
igt_assert(fence_busy(spin->out_fence));
-   for (int n = 0; n < i; n++)
+   for (int n = 1; n <= i; n++)
igt_assert_eq_u32(out[n], 0);
 
igt_spin_end(spin);
@@ -366,8 +366,11 @@ static void test_fence_await(int fd, const intel_ctx_t 
*ctx,
 
gem_set_domain(fd, scratch, I915_GEM_DOMAIN_GTT, 0);
igt_assert(!fence_busy(spin->out_fence));
-   while ((flags & HANG) == 0 && i--)
-   igt_assert_eq_u32(out[i], i);
+   if ((flags & HANG) == 0) {
+   do
+   igt_assert_eq_u32(out[i], i);
+   while (--i);
+   }
munmap(out, 4096);
 
igt_spin_free(fd, spin);
-- 
2.25.1



[Intel-gfx] [PATCH i-g-t 3/3] tests/gem_exec_fence: Restore pre-hang checks in *await-hang scenarios

2022-08-05 Thread Janusz Krzysztofik
Commit c8f6aaf32d83 "tests/gem_exec_fence: Check stored values only for
valid workloads" resolved an issue, observed in *await-hang scenarios,
where a fence exposed by an invalid spin batch was signaled asynchronously
to pending checks for depended test batches still waiting for that fence.
Those checks have been disabled, weakening those scenarios.

This change takes an alternative approach: it makes the invalid spin batch
dependent on another fence so the test has full control over the moment
when that batch starts, triggers a GPU hang and its fence is signaled.
With that in place, the test is able to check synchronously if execution
of dependent test batches is still blocked on the not yet signaled fence
of the not yet completed spin batch, as it does in counterpart non-hanging
scenarios.

Signed-off-by: Janusz Krzysztofik 
---
 tests/i915/gem_exec_fence.c | 45 ++---
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index 11d99781b0..27af9718d9 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -309,10 +309,10 @@ static void test_fence_await(int fd, const intel_ctx_t 
*ctx,
 {
const struct intel_execution_engine2 *e2;
uint32_t scratch = gem_create(fd, 4096);
-   igt_spin_t *spin;
+   igt_spin_t *spin, *invalid_spin;
uint32_t *out;
uint64_t scratch_offset, ahnd = get_reloc_ahnd(fd, ctx->id);
-   int i;
+   int out_fence, i;
 
scratch_offset = get_offset(ahnd, scratch, 4096, 0);
 
@@ -325,10 +325,25 @@ static void test_fence_await(int fd, const intel_ctx_t 
*ctx,
.ctx = ctx,
.engine = e->flags,
.flags = IGT_SPIN_FENCE_OUT |
-IGT_SPIN_POLL_RUN |
-spin_hang(flags));
+IGT_SPIN_POLL_RUN);
igt_assert(spin->out_fence != -1);
 
+   if (flags & HANG) {
+   invalid_spin = igt_spin_new(fd,
+   .ahnd = ahnd,
+   .ctx = ctx,
+   .engine = e->flags,
+   .fence = spin->out_fence,
+   .flags = IGT_SPIN_FENCE_IN |
+IGT_SPIN_FENCE_OUT |
+IGT_SPIN_POLL_RUN |
+spin_hang(flags));
+   igt_assert(invalid_spin->out_fence != -1);
+   out_fence = invalid_spin->out_fence;
+   } else {
+   out_fence = spin->out_fence;
+   }
+
i = 0;
for_each_ctx_engine(fd, ctx, e2) {
if (!gem_class_can_store_dword(fd, e2->class))
@@ -337,12 +352,12 @@ static void test_fence_await(int fd, const intel_ctx_t 
*ctx,
i++;
 
if (flags & NONBLOCK) {
-   igt_store_word(fd, ahnd, ctx, e2, spin->out_fence,
+   igt_store_word(fd, ahnd, ctx, e2, out_fence,
   scratch, scratch_offset, i, i);
} else {
igt_fork(child, 1) {
ahnd = get_reloc_ahnd(fd, ctx->id);
-   igt_store_word(fd, ahnd, ctx, e2, 
spin->out_fence,
+   igt_store_word(fd, ahnd, ctx, e2, out_fence,
   scratch, scratch_offset, i, i);
put_ahnd(ahnd);
}
@@ -353,19 +368,21 @@ static void test_fence_await(int fd, const intel_ctx_t 
*ctx,
/* Long, but not too long to anger preemption disable checks */
usleep(50 * 1000); /* 50 ms, typical preempt reset is 150+ms */
 
-   if ((flags & HANG) == 0) {
-   /* Check for invalidly completing the task early */
+   /* Check for invalidly completing the task early */
+   if (flags & HANG) {
igt_assert(fence_busy(spin->out_fence));
-   for (int n = 1; n <= i; n++)
-   igt_assert_eq_u32(out[n], 0);
-
-   igt_spin_end(spin);
+   igt_fail_on(igt_spin_has_started(invalid_spin));
}
+   igt_assert(fence_busy(out_fence));
+   for (int n = 1; n <= i; n++)
+   igt_assert_eq_u32(out[n], 0);
+
+   igt_spin_end(spin);
 
igt_waitchildren();
 
gem_set_domain(fd, scratch, I915_GEM_DOMAIN_GTT, 0);
-   igt_assert(!fence_busy(spin->out_fence));
+   igt_assert(!fence_busy(out_fence));
if ((flags & HANG) == 0) {
do
igt_assert_eq_u32(out[i], i);
@@ -373,6 +390,8 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
  

[Intel-gfx] ✗ Fi.CI.IGT: failure for i915/pmu: Wire GuC backend to per-client busyness (rev4)

2022-08-05 Thread Patchwork
== Series Details ==

Series: i915/pmu: Wire GuC backend to per-client busyness (rev4)
URL   : https://patchwork.freedesktop.org/series/105085/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11970_full -> Patchwork_105085v4_full


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_105085v4_full absolutely need 
to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_105085v4_full, please notify your bug team to allow 
them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (12 -> 12)
--

  No changes in participating hosts

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_105085v4_full:

### IGT changes ###

 Possible regressions 

  * igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions:
- shard-skl:  NOTRUN -> [INCOMPLETE][1]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105085v4/shard-skl10/igt@kms_cursor_legacy@flip-vs-cur...@atomic-transitions.html

  
 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_ppgtt@blt-vs-render-ctxn:
- {shard-rkl}:NOTRUN -> [FAIL][2]
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105085v4/shard-rkl-5/igt@gem_pp...@blt-vs-render-ctxn.html

  
Known issues


  Here are the changes found in Patchwork_105085v4_full that come from known 
issues:

### CI changes ###

 Possible fixes 

  * boot:
- shard-skl:  ([PASS][3], [PASS][4], [PASS][5], [PASS][6], 
[PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], 
[PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], 
[FAIL][19], [FAIL][20], [FAIL][21], [FAIL][22], [FAIL][23], [FAIL][24]) 
([i915#5032]) -> ([PASS][25], [PASS][26], [PASS][27], [PASS][28], [PASS][29], 
[PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], 
[PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], 
[PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], 
[PASS][48])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl9/boot.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl9/boot.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl9/boot.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl7/boot.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl7/boot.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl7/boot.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl6/boot.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl6/boot.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl6/boot.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl4/boot.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl4/boot.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl3/boot.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl2/boot.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl1/boot.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl1/boot.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl1/boot.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl10/boot.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl10/boot.html
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl10/boot.html
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl10/boot.html
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl10/boot.html
   [24]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11970/shard-skl10/boot.html
   [25]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105085v4/shard-skl9/boot.html
   [26]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105085v4/shard-skl9/boot.html
   [27]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105085v4/shard-skl9/boot.html
   [28]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105085v4/shard-skl9/boot.html
   [29]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105085v4/shard-skl7/boot.html
   [30]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105085v4/shard-skl7/boot.html
   [31]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105085v4/shard-skl7/boot.html
   [32]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105085v4/shard-skl7/boot.html
   [33]: