> Subject: RE: [PATCH v2 11/24] drm/i915/lt_phy: Add xe3plpd .dump_hw_state > hook > > > Subject: [PATCH v2 11/24] drm/i915/lt_phy: Add xe3plpd .dump_hw_state > > hook > > > > Add .dump_hw_state function pointer for xe3plpd platform to support > > dpll framework. While at it, switch to use drm_printer structure to > > print hw state information. > >
Ahh ohkay so you do the change here, Maybe move the patch right after the patch where you introduce drm_printer Otherwise, Reviewed-by: Suraj Kandpal <[email protected]> > > Signed-off-by: Mika Kahola <[email protected]> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 5 ++--- > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 7 +++++++ > > drivers/gpu/drm/i915/display/intel_lt_phy.c | 16 ++++++++-------- > > drivers/gpu/drm/i915/display/intel_lt_phy.h | 3 ++- > > 4 files changed, 19 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 27354585ba92..d67ec81c0b01 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -5065,15 +5065,14 @@ pipe_config_lt_phy_pll_mismatch(struct > > drm_printer *p, bool fastset, > > const struct intel_lt_phy_pll_state *a, > > const struct intel_lt_phy_pll_state *b) { > > - struct intel_display *display = to_intel_display(crtc); > > char *chipname = "LTPHY"; > > > > pipe_config_mismatch(p, fastset, crtc, name, chipname); > > > > drm_printf(p, "expected:\n"); > > - intel_lt_phy_dump_hw_state(display, a); > > + intel_lt_phy_dump_hw_state(p, a); > > drm_printf(p, "found:\n"); > > - intel_lt_phy_dump_hw_state(display, b); > > + intel_lt_phy_dump_hw_state(p, b); > > } > > > > bool > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > index b50f02303356..26b78063dd94 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > @@ -4649,6 +4649,12 @@ static int xe3plpd_compute_dplls(struct > > intel_atomic_state *state, > > return xe3plpd_compute_non_tc_phy_dpll(state, crtc, > encoder); } > > > > +static void xe3plpd_dump_hw_state(struct drm_printer *p, > > + const struct intel_dpll_hw_state > > *dpll_hw_state) { > > + intel_lt_phy_dump_hw_state(p, &dpll_hw_state->ltpll); } > > + > > __maybe_unused > > static const struct intel_dpll_mgr xe3plpd_pll_mgr = { > > .dpll_info = xe3plpd_plls, > > @@ -4657,6 +4663,7 @@ static const struct intel_dpll_mgr > xe3plpd_pll_mgr = { > > .put_dplls = icl_put_dplls, > > .update_active_dpll = icl_update_active_dpll, > > .update_ref_clks = icl_update_dpll_ref_clks, > > + .dump_hw_state = xe3plpd_dump_hw_state, > > }; > > > > /** > > diff --git a/drivers/gpu/drm/i915/display/intel_lt_phy.c > > b/drivers/gpu/drm/i915/display/intel_lt_phy.c > > index ca31b3c1440c..923ee132ec3c 100644 > > --- a/drivers/gpu/drm/i915/display/intel_lt_phy.c > > +++ b/drivers/gpu/drm/i915/display/intel_lt_phy.c > > @@ -2146,23 +2146,23 @@ void intel_lt_phy_set_signal_levels(struct > > intel_encoder *encoder, > > intel_lt_phy_transaction_end(encoder, wakeref); } > > > > -void intel_lt_phy_dump_hw_state(struct intel_display *display, > > +void intel_lt_phy_dump_hw_state(struct drm_printer *p, > > const struct intel_lt_phy_pll_state *hw_state) > { > > int i, j; > > > > - drm_dbg_kms(display->drm, "lt_phy_pll_hw_state: ssc enabled: %d, > > tbt mode: %d\n", > > - hw_state->ssc_enabled, hw_state->tbt_mode); > > + drm_printf(p, "lt_phy_pll_hw_state: ssc enabled: %d, tbt mode: > %d\n", > > + hw_state->ssc_enabled, hw_state->tbt_mode); > > Maybe not something for this patch to fix but a separate patch adding debug > print for Lane_count since we cache it now > > Otherwise, > LGTM, > Reviewed-by: Suraj Kandpal <[email protected]> > > > > > for (i = 0; i < 3; i++) { > > - drm_dbg_kms(display->drm, "config[%d] = 0x%.4x,\n", > > - i, hw_state->config[i]); > > + drm_printf(p, "config[%d] = 0x%.4x,\n", > > + i, hw_state->config[i]); > > } > > > > for (i = 0; i <= 12; i++) > > for (j = 3; j >= 0; j--) > > - drm_dbg_kms(display->drm, "vdr_data[%d][%d] = > > 0x%.4x,\n", > > - i, j, hw_state->data[i][j]); > > + drm_printf(p, "vdr_data[%d][%d] = 0x%.4x,\n", > > + i, j, hw_state->data[i][j]); > > } > > > > bool > > @@ -2330,7 +2330,7 @@ static void intel_lt_phy_pll_verify_clock(struct > > intel_display *display, > > drm_printf(&p, "PLL state %s (%s):\n", > > pll_state_name, > > is_precomputed_state ? "precomputed" : "computed"); > > - intel_lt_phy_dump_hw_state(display, pll_state); > > + intel_lt_phy_dump_hw_state(&p, pll_state); > > } > > > > static void intel_lt_phy_pll_verify_params(struct intel_display > > *display, diff --git a/drivers/gpu/drm/i915/display/intel_lt_phy.h > > b/drivers/gpu/drm/i915/display/intel_lt_phy.h > > index 61ec0e5d8888..b208bbd6f8ca 100644 > > --- a/drivers/gpu/drm/i915/display/intel_lt_phy.h > > +++ b/drivers/gpu/drm/i915/display/intel_lt_phy.h > > @@ -8,6 +8,7 @@ > > > > #include <linux/types.h> > > > > +struct drm_printer; > > struct intel_atomic_state; > > struct intel_display; > > struct intel_encoder; > > @@ -26,7 +27,7 @@ int intel_lt_phy_calc_port_clock(struct > > intel_display *display, > > const struct intel_lt_phy_pll_state *lt_state); > void > > intel_lt_phy_set_signal_levels(struct intel_encoder *encoder, > > const struct intel_crtc_state *crtc_state); > > - > void > > intel_lt_phy_dump_hw_state(struct intel_display *display, > > +void intel_lt_phy_dump_hw_state(struct drm_printer *p, > > const struct intel_lt_phy_pll_state > *hw_state); bool > > intel_lt_phy_pll_compare_hw_state(const struct intel_lt_phy_pll_state > > *a, > > -- > > 2.43.0
