Re: [PATCH 01/53] drm/i915: Remove ironlake rc6 support

[Tomas Elf <tomas.elf@intel.com>]

On 05/03/2015 14:03, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Apparently, this has never worked reliably and is currently disabled. Also, the
> gains are not particularly impressive. Thus rather than try to keep unused code
> from decaying and having to update it for other driver changes, it was decided
> to simply remove it.
>
> For: VIZ-5115
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c  |   12 ---
>   drivers/gpu/drm/i915/i915_drv.h      |    3 -
>   drivers/gpu/drm/i915/intel_display.c |    2 -
>   drivers/gpu/drm/i915/intel_drv.h     |    1 -
>   drivers/gpu/drm/i915/intel_pm.c      |  155 ----------------------------------
>   5 files changed, 173 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 68a1e6e..18e4900 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1846,18 +1846,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   	if (ret)
>   		return ret;
>
> -	if (dev_priv->ips.pwrctx) {
> -		seq_puts(m, "power context ");
> -		describe_obj(m, dev_priv->ips.pwrctx);
> -		seq_putc(m, '\n');
> -	}
> -
> -	if (dev_priv->ips.renderctx) {
> -		seq_puts(m, "render context ");
> -		describe_obj(m, dev_priv->ips.renderctx);
> -		seq_putc(m, '\n');
> -	}
> -
>   	list_for_each_entry(ctx, &dev_priv->context_list, link) {
>   		if (!i915.enable_execlists &&
>   		    ctx->legacy_hw_ctx.rcs_state == NULL)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 798fa88..05f106d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1053,9 +1053,6 @@ struct intel_ilk_power_mgmt {
>
>   	int c_m;
>   	int r_t;
> -
> -	struct drm_i915_gem_object *pwrctx;
> -	struct drm_i915_gem_object *renderctx;
>   };
>
>   struct drm_i915_private;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5c35098..51f7ed4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13904,8 +13904,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
>
>   	intel_fbc_disable(dev);
>
> -	ironlake_teardown_rc6(dev);
> -
>   	mutex_unlock(&dev->struct_mutex);
>
>   	/* flush any delayed tasks or pending work */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f4305be..b793558 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1229,7 +1229,6 @@ void intel_enable_gt_powersave(struct drm_device *dev);
>   void intel_disable_gt_powersave(struct drm_device *dev);
>   void intel_suspend_gt_powersave(struct drm_device *dev);
>   void intel_reset_gt_powersave(struct drm_device *dev);
> -void ironlake_teardown_rc6(struct drm_device *dev);
>   void gen6_update_ring_freq(struct drm_device *dev);
>   void gen6_rps_idle(struct drm_i915_private *dev_priv);
>   void gen6_rps_boost(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 542cf68..0762572 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3539,41 +3539,6 @@ void intel_update_sprite_watermarks(struct drm_plane *plane,
>   						   pixel_size, enabled, scaled);
>   }
>
> -static struct drm_i915_gem_object *
> -intel_alloc_context_page(struct drm_device *dev)
> -{
> -	struct drm_i915_gem_object *ctx;
> -	int ret;
> -
> -	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> -
> -	ctx = i915_gem_alloc_object(dev, 4096);
> -	if (!ctx) {
> -		DRM_DEBUG("failed to alloc power context, RC6 disabled\n");
> -		return NULL;
> -	}
> -
> -	ret = i915_gem_obj_ggtt_pin(ctx, 4096, 0);
> -	if (ret) {
> -		DRM_ERROR("failed to pin power context: %d\n", ret);
> -		goto err_unref;
> -	}
> -
> -	ret = i915_gem_object_set_to_gtt_domain(ctx, 1);
> -	if (ret) {
> -		DRM_ERROR("failed to set-domain on power context: %d\n", ret);
> -		goto err_unpin;
> -	}
> -
> -	return ctx;
> -
> -err_unpin:
> -	i915_gem_object_ggtt_unpin(ctx);
> -err_unref:
> -	drm_gem_object_unreference(&ctx->base);
> -	return NULL;
> -}
> -
>   /**
>    * Lock protecting IPS related data structures
>    */
> @@ -4990,124 +4955,6 @@ static void valleyview_enable_rps(struct drm_device *dev)
>   	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>   }
>
> -void ironlake_teardown_rc6(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	if (dev_priv->ips.renderctx) {
> -		i915_gem_object_ggtt_unpin(dev_priv->ips.renderctx);
> -		drm_gem_object_unreference(&dev_priv->ips.renderctx->base);
> -		dev_priv->ips.renderctx = NULL;
> -	}
> -
> -	if (dev_priv->ips.pwrctx) {
> -		i915_gem_object_ggtt_unpin(dev_priv->ips.pwrctx);
> -		drm_gem_object_unreference(&dev_priv->ips.pwrctx->base);
> -		dev_priv->ips.pwrctx = NULL;
> -	}
> -}
> -
> -static void ironlake_disable_rc6(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	if (I915_READ(PWRCTXA)) {
> -		/* Wake the GPU, prevent RC6, then restore RSTDBYCTL */
> -		I915_WRITE(RSTDBYCTL, I915_READ(RSTDBYCTL) | RCX_SW_EXIT);
> -		wait_for(((I915_READ(RSTDBYCTL) & RSX_STATUS_MASK) == RSX_STATUS_ON),
> -			 50);
> -
> -		I915_WRITE(PWRCTXA, 0);
> -		POSTING_READ(PWRCTXA);
> -
> -		I915_WRITE(RSTDBYCTL, I915_READ(RSTDBYCTL) & ~RCX_SW_EXIT);
> -		POSTING_READ(RSTDBYCTL);
> -	}
> -}
> -
> -static int ironlake_setup_rc6(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	if (dev_priv->ips.renderctx == NULL)
> -		dev_priv->ips.renderctx = intel_alloc_context_page(dev);
> -	if (!dev_priv->ips.renderctx)
> -		return -ENOMEM;
> -
> -	if (dev_priv->ips.pwrctx == NULL)
> -		dev_priv->ips.pwrctx = intel_alloc_context_page(dev);
> -	if (!dev_priv->ips.pwrctx) {
> -		ironlake_teardown_rc6(dev);
> -		return -ENOMEM;
> -	}
> -
> -	return 0;
> -}
> -
> -static void ironlake_enable_rc6(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_engine_cs *ring = &dev_priv->ring[RCS];
> -	bool was_interruptible;
> -	int ret;
> -
> -	/* rc6 disabled by default due to repeated reports of hanging during
> -	 * boot and resume.
> -	 */
> -	if (!intel_enable_rc6(dev))
> -		return;
> -
> -	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> -
> -	ret = ironlake_setup_rc6(dev);
> -	if (ret)
> -		return;
> -
> -	was_interruptible = dev_priv->mm.interruptible;
> -	dev_priv->mm.interruptible = false;
> -
> -	/*
> -	 * GPU can automatically power down the render unit if given a page
> -	 * to save state.
> -	 */
> -	ret = intel_ring_begin(ring, 6);
> -	if (ret) {
> -		ironlake_teardown_rc6(dev);
> -		dev_priv->mm.interruptible = was_interruptible;
> -		return;
> -	}
> -
> -	intel_ring_emit(ring, MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN);
> -	intel_ring_emit(ring, MI_SET_CONTEXT);
> -	intel_ring_emit(ring, i915_gem_obj_ggtt_offset(dev_priv->ips.renderctx) |
> -			MI_MM_SPACE_GTT |
> -			MI_SAVE_EXT_STATE_EN |
> -			MI_RESTORE_EXT_STATE_EN |
> -			MI_RESTORE_INHIBIT);
> -	intel_ring_emit(ring, MI_SUSPEND_FLUSH);
> -	intel_ring_emit(ring, MI_NOOP);
> -	intel_ring_emit(ring, MI_FLUSH);
> -	intel_ring_advance(ring);
> -
> -	/*
> -	 * Wait for the command parser to advance past MI_SET_CONTEXT. The HW
> -	 * does an implicit flush, combined with MI_FLUSH above, it should be
> -	 * safe to assume that renderctx is valid
> -	 */
> -	ret = intel_ring_idle(ring);
> -	dev_priv->mm.interruptible = was_interruptible;
> -	if (ret) {
> -		DRM_ERROR("failed to enable ironlake power savings\n");
> -		ironlake_teardown_rc6(dev);
> -		return;
> -	}
> -
> -	I915_WRITE(PWRCTXA, i915_gem_obj_ggtt_offset(dev_priv->ips.pwrctx) | PWRCTX_EN);
> -	I915_WRITE(RSTDBYCTL, I915_READ(RSTDBYCTL) & ~RCX_SW_EXIT);
> -
> -	intel_print_rc6_info(dev, GEN6_RC_CTL_RC6_ENABLE);
> -}
> -
>   static unsigned long intel_pxfreq(u32 vidfreq)
>   {
>   	unsigned long freq;
> @@ -5655,7 +5502,6 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>
>   	if (IS_IRONLAKE_M(dev)) {
>   		ironlake_disable_drps(dev);
> -		ironlake_disable_rc6(dev);
>   	} else if (INTEL_INFO(dev)->gen >= 6) {
>   		intel_suspend_gt_powersave(dev);
>
> @@ -5726,7 +5572,6 @@ void intel_enable_gt_powersave(struct drm_device *dev)
>   	if (IS_IRONLAKE_M(dev)) {
>   		mutex_lock(&dev->struct_mutex);
>   		ironlake_enable_drps(dev);
> -		ironlake_enable_rc6(dev);
>   		intel_init_emon(dev);
>   		mutex_unlock(&dev->struct_mutex);
>   	} else if (INTEL_INFO(dev)->gen >= 6) {
>
 >

Reviewed-by: Tomas Elf <tomas.elf@intel.com>

Thanks,
Tomas

(I know this one has already been queued for merge... I'm going through 
these patches regardless, for my own bookkeeping)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx