On Friday 09 May 2014 06:49 PM, Mika Kuoppala wrote:
Hi Deepak,
deepak.s@xxxxxxxxxxxxxxx writes:
From: Deepak S <deepak.s@xxxxxxxxxxxxxxx>
v2: Configure PCBR if BIOS fails allocate pcbr (deepak)
v3: Fix PCBR condition check during CHV RC6 Enable flag set
v4: Fixup PCBR comment msg. (Chris)
Rebase against latest code (Deak)
Fixup Spurious hunk (Ben)
Signed-off-by: Deepak S <deepak.s@xxxxxxxxxxxxxxx>
Acked-by: Ben Widawsky <ben@xxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_reg.h | 2 +
drivers/gpu/drm/i915/intel_pm.c | 115 +++++++++++++++++++++++++++++++++++++---
2 files changed, 111 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c850254..b4074fd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -962,6 +962,8 @@ enum punit_power_well {
#define VLV_IMR (VLV_DISPLAY_BASE + 0x20a8)
#define VLV_ISR (VLV_DISPLAY_BASE + 0x20ac)
#define VLV_PCBR (VLV_DISPLAY_BASE + 0x2120)
+#define VLV_PCBR_ADDR_SHIFT 12
+
#define DISPLAY_PLANE_FLIP_PENDING(plane) (1<<(11-(plane))) /* A and B only */
#define EIR 0x020b0
#define EMR 0x020b4
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ebb5c88..f0359b6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3300,6 +3300,13 @@ static void gen6_disable_rps(struct drm_device *dev)
gen6_disable_rps_interrupts(dev);
}
+static void cherryview_disable_rps(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ I915_WRITE(GEN6_RC_CONTROL, 0);
+}
+
static void valleyview_disable_rps(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3722,6 +3729,33 @@ static void valleyview_check_pctx(struct drm_i915_private *dev_priv)
dev_priv->vlv_pctx->stolen->start);
}
+
+/* Check that the pcbr address is not empty. */
+static void cherryview_check_pctx(struct drm_i915_private *dev_priv)
+{
+ unsigned long pctx_addr = I915_READ(VLV_PCBR) & ~4095;
+
+ WARN_ON((pctx_addr >> VLV_PCBR_ADDR_SHIFT) == 0);
+}
+
+static void cherryview_setup_pctx(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long pctx_paddr;
+ struct i915_gtt *gtt = &dev_priv->gtt;
+ u32 pcbr;
+ int pctx_size = 32*1024;
+
+ WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+
+ pcbr = I915_READ(VLV_PCBR);
+ if ((pcbr >> VLV_PCBR_ADDR_SHIFT) == 0) {
I admit that address zero locked by bios is prolly in
realms of paranoia. But I would still omit shift here to
get lock bit taken into consideration.
Agreed, But verifying address is more important right? We are more concerned with address than lock bit right?
+ pctx_paddr = (dev_priv->mm.stolen_base +
+ (gtt->stolen_size - pctx_size));
+ I915_WRITE(VLV_PCBR, pctx_paddr);
In here tho I would mask the low bits out, just to be on
the safe side. If we get off by one on stolen calculation we end up
writing the lock bit.
I will add the mask bit
I am thinking that we should just sanity check that bios
has set this up and that it seems to be in correct place. If not, spit a
warning and leave rc6 disabled.
The BIOS should have setup everything for us. Why do we need this
PCBR setup?
Nice Point, We need to make sure BIOS is setting up the PCBR properly.
+ }
+}
+
static void valleyview_setup_pctx(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3811,11 +3845,72 @@ static void valleyview_init_gt_powersave(struct drm_device *dev)
mutex_unlock(&dev_priv->rps.hw_lock);
}
+static void cherryview_init_gt_powersave(struct drm_device *dev)
+{
+ cherryview_setup_pctx(dev);
+}
+
static void valleyview_cleanup_gt_powersave(struct drm_device *dev)
{
valleyview_cleanup_pctx(dev);
}
+static void cherryview_enable_rps(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_ring_buffer *ring;
+ u32 gtfifodbg, rc6_mode = 0, pcbr;
+ int i;
+
+ WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
+
+ gtfifodbg = I915_READ(GTFIFODBG);
+ if (gtfifodbg) {
+ DRM_DEBUG_DRIVER("GT fifo had a previous error %x\n",
+ gtfifodbg);
+ I915_WRITE(GTFIFODBG, gtfifodbg);
+ }
+
+ cherryview_check_pctx(dev_priv);
+
+ /* 1a & 1b: Get forcewake during program sequence. Although the driver
+ * hasn't enabled a state yet where we need forcewake, BIOS may have.*/
+ gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
+
I915_WRITE(GEN6_RC_CONTROL, 0);
+ /* 2a: Program RC6 thresholds.*/
+ I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16);
+ I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); /* 12500 * 1280ns */
+ I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); /* 25 * 1280ns */
+
+ for_each_ring(ring, dev_priv, i)
+ I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
+
I915_WRITE(GEN6_RC_SLEEP, 0);
missing?
This might not be need. let me check back. As the default is already 0. I will check the spec and add if needed.
+ I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
+
+ /* allows RC6 residency counter to work */
+ I915_WRITE(VLV_COUNTER_CONTROL,
+ _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH |
+ VLV_MEDIA_RC6_COUNT_EN |
+ VLV_RENDER_RC6_COUNT_EN));
+
+ /* Todo: If BIOS has not configured PCBR
+ * then allocate in BIOS Reserved */
+
+ /* For now we assume BIOS is allocating and populating the PCBR
*/
Both comments do not match with the code.
I will address the comments
+ pcbr = I915_READ(VLV_PCBR);
+
+ DRM_DEBUG_DRIVER("PCBR offset : 0x%x\n", pcbr);
+
+ /* 3: Enable RC6 */
+ if ((intel_enable_rc6(dev) & INTEL_RC6_ENABLE) &&
+ (pcbr >> VLV_PCBR_ADDR_SHIFT))
+ rc6_mode = GEN6_RC_CTL_EI_MODE(1) | VLV_RC_CTL_CTX_RST_PARALLEL;
^^
Bit 24 is not used with chv.
-Mika
I will remove this :) Missed while addressing previous comments
+
+ I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
+
+ gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
+}
+
static void valleyview_enable_rps(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -4624,13 +4719,17 @@ void intel_init_gt_powersave(struct drm_device *dev)
{
i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
- if (IS_VALLEYVIEW(dev))
+ if (IS_CHERRYVIEW(dev))
+ cherryview_init_gt_powersave(dev);
+ else if (IS_VALLEYVIEW(dev))
valleyview_init_gt_powersave(dev);
}
void intel_cleanup_gt_powersave(struct drm_device *dev)
{
- if (IS_VALLEYVIEW(dev))
+ if (IS_CHERRYVIEW(dev))
+ return;
+ else if (IS_VALLEYVIEW(dev))
valleyview_cleanup_gt_powersave(dev);
}
@@ -4644,11 +4743,13 @@ void intel_disable_gt_powersave(struct drm_device *dev)
if (IS_IRONLAKE_M(dev)) {
ironlake_disable_drps(dev);
ironlake_disable_rc6(dev);
- } else if (IS_GEN6(dev) || IS_GEN7(dev)) {
+ } else if (INTEL_INFO(dev)->gen >= 6) {
cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
cancel_work_sync(&dev_priv->rps.work);
mutex_lock(&dev_priv->rps.hw_lock);
- if (IS_VALLEYVIEW(dev))
+ if (IS_CHERRYVIEW(dev))
+ cherryview_disable_rps(dev);
+ else if (IS_VALLEYVIEW(dev))
valleyview_disable_rps(dev);
else
gen6_disable_rps(dev);
@@ -4666,7 +4767,9 @@ static void intel_gen6_powersave_work(struct work_struct *work)
mutex_lock(&dev_priv->rps.hw_lock);
- if (IS_VALLEYVIEW(dev)) {
+ if (IS_CHERRYVIEW(dev)) {
+ cherryview_enable_rps(dev);
+ } else if (IS_VALLEYVIEW(dev)) {
valleyview_enable_rps(dev);
} else if (IS_BROADWELL(dev)) {
gen8_enable_rps(dev);
@@ -4691,7 +4794,7 @@ void intel_enable_gt_powersave(struct drm_device *dev)
ironlake_enable_rc6(dev);
intel_init_emon(dev);
mutex_unlock(&dev->struct_mutex);
- } else if (IS_GEN6(dev) || IS_GEN7(dev)) {
+ } else if (INTEL_INFO(dev)->gen >= 6) {
/*
* PCU communication is slow and this doesn't need to be
* done at any specific time, so do this out of our fast path
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Thanks for the review.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
