diff mbox

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

Message ID 1387258107-19232-5-git-send-email-vandana.kannan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

vandana.kannan@intel.com Dec. 17, 2013, 5:28 a.m. UTC
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.

Change-Id: I17b011b3867a39588375f2b97b992444972f7760
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      |   19 ++++++
 drivers/gpu/drm/i915/intel_display.c |   13 ++++
 drivers/gpu/drm/i915/intel_dp.c      |    9 +++
 drivers/gpu/drm/i915/intel_pm.c      |  112 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_sprite.c  |    3 +
 5 files changed, 156 insertions(+)

Comments

Chris Wilson Dec. 17, 2013, 12:29 p.m. UTC | #1
On Tue, Dec 17, 2013 at 10:58:26AM +0530, Vandana Kannan 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.

Looks very inconsistent next to intel_fbc_disable/intel_fbc_update.
-Chris
vandana.kannan@intel.com Dec. 18, 2013, 8:18 a.m. UTC | #2
On Dec-17-2013 5:59 PM, Chris Wilson wrote:
> On Tue, Dec 17, 2013 at 10:58:26AM +0530, Vandana Kannan 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.
> 
> Looks very inconsistent next to intel_fbc_disable/intel_fbc_update.
> -Chris
> 
We have implemented this in a way that it is similar to fbc
implementation. Could you explain some more about your review comment?
Chris Wilson Dec. 18, 2013, 9:04 a.m. UTC | #3
On Wed, Dec 18, 2013 at 01:48:12PM +0530, Vandana Kannan wrote:
> On Dec-17-2013 5:59 PM, Chris Wilson wrote:
> > On Tue, Dec 17, 2013 at 10:58:26AM +0530, Vandana Kannan 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.
> > 
> > Looks very inconsistent next to intel_fbc_disable/intel_fbc_update.
> > -Chris
> > 
> We have implemented this in a way that it is similar to fbc
> implementation. Could you explain some more about your review comment?

See the split between intel_fbc_disable and intel_fbc_update and how it
would make your code more tidy, your API harder to get wrong and make it
easier to integrate all of these triggers into a single routine. Also
note that you miss out on frontbuffer rendering detection.
-Chris
vandana.kannan@intel.com Dec. 18, 2013, 10:09 a.m. UTC | #4
On Dec-18-2013 2:34 PM, Chris Wilson wrote:
> On Wed, Dec 18, 2013 at 01:48:12PM +0530, Vandana Kannan wrote:
>> On Dec-17-2013 5:59 PM, Chris Wilson wrote:
>>> On Tue, Dec 17, 2013 at 10:58:26AM +0530, Vandana Kannan 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.
>>>
>>> Looks very inconsistent next to intel_fbc_disable/intel_fbc_update.
>>> -Chris
>>>
>> We have implemented this in a way that it is similar to fbc
>> implementation. Could you explain some more about your review comment?
> 
> See the split between intel_fbc_disable and intel_fbc_update and how it
> would make your code more tidy, your API harder to get wrong and make it
> easier to integrate all of these triggers into a single routine. Also
> note that you miss out on frontbuffer rendering detection.
> -Chris
> 
The current implementation makes use of "bool update" to differentiate
between an update/disable. I will make changes so that the
implementation is similar to intel_fbc_disable/intel_fbc_update.
Could you give more information on the miss on frontbuffer rendering
detection?
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c9bca16..ec29603 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -713,6 +713,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;
+	} *drrs_work;
+};
+
 struct i915_psr {
 	bool sink_support;
 	bool source_ok;
@@ -1397,6 +1412,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;
 
@@ -2422,6 +2438,9 @@  extern void intel_modeset_setup_hw_state(struct drm_device *dev,
 extern void i915_redisable_vga(struct drm_device *dev);
 extern bool intel_fbc_enabled(struct drm_device *dev);
 extern void intel_disable_fbc(struct drm_device *dev);
+extern void intel_init_drrs_idleness_detection(struct drm_device *dev,
+		struct intel_connector *connector, struct intel_dp *dp);
+extern void intel_update_drrs(struct drm_device *dev, bool update);
 extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
 extern void intel_init_pch_refclk(struct drm_device *dev);
 extern void gen6_set_rps(struct drm_device *dev, u8 val);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0332d7c..9a699cf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2385,6 +2385,7 @@  intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 	}
 
 	intel_update_fbc(dev);
+	intel_update_drrs(dev, true);
 	intel_edp_psr_update(dev);
 	mutex_unlock(&dev->struct_mutex);
 
@@ -3549,6 +3550,7 @@  static void ironlake_crtc_enable(struct drm_crtc *crtc)
 
 	mutex_lock(&dev->struct_mutex);
 	intel_update_fbc(dev);
+	intel_update_drrs(dev, true);
 	mutex_unlock(&dev->struct_mutex);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
@@ -3590,6 +3592,7 @@  static void haswell_crtc_enable_planes(struct drm_crtc *crtc)
 
 	mutex_lock(&dev->struct_mutex);
 	intel_update_fbc(dev);
+	intel_update_drrs(dev, true);
 	mutex_unlock(&dev->struct_mutex);
 }
 
@@ -3796,6 +3799,7 @@  static void ironlake_crtc_disable(struct drm_crtc *crtc)
 
 	mutex_lock(&dev->struct_mutex);
 	intel_update_fbc(dev);
+	intel_update_drrs(dev, true);
 	mutex_unlock(&dev->struct_mutex);
 }
 
@@ -3843,6 +3847,7 @@  static void haswell_crtc_disable(struct drm_crtc *crtc)
 
 	mutex_lock(&dev->struct_mutex);
 	intel_update_fbc(dev);
+	intel_update_drrs(dev, true);
 	mutex_unlock(&dev->struct_mutex);
 }
 
@@ -8184,6 +8189,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, true);
 	mutex_unlock(&dev->struct_mutex);
 
 	BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0);
@@ -8623,6 +8633,7 @@  static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		goto cleanup_pending;
 
 	intel_disable_fbc(dev);
+	intel_update_drrs(dev, false);
 	intel_mark_fb_busy(obj, NULL);
 	mutex_unlock(&dev->struct_mutex);
 
@@ -11250,6 +11261,8 @@  void intel_modeset_cleanup(struct drm_device *dev)
 
 	intel_disable_fbc(dev);
 
+	intel_update_drrs(dev, false);
+
 	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 fbf71ed..209be3c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3293,11 +3293,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.is_drrs_supported
+					== 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);
@@ -3665,6 +3672,8 @@  intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
 		dev_priv->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.is_drrs_supported
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ff47520..4866e53 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -615,6 +615,118 @@  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;
+
+	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));
+	}
+}
+
+/**
+ * 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.
+*/
+void intel_update_drrs(struct drm_device *dev, bool update)
+{
+	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;
+	}
+
+	intel_cancel_drrs_work(dev_priv);
+
+	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");
+				update = false;
+				break;
+			}
+			crtc = tmp_crtc;
+		}
+	}
+
+	if (crtc == NULL) {
+		DRM_INFO("DRRS: crtc not initialized\n");
+		return;
+	}
+
+	/* 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_dp_set_drrs_state(dev,
+			dev_priv->drrs.connector->panel.fixed_mode->vrefresh);
+
+	if (update == true) {
+		/* re-enable idleness detection */
+		intel_enable_drrs(crtc);
+	}
+}
+
+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..71ca8af 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, true);
 	mutex_unlock(&dev->struct_mutex);
 }
 
@@ -579,6 +580,8 @@  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_update_drrs(dev, false);
 	mutex_unlock(&dev->struct_mutex);
 
 	/*