On Thu, Jun 13, 2019 at 12:24:59PM +0300, Ville Syrjälä wrote: > On Wed, Jun 12, 2019 at 08:24:23PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > We're now calling intel_pipe_config_compare(..., true) uncoditionally > > which means we're always going clobber the calculated M/N values with > > the old values if the fuzzy M/N check passes. That causes problems > > because the fuzzy check allows for a huge difference in the values. > > > > I'm actually tempted to just make the M/N checks exact, but that might > > prevent fastboot from kicking in when people want it. So for now let's > > overwrite the computed values with the old values only if decide to skip > > the modeset. > > > > v2: Copy has_drrs along with M/N M2/N2 values > > > > Cc: stable@xxxxxxxxxxxxxxx > > Cc: Blubberbub@xxxxxxxxxxxxxx > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > Cc: Hans de Goede <hdegoede@xxxxxxxxxx> > > Tested-by: Blubberbub@xxxxxxxxxxxxxx > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110782 > > Fixes: d19f958db23c ("drm/i915: Enable fastset for non-boot modesets.") > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Looks like also > https://bugs.freedesktop.org/show_bug.cgi?id=110675 Ok, the copying from old-state to new-state is needed to keep HW/SW state verification later pass, but we want to preserve the calculated state if we'll need to reprogram everything based on that. Makes sense: Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx> > > > --- > > drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++------- > > 1 file changed, 28 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 1b1ddb48ca7a..3d8ed1cf0ab7 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -12299,9 +12299,6 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n, > > m2_n2->gmch_m, m2_n2->gmch_n, !adjust) && > > intel_compare_m_n(m_n->link_m, m_n->link_n, > > m2_n2->link_m, m2_n2->link_n, !adjust)) { > > - if (adjust) > > - *m2_n2 = *m_n; > > - > > return true; > > } > > > > @@ -13433,6 +13430,33 @@ static int calc_watermark_data(struct intel_atomic_state *state) > > return 0; > > } > > > > +static void intel_crtc_check_fastset(struct intel_crtc_state *old_crtc_state, > > + struct intel_crtc_state *new_crtc_state) > > +{ > > + struct drm_i915_private *dev_priv = > > + to_i915(new_crtc_state->base.crtc->dev); > > + > > + if (!intel_pipe_config_compare(dev_priv, old_crtc_state, > > + new_crtc_state, true)) > > + return; > > + > > + new_crtc_state->base.mode_changed = false; > > + new_crtc_state->update_pipe = true; > > + > > + /* > > + * If we're not doing the full modeset we want to > > + * keep the current M/N values as they may be > > + * sufficiently different to the computed values > > + * to cause problems. > > + * > > + * FIXME: should really copy more fuzzy state here > > + */ > > + new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n; > > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > > + new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2; > > + new_crtc_state->has_drrs = old_crtc_state->has_drrs; > > +} > > + > > /** > > * intel_atomic_check - validate state object > > * @dev: drm device > > @@ -13474,11 +13498,7 @@ static int intel_atomic_check(struct drm_device *dev, > > if (ret) > > goto fail; > > > > - if (intel_pipe_config_compare(dev_priv, old_crtc_state, > > - new_crtc_state, true)) { > > - new_crtc_state->base.mode_changed = false; > > - new_crtc_state->update_pipe = true; > > - } > > + intel_crtc_check_fastset(old_crtc_state, new_crtc_state); > > > > if (needs_modeset(&new_crtc_state->base)) > > any_ms = true; > > -- > > 2.21.0 > > -- > Ville Syrjälä > Intel > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx