[Intel-gfx] [RESEND PATCHv2] drm/i915/display/dp: 128/132b LT requirement
Ville Syrjälä
ville.syrjala at linux.intel.com
Mon Apr 24 15:27:25 UTC 2023
On Wed, Apr 19, 2023 at 10:07:46AM +0000, Murthy, Arun R wrote:
> > -----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)
Looks like an unordered list to me
>
> 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
whereas this is an ordered list.
>
>
> 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
--
Ville Syrjälä
Intel
More information about the Intel-gfx
mailing list