On Thu, 2023-03-30 at 07:22 +0000, Hogander, Jouni wrote:
> On Wed, 2023-03-29 at 20:24 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Replace the hardcoded final numbers in the AUX SYNC/precharge
> > setup, and derive those from numbers from the (e)DP specs.
> >
> > The new functions can serve as the single point of truth for
> > the number of SYNC pulses we use.
> >
> > Cc: Jouni Högander <jouni.hogander@xxxxxxxxx>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Let's take care of that io/fast wake calculation separately. I think we
want to have these in as currently fast wake SYNC pulse count is not
according to spec.
Reviewed-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_dp_aux.c | 32
> > +++++++++++++++++++--
> > 1 file changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > index ad0aac707219..374492293392 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > @@ -118,6 +118,32 @@ static u32 skl_get_aux_clock_divider(struct
> > intel_dp *intel_dp, int index)
> > return index ? 0 : 1;
> > }
> >
> > +static int intel_dp_aux_sync_len(void)
> > +{
> > + int precharge = 16; /* 10-16 */
> > + int preamble = 16;
> > +
> > + return precharge + preamble;
> > +}
> > +
> > +static int intel_dp_aux_fw_sync_len(void)
> > +{
> > + int precharge = 16; /* 10-16 */
> > + int preamble = 8;
> > +
> > + return precharge + preamble;
> > +}
>
> What do you think if we move this into intel_dp_aux.h and use that to
> calculate io wake time and fast wake time in
> intel_psr.c:_compute_psr2_wake_time.
>
> > +
> > +static int g4x_dp_aux_precharge_len(void)
> > +{
> > + int precharge_min = 10;
> > + int preamble = 16;
> > +
> > + /* HW wants the length of the extra precharge in 2us units
> > */
> > + return (intel_dp_aux_sync_len() -
> > + precharge_min - preamble) / 2;
> > +}
> > +
> > static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > int send_bytes,
> > u32 aux_clock_divider)
> > @@ -140,7 +166,7 @@ static u32 g4x_get_aux_send_ctl(struct intel_dp
> > *intel_dp,
> > timeout |
> > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> > - (3 << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> > + (g4x_dp_aux_precharge_len() <<
> > DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> > (aux_clock_divider <<
> > DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT);
> > }
> >
> > @@ -164,8 +190,8 @@ static u32 skl_get_aux_send_ctl(struct intel_dp
> > *intel_dp,
> > DP_AUX_CH_CTL_TIME_OUT_MAX |
> > DP_AUX_CH_CTL_RECEIVE_ERROR |
> > (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> > - DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(24) |
> > - DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> > +
> > DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(intel_dp_aux_fw_sync_len()) |
> > +
> > DP_AUX_CH_CTL_SYNC_PULSE_SKL(intel_dp_aux_sync_len());
> >
> > if (intel_tc_port_in_tbt_alt_mode(dig_port))
> > ret |= DP_AUX_CH_CTL_TBT_IO;
>
