[Intel-gfx] [RESEND PATCHv2] drm/i915/display/dp: 128/132b LT requirement
Murthy, Arun R
arun.r.murthy at intel.com
Wed Apr 19 10:07:46 UTC 2023
> -----Original Message-----
> From: Nikula, Jani <jani.nikula at intel.com>
> Sent: Wednesday, April 19, 2023 3:26 PM
> To: Murthy, Arun R <arun.r.murthy at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: ville.syrjala at linux.intel.com
> Subject: RE: [RESEND PATCHv2] drm/i915/display/dp: 128/132b LT
> requirement
>
> On Wed, 19 Apr 2023, "Murthy, Arun R" <arun.r.murthy at intel.com> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani <jani.nikula at intel.com>
> >> Sent: Wednesday, April 19, 2023 12:48 PM
> >> To: Murthy, Arun R <arun.r.murthy at intel.com>; intel-
> >> gfx at lists.freedesktop.org
> >> Cc: Murthy, Arun R <arun.r.murthy at intel.com>
> >> Subject: Re: [RESEND PATCHv2] drm/i915/display/dp: 128/132b LT
> >> requirement
> >>
> >> On Wed, 19 Apr 2023, Arun R Murthy <arun.r.murthy at intel.com> wrote:
> >> > For 128b/132b LT prior to LT DPTX should set power state, DP
> >> > channel coding and then link rate.
> >> >
> >> > v2: added separate function to avoid code duplication(Jani N)
> >> >
> >> > Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>
> >>
> >> RESEND for what reason?
> > Typo is sending V2 patch hence corrected and sent it again.
> >
> >>
> >> Two v2 and neither fixes
> >> https://lore.kernel.org/r/87o7nmergw.fsf@intel.com
> > This is pointing to the v1 patch.
> > V2 patch addressing review comments can be located @
> > https://lore.kernel.org/all/20230419022522.3457924-1-arun.r.murthy@int
> > el.com/
>
> Argh.
>
> RESEND means you're sending the exact same patch again. Hence *re-send*.
> That's what I thought. That's what everyone would think.
>
> It's even documented in submitting-patches.rst [1].
>
> ---
>
> There's still the question of whether we could just change the order for
> 8b/10b too [2]. On IRC, Ville thinks we could, "i don't think there is any order
> specified. just use the same alwas imo".
>
Spec DP2.1 section 3.5.1.2 (for 8b/10b LT)
write the following Link Configuration parameters:
* LINK_BW_SET register (DPCD 00100h)
* LANE_COUNT_SET field in the LANE_COUNT_SET register (DPCD 00101h[4:0])
* DOWNSPREAD_CTRL register (DPCD 00107h)
* MAIN_LINK_CHANNEL_CODING_SET register (DPCD 00108h)
Whereas for 128b/132b section 3.5.2.16 says
Prior to link training, a DPTX should perform the following:
1 Verify that the SET_POWER_STATE field in the
SET_POWER_AND_SET_DP_PWR_VOLTAGE register is programmed to D0 normal
operation (DPCD 00600h[2:0] = 001b).
2 Write DPCD 00108h = 02h to select 128b/132b DP channel coding.
3 Program the target link rate and lane count by way of an AUX write transaction to
DPCD 00100h and 00101h, respectively
Thanks and Regards,
Arun R Murthy
-------------------
>
> BR,
> Jani.
>
>
> [1] https://docs.kernel.org/process/submitting-patches.html#don-t-get-
> discouraged-or-impatient
> [2] https://lore.kernel.org/r/87r0siernf.fsf@intel.com
>
>
>
>
>
>
> >
> > Thanks and Regards,
> > Arun R Murthy
> > --------------------
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >> > ---
> >> > .../drm/i915/display/intel_dp_link_training.c | 62
> >> > +++++++++++++------
> >> > 1 file changed, 44 insertions(+), 18 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> >> > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> >> > index 6aa4ae5e7ebe..e5809cf7d0c4 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> >> > @@ -637,6 +637,37 @@ static bool
> >> intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp,
> >> > return true;
> >> > }
> >> >
> >> > +static void
> >> > +intel_dp_update_downspread_ctrl(struct intel_dp *intel_dp,
> >> > + const struct intel_crtc_state *crtc_state) {
> >> > + u8 link_config[2];
> >> > +
> >> > + link_config[0] = crtc_state->vrr.flipline ?
> >> DP_MSA_TIMING_PAR_IGNORE_EN : 0;
> >> > + link_config[1] = intel_dp_is_uhbr(crtc_state) ?
> >> > + DP_SET_ANSI_128B132B : DP_SET_ANSI_8B10B;
> >> > + drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL,
> >> link_config,
> >> > +2); }
> >> > +
> >> > +static void
> >> > +intel_dp_update_link_bw_set(struct intel_dp *intel_dp,
> >> > + const struct intel_crtc_state *crtc_state,
> >> > + u8 link_bw, u8 rate_select) {
> >> > + u8 link_config[2];
> >> > +
> >> > + /* Write the link configuration data */
> >> > + link_config[0] = link_bw;
> >> > + link_config[1] = crtc_state->lane_count;
> >> > + if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
> >> > + link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> >> > + drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config,
> >> 2);
> >> > + /* eDP 1.4 rate select method. */
> >> > + if (!link_bw)
> >> > + drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_RATE_SET,
> >> > + &rate_select, 1); }
> >> > +
> >> > /*
> >> > * Prepare link training by configuring the link parameters. On
> >> > DDI
> >> platforms
> >> > * also enable the port here.
> >> > @@ -647,7 +678,6 @@ intel_dp_prepare_link_train(struct intel_dp
> >> > *intel_dp, {
> >> > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> >> > struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >> > - u8 link_config[2];
> >> > u8 link_bw, rate_select;
> >> >
> >> > if (intel_dp->prepare_link_retrain) @@ -686,23 +716,19 @@
> >> > intel_dp_prepare_link_train(struct intel_dp
> >> *intel_dp,
> >> > drm_dbg_kms(&i915->drm,
> >> > "[ENCODER:%d:%s] Using LINK_RATE_SET value
> >> %02x\n",
> >> > encoder->base.base.id, encoder->base.name,
> >> rate_select);
> >> > -
> >> > - /* Write the link configuration data */
> >> > - link_config[0] = link_bw;
> >> > - link_config[1] = crtc_state->lane_count;
> >> > - if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
> >> > - link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> >> > - drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config,
> >> 2);
> >> > -
> >> > - /* eDP 1.4 rate select method. */
> >> > - if (!link_bw)
> >> > - drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_RATE_SET,
> >> > - &rate_select, 1);
> >> > -
> >> > - link_config[0] = crtc_state->vrr.flipline ?
> >> DP_MSA_TIMING_PAR_IGNORE_EN : 0;
> >> > - link_config[1] = intel_dp_is_uhbr(crtc_state) ?
> >> > - DP_SET_ANSI_128B132B : DP_SET_ANSI_8B10B;
> >> > - drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL,
> >> link_config, 2);
> >> > + if (intel_dp_is_uhbr(crtc_state)) {
> >> > + /*
> >> > + * Spec DP2.1 Section 3.5.2.16
> >> > + * Prior to LT DPTX should set 128b/132b DP Channel
> >> > + coding
> >> and then set link rate
> >> > + */
> >> > + intel_dp_update_downspread_ctrl(intel_dp, crtc_state);
> >> > + intel_dp_update_link_bw_set(intel_dp, crtc_state, link_bw,
> >> > + rate_select);
> >> > + } else {
> >> > + intel_dp_update_link_bw_set(intel_dp, crtc_state, link_bw,
> >> > + rate_select);
> >> > + intel_dp_update_downspread_ctrl(intel_dp, crtc_state);
> >> > + }
> >> >
> >> > return true;
> >> > }
> >>
> >> --
> >> Jani Nikula, Intel Open Source Graphics Center
>
> --
> Jani Nikula, Intel Open Source Graphics Center
More information about the Intel-gfx
mailing list