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 <mika.kahola@xxxxxxxxx>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> 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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
