On Thu, Apr 02, 2015 at 01:05:31PM +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()'
>
> Signed-off-by: Mika Kahola <mika.kah...@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 191
> ++++++++++++++++-------------------
> 1 file changed, 88 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 7b97907..18a1262 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5179,66 +5179,62 @@ static void cherryview_set_cdclk(struct drm_device
> *dev, int cdclk)
> intel_update_cdclk(dev);
> }
>
> -static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> +static int intel_calc_cdclk(struct drm_i915_private *dev_priv,
> int max_pixclk)
> {
> - int freq_320 = (dev_priv->hpll_freq << 1) % 320000 != 0 ? 333333 :
> 320000;
> - int limit = IS_CHERRYVIEW(dev_priv) ? 95 : 90;
> + int cdclk = 200000;
>
> - /*
> - * Really only a few cases to deal with, as only 4 CDclks are supported:
> - * 200MHz
> - * 267MHz
> - * 320/333MHz (depends on HPLL freq)
> - * 400MHz (VLV only)
> - * So we check to see whether we're above 90% (VLV) or 95% (CHV)
> - * of the lower bin and adjust if needed.
> - *
> - * We seem to get an unstable or solid color picture at 200MHz.
> - * Not sure what's wrong. For now use 200MHz only when all pipes
> - * are off.
> - */
> - if (!IS_CHERRYVIEW(dev_priv) &&
> - max_pixclk > freq_320*limit/100)
> - return 400000;
> - else if (max_pixclk > 266667*limit/100)
> - return freq_320;
> - else if (max_pixclk > 0)
> - return 266667;
> - else
> - return 200000;
> -}
I'd leave the valleyview_calc_cdclk() and haswell_calc_cdclk() alone.
Stuffing them into a single function makes the patch quite ugly. Also
I suppose we might want to turn it into a vfunc in the future and
having separate funcs for different platforms would make that easier.
> -
> -/* compute the max pixel clock for new configuration */
> -static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
> -{
> - struct drm_device *dev = dev_priv->dev;
> - struct intel_crtc *intel_crtc;
> - int max_pixclk = 0;
> + if (IS_VALLEYVIEW(dev_priv)) {
> + int freq_320 = (dev_priv->hpll_freq << 1) % 320000 != 0 ?
> 333333 : 320000;
> + int limit = IS_CHERRYVIEW(dev_priv) ? 95 : 90;
> + /*
> + * Really only a few cases to deal with, as only 4 CDclks are
> supported:
> + * 200MHz
> + * 267MHz
> + * 320/333MHz (depends on HPLL freq)
> + * 400MHz (VLV only)
> + * So we check to see whether we're above 90% (VLV) or 95% (CHV)
> + * of the lower bin and adjust if needed.
> + *
> + * We seem to get an unstable or solid color picture at 200MHz.
> + * Not sure what's wrong. For now use 200MHz only when all pipes
> + * are off.
> + */
> + if (!IS_CHERRYVIEW(dev_priv) &&
> + max_pixclk > freq_320*limit/100)
> + cdclk = 400000;
> + else if (max_pixclk > 266667*limit/100)
> + cdclk = freq_320;
> + else if (max_pixclk > 0)
> + cdclk = 266667;
> + else
> + cdclk = 200000;
> + } else if (IS_HASWELL(dev_priv)) {
> + /*
> + * FIXME should also account for plane ratio
> + * once 64bpp pixel formats are supported.
> + */
> + if (max_pixclk > 540000)
> + cdclk = 675000;
> + else if (max_pixclk > 450000)
> + cdclk = 540000;
> + else if (max_pixclk > 337500 || !IS_HSW_ULX(dev_priv))
> + cdclk = 450000;
> + else
> + cdclk = 337500;
>
> - for_each_intel_crtc(dev, intel_crtc) {
> - if (intel_crtc->new_enabled)
> - max_pixclk = max(max_pixclk,
> -
> intel_crtc->new_config->base.adjusted_mode.crtc_clock);
> + /*
> + * FIXME move the cdclk caclulation to
> + * compute_config() so we can fail gracegully.
> + */
> + if (cdclk > dev_priv->max_cdclk_freq) {
> + DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d
> kHz)\n",
> + cdclk, dev_priv->max_cdclk_freq);
> + cdclk = dev_priv->max_cdclk_freq;
> + }
> }
>
> - return max_pixclk;
> -}
> -
> -static void valleyview_modeset_global_pipes(struct drm_device *dev,
> - unsigned *prepare_pipes)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc;
> - int max_pixclk = intel_mode_max_pixclk(dev_priv);
> -
> - if (valleyview_calc_cdclk(dev_priv, max_pixclk) == dev_priv->cdclk_freq)
> - return;
> -
> - /* disable/enable all currently active pipes while we change cdclk */
> - for_each_intel_crtc(dev, intel_crtc)
> - if (intel_crtc->base.state->enable)
> - *prepare_pipes |= (1 << intel_crtc->pipe);
> + return cdclk;
> }
>
> static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
> @@ -5277,12 +5273,28 @@ static void vlv_program_pfi_credits(struct
> drm_i915_private *dev_priv)
> WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
> }
>
> +/* compute the max pixel clock for new configuration */
> +static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + struct intel_crtc *intel_crtc;
> + int max_pixclk = 0;
> +
> + for_each_intel_crtc(dev, intel_crtc) {
> + if (intel_crtc->new_enabled)
> + max_pixclk = max(max_pixclk,
> +
> intel_crtc->new_config->base.adjusted_mode.crtc_clock);
> + }
> +
> + return max_pixclk;
> +}
> +
> static void valleyview_modeset_global_resources(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_priv);
> - int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
> + int req_cdclk = intel_calc_cdclk(dev_priv, max_pixclk);
>
> if (req_cdclk != dev_priv->cdclk_freq) {
> /*
> @@ -8634,37 +8646,6 @@ static int ilk_max_pixel_rate(struct drm_i915_private
> *dev_priv)
> return max_pixel_rate;
> }
>
> -static int haswell_calc_cdclk(struct drm_i915_private *dev_priv,
> - int max_pixel_rate)
> -{
> - int cdclk;
> -
> - /*
> - * FIXME should also account for plane ratio
> - * once 64bpp pixel formats are supported.
> - */
> - if (max_pixel_rate > 540000)
> - cdclk = 675000;
> - else if (max_pixel_rate > 450000)
> - cdclk = 540000;
> - else if (max_pixel_rate > 337500 || !IS_HSW_ULX(dev_priv))
> - cdclk = 450000;
> - else
> - cdclk = 337500;
> -
> - /*
> - * FIXME move the cdclk caclulation to
> - * compute_config() so we can fail gracegully.
> - */
> - if (cdclk > dev_priv->max_cdclk_freq) {
> - DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> - cdclk, dev_priv->max_cdclk_freq);
> - cdclk = dev_priv->max_cdclk_freq;
> - }
> -
> - return cdclk;
> -}
> -
> static void haswell_set_cdclk(struct drm_device *dev, int cdclk)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -8787,21 +8768,33 @@ static void broadwell_set_cdclk(struct drm_device
> *dev, int cdclk)
> cdclk, dev_priv->cdclk_freq);
> }
>
> -static void haswell_modeset_global_pipes(struct drm_device *dev,
> - unsigned *prepare_pipes)
> +static void intel_modeset_global_pipes(struct drm_device *dev,
> + unsigned *prepare_pipes,
> + unsigned disable_pipes)
> {
> 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;
> +
> + /* 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(dev_priv);
> + else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + max_pixclk = ilk_max_pixel_rate(dev_priv);
>
> - if (haswell_calc_cdclk(dev_priv, max_pixel_rate) ==
> - dev_priv->cdclk_freq)
> + if (intel_calc_cdclk(dev_priv, 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)
> - *prepare_pipes |= 1 << crtc->pipe;
> + 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)
> @@ -8809,7 +8802,7 @@ static void haswell_modeset_global_resources(struct
> drm_atomic_state *state)
> struct drm_device *dev = state->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> - int req_cdclk = haswell_calc_cdclk(dev_priv, max_pixel_rate);
> + int req_cdclk = intel_calc_cdclk(dev_priv, max_pixel_rate);
>
> if (req_cdclk != dev_priv->cdclk_freq) {
> if (IS_BROADWELL(dev))
> @@ -11933,15 +11926,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(dev, &prepare_pipes);
> - else
> - haswell_modeset_global_pipes(dev, &prepare_pipes);
> -
> - /* may have added more to prepare_pipes than we should */
> - prepare_pipes &= ~disable_pipes;
> - }
> + intel_modeset_global_pipes(dev, &prepare_pipes, disable_pipes);
>
> ret = __intel_set_mode_setup_plls(dev, 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
