2013/5/21 Daniel Vetter <daniel.vetter at ffwll.ch>:
> All this pipe config abstraction adds another layer of complexity, so
> it's good to have better visibility into what's going on exactly.
> Doesn't dump out everything yet, and some bits are a bit duplicated
> but this should be a good start.
>
> v2: Remove a few more now redudant debug output lines.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 74 +++++++++++++++++++++---------------
> 1 file changed, 44 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a42a7fd..5efb00d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3621,18 +3621,12 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
> if (!crtc->config.gmch_pfit.control)
> return;
>
> - WARN_ON(I915_READ(PFIT_CONTROL) & PFIT_ENABLE);
> - assert_pipe_disabled(dev_priv, crtc->pipe);
> -
> /*
> - * Enable automatic panel scaling so that non-native modes
> - * fill the screen. The panel fitter should only be
> - * adjusted whilst the pipe is disabled, according to
> - * register description and PRM.
> + * The panel fitter should only be adjusted whilst the pipe is disabled,
> + * according to register description and PRM.
> */
> - DRM_DEBUG_KMS("applying panel-fitter: %x, %x\n",
> - pipe_config->gmch_pfit.control,
> - pipe_config->gmch_pfit.pgm_ratios);
> + WARN_ON(I915_READ(PFIT_CONTROL) & PFIT_ENABLE);
> + assert_pipe_disabled(dev_priv, crtc->pipe);
>
> I915_WRITE(PFIT_PGM_RATIOS, pipe_config->gmch_pfit.pgm_ratios);
> I915_WRITE(PFIT_CONTROL, pipe_config->gmch_pfit.control);
> @@ -4955,9 +4949,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> dspcntr |= DISPPLANE_SEL_PIPE_B;
> }
>
> - DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe_name(pipe));
> - drm_mode_debug_printmodeline(mode);
> -
> intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
>
> /* pipesrc and dspsize control the size that is scaled from,
> @@ -5759,9 +5750,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> /* Ensure that the cursor is valid for the new mode before changing... */
> intel_crtc_update_cursor(crtc, true);
>
> - DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe_name(pipe));
> - drm_mode_debug_printmodeline(mode);
> -
> /* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
> if (intel_crtc->config.has_pch_encoder) {
> struct intel_pch_pll *pll;
> @@ -5972,9 +5960,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> /* Ensure that the cursor is valid for the new mode before changing... */
> intel_crtc_update_cursor(crtc, true);
>
> - DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe_name(pipe));
> - drm_mode_debug_printmodeline(mode);
> -
> if (intel_crtc->config.has_dp_encoder)
> intel_dp_set_m_n(intel_crtc);
>
> @@ -7783,6 +7768,35 @@ pipe_config_set_bpp(struct drm_crtc *crtc,
> return bpp;
> }
>
> +static void intel_dump_pipe_config(struct intel_crtc *crtc,
> + struct intel_crtc_config *pipe_config,
> + const char *context)
> +{
> + DRM_DEBUG_KMS("[CRTC:%d]%s config for pipe %c\n", crtc->base.base.id,
> + context, pipe_name(crtc->pipe));
> +
> + DRM_DEBUG_KMS("cpu_transcoder: %i\n", pipe_name(pipe_config->cpu_transcoder));
You should use %c, not %i. We also have the transcoder_name macro,
which will be useful if we ever decide we want to print "EDP" for the
EDP transcoder.
> + DRM_DEBUG_KMS("pipe bpp: %i, dithering: %i\n",
> + pipe_config->pipe_bpp, pipe_config->dither);
> + DRM_DEBUG_KMS("fdi/pch: %i, lanes: %i, gmch_m: %i, gmch_n %i, link_m: %i, link_n: %i, tu: %i\n",
You're missing a ':' character after gmch_n.
> + pipe_config->has_pch_encoder,
> + pipe_config->fdi_lanes,
> + pipe_config->fdi_m_n.gmch_m, pipe_config->fdi_m_n.gmch_n,
> + pipe_config->fdi_m_n.link_m, pipe_config->fdi_m_n.link_n,
These 4 are unsigned, so maybe %u or 0x%x?
> + pipe_config->fdi_m_n.tu);
> + DRM_DEBUG_KMS("requested mode:\n");
> + drm_mode_debug_printmodeline(&pipe_config->requested_mode);
> + DRM_DEBUG_KMS("adjusted mode:\n");
> + drm_mode_debug_printmodeline(&pipe_config->adjusted_mode);
> + DRM_DEBUG_KMS("gmch pfit: control: 0x%08x, ratios: 0x%08x, lvds border: 0x%08x\n",
> + pipe_config->gmch_pfit.control,
> + pipe_config->gmch_pfit.pgm_ratios,
> + pipe_config->gmch_pfit.lvds_border_bits);
> + DRM_DEBUG_KMS("pch pfit: pos: 0x%08x, size: 0x%08x\n",
> + pipe_config->pch_pfit.pos,
> + pipe_config->pch_pfit.size);
You could maybe break pos and size into x and y. Or just wait until
someone fully splits the struct itself into more pieces, as Ville
suggested a few days ago.
> +}
> +
> static struct intel_crtc_config *
> intel_modeset_pipe_config(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> @@ -7853,8 +7867,6 @@ encoder_retry:
> goto encoder_retry;
> }
>
> - DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
> -
> pipe_config->dither = pipe_config->pipe_bpp != plane_bpp;
> DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
> plane_bpp, pipe_config->pipe_bpp, pipe_config->dither);
> @@ -8211,9 +8223,14 @@ intel_modeset_check_state(struct drm_device *dev)
> "crtc active state doesn't match with hw state "
> "(expected %i, found %i)\n", crtc->active, active);
>
> - WARN(active &&
> - !intel_pipe_config_compare(dev, &crtc->config, &pipe_config),
> - "pipe state doesn't match!\n");
> + if (active &&
> + !intel_pipe_config_compare(dev, &crtc->config, &pipe_config)) {
> + WARN(1, "pipe state doesn't match!\n");
> + intel_dump_pipe_config(crtc, &pipe_config,
> + "[hw state]");
> + intel_dump_pipe_config(crtc, &crtc->config,
> + "[sw state]");
> + }
> }
> }
>
> @@ -8253,6 +8270,8 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>
> goto out;
> }
> + intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> + "[modeset]");
> }
>
> for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
> @@ -8609,12 +8628,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> goto fail;
>
> if (config->mode_changed) {
> - if (set->mode) {
> - DRM_DEBUG_KMS("attempting to set mode from"
> - " userspace\n");
> - drm_mode_debug_printmodeline(set->mode);
> - }
> -
> ret = intel_set_mode(set->crtc, set->mode,
> set->x, set->y, set->fb);
> } else if (config->fb_changed) {
Patch doesn't apply anymore due to a recent code change here.
Besides this, I also booted a kernel with this patch, and on the first
time we print these things, almost everything is zero, even stuff like
pipe_bpp, even for enabled pipes. Is this expected?
> @@ -9629,6 +9642,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> for_each_pipe(pipe) {
> crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> intel_sanitize_crtc(crtc);
> + intel_dump_pipe_config(crtc, &crtc->config, "[setup_hw_state]");
> }
>
> if (force_restore) {
> --
> 1.8.1.4
>
--
Paulo Zanoni
