Re: [PATCH v4 10/12] drm/i915: Program PFI credits for VLV

Purushothaman, Vijay A
2015-03-10T10:05:22Z

On 3/6/2015 12:49 AM, ville.syrjala@linux.intel.com wrote:
> From: Vidya Srinivas <vidya.srinivas@intel.com>
>
> PFI credit programming is required when CD clock (related to data flow from
> display pipeline to end display) is greater than CZ clock (related to data
> flow from memory to display plane). This programming should be done when all
> planes are OFF to avoid intermittent hangs while accessing memory even from
> different Gfx units (not just display).
>
> If cdclk/czclk >=1, PFI credits could be set as any number. To get better
> performance, larger PFI credit can be assigned to PND. Otherwise if
> cdclk/czclk<1, the default PFI credit of 8 should be set.
>
> v2:
> - Change log to lower log level instead of DRM_ERROR
> - Change function name to valleyview_program_pfi_credits
> - Move program PFI credits to modeset_init instead of intel_set_mode
> - Change magic numbers to logical constants
>
> [vsyrjala v3:
> - only program in response to cdclk update
> - program the credits also when cdclk<czclk
> - add CHV bits
> v4:
> - Change CHV cdclk<czclk credits to 12 (Vijay)]
>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

Reviewed-by: Vijay Purushothaman <vijay.a.purushothaman@linux.intel.com>

Thanks,
Vijay

> drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
> drivers/gpu/drm/i915/intel_display.c | 38 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9f98384..145f0d4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2061,6 +2061,14 @@ enum skl_disp_power_wells {
> #define CDCLK_FREQ_SHIFT 4
> #define CDCLK_FREQ_MASK (0x1f << CDCLK_FREQ_SHIFT)
> #define CZCLK_FREQ_MASK 0xf
> +
> +#define GCI_CONTROL (VLV_DISPLAY_BASE + 0x650C)
> +#define PFI_CREDIT_63 (9 << 28) /* chv only */
> +#define PFI_CREDIT_31 (8 << 28) /* chv only */
> +#define PFI_CREDIT(x) (((x) - 8) << 28) /* 8-15 */
> +#define PFI_CREDIT_RESEND (1 << 27)
> +#define VGA_FAST_MODE_DISABLE (1 << 14)
> +
> #define GMBUSFREQ_VLV (VLV_DISPLAY_BASE + 0x6510)
>
> /*
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3fe9598..29ee72d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4987,6 +4987,42 @@ static void valleyview_modeset_global_pipes(struct drm_device *dev,
> *prepare_pipes |= (1 << intel_crtc->pipe);
> }
>
> +static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
> +{
> + unsigned int credits, default_credits;
> +
> + if (IS_CHERRYVIEW(dev_priv))
> + default_credits = PFI_CREDIT(12);
> + else
> + default_credits = PFI_CREDIT(8);
> +
> + if (DIV_ROUND_CLOSEST(dev_priv->vlv_cdclk_freq, 1000) >= dev_priv->rps.cz_freq) {
> + /* CHV suggested value is 31 or 63 */
> + if (IS_CHERRYVIEW(dev_priv))
> + credits = PFI_CREDIT_31;
> + else
> + credits = PFI_CREDIT(15);
> + } else {
> + credits = default_credits;
> + }
> +
> + /*
> + * WA - write default credits before re-programming
> + * FIXME: should we also set the resend bit here?
> + */
> + I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
> + default_credits);
> +
> + I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
> + credits | PFI_CREDIT_RESEND);
> +
> + /*
> + * FIXME is this guaranteed to clear
> + * immediately or should we poll for it?
> + */
> + WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
> +}
> +
> static void valleyview_modeset_global_resources(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5010,6 +5046,8 @@ static void valleyview_modeset_global_resources(struct drm_device *dev)
> else
> valleyview_set_cdclk(dev, req_cdclk);
>
> + vlv_program_pfi_credits(dev_priv);
> +
> intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
> }
> }