| Message ID | 1423574909-1074-12-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. 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); > } > }
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. 8 is the minimum. So I think on CHV we actually want PFI_CREDIT(12). The reason the CHV value is wrong here is because I wrote the patch based on the spec, and sadly the new recommended value isn't in the spec. > > > + } > > + > > + /* > > + * 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. Right. So exactly as I thought too. I'll respin the patch with the new value. > > 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
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
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); + } + + /* + * 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)); + + 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); } }