[Intel-gfx] [PATCH 2/2] drm/i915: add a display info file to debugfs

Rodrigo Vivi rodrigo.vivi at gmail.com
Thu Jan 30 22:58:43 CET 2014


On Mon, Jan 20, 2014 at 9:55 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> Can be expanded up on to include all sorts of things (HDMI infoframe
> data, more DP status, etc).  Should be useful for bug reports to get a
> baseline on the display config and info.
>
> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 155 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h     |   4 +
>  2 files changed, 159 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 75a489e..20ae7ed 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1992,6 +1992,160 @@ static int i915_power_domain_info(struct seq_file *m, void *unused)
>         return 0;
>  }
>
> +static void intel_encoder_info(struct seq_file *m,
> +                              struct intel_crtc *intel_crtc,
> +                              struct intel_encoder *intel_encoder)
> +{
> +       struct drm_info_node *node = (struct drm_info_node *) m->private;
> +       struct drm_device *dev = node->minor->dev;
> +       struct drm_crtc *crtc = &intel_crtc->base;
> +       struct intel_connector *intel_connector;
> +       struct drm_encoder *encoder;
> +
> +       encoder = &intel_encoder->base;
> +       seq_printf(m, "\tencoder %d: type: %s, connectors:\n",
> +                  encoder->base.id, drm_get_encoder_name(encoder));
> +       for_each_connector_on_encoder(dev, encoder, intel_connector) {
> +               struct drm_connector *connector = &intel_connector->base;
> +               seq_printf(m, "\t\tconnector %d: type: %s, status: %s",
> +                          connector->base.id,
> +                          drm_get_connector_name(connector),
> +                          drm_get_connector_status_name(connector->status));
> +               if (connector->status == connector_status_connected) {
> +                       struct drm_display_mode *mode = &crtc->mode;
> +                       seq_printf(m, ", mode:\n");
> +                       seq_printf(m, "\t\t%d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x\n",
> +                                  mode->base.id, mode->name,
> +                                  mode->vrefresh, mode->clock,
> +                                  mode->hdisplay, mode->hsync_start,
> +                                  mode->hsync_end, mode->htotal,
> +                                  mode->vdisplay, mode->vsync_start,
> +                                  mode->vsync_end, mode->vtotal,
> +                                  mode->type, mode->flags);
> +               } else {
> +                       seq_printf(m, "\n");

for cases like this shouldn't we use seq_put instead of seq_printf?

> +               }
> +       }
> +}
> +
> +static void intel_crtc_info(struct seq_file *m, struct intel_crtc *intel_crtc)
> +{
> +       struct drm_info_node *node = (struct drm_info_node *) m->private;
> +       struct drm_device *dev = node->minor->dev;
> +       struct drm_crtc *crtc = &intel_crtc->base;
> +       struct intel_encoder *intel_encoder;
> +
> +       seq_printf(m, "\tfb: %d, pos: %dx%d, size: %dx%d\n",
> +                  crtc->fb->base.id, crtc->x, crtc->y,
> +                  crtc->fb->width, crtc->fb->height);
> +       for_each_encoder_on_crtc(dev, crtc, intel_encoder)
> +               intel_encoder_info(m, intel_crtc, intel_encoder);
> +}
> +
> +static void intel_panel_info(struct seq_file *m, struct intel_panel *panel)
> +{
> +       struct drm_display_mode *mode = panel->fixed_mode;
> +
> +       seq_printf(m, "\tfixed mode:\n");
> +       seq_printf(m, "\t\t%d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x\n",
> +                  mode->base.id, mode->name,
> +                  mode->vrefresh, mode->clock,
> +                  mode->hdisplay, mode->hsync_start,
> +                  mode->hsync_end, mode->htotal,
> +                  mode->vdisplay, mode->vsync_start,
> +                  mode->vsync_end, mode->vtotal,
> +                  mode->type, mode->flags);
> +}

I would prefer a more bloated info, with variable_name =
vriable_value... I know this is bloated, but I'm also think on our
lazyness when reading bug reports ;)

> +
> +static void intel_dp_info(struct seq_file *m,
> +                         struct intel_connector *intel_connector)
> +{
> +       struct intel_encoder *intel_encoder = intel_connector->encoder;
> +       struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
> +
> +       seq_printf(m, "\tDPCD rev: %x\n", intel_dp->dpcd[DP_DPCD_REV]);
> +       seq_printf(m, "\taudio support: %s\n", intel_dp->has_audio ? "yes" :
> +                  "no");
> +       if (intel_encoder->type == INTEL_OUTPUT_EDP)
> +               intel_panel_info(m, &intel_connector->panel);
> +}
> +
> +static void intel_hdmi_info(struct seq_file *m,
> +                           struct intel_connector *intel_connector)
> +{
> +       struct intel_encoder *intel_encoder = intel_connector->encoder;
> +       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
> +
> +       seq_printf(m, "\taudio support: %s\n", intel_hdmi->has_audio ? "yes" :
> +                  "no");
> +}
> +
> +static void intel_lvds_info(struct seq_file *m,
> +                           struct intel_connector *intel_connector)
> +{
> +       intel_panel_info(m, &intel_connector->panel);
> +}
> +
> +static void intel_connector_info(struct seq_file *m,
> +                                struct drm_connector *connector)
> +{
> +       struct intel_connector *intel_connector = to_intel_connector(connector);
> +       struct intel_encoder *intel_encoder = intel_connector->encoder;
> +
> +       seq_printf(m, "connector %d: type %s, status: %s\n",
> +                  connector->base.id, drm_get_connector_name(connector),
> +                  drm_get_connector_status_name(connector->status));
> +       if (connector->status == connector_status_connected) {
> +               seq_printf(m, "\tname: %s\n", connector->display_info.name);
> +               seq_printf(m, "\tphysical dimensions: %dx%dmm\n",
> +                          connector->display_info.width_mm,
> +                          connector->display_info.height_mm);
> +               seq_printf(m, "\tsubpixel order: %s\n",
> +                          drm_get_subpixel_order_name(connector->display_info.subpixel_order));
> +               seq_printf(m, "\tCEA rev: %d\n",
> +                          connector->display_info.cea_rev);
> +       }
> +       if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> +           intel_encoder->type == INTEL_OUTPUT_EDP)
> +               intel_dp_info(m, intel_connector);
> +       else if (intel_encoder->type == INTEL_OUTPUT_HDMI)
> +               intel_hdmi_info(m, intel_connector);
> +       else if (intel_encoder->type == INTEL_OUTPUT_LVDS)
> +               intel_lvds_info(m, intel_connector);
> +
> +}
> +
> +static int i915_display_info(struct seq_file *m, void *unused)
> +{
> +       struct drm_info_node *node = (struct drm_info_node *) m->private;
> +       struct drm_device *dev = node->minor->dev;
> +       struct drm_crtc *crtc;
> +       struct drm_connector *connector;
> +
> +       drm_modeset_lock_all(dev);
> +       seq_printf(m, "CRTC info\n");
> +       seq_printf(m, "---------\n");
> +       list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +               struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +               seq_printf(m, "CRTC %d: pipe: %c, active: %s\n",
> +                          crtc->base.id, pipe_name(intel_crtc->pipe),
> +                          intel_crtc->active ? "yes" : "no");
> +               if (intel_crtc->active)
> +                       intel_crtc_info(m, intel_crtc);
> +       }
> +
> +       seq_printf(m, "\n");
> +       seq_printf(m, "Connector info\n");
> +       seq_printf(m, "--------------\n");
> +       list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> +               intel_connector_info(m, connector);
> +       }
> +       drm_modeset_unlock_all(dev);
> +
> +       return 0;
> +}
> +
>  struct pipe_crc_info {
>         const char *name;
>         struct drm_device *dev;
> @@ -3235,6 +3389,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
>         {"i915_energy_uJ", i915_energy_uJ, 0},
>         {"i915_pc8_status", i915_pc8_status, 0},
>         {"i915_power_domain_info", i915_power_domain_info, 0},
> +       {"i915_display_info", i915_display_info, 0},
>  };
>  #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cc8afff..741d14e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -162,6 +162,10 @@ enum hpd_pin {
>         list_for_each_entry((intel_encoder), &(dev)->mode_config.encoder_list, base.head) \
>                 if ((intel_encoder)->base.crtc == (__crtc))
>
> +#define for_each_connector_on_encoder(dev, __encoder, intel_connector) \
> +       list_for_each_entry((intel_connector), &(dev)->mode_config.connector_list, base.head) \
> +               if ((intel_connector)->base.encoder == (__encoder))

are all of this parenthesis needed?

> +
>  struct drm_i915_private;
>
>  enum intel_dpll_id {
> --
> 1.8.3.2
>

with or without my bikesheds, feel free to use:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>


> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br



More information about the Intel-gfx mailing list