[PATCH 1/2] drm/i915: Add pretty printer for device info flags

[PATCH 1/2] drm/i915: Add pretty printer for device info flags

[Michal Wajdeczko]

We dump device flags in few places (init_early, debugfs, gpu_error)
using different functions. Lets add reusable function to avoid
code duplication.

add/remove: 1/0 grow/shrink: 0/3 up/down: 1296/-3572 (-2276)
Function                                     old     new   delta
intel_device_info_dump_flags                   -    1296   +1296
i915_capabilities                           2435    1353   -1082
i915_error_state_to_str                     6642    5507   -1135
intel_device_info_dump                      1507     152   -1355
Total: Before=1287992, After=1285716, chg -0.18%

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c      |  5 ++---
 drivers/gpu/drm/i915/i915_drv.h          |  2 ++
 drivers/gpu/drm/i915/i915_gpu_error.c    |  6 +++---
 drivers/gpu/drm/i915/intel_device_info.c | 20 ++++++++++++++++----
 4 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0ddce72..e384e28 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -58,14 +58,13 @@ static int i915_capabilities(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
 	const struct intel_device_info *info = INTEL_INFO(dev_priv);
+	struct drm_printer p = drm_seq_file_printer(m);
 
 	seq_printf(m, "gen: %d\n", INTEL_GEN(dev_priv));
 	seq_printf(m, "platform: %s\n", intel_platform_name(info->platform));
 	seq_printf(m, "pch: %d\n", INTEL_PCH_TYPE(dev_priv));
 
-#define PRINT_FLAG(x)  seq_printf(m, #x ": %s\n", yesno(info->x))
-	DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
-#undef PRINT_FLAG
+	intel_device_info_dump_flags(info, &p);
 
 	kernel_param_lock(THIS_MODULE);
 #define PRINT_PARAM(T, x, ...) seq_print_param(m, #x, #T, &i915_modparams.x);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1aba565..26ebc70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -4165,6 +4165,8 @@ static inline int intel_opregion_get_panel_type(struct drm_i915_private *dev)
 const char *intel_platform_name(enum intel_platform platform);
 void intel_device_info_runtime_init(struct drm_i915_private *dev_priv);
 void intel_device_info_dump(struct drm_i915_private *dev_priv);
+void intel_device_info_dump_flags(const struct intel_device_info *info,
+				  struct drm_printer *p);
 
 /* modesetting */
 extern void intel_modeset_init_hw(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index aba50aa..664f55c 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -564,9 +564,9 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
 static void err_print_capabilities(struct drm_i915_error_state_buf *m,
 				   const struct intel_device_info *info)
 {
-#define PRINT_FLAG(x)  err_printf(m, #x ": %s\n", yesno(info->x))
-	DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
-#undef PRINT_FLAG
+	struct drm_printer p = i915_error_printer(m);
+
+	intel_device_info_dump_flags(info, &p);
 }
 
 static __always_inline void err_print_param(struct drm_i915_error_state_buf *m,
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index f478be3..5a385a9 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -22,6 +22,8 @@
  *
  */
 
+#include <drm/drm_print.h>
+
 #include "i915_drv.h"
 
 #define PLATFORM_NAME(x) [INTEL_##x] = #x
@@ -67,6 +69,14 @@ const char *intel_platform_name(enum intel_platform platform)
 	return platform_names[platform];
 }
 
+void intel_device_info_dump_flags(const struct intel_device_info *info,
+				  struct drm_printer *p)
+{
+#define PRINT_FLAG(name) drm_printf(p, "%s: %s\n", #name, yesno(info->name));
+	DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
+#undef PRINT_FLAG
+}
+
 void intel_device_info_dump(struct drm_i915_private *dev_priv)
 {
 	const struct intel_device_info *info = &dev_priv->info;
@@ -76,10 +86,12 @@ void intel_device_info_dump(struct drm_i915_private *dev_priv)
 			 info->gen,
 			 dev_priv->drm.pdev->device,
 			 dev_priv->drm.pdev->revision);
-#define PRINT_FLAG(name) \
-	DRM_DEBUG_DRIVER("i915 device info: " #name ": %s", yesno(info->name))
-	DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
-#undef PRINT_FLAG
+
+	if (drm_debug & DRM_UT_DRIVER) {
+		struct drm_printer p = drm_debug_printer("i915 device info: ");
+
+		intel_device_info_dump_flags(info, &p);
+	}
 }
 
 static void gen10_sseu_info_init(struct drm_i915_private *dev_priv)
-- 
1.9.1

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

[PATCH 2/2] drm/i915: Add pretty printer for modparams

[Michal Wajdeczko]

We dump modparams in few places (debugfs, gpu_error) using different
functions. Lets add reusable function to avoid code duplication.

add/remove: 1/0 grow/shrink: 0/2 up/down: 1096/-2339 (-1243)
Function                                     old     new   delta
i915_params_dump                               -    1096   +1096
i915_capabilities                           1353     185   -1168
i915_error_state_to_str                     5507    4336   -1171
Total: Before=1285716, After=1284473, chg -0.10%

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c   | 21 +--------------------
 drivers/gpu/drm/i915/i915_gpu_error.c | 25 ++++---------------------
 drivers/gpu/drm/i915/i915_params.c    | 31 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_params.h    |  3 +++
 4 files changed, 39 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e384e28..c4780f0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -37,23 +37,6 @@ static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
 	return to_i915(node->minor->dev);
 }
 
-static __always_inline void seq_print_param(struct seq_file *m,
-					    const char *name,
-					    const char *type,
-					    const void *x)
-{
-	if (!__builtin_strcmp(type, "bool"))
-		seq_printf(m, "i915.%s=%s\n", name, yesno(*(const bool *)x));
-	else if (!__builtin_strcmp(type, "int"))
-		seq_printf(m, "i915.%s=%d\n", name, *(const int *)x);
-	else if (!__builtin_strcmp(type, "unsigned int"))
-		seq_printf(m, "i915.%s=%u\n", name, *(const unsigned int *)x);
-	else if (!__builtin_strcmp(type, "char *"))
-		seq_printf(m, "i915.%s=%s\n", name, *(const char **)x);
-	else
-		BUILD_BUG();
-}
-
 static int i915_capabilities(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -67,9 +50,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
 	intel_device_info_dump_flags(info, &p);
 
 	kernel_param_lock(THIS_MODULE);
-#define PRINT_PARAM(T, x, ...) seq_print_param(m, #x, #T, &i915_modparams.x);
-	I915_PARAMS_FOR_EACH(PRINT_PARAM);
-#undef PRINT_PARAM
+	i915_params_dump(&i915_modparams, &p);
 	kernel_param_unlock(THIS_MODULE);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 664f55c..84831e9 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -569,29 +569,12 @@ static void err_print_capabilities(struct drm_i915_error_state_buf *m,
 	intel_device_info_dump_flags(info, &p);
 }
 
-static __always_inline void err_print_param(struct drm_i915_error_state_buf *m,
-					    const char *name,
-					    const char *type,
-					    const void *x)
-{
-	if (!__builtin_strcmp(type, "bool"))
-		err_printf(m, "i915.%s=%s\n", name, yesno(*(const bool *)x));
-	else if (!__builtin_strcmp(type, "int"))
-		err_printf(m, "i915.%s=%d\n", name, *(const int *)x);
-	else if (!__builtin_strcmp(type, "unsigned int"))
-		err_printf(m, "i915.%s=%u\n", name, *(const unsigned int *)x);
-	else if (!__builtin_strcmp(type, "char *"))
-		err_printf(m, "i915.%s=%s\n", name, *(const char **)x);
-	else
-		BUILD_BUG();
-}
-
 static void err_print_params(struct drm_i915_error_state_buf *m,
-			     const struct i915_params *p)
+			     const struct i915_params *params)
 {
-#define PRINT(T, x, ...) err_print_param(m, #x, #T, &p->x);
-	I915_PARAMS_FOR_EACH(PRINT);
-#undef PRINT
+	struct drm_printer p = i915_error_printer(m);
+
+	i915_params_dump(params, &p);
 }
 
 static void err_print_pciid(struct drm_i915_error_state_buf *m,
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8dfea03..d8d7189 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -172,3 +172,34 @@ struct i915_params i915_modparams __read_mostly = {
 
 i915_param_named(enable_gvt, bool, 0400,
 	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
+
+static __always_inline void _print_param(struct drm_printer *p,
+					 const char *name,
+					 const char *type,
+					 const void *x)
+{
+	if (!__builtin_strcmp(type, "bool"))
+		drm_printf(p, "i915.%s=%s\n", name, yesno(*(const bool *)x));
+	else if (!__builtin_strcmp(type, "int"))
+		drm_printf(p, "i915.%s=%d\n", name, *(const int *)x);
+	else if (!__builtin_strcmp(type, "unsigned int"))
+		drm_printf(p, "i915.%s=%u\n", name, *(const unsigned int *)x);
+	else if (!__builtin_strcmp(type, "char *"))
+		drm_printf(p, "i915.%s=%s\n", name, *(const char **)x);
+	else
+		BUILD_BUG();
+}
+
+/**
+ * i915_params_dump - dump i915 modparams
+ * @params: i915 modparams
+ * @p: the &drm_printer
+ *
+ * Pretty printer for i915 modparams.
+ */
+void i915_params_dump(const struct i915_params *params, struct drm_printer *p)
+{
+#define PRINT(T, x, ...) _print_param(p, #x, #T, &params->x);
+	I915_PARAMS_FOR_EACH(PRINT);
+#undef PRINT
+}
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 792ce26..2af22bb 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -27,6 +27,7 @@
 
 #include <linux/bitops.h>
 #include <linux/cache.h> /* for __read_mostly */
+#include <drm/drm_print.h>
 
 #define ENABLE_GUC_SUBMISSION		BIT(0)
 #define ENABLE_GUC_LOAD_HUC		BIT(1)
@@ -77,5 +78,7 @@ struct i915_params {
 
 extern struct i915_params i915_modparams __read_mostly;
 
+void i915_params_dump(const struct i915_params *params, struct drm_printer *p);
+
 #endif
 
-- 
1.9.1

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

Re: [PATCH 1/2] drm/i915: Add pretty printer for device info flags

[Chris Wilson]

Quoting Michal Wajdeczko (2017-12-18 15:51:31)
> We dump device flags in few places (init_early, debugfs, gpu_error)
> using different functions. Lets add reusable function to avoid
> code duplication.
> 
> add/remove: 1/0 grow/shrink: 0/3 up/down: 1296/-3572 (-2276)
> Function                                     old     new   delta
> intel_device_info_dump_flags                   -    1296   +1296
> i915_capabilities                           2435    1353   -1082
> i915_error_state_to_str                     6642    5507   -1135
> intel_device_info_dump                      1507     152   -1355
> Total: Before=1287992, After=1285716, chg -0.18%
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c      |  5 ++---
>  drivers/gpu/drm/i915/i915_drv.h          |  2 ++
>  drivers/gpu/drm/i915/i915_gpu_error.c    |  6 +++---
>  drivers/gpu/drm/i915/intel_device_info.c | 20 ++++++++++++++++----
>  4 files changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0ddce72..e384e28 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -58,14 +58,13 @@ static int i915_capabilities(struct seq_file *m, void *data)
>  {
>         struct drm_i915_private *dev_priv = node_to_i915(m->private);
>         const struct intel_device_info *info = INTEL_INFO(dev_priv);
> +       struct drm_printer p = drm_seq_file_printer(m);
>  
>         seq_printf(m, "gen: %d\n", INTEL_GEN(dev_priv));
>         seq_printf(m, "platform: %s\n", intel_platform_name(info->platform));
>         seq_printf(m, "pch: %d\n", INTEL_PCH_TYPE(dev_priv));
>  
> -#define PRINT_FLAG(x)  seq_printf(m, #x ": %s\n", yesno(info->x))
> -       DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
> -#undef PRINT_FLAG
> +       intel_device_info_dump_flags(info, &p);
>  
>         kernel_param_lock(THIS_MODULE);
>  #define PRINT_PARAM(T, x, ...) seq_print_param(m, #x, #T, &i915_modparams.x);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1aba565..26ebc70 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -4165,6 +4165,8 @@ static inline int intel_opregion_get_panel_type(struct drm_i915_private *dev)
>  const char *intel_platform_name(enum intel_platform platform);
>  void intel_device_info_runtime_init(struct drm_i915_private *dev_priv);
>  void intel_device_info_dump(struct drm_i915_private *dev_priv);
> +void intel_device_info_dump_flags(const struct intel_device_info *info,
> +                                 struct drm_printer *p);
>  
>  /* modesetting */
>  extern void intel_modeset_init_hw(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index aba50aa..664f55c 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -564,9 +564,9 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
>  static void err_print_capabilities(struct drm_i915_error_state_buf *m,
>                                    const struct intel_device_info *info)
>  {
> -#define PRINT_FLAG(x)  err_printf(m, #x ": %s\n", yesno(info->x))
> -       DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
> -#undef PRINT_FLAG
> +       struct drm_printer p = i915_error_printer(m);
> +
> +       intel_device_info_dump_flags(info, &p);
>  }
>  
>  static __always_inline void err_print_param(struct drm_i915_error_state_buf *m,
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index f478be3..5a385a9 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -22,6 +22,8 @@
>   *
>   */
>  
> +#include <drm/drm_print.h>
> +
>  #include "i915_drv.h"
>  
>  #define PLATFORM_NAME(x) [INTEL_##x] = #x
> @@ -67,6 +69,14 @@ const char *intel_platform_name(enum intel_platform platform)
>         return platform_names[platform];
>  }
>  
> +void intel_device_info_dump_flags(const struct intel_device_info *info,
> +                                 struct drm_printer *p)
> +{
> +#define PRINT_FLAG(name) drm_printf(p, "%s: %s\n", #name, yesno(info->name));
> +       DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
> +#undef PRINT_FLAG
> +}
> +
>  void intel_device_info_dump(struct drm_i915_private *dev_priv)
>  {
>         const struct intel_device_info *info = &dev_priv->info;
> @@ -76,10 +86,12 @@ void intel_device_info_dump(struct drm_i915_private *dev_priv)
>                          info->gen,
>                          dev_priv->drm.pdev->device,
>                          dev_priv->drm.pdev->revision);
> -#define PRINT_FLAG(name) \
> -       DRM_DEBUG_DRIVER("i915 device info: " #name ": %s", yesno(info->name))
> -       DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
> -#undef PRINT_FLAG
> +
> +       if (drm_debug & DRM_UT_DRIVER) {
> +               struct drm_printer p = drm_debug_printer("i915 device info: ");
> +
> +               intel_device_info_dump_flags(info, &p);
> +       }


Looks good, I'm thinking we might push the drm_printer to the caller of
intel_device_info_dump(), but this is already a substantial improvement

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Add pretty printer for modparams

[Chris Wilson]

Quoting Michal Wajdeczko (2017-12-18 15:51:32)
> We dump modparams in few places (debugfs, gpu_error) using different
> functions. Lets add reusable function to avoid code duplication.
> 
> add/remove: 1/0 grow/shrink: 0/2 up/down: 1096/-2339 (-1243)
> Function                                     old     new   delta
> i915_params_dump                               -    1096   +1096
> i915_capabilities                           1353     185   -1168
> i915_error_state_to_str                     5507    4336   -1171
> Total: Before=1285716, After=1284473, chg -0.10%
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   | 21 +--------------------
>  drivers/gpu/drm/i915/i915_gpu_error.c | 25 ++++---------------------
>  drivers/gpu/drm/i915/i915_params.c    | 31 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_params.h    |  3 +++
>  4 files changed, 39 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e384e28..c4780f0 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -37,23 +37,6 @@ static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
>         return to_i915(node->minor->dev);
>  }
>  
> -static __always_inline void seq_print_param(struct seq_file *m,
> -                                           const char *name,
> -                                           const char *type,
> -                                           const void *x)
> -{
> -       if (!__builtin_strcmp(type, "bool"))
> -               seq_printf(m, "i915.%s=%s\n", name, yesno(*(const bool *)x));
> -       else if (!__builtin_strcmp(type, "int"))
> -               seq_printf(m, "i915.%s=%d\n", name, *(const int *)x);
> -       else if (!__builtin_strcmp(type, "unsigned int"))
> -               seq_printf(m, "i915.%s=%u\n", name, *(const unsigned int *)x);
> -       else if (!__builtin_strcmp(type, "char *"))
> -               seq_printf(m, "i915.%s=%s\n", name, *(const char **)x);
> -       else
> -               BUILD_BUG();
> -}
> -
>  static int i915_capabilities(struct seq_file *m, void *data)
>  {
>         struct drm_i915_private *dev_priv = node_to_i915(m->private);
> @@ -67,9 +50,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
>         intel_device_info_dump_flags(info, &p);
>  
>         kernel_param_lock(THIS_MODULE);
> -#define PRINT_PARAM(T, x, ...) seq_print_param(m, #x, #T, &i915_modparams.x);
> -       I915_PARAMS_FOR_EACH(PRINT_PARAM);
> -#undef PRINT_PARAM
> +       i915_params_dump(&i915_modparams, &p);
>         kernel_param_unlock(THIS_MODULE);
>  
>         return 0;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 664f55c..84831e9 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -569,29 +569,12 @@ static void err_print_capabilities(struct drm_i915_error_state_buf *m,
>         intel_device_info_dump_flags(info, &p);
>  }
>  
> -static __always_inline void err_print_param(struct drm_i915_error_state_buf *m,
> -                                           const char *name,
> -                                           const char *type,
> -                                           const void *x)
> -{
> -       if (!__builtin_strcmp(type, "bool"))
> -               err_printf(m, "i915.%s=%s\n", name, yesno(*(const bool *)x));
> -       else if (!__builtin_strcmp(type, "int"))
> -               err_printf(m, "i915.%s=%d\n", name, *(const int *)x);
> -       else if (!__builtin_strcmp(type, "unsigned int"))
> -               err_printf(m, "i915.%s=%u\n", name, *(const unsigned int *)x);
> -       else if (!__builtin_strcmp(type, "char *"))
> -               err_printf(m, "i915.%s=%s\n", name, *(const char **)x);
> -       else
> -               BUILD_BUG();
> -}
> -
>  static void err_print_params(struct drm_i915_error_state_buf *m,
> -                            const struct i915_params *p)
> +                            const struct i915_params *params)
>  {
> -#define PRINT(T, x, ...) err_print_param(m, #x, #T, &p->x);
> -       I915_PARAMS_FOR_EACH(PRINT);
> -#undef PRINT
> +       struct drm_printer p = i915_error_printer(m);
> +
> +       i915_params_dump(params, &p);
>  }
>  
>  static void err_print_pciid(struct drm_i915_error_state_buf *m,
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 8dfea03..d8d7189 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -172,3 +172,34 @@ struct i915_params i915_modparams __read_mostly = {
>  
>  i915_param_named(enable_gvt, bool, 0400,
>         "Enable support for Intel GVT-g graphics virtualization host support(default:false)");
> +
> +static __always_inline void _print_param(struct drm_printer *p,
> +                                        const char *name,
> +                                        const char *type,
> +                                        const void *x)
> +{
> +       if (!__builtin_strcmp(type, "bool"))
> +               drm_printf(p, "i915.%s=%s\n", name, yesno(*(const bool *)x));
> +       else if (!__builtin_strcmp(type, "int"))
> +               drm_printf(p, "i915.%s=%d\n", name, *(const int *)x);
> +       else if (!__builtin_strcmp(type, "unsigned int"))
> +               drm_printf(p, "i915.%s=%u\n", name, *(const unsigned int *)x);
> +       else if (!__builtin_strcmp(type, "char *"))
> +               drm_printf(p, "i915.%s=%s\n", name, *(const char **)x);
> +       else
> +               BUILD_BUG();
> +}
> +
> +/**
> + * i915_params_dump - dump i915 modparams
> + * @params: i915 modparams
> + * @p: the &drm_printer
> + *
> + * Pretty printer for i915 modparams.
> + */
> +void i915_params_dump(const struct i915_params *params, struct drm_printer *p)
> +{
> +#define PRINT(T, x, ...) _print_param(p, #x, #T, &params->x);
> +       I915_PARAMS_FOR_EACH(PRINT);
> +#undef PRINT
> +}
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 792ce26..2af22bb 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -27,6 +27,7 @@
>  
>  #include <linux/bitops.h>
>  #include <linux/cache.h> /* for __read_mostly */
> +#include <drm/drm_print.h>
>  
>  #define ENABLE_GUC_SUBMISSION          BIT(0)
>  #define ENABLE_GUC_LOAD_HUC            BIT(1)
> @@ -77,5 +78,7 @@ struct i915_params {
>  
>  extern struct i915_params i915_modparams __read_mostly;
>  
struct drm_printer; forward decl rather than include drm_print.h

> +void i915_params_dump(const struct i915_params *params, struct drm_printer *p);

Same for patch 1 if I missed it there.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add pretty printer for device info flags

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Add pretty printer for device info flags
URL   : https://patchwork.freedesktop.org/series/35521/
State : success

== Summary ==

Series 35521v1 series starting with [1/2] drm/i915: Add pretty printer for device info flags
https://patchwork.freedesktop.org/api/1.0/series/35521/revisions/1/mbox/

Test kms_busy:
        Subgroup basic-flip-a:
                pass       -> DMESG-WARN (fi-elk-e7500) fdo#103989
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (fi-hsw-4770r) fdo#100368
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-kbl-r) fdo#104172 +1
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (fi-bdw-5557u) fdo#104162

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172
fdo#104162 https://bugs.freedesktop.org/show_bug.cgi?id=104162

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:431s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:438s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:382s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:490s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:275s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:492s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:494s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:480s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:468s
fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:262s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:529s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:402s
fi-hsw-4770r     total:288  pass:260  dwarn:0   dfail:0   fail:1   skip:27  time:402s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:393s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:477s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:424s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:479s
fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:516s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:466s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:521s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:588s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:450s
fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:534s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:556s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:507s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:503s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:445s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:556s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:408s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:598s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:612s
fi-glk-dsi       total:288  pass:181  dwarn:1   dfail:4   fail:2   skip:100 time:385s

913d6a0d4d78fba4cb62a1ac14d539d959fe422a drm-tip: 2017y-12m-18d-13h-25m-28s UTC integration manifest
c58426754088 drm/i915: Add pretty printer for modparams
9ae562ebfe10 drm/i915: Add pretty printer for device info flags

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7528/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Add pretty printer for device info flags

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Add pretty printer for device info flags
URL   : https://patchwork.freedesktop.org/series/35521/
State : success

== Summary ==

Test drv_selftest:
        Subgroup live_hangcheck:
                incomplete -> PASS       (shard-snb) fdo#104313
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                fail       -> PASS       (shard-snb) fdo#101623
Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-c-planes:
                pass       -> SKIP       (shard-hsw) fdo#103375
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912

fdo#104313 https://bugs.freedesktop.org/show_bug.cgi?id=104313
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2712 pass:1536 dwarn:1   dfail:0   fail:10  skip:1165 time:9375s
shard-snb        total:2712 pass:1308 dwarn:1   dfail:0   fail:12  skip:1391 time:7973s
Blacklisted hosts:
shard-apl        total:2693 pass:1669 dwarn:1   dfail:1   fail:24  skip:997 time:13477s
shard-kbl        total:2712 pass:1791 dwarn:12  dfail:6   fail:23  skip:880 time:11178s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7528/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Add pretty printer for device info flags

[Jani Nikula]

On Mon, 18 Dec 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Michal Wajdeczko (2017-12-18 15:51:31)
>> We dump device flags in few places (init_early, debugfs, gpu_error)
>> using different functions. Lets add reusable function to avoid
>> code duplication.
>> 
>> add/remove: 1/0 grow/shrink: 0/3 up/down: 1296/-3572 (-2276)
>> Function                                     old     new   delta
>> intel_device_info_dump_flags                   -    1296   +1296
>> i915_capabilities                           2435    1353   -1082
>> i915_error_state_to_str                     6642    5507   -1135
>> intel_device_info_dump                      1507     152   -1355
>> Total: Before=1287992, After=1285716, chg -0.18%
>> 
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c      |  5 ++---
>>  drivers/gpu/drm/i915/i915_drv.h          |  2 ++
>>  drivers/gpu/drm/i915/i915_gpu_error.c    |  6 +++---
>>  drivers/gpu/drm/i915/intel_device_info.c | 20 ++++++++++++++++----
>>  4 files changed, 23 insertions(+), 10 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 0ddce72..e384e28 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -58,14 +58,13 @@ static int i915_capabilities(struct seq_file *m, void *data)
>>  {
>>         struct drm_i915_private *dev_priv = node_to_i915(m->private);
>>         const struct intel_device_info *info = INTEL_INFO(dev_priv);
>> +       struct drm_printer p = drm_seq_file_printer(m);
>>  
>>         seq_printf(m, "gen: %d\n", INTEL_GEN(dev_priv));
>>         seq_printf(m, "platform: %s\n", intel_platform_name(info->platform));
>>         seq_printf(m, "pch: %d\n", INTEL_PCH_TYPE(dev_priv));
>>  
>> -#define PRINT_FLAG(x)  seq_printf(m, #x ": %s\n", yesno(info->x))
>> -       DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
>> -#undef PRINT_FLAG
>> +       intel_device_info_dump_flags(info, &p);
>>  
>>         kernel_param_lock(THIS_MODULE);
>>  #define PRINT_PARAM(T, x, ...) seq_print_param(m, #x, #T, &i915_modparams.x);
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 1aba565..26ebc70 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -4165,6 +4165,8 @@ static inline int intel_opregion_get_panel_type(struct drm_i915_private *dev)
>>  const char *intel_platform_name(enum intel_platform platform);
>>  void intel_device_info_runtime_init(struct drm_i915_private *dev_priv);
>>  void intel_device_info_dump(struct drm_i915_private *dev_priv);
>> +void intel_device_info_dump_flags(const struct intel_device_info *info,
>> +                                 struct drm_printer *p);
>>  
>>  /* modesetting */
>>  extern void intel_modeset_init_hw(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
>> index aba50aa..664f55c 100644
>> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
>> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
>> @@ -564,9 +564,9 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
>>  static void err_print_capabilities(struct drm_i915_error_state_buf *m,
>>                                    const struct intel_device_info *info)
>>  {
>> -#define PRINT_FLAG(x)  err_printf(m, #x ": %s\n", yesno(info->x))
>> -       DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
>> -#undef PRINT_FLAG
>> +       struct drm_printer p = i915_error_printer(m);
>> +
>> +       intel_device_info_dump_flags(info, &p);
>>  }
>>  
>>  static __always_inline void err_print_param(struct drm_i915_error_state_buf *m,
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> index f478be3..5a385a9 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -22,6 +22,8 @@
>>   *
>>   */
>>  
>> +#include <drm/drm_print.h>
>> +
>>  #include "i915_drv.h"
>>  
>>  #define PLATFORM_NAME(x) [INTEL_##x] = #x
>> @@ -67,6 +69,14 @@ const char *intel_platform_name(enum intel_platform platform)
>>         return platform_names[platform];
>>  }
>>  
>> +void intel_device_info_dump_flags(const struct intel_device_info *info,
>> +                                 struct drm_printer *p)
>> +{
>> +#define PRINT_FLAG(name) drm_printf(p, "%s: %s\n", #name, yesno(info->name));
>> +       DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
>> +#undef PRINT_FLAG
>> +}
>> +
>>  void intel_device_info_dump(struct drm_i915_private *dev_priv)
>>  {
>>         const struct intel_device_info *info = &dev_priv->info;
>> @@ -76,10 +86,12 @@ void intel_device_info_dump(struct drm_i915_private *dev_priv)
>>                          info->gen,
>>                          dev_priv->drm.pdev->device,
>>                          dev_priv->drm.pdev->revision);
>> -#define PRINT_FLAG(name) \
>> -       DRM_DEBUG_DRIVER("i915 device info: " #name ": %s", yesno(info->name))
>> -       DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
>> -#undef PRINT_FLAG
>> +
>> +       if (drm_debug & DRM_UT_DRIVER) {
>> +               struct drm_printer p = drm_debug_printer("i915 device info: ");
>> +
>> +               intel_device_info_dump_flags(info, &p);
>> +       }
>
>
> Looks good, I'm thinking we might push the drm_printer to the caller of
> intel_device_info_dump(), but this is already a substantial improvement

Param ordering seems a bit surprising in both patches. Target first?

BR,
Jani.


>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Add pretty printer for device info flags

[Chris Wilson]

Quoting Jani Nikula (2017-12-18 17:37:13)
> On Mon, 18 Dec 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Looks good, I'm thinking we might push the drm_printer to the caller of
> > intel_device_info_dump(), but this is already a substantial improvement
> 
> Param ordering seems a bit surprising in both patches. Target first?

The param order at the moment tries to be (object-doing-the-dump,
stream-as-parameter). I don't think it is quite so backwards as it first
appears, when compared to printf(stderr, fmt,...) or even seq_printf(m,
fmt, ..). In both of those printf cases, it is the stream that can be
argued is providing the object (even more so if you create your own
stream using cookie_io_functions_t). But for us, it is clearer to argue
that the object we control is the object providing the method to output
into the stream.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx