Re: [Intel-gfx] [RESEND PATCHv2] drm/i915/display/dp: 128/132b LT requirement

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 19 Apr 2023, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Wednesday, April 19, 2023 12:48 PM
>> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Murthy, Arun R <arun.r.murthy@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@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@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@intel.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".


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