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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>


On Sat, 05 Apr 2014, Vandana Kannan <vandana.kannan@xxxxxxxxx> 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
>
> v3: Internal review comments incorporated
> Add NULL pointer check in intel_disable_drrs.
> Add drrs calls in i9xx_crtc_enable/disable and valleyview_crtc_enable.
>
> v4: Jani's review comments incorporated.
> Change in sequence in intel_update_drrs. Comment modified to remove details
> of update param. Modified DRRS idleness interval to a module parameter.
>
> v5: Chris's review comments incorporated.
> Initialize connector in idleness detection init. Modifications made to
> use only intel_connector in i915_drrs and derive intel_dp when required.
> Added a function drrs_fini to cleanup DRRS work.
>
> v6: Internal review comments. Removed check for primary enabled, which is
> a redundant check, in the case of clone mode. Added a flag to track
> dual-display configuration. Remove print statement for "cancel DRR work"
> and print "DRRS not supported" only once.
>
> v7: As per internal review comments, removing calls to update/disable drrs
> from sprite update path. For sprite, all drrs related updates would be
> taken care of with calls to crtc page flip itself. This will have to be
> revisited later if flip infrastructure changes for sprite.
>
> v8: Incorporated Jani's review comments. Added space after the periods in the
> module param description. Changes around drrs-fini to remove seamless DRRS
> check.
>
> v9: Added checks for PSR before updating DRRS. Added check for module
> param drrs_interval before updating DRRS (this is required if the interval
> is modified by the user during system use). DRRS disabled by default. Changes
> based on Jani's review comments
>
> v10: Disable/enable DRRS when PSR is enable/disabled.
>
> v11: Moved DRRS not supported log to patch2.
>
> Signed-off-by: Vandana Kannan <vandana.kannan@xxxxxxxxx>
> Signed-off-by: Pradeep Bhat <pradeep.bhat@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
>  drivers/gpu/drm/i915/i915_params.c   |   8 ++
>  drivers/gpu/drm/i915/intel_display.c |  16 ++++
>  drivers/gpu/drm/i915/intel_dp.c      |  27 ++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |   5 +-
>  drivers/gpu/drm/i915/intel_pm.c      | 142 +++++++++++++++++++++++++++++++++++
>  6 files changed, 203 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9da8d83..fb9f9f0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -635,6 +635,12 @@ struct i915_fbc {
>  
>  struct i915_drrs {
>  	struct intel_connector *connector;
> +	bool is_clone;
> +	struct intel_drrs_work {
> +		struct delayed_work work;
> +		struct drm_crtc *crtc;
> +		int interval;
> +	} *drrs_work;
>  };
>  
>  struct i915_psr {
> @@ -1922,6 +1928,7 @@ struct i915_params {
>  	int enable_ips;
>  	int invert_brightness;
>  	int enable_cmd_parser;
> +	int drrs_interval;
>  	/* leave bools at the end to not create holes */
>  	bool enable_hangcheck;
>  	bool fastboot;
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index d05a2af..144ab0b 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -48,6 +48,7 @@ struct i915_params i915 __read_mostly = {
>  	.disable_display = 0,
>  	.enable_cmd_parser = 1,
>  	.disable_vtd_wa = 0,
> +	.drrs_interval = 0,
>  };
>  
>  module_param_named(modeset, i915.modeset, int, 0400);
> @@ -156,3 +157,10 @@ MODULE_PARM_DESC(disable_vtd_wa, "Disable all VT-d workarounds (default: false)"
>  module_param_named(enable_cmd_parser, i915.enable_cmd_parser, int, 0600);
>  MODULE_PARM_DESC(enable_cmd_parser,
>  		 "Enable command parsing (1=enabled [default], 0=disabled)");
> +
> +module_param_named(drrs_interval, i915.drrs_interval, int, 0600);
> +MODULE_PARM_DESC(drrs_interval,
> +	"DRRS idleness detection interval  (default: 0 ms). "
> +	"If this field is set to 0, then seamless DRRS feature "
> +	"based on idleness detection is disabled. "
> +	"The interval is to be set in milliseconds.");
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3697433..363ca22 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2523,6 +2523,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>  
>  	mutex_lock(&dev->struct_mutex);
>  	intel_update_fbc(dev);
> +	intel_update_drrs(dev);
>  	intel_edp_psr_update(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
> @@ -3695,6 +3696,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)
> @@ -3738,6 +3740,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);
>  
>  	drm_vblank_on(dev, pipe);
> @@ -3935,6 +3938,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);
>  }
>  
> @@ -3982,6 +3986,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);
>  }
>  
> @@ -4376,6 +4381,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>  	intel_crtc_update_cursor(crtc, true);
>  
>  	intel_update_fbc(dev);
> +	intel_update_drrs(dev);
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		encoder->enable(encoder);
> @@ -4423,6 +4429,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>  	intel_crtc_dpms_overlay(intel_crtc, true);
>  
>  	intel_update_fbc(dev);
> +	intel_update_drrs(dev);
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		encoder->enable(encoder);
> @@ -4490,6 +4497,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>  	intel_update_watermarks(crtc);
>  
>  	intel_update_fbc(dev);
> +	intel_update_drrs(dev);
>  }
>  
>  static void i9xx_crtc_off(struct drm_crtc *crtc)
> @@ -8510,6 +8518,10 @@ static void intel_unpin_work_fn(struct work_struct *__work)
>  	drm_gem_object_unreference(&work->pending_flip_obj->base);
>  	drm_gem_object_unreference(&work->old_fb_obj->base);
>  
> +	/* disable current DRRS work scheduled and restart
> +	 * to push work by another x seconds
> +	 */
> +	intel_update_drrs(dev);
>  	intel_update_fbc(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
> @@ -8968,6 +8980,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);
>  
> @@ -11341,6 +11354,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);
>  
>  	mutex_lock(&dev->mode_config.mutex);
>  	intel_modeset_setup_hw_state(dev, false);
> @@ -11821,6 +11835,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 b50b170..5fc4060 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1739,6 +1739,11 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
>  	    intel_edp_is_psr_enabled(dev))
>  		return;
>  
> +	if (INTEL_INFO(dev)->gen < 8) {
> +		DRM_DEBUG_KMS("Enabling PSR, disabling DRRS\n");
> +		intel_disable_drrs(dev);
> +	}
> +
>  	/* Setup PSR once */
>  	intel_edp_psr_setup(intel_dp);
>  
> @@ -1773,6 +1778,9 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
>  	if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev)) &
>  		       EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
>  		DRM_ERROR("Timed out waiting for PSR Idle State\n");
> +
> +	if (INTEL_INFO(dev)->gen < 8)
> +		intel_update_drrs(dev);
>  }
>  
>  void intel_edp_psr_update(struct drm_device *dev)
> @@ -3371,16 +3379,33 @@ intel_dp_connector_destroy(struct drm_connector *connector)
>  	kfree(connector);
>  }
>  
> +static void
> +intel_dp_drrs_fini(struct drm_i915_private *dev_priv)
> +{
> +	if (cancel_delayed_work_sync(&dev_priv->drrs.drrs_work->work)) {
> +		kfree(dev_priv->drrs.drrs_work);
> +		dev_priv->drrs.drrs_work = NULL;
> +		dev_priv->drrs.connector = NULL;
> +	}
> +}
> +
>  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;
>  
>  	drm_dp_aux_unregister_i2c_bus(&intel_dp->aux);
>  	drm_encoder_cleanup(encoder);
>  	if (is_edp(intel_dp)) {
>  		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
> +
> +		if (dev_priv->drrs.connector && dev_priv->drrs.drrs_work &&
> +			intel_dp == enc_to_intel_dp(
> +			&dev_priv->drrs.connector->encoder->base))
> +			intel_dp_drrs_fini(dev_priv);
> +
>  		mutex_lock(&dev->mode_config.mutex);
>  		edp_panel_vdd_off_sync(intel_dp);
>  		mutex_unlock(&dev->mode_config.mutex);
> @@ -3767,7 +3792,7 @@ intel_dp_drrs_init(struct intel_digital_port *intel_dig_port,
>  		return NULL;
>  	}
>  
> -	dev_priv->drrs.connector = intel_connector;
> +	intel_init_drrs_idleness_detection(dev, intel_connector);
>  
>  	mutex_init(&intel_dp->drrs_state.mutex);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 37d56e5..de1357b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -932,7 +932,10 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
>  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);
> +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 04af065..0c43329 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -621,6 +621,148 @@ 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;
> +
> +	/* Double check if the dual-display mode is active. */
> +	if (dev_priv->drrs.is_clone)
> +		return;
> +
> +	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;
> +
> +	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;
> +	struct intel_dp *intel_dp = NULL;
> +
> +	intel_dp = enc_to_intel_dp(&dev_priv->drrs.connector->encoder->base);
> +
> +	if (intel_dp == NULL)
> +		return;
> +
> +	intel_cancel_drrs_work(dev_priv);
> +
> +	if (intel_dp->drrs_state.refresh_rate_type != DRRS_LOW_RR) {
> +		dev_priv->drrs.drrs_work->crtc = crtc;
> +		dev_priv->drrs.drrs_work->interval = i915.drrs_interval;
> +
> +		/* 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;
> +	struct intel_dp *intel_dp = NULL;
> +
> +	if (dev_priv->drrs.connector == NULL)
> +		return;
> +
> +	intel_dp = enc_to_intel_dp(&dev_priv->drrs.connector->encoder->base);
> +
> +	if (intel_dp == NULL)
> +		return;
> +
> +	/* as part of disable DRRS, reset refresh rate to HIGH_RR */
> +	if (intel_dp->drrs_state.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
> +*/
> +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 (i915.drrs_interval == 0) {
> +		intel_disable_drrs(dev);
> +		return;
> +	}
> +
> +	/* if drrs.connector is NULL, then drrs_init did not get called.
> +	 * which means DRRS is not supported.
> +	 */
> +	if (dev_priv->drrs.connector == NULL)
> +		return;
> +
> +	if (dev_priv->drrs.connector->panel.downclock_mode == NULL)
> +		return;
> +
> +	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
> +		if (intel_crtc_active(tmp_crtc)) {
> +			if (crtc) {
> +				DRM_DEBUG_KMS(
> +				"more than one pipe active, disabling DRRS\n");
> +				dev_priv->drrs.is_clone = true;
> +				intel_disable_drrs(dev);
> +				return;
> +			}
> +			crtc = tmp_crtc;
> +		}
> +	}
> +
> +	if (crtc == NULL) {
> +		DRM_DEBUG_KMS("DRRS: crtc not initialized\n");
> +		return;
> +	}
> +
> +	dev_priv->drrs.is_clone = false;
> +	intel_disable_drrs(dev);
> +
> +	/* re-enable idleness detection */
> +	intel_enable_drrs(crtc);
> +}
> +
> +void intel_init_drrs_idleness_detection(struct drm_device *dev,
> +					struct intel_connector *connector)
> +{
> +	struct intel_drrs_work *work;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (i915.drrs_interval == 0)
> +		DRM_INFO("DRRS disable by flag\n");
> +
> +	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.is_clone = false;
> +
> +	work->interval = i915.drrs_interval;
> +	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)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -- 
> 1.9.1
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux