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

Ville Syrjälä
2015-03-04T15:26:27Z

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