On Thu, 2015-11-19 at 16:07 +0100, Maarten Lankhorst wrote: > On skylake when calculating plane visibility with the crtc in > dpms off mode the real cdclk may be different from what it would be > if the crtc was active. This may result in a WARN_ON(cdclk < crtc_clock) > from skl_max_scale. The fix is to keep a atomic_cdclk that would be true > if all crtc's were active. > > This is required to get the same calculations done correctly regardless > of dpms mode. > Reviewed-by: Mika Kahola <[email protected]> > Signed-off-by: Maarten Lankhorst <[email protected]> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 55 > ++++++++++++++++++++++++------------ > drivers/gpu/drm/i915/intel_drv.h | 6 ++++ > 3 files changed, 44 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 3caecf896f17..f3ad72abeea7 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1776,7 +1776,7 @@ struct drm_i915_private { > > unsigned int fsb_freq, mem_freq, is_ddr3; > unsigned int skl_boot_cdclk; > - unsigned int cdclk_freq, max_cdclk_freq; > + unsigned int cdclk_freq, max_cdclk_freq, atomic_cdclk_freq; > unsigned int max_dotclk_freq; > unsigned int hpll_freq; > unsigned int czclk_freq; > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 7f69f98d8b23..b2bf92a3b701 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5312,6 +5312,7 @@ static void modeset_put_power_domains(struct > drm_i915_private *dev_priv, > > static void modeset_update_crtc_power_domains(struct drm_atomic_state *state) > { > + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > struct drm_device *dev = state->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > unsigned long put_domains[I915_MAX_PIPES] = {}; > @@ -5325,13 +5326,9 @@ static void modeset_update_crtc_power_domains(struct > drm_atomic_state *state) > modeset_get_crtc_power_domains(crtc); > } > > - if (dev_priv->display.modeset_commit_cdclk) { > - unsigned int cdclk = to_intel_atomic_state(state)->cdclk; > - > - if (cdclk != dev_priv->cdclk_freq && > - !WARN_ON(!state->allow_modeset)) > - dev_priv->display.modeset_commit_cdclk(state); > - } > + if (dev_priv->display.modeset_commit_cdclk && > + intel_state->dev_cdclk != dev_priv->cdclk_freq) > + dev_priv->display.modeset_commit_cdclk(state); > > for (i = 0; i < I915_MAX_PIPES; i++) > if (put_domains[i]) > @@ -6039,13 +6036,18 @@ static int valleyview_modeset_calc_cdclk(struct > drm_atomic_state *state) > struct drm_device *dev = state->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > int max_pixclk = intel_mode_max_pixclk(dev, state); > + struct intel_atomic_state *intel_state = > + to_intel_atomic_state(state); > > if (max_pixclk < 0) > return max_pixclk; > > - to_intel_atomic_state(state)->cdclk = > + intel_state->cdclk = intel_state->dev_cdclk = > valleyview_calc_cdclk(dev_priv, max_pixclk); > > + if (!intel_state->active_crtcs) > + intel_state->dev_cdclk = valleyview_calc_cdclk(dev_priv, 0); > + > return 0; > } > > @@ -6054,13 +6056,18 @@ static int broxton_modeset_calc_cdclk(struct > drm_atomic_state *state) > struct drm_device *dev = state->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > int max_pixclk = intel_mode_max_pixclk(dev, state); > + struct intel_atomic_state *intel_state = > + to_intel_atomic_state(state); > > if (max_pixclk < 0) > return max_pixclk; > > - to_intel_atomic_state(state)->cdclk = > + intel_state->cdclk = intel_state->dev_cdclk = > broxton_calc_cdclk(dev_priv, max_pixclk); > > + if (!intel_state->active_crtcs) > + intel_state->dev_cdclk = broxton_calc_cdclk(dev_priv, 0); > + > return 0; > } > > @@ -6103,8 +6110,10 @@ static void vlv_program_pfi_credits(struct > drm_i915_private *dev_priv) > static void valleyview_modeset_commit_cdclk(struct drm_atomic_state > *old_state) > { > struct drm_device *dev = old_state->dev; > - unsigned int req_cdclk = to_intel_atomic_state(old_state)->cdclk; > struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_atomic_state *old_intel_state = > + to_intel_atomic_state(old_state); > + unsigned req_cdclk = old_intel_state->dev_cdclk; > > /* > * FIXME: We can end up here with all power domains off, yet > @@ -9574,7 +9583,9 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv) > static void broxton_modeset_commit_cdclk(struct drm_atomic_state *old_state) > { > struct drm_device *dev = old_state->dev; > - unsigned int req_cdclk = to_intel_atomic_state(old_state)->cdclk; > + struct intel_atomic_state *old_intel_state = > + to_intel_atomic_state(old_state); > + unsigned int req_cdclk = old_intel_state->dev_cdclk; > > broxton_set_cdclk(dev, req_cdclk); > } > @@ -9700,6 +9711,7 @@ static void broadwell_set_cdclk(struct drm_device *dev, > int cdclk) > static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state) > { > struct drm_i915_private *dev_priv = to_i915(state->dev); > + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); > int max_pixclk = ilk_max_pixel_rate(state); > int cdclk; > > @@ -9722,7 +9734,9 @@ static int broadwell_modeset_calc_cdclk(struct > drm_atomic_state *state) > return -EINVAL; > } > > - to_intel_atomic_state(state)->cdclk = cdclk; > + intel_state->cdclk = intel_state->dev_cdclk = cdclk; > + if (!intel_state->active_crtcs) > + intel_state->dev_cdclk = 337500; > > return 0; > } > @@ -9730,7 +9744,9 @@ static int broadwell_modeset_calc_cdclk(struct > drm_atomic_state *state) > static void broadwell_modeset_commit_cdclk(struct drm_atomic_state > *old_state) > { > struct drm_device *dev = old_state->dev; > - unsigned int req_cdclk = to_intel_atomic_state(old_state)->cdclk; > + struct intel_atomic_state *old_intel_state = > + to_intel_atomic_state(old_state); > + unsigned req_cdclk = old_intel_state->dev_cdclk; > > broadwell_set_cdclk(dev, req_cdclk); > } > @@ -13066,18 +13082,15 @@ static int intel_modeset_checks(struct > drm_atomic_state *state) > * adjusted_mode bits in the crtc directly. > */ > if (dev_priv->display.modeset_calc_cdclk) { > - unsigned int cdclk; > - > ret = dev_priv->display.modeset_calc_cdclk(state); > > - cdclk = to_intel_atomic_state(state)->cdclk; > - if (!ret && cdclk != dev_priv->cdclk_freq) > + if (!ret && intel_state->dev_cdclk != dev_priv->cdclk_freq) > ret = intel_modeset_all_pipes(state); > > if (ret < 0) > return ret; > } else > - to_intel_atomic_state(state)->cdclk = dev_priv->cdclk_freq; > + to_intel_atomic_state(state)->cdclk = > dev_priv->atomic_cdclk_freq; > > intel_modeset_clear_plls(state); > > @@ -13358,6 +13371,7 @@ static int intel_atomic_commit(struct drm_device *dev, > memcpy(dev_priv->min_pixclk, intel_state->min_pixclk, > sizeof(intel_state->min_pixclk)); > dev_priv->active_crtcs = intel_state->active_crtcs; > + dev_priv->atomic_cdclk_freq = intel_state->cdclk; > } > > for_each_crtc_in_state(state, crtc, crtc_state, i) { > @@ -15026,7 +15040,12 @@ static void i915_disable_vga(struct drm_device *dev) > > void intel_modeset_init_hw(struct drm_device *dev) > { > + struct drm_i915_private *dev_priv = dev->dev_private; > + > intel_update_cdclk(dev); > + > + dev_priv->atomic_cdclk_freq = dev_priv->cdclk_freq; > + > intel_prepare_ddi(dev); > intel_init_clock_gating(dev); > intel_enable_gt_powersave(dev); > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index d2e1dc698fca..7fd025f64f4c 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -247,6 +247,12 @@ struct intel_atomic_state { > > unsigned int cdclk; > > + /* > + * Calculated device cdclk, can be different from cdclk > + * only when all crtc's are DPMS off. > + */ > + unsigned int dev_cdclk; > + > bool dpll_set, modeset; > > unsigned int active_crtcs;
_______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
