Re: [PATCH 14/14] drm/i915: Modeset global_pipes() update

Ville Syrjälä
2015-04-15T19:19:22Z

On Wed, Apr 15, 2015 at 04:07:24PM +0300, Mika Kahola wrote:
> Combined Valleyview, Haswell and Broadwell '*_modeset_global_pipes()'
> into one function 'intel_modeset_global_pipes()'
>
> v2:
> - we don't modify 'disable_pipes', so passing this as a pointer
> is removed (based on Ville's comment)
> - introduced a new function 'intel_calc_cdclk()' that combines
> routines from 'valleyview_calc_cdclk()' and 'haswell_calc_cdclk()'
>
> v3:
> - Let's take a step back and not remove the routines 'valleyview_calc_cdclk()'
> and 'haswell_calc_cdclk()' from newly introduced routine
> 'intel_calc_cdclk()' (based on Ville's comment)
>
> v4:
> - Rebased to the latest
>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
> drivers/gpu/drm/i915/intel_display.c | 70 +++++++++++++++++-------------------
> 1 file changed, 32 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d79421a..f199faa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5433,28 +5433,6 @@ static int intel_mode_max_pixclk(struct drm_atomic_state *state)
> return max_pixclk;
> }
>
> -static int valleyview_modeset_global_pipes(struct drm_atomic_state *state,
> - unsigned *prepare_pipes)
> -{
> - struct drm_i915_private *dev_priv = to_i915(state->dev);
> - struct intel_crtc *intel_crtc;
> - int max_pixclk = intel_mode_max_pixclk(state);
> -
> - if (max_pixclk < 0)
> - return max_pixclk;
> -
> - if (valleyview_calc_cdclk(dev_priv, max_pixclk) ==
> - dev_priv->cdclk_freq)
> - return 0;
> -
> - /* disable/enable all currently active pipes while we change cdclk */
> - for_each_intel_crtc(state->dev, intel_crtc)
> - if (intel_crtc->base.state->enable)
> - *prepare_pipes |= (1 << intel_crtc->pipe);
> -
> - return 0;
> -}
> -
> static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
> {
> unsigned int credits, default_credits;
> @@ -9265,21 +9243,47 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
> cdclk, dev_priv->cdclk_freq);
> }
>
> -static void haswell_modeset_global_pipes(struct drm_atomic_state *state,
> - unsigned *prepare_pipes)
> +static int intel_calc_cdclk(struct drm_device *dev, int max_pixclk)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + int cdclk = 200000;
> +
> + if (IS_VALLEYVIEW(dev))
> + cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
> + else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + cdclk = haswell_calc_cdclk(dev_priv, max_pixclk);
> +
> + return cdclk;
> +}
> +
> +static void intel_modeset_global_pipes(struct drm_atomic_state *state,
> + unsigned *prepare_pipes,
> + unsigned disable_pipes)
> {
> struct drm_device *dev = state->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *crtc;
> - int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> + int max_pixclk;
>
> - if (haswell_calc_cdclk(dev_priv, max_pixel_rate) == dev_priv->cdclk_freq)
> + /* this modeset is valid only for VLV, HSW, and BDW */
> + if (!IS_VALLEYVIEW(dev) && !IS_HASWELL(dev) && !IS_BROADWELL(dev))
> + return;
> +
> + if (IS_VALLEYVIEW(dev))
> + max_pixclk = intel_mode_max_pixclk(state);
> + else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + max_pixclk = ilk_max_pixel_rate(dev_priv);
> +
> + if (intel_calc_cdclk(dev, max_pixclk) == dev_priv->cdclk_freq)
> return;
>
> /* disable/enable all currently active pipes while we change cdclk */
> for_each_intel_crtc(dev, crtc)
> - if (crtc->base.enabled)
> + if (crtc->base.state->enable)
> *prepare_pipes |= 1 << crtc->pipe;
> +
> + /* may have added more to prepare_pipes than we should */
> + *prepare_pipes &= ~disable_pipes;
> }
>
> static void haswell_modeset_global_resources(struct drm_atomic_state *state)
> @@ -12453,17 +12457,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
> * mode set on this crtc. For other crtcs we need to use the
> * adjusted_mode bits in the crtc directly.
> */
> - if (IS_VALLEYVIEW(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> - if (IS_VALLEYVIEW(dev))
> - valleyview_modeset_global_pipes(state, &prepare_pipes);
> - else
> - haswell_modeset_global_pipes(state, &prepare_pipes);
> - if (ret)
> - goto done;
> -
> - /* may have added more to prepare_pipes than we should */
> - prepare_pipes &= ~disable_pipes;
> - }
> + intel_modeset_global_pipes(state, &prepare_pipes, disable_pipes);
>
> ret = __intel_set_mode_setup_plls(state, modeset_pipes, disable_pipes);
> if (ret)
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC