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

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "Purushothaman, Vijay A" <vijay.a.purushothaman@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 11/12] drm/i915: Program PFI credits for VLV
Date: Wed, 4 Mar 2015 17:26:22 +0200	[thread overview]
Message-ID: <20150304152622.GT11371@intel.com> (raw)
In-Reply-To: <54F715D1.2070805@linux.intel.com>

On Wed, Mar 04, 2015 at 07:55:21PM +0530, Purushothaman, Vijay A wrote:
> On 2/10/2015 6:58 PM, 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]
> >
> > 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>
> > ---
> >   drivers/gpu/drm/i915/i915_reg.h      |  8 ++++++++
> >   drivers/gpu/drm/i915/intel_display.c | 33 +++++++++++++++++++++++++++++++++
> >   2 files changed, 41 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index aacf90b..a0a7688 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..9dcab4b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4987,6 +4987,37 @@ 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;
> > +
> > +	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 = PFI_CREDIT(8);
> The default value should be 4 credits for CHV and 0 for VLV.
> 
> > +	}
> > +
> > +	/*
> > +	 * WA - write default credits before re-programming
> > +	 * FIXME: should we also set the resend bit here?
> > +	 */
> > +	I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
> > +		   PFI_CREDIT(8));
> Default credit should be 4 credits for CHV.  PFI_CREDIT(12). Document 
> update is pending. But this is the latest recommendation to windows team.

BTW do you have any specific information about this workaround? I asked
Vandana about it when she first submitted the patch and she said she'd
get back to me on that, but I don't think I ever got an actual answer.
The FIXMEs in particular would need to be answered.

Also if we still need this workaround on CHV, I don't know if we should
set the credits to 8 or 12 here. Any ideas?

> 
> Thanks,
> Vijay
> 
> > +
> > +	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 +5041,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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx