On Thu, 2024-03-07 at 19:16 +0200, Ville Syrjälä wrote:
> On Wed, Mar 06, 2024 at 10:34:25AM +0200, Jouni Högander wrote:
> > Bspec mentions 50 us for IO wake time and 32 us for fast wake time.
> > 32 us
> > is most probably wrong as it doesn't meet the specification as fast
> > wake
> > time is calculated in Bspec like this:
> >
> > 10..16 us (precharge) + 8 us (preamble) + 4 us (phy_wake) + 20 us
> > (tfw_exit_latency)
> >
> > Instead of using these constants calculate IO wake and fast wake
> > for
> > DISPLAY_VER < 12 as well.
> >
> > v2:
> > - initialize io/fast_wake_time for display version < 9
> >
> > Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_psr.c | 34 ++++++++++++++++++--
> > ----
> > 1 file changed, 26 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 905208e1c771..e1df0ece9fa3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1150,9 +1150,25 @@ static bool _lnl_compute_alpm_params(struct
> > intel_dp *intel_dp,
> > return true;
> > }
> >
> > -static int get_io_buffer_wake_time(void)
> > +/*
> > + * From Bspec:
> > + *
> > + * For DISPLAY_VER >= 12
> > + * 10 us
> > + *
> > + * For DISPLAY_VER < 12
> > + * This is not directly mentioned in Bspec. There are 50 us io
> > wake time and 32
> > + * us fast wake time. Clearly preharge pulses are not (improperly)
> > included in
> > + * 32 us fast wake time. 50 us - 32 us = 18 us.
> > + */
> > +static int get_io_buffer_wake_time(const struct intel_crtc_state
> > *crtc_state)
> > {
> > - return 10;
> > + struct drm_i915_private *i915 = to_i915(crtc_state-
> > >uapi.crtc->dev);
> > +
> > + if (DISPLAY_VER(i915) < 12)
> > + return 18;
> > + else
> > + return 10;
> > }
> >
> > static bool _compute_alpm_params(struct intel_dp *intel_dp,
> > @@ -1162,8 +1178,8 @@ static bool _compute_alpm_params(struct
> > intel_dp *intel_dp,
> > int io_wake_lines, io_wake_time, fast_wake_lines,
> > fast_wake_time;
> > u8 max_wake_lines;
> >
> > - if (DISPLAY_VER(i915) >= 12) {
> > - int io_buffer_wake_time =
> > get_io_buffer_wake_time();
> > + if (DISPLAY_VER(i915) >= 9) {
> > + int io_buffer_wake_time =
> > get_io_buffer_wake_time(crtc_state);
> > int tfw_exit_latency = 20; /* eDP spec */
> > int phy_wake = 4; /* eDP spec */
> > int preamble = 8; /* eDP spec */
> > @@ -1173,15 +1189,17 @@ static bool _compute_alpm_params(struct
> > intel_dp *intel_dp,
> > phy_wake + tfw_exit_latency;
> > fast_wake_time = precharge + preamble + phy_wake +
> > tfw_exit_latency;
> > -
> > - /* TODO: Check how we can use ALPM_CTL fast wake
> > extended field */
> > - max_wake_lines = 12;
> > } else {
> > io_wake_time = 50;
> > fast_wake_time = 32;
> > - max_wake_lines = 8;
>
> Isn't the whole else branch dead code now?
Yes, that shouldn't actually happen (due to transcoder_has_psr2). Maybe
best way is to remove if and else. I will change that unless you have
better idea.
BR,
Jouni Högander
>
> > }
> >
> > + if (DISPLAY_VER(i915) >= 12)
> > + /* TODO: Check how we can use ALPM_CTL fast wake
> > extended field */
> > + max_wake_lines = 12;
> > + else
> > + max_wake_lines = 8;
> > +
> > io_wake_lines = intel_usecs_to_scanlines(
> > &crtc_state->hw.adjusted_mode, io_wake_time);
> > fast_wake_lines = intel_usecs_to_scanlines(
> > --
> > 2.34.1
>
