| Message ID | 20240306083427.2040475-4-jouni.hogander@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IO and fast wake lines calculation and increase fw sync length | expand |
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@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? > } > > + 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
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@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 >
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; } + 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(
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@intel.com> --- drivers/gpu/drm/i915/display/intel_psr.c | 34 ++++++++++++++++++------ 1 file changed, 26 insertions(+), 8 deletions(-)