Re: [Intel-gfx] [PATCH 3/3] drm/i915: Program PFI credits for VLV
Hi Ville,
Apologize for the delay in reply.
For your inputs on the programming side (modeset global resources
handling and self documenting code parts), I agree with you and will
make the changes.
For opens related to info on the feature, internal discussions are
ongoing. I will get back to you on them next week..
Thanks,
Vandana
On Aug-08-2014 6:33 PM, Ville Syrjälä wrote:
On Thu, Aug 07, 2014 at 06:40:03PM +0530, Vandana Kannan wrote:
From: Vidya Srinivas vidya.srini...@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/czclk1, the default PFI credit of 8 should be set.
Do we have any docs for this? I see the register in the docs but nothing
about the correct programming. Some kind of refrence to the used
documentation would be nice.
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
Signed-off-by: Vidya Srinivas vidya.srini...@intel.com
Signed-off-by: Gajanan Bhat gajanan.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 6 ++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_display.c | 4 +++-
drivers/gpu/drm/i915/intel_pm.c | 22 ++
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index 6c4b25c..00e0b4a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
console_unlock();
+if (IS_VALLEYVIEW(dev))
+valleyview_program_pfi_credits(dev_priv, false);
If we want to do this for system suspend I think we should stick it
into i915_drm_freeze() (just after turning off the pipes). Not sure if
we should also force cdclk to minimum there...
Hmm. Is the GCI_CONTROL register is part of the disp2d power well? If it
is we probably need to enable the power well around the register write.
+
dev_priv-suspend_count++;
intel_display_set_init_power(dev_priv, false);
@@ -693,6 +696,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
dev_priv-modeset_restore = MODESET_DONE;
mutex_unlock(dev_priv-modeset_restore_lock);
+if (IS_VALLEYVIEW(dev))
+valleyview_program_pfi_credits(dev_priv, true);
I think this thing can be dropped. We need to do the reprogramming as
part of the modeset global resources handling.
+
intel_opregion_notify_adapter(dev, PCI_D0);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 881e0a6..88fd4c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2172,6 +2172,8 @@ extern struct i915_params i915 __read_mostly;
/* i915_dma.c */
void i915_update_dri1_breadcrumb(struct drm_device *dev);
+extern void valleyview_program_pfi_credits(struct drm_i915_private
*dev_priv,
+ bool flag);
extern void i915_kernel_lost_context(struct drm_device * dev);
extern int i915_driver_load(struct drm_device *, unsigned long flags);
extern int i915_driver_unload(struct drm_device *);
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index fb111cd..7f4c2f5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1937,6 +1937,11 @@ enum punit_power_well {
#define CZCLK_FREQ_MASK 0xf
#define GMBUSFREQ_VLV (VLV_DISPLAY_BASE + 0x6510)
+#define GCI_CONTROL (VLV_DISPLAY_BASE + 0x650C)
+#define PFI_CREDIT(7 28)
Maybe something like this for a bit more self documenting code.
#define PFI_CREDIT(x) (((x)-8) 28) /* 8-15 */
+#define PFI_CREDIT_RESEND (1 27)
+#define VGA_FAST_MODE_DISABLE (1 14)
+
/*
* Palette regs
*/
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2089319..2af2e60 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12691,8 +12691,10 @@ void intel_modeset_init_hw(struct drm_device *dev)
{
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Program PFI credits for VLV
On Thu, Aug 07, 2014 at 06:40:03PM +0530, Vandana Kannan wrote:
From: Vidya Srinivas vidya.srini...@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/czclk1, the default PFI credit of 8 should be set.
Do we have any docs for this? I see the register in the docs but nothing
about the correct programming. Some kind of refrence to the used
documentation would be nice.
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
Signed-off-by: Vidya Srinivas vidya.srini...@intel.com
Signed-off-by: Gajanan Bhat gajanan.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 6 ++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_display.c | 4 +++-
drivers/gpu/drm/i915/intel_pm.c | 22 ++
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6c4b25c..00e0b4a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
console_unlock();
+ if (IS_VALLEYVIEW(dev))
+ valleyview_program_pfi_credits(dev_priv, false);
If we want to do this for system suspend I think we should stick it
into i915_drm_freeze() (just after turning off the pipes). Not sure if
we should also force cdclk to minimum there...
Hmm. Is the GCI_CONTROL register is part of the disp2d power well? If it
is we probably need to enable the power well around the register write.
+
dev_priv-suspend_count++;
intel_display_set_init_power(dev_priv, false);
@@ -693,6 +696,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
dev_priv-modeset_restore = MODESET_DONE;
mutex_unlock(dev_priv-modeset_restore_lock);
+ if (IS_VALLEYVIEW(dev))
+ valleyview_program_pfi_credits(dev_priv, true);
I think this thing can be dropped. We need to do the reprogramming as
part of the modeset global resources handling.
+
intel_opregion_notify_adapter(dev, PCI_D0);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 881e0a6..88fd4c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2172,6 +2172,8 @@ extern struct i915_params i915 __read_mostly;
/* i915_dma.c */
void i915_update_dri1_breadcrumb(struct drm_device *dev);
+extern void valleyview_program_pfi_credits(struct drm_i915_private *dev_priv,
+bool flag);
extern void i915_kernel_lost_context(struct drm_device * dev);
extern int i915_driver_load(struct drm_device *, unsigned long flags);
extern int i915_driver_unload(struct drm_device *);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fb111cd..7f4c2f5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1937,6 +1937,11 @@ enum punit_power_well {
#define CZCLK_FREQ_MASK0xf
#define GMBUSFREQ_VLV(VLV_DISPLAY_BASE + 0x6510)
+#define GCI_CONTROL (VLV_DISPLAY_BASE + 0x650C)
+#define PFI_CREDIT (7 28)
Maybe something like this for a bit more self documenting code.
#define PFI_CREDIT(x) (((x)-8) 28) /* 8-15 */
+#define PFI_CREDIT_RESEND (1 27)
+#define VGA_FAST_MODE_DISABLE (1 14)
+
/*
* Palette regs
*/
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2089319..2af2e60 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12691,8 +12691,10 @@ void intel_modeset_init_hw(struct drm_device *dev)
{
intel_prepare_ddi(dev);
- if (IS_VALLEYVIEW(dev))
+ if (IS_VALLEYVIEW(dev)) {
vlv_update_cdclk(dev);
+ valleyview_program_pfi_credits(dev-dev_private, true);
The planes may be be active here. So for the initial state we just need
to trust that the BIOS got it right.
For runtime cdclk changes this needs to be handled in
valleyview_modeset_global_resources().
[Intel-gfx] [PATCH 3/3] drm/i915: Program PFI credits for VLV
From: Vidya Srinivas vidya.srini...@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/czclk1, 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
Signed-off-by: Vidya Srinivas vidya.srini...@intel.com
Signed-off-by: Gajanan Bhat gajanan.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 6 ++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_display.c | 4 +++-
drivers/gpu/drm/i915/intel_pm.c | 22 ++
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6c4b25c..00e0b4a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
console_unlock();
+ if (IS_VALLEYVIEW(dev))
+ valleyview_program_pfi_credits(dev_priv, false);
+
dev_priv-suspend_count++;
intel_display_set_init_power(dev_priv, false);
@@ -693,6 +696,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
dev_priv-modeset_restore = MODESET_DONE;
mutex_unlock(dev_priv-modeset_restore_lock);
+ if (IS_VALLEYVIEW(dev))
+ valleyview_program_pfi_credits(dev_priv, true);
+
intel_opregion_notify_adapter(dev, PCI_D0);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 881e0a6..88fd4c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2172,6 +2172,8 @@ extern struct i915_params i915 __read_mostly;
/* i915_dma.c */
void i915_update_dri1_breadcrumb(struct drm_device *dev);
+extern void valleyview_program_pfi_credits(struct drm_i915_private *dev_priv,
+ bool flag);
extern void i915_kernel_lost_context(struct drm_device * dev);
extern int i915_driver_load(struct drm_device *, unsigned long flags);
extern int i915_driver_unload(struct drm_device *);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fb111cd..7f4c2f5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1937,6 +1937,11 @@ enum punit_power_well {
#define CZCLK_FREQ_MASK 0xf
#define GMBUSFREQ_VLV (VLV_DISPLAY_BASE + 0x6510)
+#define GCI_CONTROL(VLV_DISPLAY_BASE + 0x650C)
+#define PFI_CREDIT (7 28)
+#define PFI_CREDIT_RESEND(1 27)
+#define VGA_FAST_MODE_DISABLE (1 14)
+
/*
* Palette regs
*/
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2089319..2af2e60 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12691,8 +12691,10 @@ void intel_modeset_init_hw(struct drm_device *dev)
{
intel_prepare_ddi(dev);
- if (IS_VALLEYVIEW(dev))
+ if (IS_VALLEYVIEW(dev)) {
vlv_update_cdclk(dev);
+ valleyview_program_pfi_credits(dev-dev_private, true);
+ }
intel_init_clock_gating(dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ba8dfeb..ad8e190 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7154,6 +7154,28 @@ void intel_fini_runtime_pm(struct drm_i915_private
*dev_priv)
pm_runtime_disable(device);
}
+void valleyview_program_pfi_credits(struct drm_i915_private *dev_priv,
+ bool flag)
+{
+ int cd_clk, cz_clk;
+
+ if (!flag) {
+ I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE);
+ return;
+ }
+
+ cd_clk = dev_priv-display.get_display_clock_speed(dev_priv-dev);
+ cz_clk = valleyview_get_cz_clock_speed(dev_priv-dev);
+
+ if (cd_clk = cz_clk) {
+ /* WA - write default credits before re-programming */
+ I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE);
+ I915_WRITE(GCI_CONTROL, (PFI_CREDIT | PFI_CREDIT_RESEND |
+
