On Tue, Nov 11, 2025 at 01:16:46PM +0200, Jani Nikula wrote:
> On Tue, 11 Nov 2025, Jani Nikula <[email protected]> wrote:
> > On Fri, 31 Oct 2025, Mika Kahola <[email protected]> wrote:
> >> From: Imre Deak <[email protected]>
> >>
> >> Print all the Cx0 PLL state in the PLL state dumper.
> >>
> >> Signed-off-by: Imre Deak <[email protected]>
> >> Signed-off-by: Mika Kahola <[email protected]>
> >> ---
> >> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 18 +++++++++++++++---
> >> 1 file changed, 15 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> index 3418fc560faf..1e68a73c2ca8 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> @@ -2311,8 +2311,8 @@ static void intel_c10pll_dump_hw_state(struct
> >> intel_display *display,
> >> unsigned int multiplier, tx_clk_div;
> >>
> >> fracen = hw_state->pll[0] & C10_PLL0_FRACEN;
> >> - drm_dbg_kms(display->drm, "c10pll_hw_state: fracen: %s, ",
> >> - str_yes_no(fracen));
> >> + drm_dbg_kms(display->drm, "c10pll_hw_state: clock: %d, fracen: %s, ",
> >> + hw_state->clock, str_yes_no(fracen));
> >>
> >> if (fracen) {
> >> frac_quot = hw_state->pll[12] << 8 | hw_state->pll[11];
> >> @@ -2835,7 +2835,7 @@ static void intel_c20pll_dump_hw_state(struct
> >> intel_display *display,
> >> {
> >> int i;
> >>
> >> - drm_dbg_kms(display->drm, "c20pll_hw_state:\n");
> >> + drm_dbg_kms(display->drm, "c20pll_hw_state clock: %d:\n",
> >> hw_state->clock);
> >> drm_dbg_kms(display->drm,
> >> "tx[0] = 0x%.4x, tx[1] = 0x%.4x, tx[2] = 0x%.4x\n",
> >> hw_state->tx[0], hw_state->tx[1], hw_state->tx[2]);
> >> @@ -2851,12 +2851,24 @@ static void intel_c20pll_dump_hw_state(struct
> >> intel_display *display,
> >> for (i = 0; i < ARRAY_SIZE(hw_state->mplla); i++)
> >> drm_dbg_kms(display->drm, "mplla[%d] = 0x%.4x\n", i,
> >> hw_state->mplla[i]);
> >> +
> >> + /* For full coverage, also print the additional PLL B entry. */
> >> + WARN_ON(i + 1 != ARRAY_SIZE(hw_state->mpllb));
> >
> > Why? What if we hit this? At the very least please use
> > drm_WARN_ON(). What does the comment have to do with the WARN?
The WARN verifies that the additional entry to include in the print
exists and it is the only entry to print.
> Besides after the loop i == ARRAY_SIZE(hw_state->mplla), i.e. the whole
> thing can be
>
> BUILD_BUG_ON(ARRAY_SIZE(hw_state->mplla) + 1 !=
> ARRAY_SIZE(hw_state->mpllb));
Yes, BUILD_BUG_ON() is better here, no reason for delaying the check
until runtime.
> >
> >> + drm_dbg_kms(display->drm, "mpllb[%d] = 0x%.4x\n", i,
> >> hw_state->mpllb[i]);
> >> }
> >> +
> >> + drm_dbg_kms(display->drm, "vdr: custom width: 0x%02x, serdes rate:
> >> 0x%02x, hdmi rate: 0x%02x\n",
> >> + hw_state->vdr.custom_width, hw_state->vdr.serdes_rate,
> >> hw_state->vdr.hdmi_rate);
> >> }
> >>
> >> void intel_cx0pll_dump_hw_state(struct intel_display *display,
> >> const struct intel_cx0pll_state *hw_state)
> >> {
> >> + drm_dbg_kms(display->drm,
> >> + "cx0pll_hw_state: lane_count: %d, ssc_enabled: %s, use_c10:
> >> %s, tbt_mode: %s\n",
> >> + hw_state->lane_count, str_yes_no(hw_state->ssc_enabled),
> >> + str_yes_no(hw_state->use_c10),
> >> str_yes_no(hw_state->tbt_mode));
> >> +
> >> if (hw_state->use_c10)
> >> intel_c10pll_dump_hw_state(display, &hw_state->c10);
> >> else
>
> --
> Jani Nikula, Intel
