On Fri, 2020-07-10 at 15:02 +0300, Ville Syrjälä wrote: > On Thu, Jul 09, 2020 at 08:14:05PM +0000, Souza, Jose wrote: > > On Thu, 2020-07-09 at 17:58 +0300, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > According to the DP spec a DPTX must support vswing/pre-emphasis > > > up to and including level 2. Level 3 is optional (actually DP 1.4a > > > seems to make even level 3 mandatory for HBR2/3, while leaving it > > > optional for RBR/HBR1). > > > > > > WARN if out encoders' .voltage_max()/.preemph_max() return > > > an illegal value. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/display/intel_dp_link_training.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > 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 2493142a70e9..a23ed7290843 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > > @@ -52,6 +52,7 @@ static u8 dp_voltage_max(u8 preemph) > > > void intel_dp_get_adjust_train(struct intel_dp *intel_dp, > > > const u8 link_status[DP_LINK_STATUS_SIZE]) > > > { > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > u8 v = 0; > > > u8 p = 0; > > > int lane; > > > @@ -64,12 +65,20 @@ void intel_dp_get_adjust_train(struct intel_dp *intel_dp, > > > } > > > > > > preemph_max = intel_dp->preemph_max(intel_dp); > > > + drm_WARN_ON_ONCE(&i915->drm, > > > + preemph_max != DP_TRAIN_PRE_EMPH_LEVEL_2 && > > > + preemph_max != DP_TRAIN_PRE_EMPH_LEVEL_3); > > > > Okay matches but I guess the preemph_max function should receive as parameter the voltage level to it return the max preemph of that voltage level, > > no? > > That was the old way which didn't match the spec. With > the new way those just return the overall maximum that > we support. And the dp_voltage_max() call below fixes up > any out of spec requests. It still do not handle a table that do not have DP_TRAIN_VOLTAGE_SWING_LEVEL_0 | DP_TRAIN_PRE_EMPH_LEVEL_3, but in this case we would be using a wrong entry. The warnings LGTM. Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > > > > + > > > if (p >= preemph_max) > > > p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; > > > > > > v = min(v, dp_voltage_max(p)); > > > > > > voltage_max = intel_dp->voltage_max(intel_dp); > > > + drm_WARN_ON_ONCE(&i915->drm, > > > + voltage_max != DP_TRAIN_VOLTAGE_SWING_LEVEL_2 && > > > + voltage_max != DP_TRAIN_VOLTAGE_SWING_LEVEL_3); > > > + > > > if (v >= voltage_max) > > > v = voltage_max | DP_TRAIN_MAX_SWING_REACHED; > > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx