Re: [PATCH 8/8] drm/i915: Add drrs_interval module parameter

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

 





On 15-Dec-14 3:17 PM, Daniel Vetter wrote:
On Thu, Dec 11, 2014 at 02:22:57AM +0530, Vandana Kannan wrote:
Adding i915 module parameter for setting drrs_interval. If this param is
set to 0, then drrs is disabled. If changed in runtime, then the new interval
value will be considered for scheduling the next drrs work.
drrs_interval is set to 0 by default, i.e. DRRS is disabled by default.

Nope, please don't hide power saving features behind module options by
default. New stuff must be enabled by default, otherwise it'll bitrot and
merging to upstream is fairly useless since we still have the rebase pain
(just spread out over more people) with none of the testing.
ok.. so, shall I just make the delay (drrs_interval) fixed at 1 second
or let user set this delay at runtime through the same module param (excluding the disable feature if interval is 0 part) ?

Also such debug options need to be marked using the _debug variants to
make sure that people report issues and don't keep using them.

Now talking about validation gets me to the next point: Do we have a basic
igt to make sure DRRS doesn't break right after merging? I don't think we
need the full-blown test setup like for psr/fbc, but a basic test to make
sure it enters/exit RR mode should be there.
libdrm has vbltest which prints refresh rate..
Apart from this, I'm adding downclock mode (if supported) in kms_flip.
Shall I add a test which involves low RR trigger after idle time and then forces some activity on screen and switches back to high RR ?
- Vandana

-Daniel


Signed-off-by: Vandana Kannan <vandana.kannan@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_drv.h    |  1 +
  drivers/gpu/drm/i915/i915_params.c |  8 ++++++++
  drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++++++-
  3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 370cbaa..dba5844 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2381,6 +2381,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 c91cb20..80492e8 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -51,6 +51,7 @@ struct i915_params i915 __read_mostly = {
  	.disable_vtd_wa = 0,
  	.use_mmio_flip = 0,
  	.mmio_debug = 0,
+	.drrs_interval = 0,
  };

  module_param_named(modeset, i915.modeset, int, 0400);
@@ -173,3 +174,10 @@ module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
  MODULE_PARM_DESC(mmio_debug,
  	"Enable the MMIO debug code (default: false). This may negatively "
  	"affect performance.");
+
+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_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 092ef91..88f46906 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4931,6 +4931,9 @@ void intel_edp_drrs_invalidate(struct drm_device *dev,
  	if (!dev_priv->drrs.dp)
  		return;

+	if (i915.drrs_interval == 0)
+		return;
+
  	mutex_lock(&dev_priv->drrs.mutex);
  	crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;
  	pipe = to_intel_crtc(crtc)->pipe;
@@ -4958,6 +4961,9 @@ void intel_edp_drrs_flush(struct drm_device *dev,
  	if (!dev_priv->drrs.dp)
  		return;

+	if (i915.drrs_interval == 0)
+		return;
+
  	mutex_lock(&dev_priv->drrs.mutex);
  	crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;
  	pipe = to_intel_crtc(crtc)->pipe;
@@ -4968,7 +4974,7 @@ void intel_edp_drrs_flush(struct drm_device *dev,
  	if (dev_priv->drrs.refresh_rate_type != DRRS_LOW_RR &&
  			!dev_priv->drrs.busy_frontbuffer_bits)
  		schedule_delayed_work(&dev_priv->drrs.work,
-				msecs_to_jiffies(1000));
+				msecs_to_jiffies(i915.drrs_interval));
  	mutex_unlock(&dev_priv->drrs.mutex);
  }

@@ -4991,6 +4997,9 @@ intel_dp_drrs_init(struct intel_connector *intel_connector,
  		return NULL;
  	}

+	if (i915.drrs_interval == 0)
+		DRM_DEBUG_KMS("DRRS disable by flag\n");
+
  	downclock_mode = intel_find_panel_downclock
  					(dev, fixed_mode, connector);

--
2.0.1


_______________________________________________
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