Re: [PATCH 4/5] drm/i915: Idleness detection for DRRS

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 23 Dec 2013, Vandana Kannan <vandana.kannan@intel.com> wrote:
> Adding support to detect display idleness by tracking page flip from
> user space. Switch to low refresh rate is triggered after 2 seconds of
> idleness. The delay is configurable. If there is a page flip or call to
> update the plane, then high refresh rate is applied.
> The feature is not used in dual-display mode.
>
> v2: Chris Wilson's review comments incorporated.
> Modify idleness detection implementation to make it similar to the
> implementation of intel_update_fbc/intel_disable_fbc
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   16 +++++
>  drivers/gpu/drm/i915/intel_display.c |   14 ++++
>  drivers/gpu/drm/i915/intel_dp.c      |   10 +++
>  drivers/gpu/drm/i915/intel_drv.h     |    4 ++
>  drivers/gpu/drm/i915/intel_pm.c      |  122 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_sprite.c  |    2 +
>  6 files changed, 168 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f8fd045..d7308cc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -712,6 +712,21 @@ struct i915_fbc {
>  	} no_fbc_reason;
>  };
>  
> +/* configure the number of secs the system must be idle
> + * before DRRS is enabled
> +*/
> +#define DRRS_IDLENESS_TIME 2000 /* in millisecs */
> +
> +struct i915_drrs {
> +	struct intel_connector *connector;
> +	struct intel_dp *dp;
> +	struct intel_drrs_work {
> +		struct delayed_work work;
> +		struct drm_crtc *crtc;
> +		int interval;

I'll probably see this more useful as a module parameter than a field
here, with 0 meaning disable.

> +	} *drrs_work;
> +};
> +
>  struct i915_psr {
>  	bool sink_support;
>  	bool source_ok;
> @@ -1400,6 +1415,7 @@ typedef struct drm_i915_private {
>  	int num_plane;
>  
>  	struct i915_fbc fbc;
> +	struct i915_drrs drrs;
>  	struct intel_opregion opregion;
>  	struct intel_vbt_data vbt;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a40651e..995d117 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2395,6 +2395,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>  	}
>  
>  	intel_update_fbc(dev);
> +	intel_update_drrs(dev);
>  	intel_edp_psr_update(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
> @@ -3559,6 +3560,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  
>  	mutex_lock(&dev->struct_mutex);
>  	intel_update_fbc(dev);
> +	intel_update_drrs(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
> @@ -3600,6 +3602,7 @@ static void haswell_crtc_enable_planes(struct drm_crtc *crtc)
>  
>  	mutex_lock(&dev->struct_mutex);
>  	intel_update_fbc(dev);
> +	intel_update_drrs(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  }
>  
> @@ -3806,6 +3809,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  
>  	mutex_lock(&dev->struct_mutex);
>  	intel_update_fbc(dev);
> +	intel_update_drrs(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  }
>  
> @@ -3853,6 +3857,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  
>  	mutex_lock(&dev->struct_mutex);
>  	intel_update_fbc(dev);
> +	intel_update_drrs(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  }
>  
> @@ -8226,6 +8231,11 @@ static void intel_unpin_work_fn(struct work_struct *__work)
>  	drm_gem_object_unreference(&work->old_fb_obj->base);
>  
>  	intel_update_fbc(dev);
> +
> +	/* disable current DRRS work scheduled and restart
> +	 * to push work by another x seconds
> +	 */
> +	intel_update_drrs(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0);
> @@ -8665,6 +8675,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  		goto cleanup_pending;
>  
>  	intel_disable_fbc(dev);
> +	intel_disable_drrs(dev);
>  	intel_mark_fb_busy(obj, NULL);
>  	mutex_unlock(&dev->struct_mutex);
>  
> @@ -10880,6 +10891,7 @@ void intel_modeset_init(struct drm_device *dev)
>  
>  	/* Just in case the BIOS is doing something questionable. */
>  	intel_disable_fbc(dev);
> +	intel_disable_drrs(dev);
>  }
>  
>  static void
> @@ -11286,6 +11298,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
>  
>  	intel_disable_fbc(dev);
>  
> +	intel_disable_drrs(dev);
> +
>  	intel_disable_gt_powersave(dev);
>  
>  	ironlake_teardown_rc6(dev);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d1e1d6e..7778808 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3289,11 +3289,18 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>  	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	i2c_del_adapter(&intel_dp->adapter);
>  	drm_encoder_cleanup(encoder);
>  	if (is_edp(intel_dp)) {
>  		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
> +		/* DRRS cleanup */
> +		if (intel_dp->drrs_state.drrs_support
> +					== SEAMLESS_DRRS_SUPPORT) {
> +			kfree(dev_priv->drrs.drrs_work);
> +			dev_priv->drrs.drrs_work = NULL;
> +		}
>  		mutex_lock(&dev->mode_config.mutex);
>  		ironlake_panel_vdd_off_sync(intel_dp);
>  		mutex_unlock(&dev->mode_config.mutex);
> @@ -3659,6 +3666,9 @@ intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
>  		intel_connector->panel.edp_downclock =
>  			intel_connector->panel.downclock_mode->clock;
>  
> +		intel_init_drrs_idleness_detection(dev,
> +			intel_connector, intel_dp);
> +
>  		mutex_init(&intel_dp->drrs_state.mutex);
>  
>  		intel_dp->drrs_state.drrs_support = dev_priv->vbt.drrs_mode;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d1c60fa..84cadf8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -902,6 +902,10 @@ void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
>  void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
>  void ilk_wm_get_hw_state(struct drm_device *dev);
>  
> +void intel_init_drrs_idleness_detection(struct drm_device *dev,
> +		struct intel_connector *connector, struct intel_dp *dp);
> +void intel_update_drrs(struct drm_device *dev);
> +void intel_disable_drrs(struct drm_device *dev);
>  
>  /* intel_sdvo.c */
>  bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b35f65e..69f31c5 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -615,6 +615,128 @@ out_disable:
>  	i915_gem_stolen_cleanup_compression(dev);
>  }
>  
> +static void intel_drrs_work_fn(struct work_struct *__work)
> +{
> +	struct intel_drrs_work *work =
> +		container_of(to_delayed_work(__work),
> +			     struct intel_drrs_work, work);
> +	struct drm_device *dev = work->crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	intel_dp_set_drrs_state(work->crtc->dev,
> +		dev_priv->drrs.connector->panel.downclock_mode->vrefresh);
> +}
> +
> +static void intel_cancel_drrs_work(struct drm_i915_private *dev_priv)
> +{
> +	if (dev_priv->drrs.drrs_work == NULL)
> +		return;
> +
> +	DRM_DEBUG_KMS("cancelling pending DRRS enable\n");
> +
> +	cancel_delayed_work_sync(&dev_priv->drrs.drrs_work->work);
> +}
> +
> +static void intel_enable_drrs(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	intel_cancel_drrs_work(dev_priv);
> +
> +	if (dev_priv->drrs.dp->drrs_state.drrs_refresh_rate_type
> +							!= DRRS_LOW_RR) {
> +		dev_priv->drrs.drrs_work->crtc = crtc;
> +
> +		/* Delay the actual enabling to let pageflipping cease and the
> +		 * display to settle before starting DRRS
> +		 */
> +		schedule_delayed_work(&dev_priv->drrs.drrs_work->work,
> +			msecs_to_jiffies(dev_priv->drrs.drrs_work->interval));
> +	}
> +}
> +
> +void intel_disable_drrs(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +
> +	/* as part of disable DRRS, reset refresh rate to HIGH_RR */
> +	if (dev_priv->drrs.dp->drrs_state.drrs_refresh_rate_type
> +							== DRRS_LOW_RR) {
> +		intel_cancel_drrs_work(dev_priv);
> +		intel_dp_set_drrs_state(dev,
> +			dev_priv->drrs.connector->panel.fixed_mode->vrefresh);
> +	}
> +}
> +
> +/**
> + * intel_update_drrs - enable/disable DRRS as needed
> + * @dev: the drm_device
> + * @update: if set to true, cancel current work and schedule new work.
> +*	    if set to false, cancel current work and disable DRRS.

There's no update parameter.

> +*/
> +void intel_update_drrs(struct drm_device *dev)
> +{
> +	struct drm_crtc *crtc = NULL, *tmp_crtc;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	/* if drrs.connector is NULL, then drrs_init did not get called.
> +	 * which means DRRS is not supported.
> +	 */
> +	if (dev_priv->drrs.connector == NULL) {
> +		DRM_INFO("DRRS is not supported.\n");
> +		return;
> +	}
> +
> +	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
> +		if (tmp_crtc != NULL && intel_crtc_active(tmp_crtc) &&
> +		    to_intel_crtc(tmp_crtc)->primary_enabled) {
> +			if (crtc) {
> +				DRM_DEBUG_KMS(
> +				"more than one pipe active, disabling DRRS\n");
> +				goto out_drrs_disable;
> +			}
> +			crtc = tmp_crtc;
> +		}
> +	}
> +
> +	if (crtc == NULL) {
> +		DRM_INFO("DRRS: crtc not initialized\n");
> +		return;
> +	}
> +
> +	intel_disable_drrs(dev);
> +
> +	/* re-enable idleness detection */
> +	intel_enable_drrs(crtc);
> +
> +out_drrs_disable:
> +	intel_disable_drrs(dev);

The happy day disable, enable, disable sequence seems like a bug.

> +}
> +
> +void intel_init_drrs_idleness_detection(struct drm_device *dev,
> +					struct intel_connector *connector,
> +					struct intel_dp *dp)
> +{
> +	struct intel_drrs_work *work;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	work = kzalloc(sizeof(struct intel_drrs_work), GFP_KERNEL);
> +	if (!work) {
> +		DRM_ERROR("Failed to allocate DRRS work structure\n");
> +		return;
> +	}
> +
> +	dev_priv->drrs.connector = connector;
> +	dev_priv->drrs.dp = dp;
> +
> +	work->interval = DRRS_IDLENESS_TIME;
> +	INIT_DELAYED_WORK(&work->work, intel_drrs_work_fn);
> +
> +	dev_priv->drrs.drrs_work = work;
> +}
> +
>  static void i915_pineview_get_mem_freq(struct drm_device *dev)
>  {
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 90a3f6d..c3dcffd 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -560,6 +560,7 @@ intel_enable_primary(struct drm_crtc *crtc)
>  
>  	mutex_lock(&dev->struct_mutex);
>  	intel_update_fbc(dev);
> +	intel_update_drrs(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  }
>  
> @@ -579,6 +580,7 @@ intel_disable_primary(struct drm_crtc *crtc)
>  	mutex_lock(&dev->struct_mutex);
>  	if (dev_priv->fbc.plane == intel_crtc->plane)
>  		intel_disable_fbc(dev);
> +	intel_disable_drrs(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	/*
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center