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

[Chris Wilson <chris@chris-wilson.co.uk>]

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