On Wed, 06 Nov 2024, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > The code initializing the power sequencing delays is a bit > hard to follow. One confusing thing is that we keep doing the > +/-1 adjustment for the hardware register value in several places. > Simplify this a bit by doing the adjustment only when reading or > writing the actual register. > > This also matches how the LVDS code does things. > > Signed-off-by: Ville Syrjälä <[email protected]>
Reviewed-by: Jani Nikula <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_pps.c | 28 ++++++++++-------------- > 1 file changed, 12 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c > b/drivers/gpu/drm/i915/display/intel_pps.c > index 093fe37a3983..83d65105f32b 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.c > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > @@ -1390,7 +1390,7 @@ static void > intel_pps_readout_hw_state(struct intel_dp *intel_dp, struct edp_power_seq > *seq) > { > struct intel_display *display = to_intel_display(intel_dp); > - u32 pp_on, pp_off, pp_ctl; > + u32 pp_on, pp_off, pp_ctl, power_cycle_delay; > struct pps_registers regs; > > intel_pps_get_registers(intel_dp, ®s); > @@ -1415,10 +1415,13 @@ intel_pps_readout_hw_state(struct intel_dp *intel_dp, > struct edp_power_seq *seq) > > pp_div = intel_de_read(display, regs.pp_div); > > - seq->t11_t12 = REG_FIELD_GET(PANEL_POWER_CYCLE_DELAY_MASK, > pp_div) * 1000; > + power_cycle_delay = REG_FIELD_GET(PANEL_POWER_CYCLE_DELAY_MASK, > pp_div); > } else { > - seq->t11_t12 = REG_FIELD_GET(BXT_POWER_CYCLE_DELAY_MASK, > pp_ctl) * 1000; > + power_cycle_delay = REG_FIELD_GET(BXT_POWER_CYCLE_DELAY_MASK, > pp_ctl); > } > + > + /* hardware wants <delay>+1 in 100ms units */ > + seq->t11_t12 = power_cycle_delay ? (power_cycle_delay - 1) * 1000 : 0; > } > > static void > @@ -1494,12 +1497,6 @@ static void pps_init_delays_vbt(struct intel_dp > *intel_dp, > vbt->t11_t12); > } > > - /* T11_T12 delay is special and actually in units of 100ms, but zero > - * based in the hw (so we need to add 100 ms). But the sw vbt > - * table multiplies it with 1000 to make it in units of 100usec, > - * too. */ > - vbt->t11_t12 += 100 * 10; > - > intel_pps_dump_state(intel_dp, "vbt", vbt); > } > > @@ -1516,11 +1513,7 @@ static void pps_init_delays_spec(struct intel_dp > *intel_dp, > spec->t8 = 50 * 10; /* no limit for t8, use t7 instead */ > spec->t9 = 50 * 10; /* no limit for t9, make it symmetric with t8 */ > spec->t10 = 500 * 10; > - /* This one is special and actually in units of 100ms, but zero > - * based in the hw (so we need to add 100 ms). But the sw vbt > - * table multiplies it with 1000 to make it in units of 100usec, > - * too. */ > - spec->t11_t12 = (510 + 100) * 10; > + spec->t11_t12 = 510 * 10; > > intel_pps_dump_state(intel_dp, "spec", spec); > } > @@ -1665,11 +1658,14 @@ static void pps_init_registers(struct intel_dp > *intel_dp, bool force_disable_vdd > */ > if (i915_mmio_reg_valid(regs.pp_div)) > intel_de_write(display, regs.pp_div, > - REG_FIELD_PREP(PP_REFERENCE_DIVIDER_MASK, (100 * > div) / 2 - 1) | REG_FIELD_PREP(PANEL_POWER_CYCLE_DELAY_MASK, > DIV_ROUND_UP(seq->t11_t12, 1000))); > + REG_FIELD_PREP(PP_REFERENCE_DIVIDER_MASK, > + (100 * div) / 2 - 1) | > + REG_FIELD_PREP(PANEL_POWER_CYCLE_DELAY_MASK, > + DIV_ROUND_UP(seq->t11_t12, 1000) > + 1)); > else > intel_de_rmw(display, regs.pp_ctrl, BXT_POWER_CYCLE_DELAY_MASK, > REG_FIELD_PREP(BXT_POWER_CYCLE_DELAY_MASK, > - DIV_ROUND_UP(seq->t11_t12, 1000))); > + DIV_ROUND_UP(seq->t11_t12, 1000) + > 1)); > > drm_dbg_kms(display->drm, > "panel power sequencer register settings: PP_ON %#x, PP_OFF > %#x, PP_DIV %#x\n", -- Jani Nikula, Intel
