[Intel-gfx] [PATCH 4/6] drm/i915: Idleness detection for DRRS
Vandana Kannan
vandana.kannan at intel.com
Thu Mar 27 11:20:58 CET 2014
On Mar-26-2014 6:35 PM, Jani Nikula wrote:
> On Fri, 07 Mar 2014, Vandana Kannan <vandana.kannan at 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
>>
>> 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.
>>
>> Signed-off-by: Vandana Kannan <vandana.kannan at intel.com>
>> Signed-off-by: Pradeep Bhat <pradeep.bhat at intel.com>
>> ---
>> 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 | 26 ++++++-
>> drivers/gpu/drm/i915/intel_drv.h | 5 +-
>> drivers/gpu/drm/i915/intel_pm.c | 138 ++++++++++++++++++++++++++++++++++
>> 6 files changed, 197 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 43d3dfe..87865e9 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -776,6 +776,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 {
>> @@ -1975,6 +1981,7 @@ struct i915_params {
>> bool prefault_disable;
>> bool reset;
>> int invert_brightness;
>> + int drrs_interval;
>> };
>> extern struct i915_params i915 __read_mostly;
>>
>> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>> index c743057..69f8b83 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -47,6 +47,7 @@ struct i915_params i915 __read_mostly = {
>> .prefault_disable = 0,
>> .reset = true,
>> .invert_brightness = 0,
>> + .drrs_interval = 2000,
>> };
>>
>> module_param_named(modeset, i915.modeset, int, 0400);
>> @@ -153,3 +154,10 @@ MODULE_PARM_DESC(invert_brightness,
>> "report PCI device ID, subsystem vendor and subsystem device ID "
>> "to dri-devel at lists.freedesktop.org, if your machine needs it. "
>> "It will then be included in an upcoming module version.");
>> +
>> +module_param_named(drrs_interval, i915.drrs_interval, int, 0600);
>> +MODULE_PARM_DESC(drrs_interval,
>> + "DRRS idleness detection interval (default: 2000 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.");
>
> When the strings are concatenated there won't be a space after the
> periods.
>
Ok.. Will make appropriate changes..
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 4d4a0d9..86cd603 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2410,6 +2410,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);
>>
>> @@ -3598,6 +3599,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)
>> @@ -3639,6 +3641,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);
>> }
>>
>> @@ -3845,6 +3848,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);
>> }
>>
>> @@ -3892,6 +3896,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);
>> }
>>
>> @@ -4176,6 +4181,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);
>> @@ -4221,6 +4227,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);
>> @@ -4286,6 +4293,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)
>> @@ -8281,6 +8289,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);
>>
>> @@ -8722,6 +8734,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);
>>
>> @@ -11055,6 +11068,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
>> @@ -11461,6 +11475,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 54cde57..6a91856 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -3406,16 +3406,37 @@ intel_dp_connector_destroy(struct drm_connector *connector)
>> kfree(connector);
>> }
>>
>> +static void
>> +intel_dp_drrs_fini(struct drm_i915_private *dev_priv,
>> + struct intel_dp *intel_dp)
>> +{
>> + if (intel_dp->drrs_state.type == SEAMLESS_DRRS_SUPPORT) {
>
> It's not obvious to me why this gets only done if
> SEAMLESS_DRRS_SUPPORT. The scheduling of the work is not indirectly
> dependent on this.
>
This check was added to confine all idleness detection related
infrastructure to seamless DRRS. This check may be redundant here - I
will make appropriate changes..
>> + 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;
>>
>> i2c_del_adapter(&intel_dp->adapter);
>> drm_encoder_cleanup(encoder);
>> if (is_edp(intel_dp)) {
>> cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
>> +
>> + if (dev_priv->drrs.connector &&
>> + intel_dp == enc_to_intel_dp(
>> + &dev_priv->drrs.connector->encoder->base))
>> + intel_dp_drrs_fini(dev_priv, intel_dp);
>> +
>> mutex_lock(&dev->mode_config.mutex);
>> edp_panel_vdd_off_sync(intel_dp);
>> mutex_unlock(&dev->mode_config.mutex);
>> @@ -3805,7 +3826,7 @@ intel_dp_drrs_init(struct intel_digital_port *intel_dig_port,
>> intel_connector->panel.edp_downclock =
>> downclock_mode->clock;
>>
>> - dev_priv->drrs.connector = intel_connector;
>> + intel_init_drrs_idleness_detection(dev, intel_connector);
>>
>> mutex_init(&intel_dp->drrs_state.mutex);
>>
>> @@ -3813,7 +3834,8 @@ intel_dp_drrs_init(struct intel_digital_port *intel_dig_port,
>>
>> intel_dp->drrs_state.refresh_rate_type = DRRS_HIGH_RR;
>> DRM_INFO("seamless DRRS supported for eDP panel.\n");
>> - }
>> + } else
>> + DRM_INFO("DRRS not supported\n");
>>
>> return downclock_mode;
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 4f7c816..7b6dcc0 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -911,7 +911,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 9ab3883..e210cbc 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -618,6 +618,144 @@ 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;
>> +
>> + /* 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 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 (tmp_crtc != NULL && 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");
>> + return;
>> + }
>> +
>> + 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)
>> {
>> drm_i915_private_t *dev_priv = dev->dev_private;
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
More information about the Intel-gfx
mailing list