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

From: "Purushothaman, Vijay A" <vijay.a.purushothaman@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v4 10/12] drm/i915: Program PFI credits for VLV
Date: Tue, 10 Mar 2015 15:35:19 +0530	[thread overview]
Message-ID: <54FEC1DF.5080204@linux.intel.com> (raw)
In-Reply-To: <1425583192-2584-11-git-send-email-ville.syrjala@linux.intel.com>

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);
>   	}
>   }

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx