[PATCH v4 3/5] drm/i915/psr: Calculate IO wake and fast wake lines for DISPLAY_VER < 12

[Hogander, Jouni]

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 at intel.com>
> > ---
> >  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
>