[Intel-gfx] [RESEND PATCHv2] drm/i915/display/dp: 128/132b LT requirement
Jani Nikula
jani.nikula at intel.com
Wed Apr 19 09:55:55 UTC 2023
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@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
More information about the Intel-gfx
mailing list