On Thu, Feb 22, 2024 at 04:54:07PM -0500, Rodrigo Vivi wrote: > On Thu, Feb 15, 2024 at 06:40:49PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Utilize drm_printer in pipe_config_pll_mismatch() to avoid > > a bit of code duplication. > > > > To achieve this we need to plumb the printer all way to the > > dpll_mgr .dump_hw_state() functions. Those are also used by > > intel_crtc_state_dump() which needs to be adjusted as well. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > .../drm/i915/display/intel_crtc_state_dump.c | 5 +- > > drivers/gpu/drm/i915/display/intel_display.c | 27 ++--- > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 105 ++++++++---------- > > drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 2 + > > 4 files changed, 67 insertions(+), 72 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c > > index 4bcf446c75f4..59d2b3d39951 100644 > > --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c > > +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c > > @@ -205,9 +205,12 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config, > > struct drm_i915_private *i915 = to_i915(crtc->base.dev); > > const struct intel_plane_state *plane_state; > > struct intel_plane *plane; > > + struct drm_printer p; > > char buf[64]; > > int i; > > > > + p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL); > > + > > drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] enable: %s [%s]\n", > > crtc->base.base.id, crtc->base.name, > > str_yes_no(pipe_config->hw.enable), context); > > @@ -356,7 +359,7 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config, > > pipe_config->ips_enabled, pipe_config->double_wide, > > pipe_config->has_drrs); > > > > - intel_dpll_dump_hw_state(i915, &pipe_config->dpll_hw_state); > > + intel_dpll_dump_hw_state(i915, &p, &pipe_config->dpll_hw_state); > > > > if (IS_CHERRYVIEW(i915)) > > drm_dbg_kms(&i915->drm, > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index e5010049d52e..d7f39ad84138 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -4927,26 +4927,27 @@ pipe_config_pll_mismatch(bool fastset, > > const struct intel_dpll_hw_state *b) > > { > > struct drm_i915_private *i915 = to_i915(crtc->base.dev); > > + struct drm_printer p; > > > > if (fastset) { > > if (!drm_debug_enabled(DRM_UT_KMS)) > > return; > > > > - drm_dbg_kms(&i915->drm, > > - "[CRTC:%d:%s] fastset requirement not met in %s\n", > > - crtc->base.base.id, crtc->base.name, name); > > - drm_dbg_kms(&i915->drm, "expected:\n"); > > - intel_dpll_dump_hw_state(i915, a); > > - drm_dbg_kms(&i915->drm, "found:\n"); > > - intel_dpll_dump_hw_state(i915, b); > > + p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL); > > + > > + drm_printf(&p, "[CRTC:%d:%s] fastset requirement not met in %s\n", > > + crtc->base.base.id, crtc->base.name, name); > > } else { > > - drm_err(&i915->drm, "[CRTC:%d:%s] mismatch in %s buffer\n", > > - crtc->base.base.id, crtc->base.name, name); > > - drm_err(&i915->drm, "expected:\n"); > > - intel_dpll_dump_hw_state(i915, a); > > - drm_err(&i915->drm, "found:\n"); > > - intel_dpll_dump_hw_state(i915, b); > > + p = drm_err_printer(&i915->drm, NULL); > > + > > + drm_printf(&p, "[CRTC:%d:%s] mismatch in %s\n", > > + crtc->base.base.id, crtc->base.name, name); > > } > > + > > + drm_dbg_kms(&i915->drm, "expected:\n"); > > + intel_dpll_dump_hw_state(i915, &p, a); > > + drm_dbg_kms(&i915->drm, "found:\n"); > > + intel_dpll_dump_hw_state(i915, &p, b); > > forgot to convert here? Looks like that part ended up in the last patch. Probably a rebase fail on my part. I'll shuffle it back over here. -- Ville Syrjälä Intel