Re: [PATCH 27/71] drm/i915/chv: Enable Render Standby (RC6) for Cheeryview

Deepak S
2014-04-13T15:31:05Z

Thanks for the feedback. I will address all the comments
and I will post cherryview rc6/turbo updated patches within couple of days.

I think the patches need little bit of cleanup.

Thanks
Deepak

On Thursday 10 April 2014 10:36 PM, Ville Syrjälä wrote:
> On Thu, Apr 10, 2014 at 07:51:03PM +0300, Jani Nikula wrote:
>> On Wed, 09 Apr 2014, ville.syrjala@linux.intel.com wrote:
>>> From: Deepak S <deepak.s@intel.com>
>>>
>>> v2: Configure PCBR if BIOS fails allocate pcbr (deepak)
>>>
>>> Signed-off-by: Deepak S <deepak.s@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_pm.c | 101 ++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 98 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index 0889af7..909cc0a 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -3184,6 +3184,18 @@ 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);
>>> +
>>> + if (dev_priv->vlv_pctx) {
>>> + drm_gem_object_unreference(&dev_priv->vlv_pctx->base);
>>> + dev_priv->vlv_pctx = NULL;
>>> + }
>>> +}
>>> +
>>> static void valleyview_disable_rps(struct drm_device *dev)
>>> {
>>> struct drm_i915_private *dev_priv = dev->dev_private;
>>> @@ -3551,6 +3563,29 @@ int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
>>> return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
>>> }
>>>
>>> +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;
>>> +
>>> + pcbr = I915_READ(VLV_PCBR);
>>> + if (pcbr >> 12 == 0) {
>>> + /*
>>> + * From the Gunit register HAS:
>>> + * The Gfx driver is expected to program this register and ensure
>>> + * proper allocation within Gfx stolen memory. For example, this
>>> + * register should be programmed such than the PCBR range does not
>>> + * overlap with other relevant ranges.
>>> + */
>>> + pctx_paddr = (dev_priv->mm.stolen_base + gtt->stolen_size - pctx_size);
>>> + I915_WRITE(VLV_PCBR, pctx_paddr);
>>> + }
>>> +}
>>> +
>>> +
>>> static void valleyview_setup_pctx(struct drm_device *dev)
>>> {
>>> struct drm_i915_private *dev_priv = dev->dev_private;
>>> @@ -3595,6 +3630,61 @@ out:
>>> dev_priv->vlv_pctx = pctx;
>>> }
>>>
>>> +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));
>>> +
>>> + if ((gtfifodbg = I915_READ(GTFIFODBG))) {
>> Please no assignment within if; this one's easy to split.
>>
>> There's a bunch of other checkpatch issues in the series; I don't
>> personally care about most of them but you might want to run it and see
>> if you want to do something about it.
> Looks like it's a straight up copy-paste from the gen6 and vlv code. So
> someone might want to clean those out as well.
>
> And maybe we should just drop this check for CHV since the GT wake FIFO
> isn't used anymore. But I'm not sure if the register still hold something
> sensible or not.
>
>> BR,
>> Jani.
>>
>>
>>> + DRM_DEBUG_DRIVER("GT fifo had a previous error %x\n",
>>> + gtfifodbg);
>>> + I915_WRITE(GTFIFODBG, gtfifodbg);
>>> + }
>>> +
>>> + cherryview_setup_pctx(dev);
>>> +
>>> + /* 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);
>>> +
>>> + /* 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_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 */
>>> + 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 >> 12))
>>> + rc6_mode = GEN6_RC_CTL_EI_MODE(1) | VLV_RC_CTL_CTX_RST_PARALLEL;
>>> +
>>> + 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;
>>> @@ -4437,7 +4527,9 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>>> 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);
>>> @@ -4455,7 +4547,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);
>>> @@ -4476,7 +4570,7 @@ void intel_enable_gt_powersave(struct drm_device *dev)
>>> ironlake_enable_drps(dev);
>>> ironlake_enable_rc6(dev);
>>> intel_init_emon(dev);
>>> - } else if (IS_GEN6(dev) || IS_GEN7(dev)) {
>>> + } else if (INTEL_INFO(dev)->gen >= 6) {
>>> if (IS_VALLEYVIEW(dev))
>>> valleyview_setup_pctx(dev);
>>> /*
>>> @@ -5051,6 +5145,7 @@ static void valleyview_init_clock_gating(struct drm_device *dev)
>>> dev_priv->mem_freq = 1333;
>>> break;
>>> }
>>> +
>>> DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
>>>
>>> dev_priv->vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
>>> --
>>> 1.8.3.2
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> --
>> Jani Nikula, Intel Open Source Technology Center