[RFC 0/6] eDP DRRS based on frontbuffer tracking

[RFC 0/6] eDP DRRS based on frontbuffer tracking

[Vandana Kannan]

As part of implementing DRRS based on frontbuffer tracking, this patch series
includes some modifications related to the data for DRRS, code to enable/
disable DRRS during init or uninit, calls to switch between high and low
refresh rates based on calls to mark_fb_busy.
Instead of having a module param to specify delay before scheduling DRRS
work, (as there was in the old patch series), a delay of 100ms is used.
Additional patches including changes specific to bdw and vlv have been
included.

TODO:-
Need more analysis into clone mode scenario

Vandana Kannan (6):
  drm/i915: Modifying structures related to DRRS
  drm/i915: Initialize DRRS delayed work
  drm/i915: Enable/disable DRRS
  drm/i915: DRRS calls based on frontbuffer
  drm/i915/bdw: Add support for DRRS to switch RR
  drm/i915: Support for RR switching on VLV

 drivers/gpu/drm/i915/i915_drv.h      |  32 ++++--
 drivers/gpu/drm/i915/i915_reg.h      |   1 +
 drivers/gpu/drm/i915/intel_ddi.c     |   2 +
 drivers/gpu/drm/i915/intel_display.c |  14 +--
 drivers/gpu/drm/i915/intel_dp.c      | 201 +++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h     |  27 ++---
 6 files changed, 211 insertions(+), 66 deletions(-)

-- 
2.0.1

[RFC 1/6] drm/i915: Modifying structures related to DRRS

[Vandana Kannan]

Moving around and changing some data related to DRRS to support
DRRS based on frontbuffer tracking in the following patches

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  | 32 ++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_dp.c  | 51 ++++++++++++++++++----------------------
 drivers/gpu/drm/i915/intel_drv.h | 18 --------------
 3 files changed, 47 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 241b2bf..1713bb9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -680,11 +680,33 @@ struct i915_fbc {
 	} no_fbc_reason;
 };
 
-struct i915_drrs {
-	struct intel_connector *connector;
+/**
+ * HIGH_RR is the highest eDP panel refresh rate read from EDID
+ * LOW_RR is the lowest eDP panel refresh rate found from EDID
+ * parsing for same resolution.
+ */
+enum drrs_refresh_rate_type {
+	DRRS_HIGH_RR,
+	DRRS_LOW_RR,
+	DRRS_MAX_RR, /* RR count */
+};
+
+enum drrs_support_type {
+	DRRS_NOT_SUPPORTED = 0,
+	STATIC_DRRS_SUPPORT = 1,
+	SEAMLESS_DRRS_SUPPORT = 2
 };
 
 struct intel_dp;
+struct i915_drrs {
+	struct mutex mutex;
+	struct delayed_work work;
+	struct intel_dp *dp;
+	unsigned busy_frontbuffer_bits;
+	enum drrs_refresh_rate_type refresh_rate_type;
+	enum drrs_support_type type;
+};
+
 struct i915_psr {
 	struct mutex lock;
 	bool sink_support;
@@ -1290,12 +1312,6 @@ struct ddi_vbt_port_info {
 	uint8_t supports_dp:1;
 };
 
-enum drrs_support_type {
-	DRRS_NOT_SUPPORTED = 0,
-	STATIC_DRRS_SUPPORT = 1,
-	SEAMLESS_DRRS_SUPPORT = 2
-};
-
 struct intel_vbt_data {
 	struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
 	struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ab7cd0a..be9d90d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1139,7 +1139,7 @@ found:
 			       &pipe_config->dp_m_n);
 
 	if (intel_connector->panel.downclock_mode != NULL &&
-		intel_dp->drrs_state.type == SEAMLESS_DRRS_SUPPORT) {
+		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
 			pipe_config->has_drrs = true;
 			intel_link_compute_m_n(bpp, lane_count,
 				intel_connector->panel.downclock_mode->clock,
@@ -4794,24 +4794,24 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 		      I915_READ(pp_div_reg));
 }
 
-void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
+static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_encoder *encoder;
-	struct intel_dp *intel_dp = NULL;
+	struct intel_digital_port *dig_port = NULL;
+	struct intel_dp *intel_dp = dev_priv->drrs.dp;
 	struct intel_crtc_config *config = NULL;
 	struct intel_crtc *intel_crtc = NULL;
-	struct intel_connector *intel_connector = dev_priv->drrs.connector;
 	u32 reg, val;
-	enum edp_drrs_refresh_rate_type index = DRRS_HIGH_RR;
+	enum drrs_refresh_rate_type index = DRRS_HIGH_RR;
 
 	if (refresh_rate <= 0) {
 		DRM_DEBUG_KMS("Refresh rate should be positive non-zero.\n");
 		return;
 	}
 
-	if (intel_connector == NULL) {
-		DRM_DEBUG_KMS("DRRS supported for eDP only.\n");
+	if (intel_dp == NULL) {
+		DRM_DEBUG_KMS("DRRS not supported.\n");
 		return;
 	}
 
@@ -4825,8 +4825,8 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 		return;
 	}
 
-	encoder = intel_attached_encoder(&intel_connector->base);
-	intel_dp = enc_to_intel_dp(&encoder->base);
+	dig_port = dp_to_dig_port(intel_dp);
+	encoder = &dig_port->base;
 	intel_crtc = encoder->new_crtc;
 
 	if (!intel_crtc) {
@@ -4836,15 +4836,16 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 
 	config = &intel_crtc->config;
 
-	if (intel_dp->drrs_state.type < SEAMLESS_DRRS_SUPPORT) {
+	if (dev_priv->drrs.type < SEAMLESS_DRRS_SUPPORT) {
 		DRM_DEBUG_KMS("Only Seamless DRRS supported.\n");
 		return;
 	}
 
-	if (intel_connector->panel.downclock_mode->vrefresh == refresh_rate)
+	if (intel_dp->attached_connector->panel.downclock_mode->vrefresh ==
+								refresh_rate)
 		index = DRRS_LOW_RR;
 
-	if (index == intel_dp->drrs_state.refresh_rate_type) {
+	if (index == dev_priv->drrs.refresh_rate_type) {
 		DRM_DEBUG_KMS(
 			"DRRS requested for previously set RR...ignoring\n");
 		return;
@@ -4873,24 +4874,21 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 	 * in future when idleness detection based DRRS in kernel and
 	 * possible calls from user space to set differnt RR are made.
 	 */
+	mutex_lock(&dev_priv->drrs.mutex);
 
-	mutex_lock(&intel_dp->drrs_state.mutex);
+	dev_priv->drrs.refresh_rate_type = index;
 
-	intel_dp->drrs_state.refresh_rate_type = index;
-
-	mutex_unlock(&intel_dp->drrs_state.mutex);
+	mutex_unlock(&dev_priv->drrs.mutex);
 
 	DRM_DEBUG_KMS("eDP Refresh Rate set to : %dHz\n", refresh_rate);
 }
 
 static struct drm_display_mode *
-intel_dp_drrs_init(struct intel_digital_port *intel_dig_port,
-			struct intel_connector *intel_connector,
-			struct drm_display_mode *fixed_mode)
+intel_dp_drrs_init(struct intel_connector *intel_connector,
+		   struct drm_display_mode *fixed_mode)
 {
 	struct drm_connector *connector = &intel_connector->base;
-	struct intel_dp *intel_dp = &intel_dig_port->dp;
-	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_device *dev = connector->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_display_mode *downclock_mode = NULL;
 
@@ -4912,13 +4910,11 @@ intel_dp_drrs_init(struct intel_digital_port *intel_dig_port,
 		return NULL;
 	}
 
-	dev_priv->drrs.connector = intel_connector;
-
-	mutex_init(&intel_dp->drrs_state.mutex);
+	mutex_init(&dev_priv->drrs.mutex);
 
-	intel_dp->drrs_state.type = dev_priv->vbt.drrs_type;
+	dev_priv->drrs.type = dev_priv->vbt.drrs_type;
 
-	intel_dp->drrs_state.refresh_rate_type = DRRS_HIGH_RR;
+	dev_priv->drrs.refresh_rate_type = DRRS_HIGH_RR;
 	DRM_DEBUG_KMS("seamless DRRS supported for eDP panel.\n");
 	return downclock_mode;
 }
@@ -4969,7 +4965,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	struct drm_display_mode *scan;
 	struct edid *edid;
 
-	intel_dp->drrs_state.type = DRRS_NOT_SUPPORTED;
+	dev_priv->drrs.type = DRRS_NOT_SUPPORTED;
 
 	if (!is_edp(intel_dp))
 		return true;
@@ -5018,7 +5014,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 		if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
 			fixed_mode = drm_mode_duplicate(dev, scan);
 			downclock_mode = intel_dp_drrs_init(
-						intel_dig_port,
 						intel_connector, fixed_mode);
 			break;
 		}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 07ce046..5b613e0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -531,17 +531,6 @@ struct intel_hdmi {
 struct intel_dp_mst_encoder;
 #define DP_MAX_DOWNSTREAM_PORTS		0x10
 
-/**
- * HIGH_RR is the highest eDP panel refresh rate read from EDID
- * LOW_RR is the lowest eDP panel refresh rate found from EDID
- * parsing for same resolution.
- */
-enum edp_drrs_refresh_rate_type {
-	DRRS_HIGH_RR,
-	DRRS_LOW_RR,
-	DRRS_MAX_RR, /* RR count */
-};
-
 struct intel_dp {
 	uint32_t output_reg;
 	uint32_t aux_ch_ctl_reg;
@@ -596,12 +585,6 @@ struct intel_dp {
 				     bool has_aux_irq,
 				     int send_bytes,
 				     uint32_t aux_clock_divider);
-	struct {
-		enum drrs_support_type type;
-		enum edp_drrs_refresh_rate_type refresh_rate_type;
-		struct mutex mutex;
-	} drrs_state;
-
 };
 
 struct intel_digital_port {
@@ -938,7 +921,6 @@ void intel_edp_panel_on(struct intel_dp *intel_dp);
 void intel_edp_panel_off(struct intel_dp *intel_dp);
 void intel_edp_psr_enable(struct intel_dp *intel_dp);
 void intel_edp_psr_disable(struct intel_dp *intel_dp);
-void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate);
 void intel_edp_psr_invalidate(struct drm_device *dev,
 			      unsigned frontbuffer_bits);
 void intel_edp_psr_flush(struct drm_device *dev,
-- 
2.0.1

Re: [RFC 1/6] drm/i915: Modifying structures related to DRRS

[Daniel Vetter]

On Fri, Sep 12, 2014 at 11:13:04PM +0530, Vandana Kannan wrote:
> Moving around and changing some data related to DRRS to support
> DRRS based on frontbuffer tracking in the following patches
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>

The commit message is a bit too sparse imo. You shouldn't just describe
what you change (the diff should make that clear), but much more important
is why you make these changes.

Here the big thing seems to be to move the drrs data from struct intel_dp
to the global dev_priv so that you can access it from non-kms code. Which
we need to be able to since the frontbuffer tracking code gets also called
from GEM functions.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h  | 32 ++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_dp.c  | 51 ++++++++++++++++++----------------------
>  drivers/gpu/drm/i915/intel_drv.h | 18 --------------
>  3 files changed, 47 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 241b2bf..1713bb9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -680,11 +680,33 @@ struct i915_fbc {
>  	} no_fbc_reason;
>  };
>  
> -struct i915_drrs {
> -	struct intel_connector *connector;
> +/**
> + * HIGH_RR is the highest eDP panel refresh rate read from EDID
> + * LOW_RR is the lowest eDP panel refresh rate found from EDID
> + * parsing for same resolution.
> + */
> +enum drrs_refresh_rate_type {
> +	DRRS_HIGH_RR,
> +	DRRS_LOW_RR,
> +	DRRS_MAX_RR, /* RR count */
> +};
> +
> +enum drrs_support_type {
> +	DRRS_NOT_SUPPORTED = 0,
> +	STATIC_DRRS_SUPPORT = 1,
> +	SEAMLESS_DRRS_SUPPORT = 2
>  };
>  
>  struct intel_dp;
> +struct i915_drrs {
> +	struct mutex mutex;
> +	struct delayed_work work;
> +	struct intel_dp *dp;
> +	unsigned busy_frontbuffer_bits;
> +	enum drrs_refresh_rate_type refresh_rate_type;
> +	enum drrs_support_type type;
> +};
> +
>  struct i915_psr {
>  	struct mutex lock;
>  	bool sink_support;
> @@ -1290,12 +1312,6 @@ struct ddi_vbt_port_info {
>  	uint8_t supports_dp:1;
>  };
>  
> -enum drrs_support_type {
> -	DRRS_NOT_SUPPORTED = 0,
> -	STATIC_DRRS_SUPPORT = 1,
> -	SEAMLESS_DRRS_SUPPORT = 2
> -};
> -
>  struct intel_vbt_data {
>  	struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
>  	struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ab7cd0a..be9d90d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1139,7 +1139,7 @@ found:
>  			       &pipe_config->dp_m_n);
>  
>  	if (intel_connector->panel.downclock_mode != NULL &&
> -		intel_dp->drrs_state.type == SEAMLESS_DRRS_SUPPORT) {
> +		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
>  			pipe_config->has_drrs = true;
>  			intel_link_compute_m_n(bpp, lane_count,
>  				intel_connector->panel.downclock_mode->clock,
> @@ -4794,24 +4794,24 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  		      I915_READ(pp_div_reg));
>  }
>  
> -void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
> +static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_encoder *encoder;
> -	struct intel_dp *intel_dp = NULL;
> +	struct intel_digital_port *dig_port = NULL;
> +	struct intel_dp *intel_dp = dev_priv->drrs.dp;
>  	struct intel_crtc_config *config = NULL;
>  	struct intel_crtc *intel_crtc = NULL;
> -	struct intel_connector *intel_connector = dev_priv->drrs.connector;
>  	u32 reg, val;
> -	enum edp_drrs_refresh_rate_type index = DRRS_HIGH_RR;
> +	enum drrs_refresh_rate_type index = DRRS_HIGH_RR;
>  
>  	if (refresh_rate <= 0) {
>  		DRM_DEBUG_KMS("Refresh rate should be positive non-zero.\n");
>  		return;
>  	}
>  
> -	if (intel_connector == NULL) {
> -		DRM_DEBUG_KMS("DRRS supported for eDP only.\n");
> +	if (intel_dp == NULL) {
> +		DRM_DEBUG_KMS("DRRS not supported.\n");
>  		return;
>  	}
>  
> @@ -4825,8 +4825,8 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>  		return;
>  	}
>  
> -	encoder = intel_attached_encoder(&intel_connector->base);
> -	intel_dp = enc_to_intel_dp(&encoder->base);
> +	dig_port = dp_to_dig_port(intel_dp);
> +	encoder = &dig_port->base;
>  	intel_crtc = encoder->new_crtc;
>  
>  	if (!intel_crtc) {
> @@ -4836,15 +4836,16 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>  
>  	config = &intel_crtc->config;
>  
> -	if (intel_dp->drrs_state.type < SEAMLESS_DRRS_SUPPORT) {
> +	if (dev_priv->drrs.type < SEAMLESS_DRRS_SUPPORT) {
>  		DRM_DEBUG_KMS("Only Seamless DRRS supported.\n");
>  		return;
>  	}
>  
> -	if (intel_connector->panel.downclock_mode->vrefresh == refresh_rate)
> +	if (intel_dp->attached_connector->panel.downclock_mode->vrefresh ==
> +								refresh_rate)
>  		index = DRRS_LOW_RR;
>  
> -	if (index == intel_dp->drrs_state.refresh_rate_type) {
> +	if (index == dev_priv->drrs.refresh_rate_type) {
>  		DRM_DEBUG_KMS(
>  			"DRRS requested for previously set RR...ignoring\n");
>  		return;
> @@ -4873,24 +4874,21 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>  	 * in future when idleness detection based DRRS in kernel and
>  	 * possible calls from user space to set differnt RR are made.
>  	 */
> +	mutex_lock(&dev_priv->drrs.mutex);
>  
> -	mutex_lock(&intel_dp->drrs_state.mutex);
> +	dev_priv->drrs.refresh_rate_type = index;
>  
> -	intel_dp->drrs_state.refresh_rate_type = index;
> -
> -	mutex_unlock(&intel_dp->drrs_state.mutex);
> +	mutex_unlock(&dev_priv->drrs.mutex);
>  
>  	DRM_DEBUG_KMS("eDP Refresh Rate set to : %dHz\n", refresh_rate);
>  }
>  
>  static struct drm_display_mode *
> -intel_dp_drrs_init(struct intel_digital_port *intel_dig_port,
> -			struct intel_connector *intel_connector,
> -			struct drm_display_mode *fixed_mode)
> +intel_dp_drrs_init(struct intel_connector *intel_connector,
> +		   struct drm_display_mode *fixed_mode)
>  {
>  	struct drm_connector *connector = &intel_connector->base;
> -	struct intel_dp *intel_dp = &intel_dig_port->dp;
> -	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_device *dev = connector->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_display_mode *downclock_mode = NULL;
>  
> @@ -4912,13 +4910,11 @@ intel_dp_drrs_init(struct intel_digital_port *intel_dig_port,
>  		return NULL;
>  	}
>  
> -	dev_priv->drrs.connector = intel_connector;
> -
> -	mutex_init(&intel_dp->drrs_state.mutex);
> +	mutex_init(&dev_priv->drrs.mutex);
>  
> -	intel_dp->drrs_state.type = dev_priv->vbt.drrs_type;
> +	dev_priv->drrs.type = dev_priv->vbt.drrs_type;
>  
> -	intel_dp->drrs_state.refresh_rate_type = DRRS_HIGH_RR;
> +	dev_priv->drrs.refresh_rate_type = DRRS_HIGH_RR;
>  	DRM_DEBUG_KMS("seamless DRRS supported for eDP panel.\n");
>  	return downclock_mode;
>  }
> @@ -4969,7 +4965,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	struct drm_display_mode *scan;
>  	struct edid *edid;
>  
> -	intel_dp->drrs_state.type = DRRS_NOT_SUPPORTED;
> +	dev_priv->drrs.type = DRRS_NOT_SUPPORTED;
>  
>  	if (!is_edp(intel_dp))
>  		return true;
> @@ -5018,7 +5014,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  		if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
>  			fixed_mode = drm_mode_duplicate(dev, scan);
>  			downclock_mode = intel_dp_drrs_init(
> -						intel_dig_port,
>  						intel_connector, fixed_mode);
>  			break;
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 07ce046..5b613e0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -531,17 +531,6 @@ struct intel_hdmi {
>  struct intel_dp_mst_encoder;
>  #define DP_MAX_DOWNSTREAM_PORTS		0x10
>  
> -/**
> - * HIGH_RR is the highest eDP panel refresh rate read from EDID
> - * LOW_RR is the lowest eDP panel refresh rate found from EDID
> - * parsing for same resolution.
> - */
> -enum edp_drrs_refresh_rate_type {
> -	DRRS_HIGH_RR,
> -	DRRS_LOW_RR,
> -	DRRS_MAX_RR, /* RR count */
> -};
> -
>  struct intel_dp {
>  	uint32_t output_reg;
>  	uint32_t aux_ch_ctl_reg;
> @@ -596,12 +585,6 @@ struct intel_dp {
>  				     bool has_aux_irq,
>  				     int send_bytes,
>  				     uint32_t aux_clock_divider);
> -	struct {
> -		enum drrs_support_type type;
> -		enum edp_drrs_refresh_rate_type refresh_rate_type;
> -		struct mutex mutex;
> -	} drrs_state;
> -
>  };
>  
>  struct intel_digital_port {
> @@ -938,7 +921,6 @@ void intel_edp_panel_on(struct intel_dp *intel_dp);
>  void intel_edp_panel_off(struct intel_dp *intel_dp);
>  void intel_edp_psr_enable(struct intel_dp *intel_dp);
>  void intel_edp_psr_disable(struct intel_dp *intel_dp);
> -void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate);
>  void intel_edp_psr_invalidate(struct drm_device *dev,
>  			      unsigned frontbuffer_bits);
>  void intel_edp_psr_flush(struct drm_device *dev,
> -- 
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[RFC 2/6] drm/i915: Initialize DRRS delayed work

[Vandana Kannan]

Initializing DRRS work to program lower refresh rate.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index be9d90d..1ceec86 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4868,19 +4868,37 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 		I915_WRITE(reg, val);
 	}
 
+	dev_priv->drrs.refresh_rate_type = index;
+
+	DRM_DEBUG_KMS("eDP Refresh Rate set to : %dHz\n", refresh_rate);
+}
+
+static void intel_edp_drrs_work(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, typeof(*dev_priv), drrs.work.work);
+	struct intel_dp *intel_dp = dev_priv->drrs.dp;
+
+	mutex_lock(&dev_priv->drrs.mutex);
+
+	if (!intel_dp)
+		goto unlock;
+
 	/*
-	 * mutex taken to ensure that there is no race between differnt
-	 * drrs calls trying to update refresh rate. This scenario may occur
-	 * in future when idleness detection based DRRS in kernel and
-	 * possible calls from user space to set differnt RR are made.
+	 * The delayed work can race with an invalidate hence we need to
+	 * recheck. Since psr_flush first clears this and then reschedules we
+	 * won't ever miss a flush when bailing out here.
 	 */
-	mutex_lock(&dev_priv->drrs.mutex);
+	if (dev_priv->drrs.busy_frontbuffer_bits)
+		goto unlock;
 
-	dev_priv->drrs.refresh_rate_type = index;
+	if (dev_priv->drrs.refresh_rate_type != DRRS_LOW_RR)
+		intel_dp_set_drrs_state(dev_priv->dev,
+			intel_dp->attached_connector->panel.
+			downclock_mode->vrefresh);
 
+unlock:
 	mutex_unlock(&dev_priv->drrs.mutex);
-
-	DRM_DEBUG_KMS("eDP Refresh Rate set to : %dHz\n", refresh_rate);
 }
 
 static struct drm_display_mode *
@@ -4910,6 +4928,8 @@ intel_dp_drrs_init(struct intel_connector *intel_connector,
 		return NULL;
 	}
 
+	INIT_DELAYED_WORK(&dev_priv->drrs.work, intel_edp_drrs_work);
+
 	mutex_init(&dev_priv->drrs.mutex);
 
 	dev_priv->drrs.type = dev_priv->vbt.drrs_type;
-- 
2.0.1

[RFC 3/6] drm/i915: Enable/disable DRRS

[Vandana Kannan]

Adding code to enable DRRS during ddi enable and disable DRRS during ddi disable

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  2 ++
 drivers/gpu/drm/i915/intel_dp.c  | 48 ++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  3 +++
 3 files changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b63d4fa..930f38e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1185,6 +1185,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
 
 		intel_edp_backlight_on(intel_dp);
 		intel_edp_psr_enable(intel_dp);
+		intel_edp_drrs_enable(intel_dp);
 	}
 
 	if (intel_crtc->config.has_audio) {
@@ -1219,6 +1220,7 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
 	if (type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
+		intel_edp_drrs_disable(intel_dp);
 		intel_edp_psr_disable(intel_dp);
 		intel_edp_backlight_off(intel_dp);
 	}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1ceec86..d50249e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4873,6 +4873,54 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 	DRM_DEBUG_KMS("eDP Refresh Rate set to : %dHz\n", refresh_rate);
 }
 
+void intel_edp_drrs_enable(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct drm_crtc *crtc = dig_port->base.base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	if (!intel_crtc->config.has_drrs) {
+		DRM_DEBUG_KMS("Panel doesn't support DRRS\n");
+		return;
+	}
+
+	mutex_lock(&dev_priv->drrs.mutex);
+	if (dev_priv->drrs.dp) {
+		DRM_DEBUG_KMS("DRRS already enabled\n");
+		mutex_unlock(&dev_priv->drrs.mutex);
+		return;
+	}
+
+	dev_priv->drrs.busy_frontbuffer_bits = 0;
+
+	dev_priv->drrs.dp = intel_dp;
+	mutex_unlock(&dev_priv->drrs.mutex);
+}
+
+void intel_edp_drrs_disable(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	mutex_lock(&dev_priv->drrs.mutex);
+	if (!dev_priv->drrs.dp) {
+		mutex_unlock(&dev_priv->drrs.mutex);
+		return;
+	}
+
+	if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR)
+		intel_dp_set_drrs_state(dev_priv->dev,
+			intel_dp->attached_connector->panel.
+			fixed_mode->vrefresh);
+
+	dev_priv->drrs.dp = NULL;
+	mutex_unlock(&dev_priv->drrs.mutex);
+
+	cancel_delayed_work_sync(&dev_priv->drrs.work);
+}
+
 static void intel_edp_drrs_work(struct work_struct *work)
 {
 	struct drm_i915_private *dev_priv =
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5b613e0..a728cc8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -927,6 +927,9 @@ void intel_edp_psr_flush(struct drm_device *dev,
 			 unsigned frontbuffer_bits);
 void intel_edp_psr_init(struct drm_device *dev);
 
+void intel_edp_drrs_enable(struct intel_dp *intel_dp);
+void intel_edp_drrs_disable(struct intel_dp *intel_dp);
+
 int intel_dp_handle_hpd_irq(struct intel_digital_port *digport, bool long_hpd);
 void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector);
 void intel_dp_mst_suspend(struct drm_device *dev);
-- 
2.0.1

[RFC 4/6] drm/i915: DRRS calls based on frontbuffer

[Vandana Kannan]

Calls to switch between high and low refresh rates based on calls made to
fb_invalidate and fb_flush.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  5 ++++
 drivers/gpu/drm/i915/intel_dp.c      | 51 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  3 +++
 3 files changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6d6214a..3bf1732 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9146,6 +9146,11 @@ static void intel_mark_fb_busy(struct drm_device *dev,
 		intel_increase_pllclock(dev, pipe);
 		if (ring && intel_fbc_enabled(dev))
 			ring->fbc_dirty = true;
+
+		if (ring)
+			intel_edp_drrs_invalidate(dev, frontbuffer_bits);
+		else
+			intel_edp_drrs_flush(dev, frontbuffer_bits);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d50249e..bd0415b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4949,6 +4949,57 @@ unlock:
 	mutex_unlock(&dev_priv->drrs.mutex);
 }
 
+void intel_edp_drrs_invalidate(struct drm_device *dev,
+			      unsigned frontbuffer_bits)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	enum pipe pipe;
+
+	mutex_lock(&dev_priv->drrs.mutex);
+	if (!dev_priv->drrs.dp) {
+		mutex_unlock(&dev_priv->drrs.mutex);
+		return;
+	}
+
+	crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;
+	pipe = to_intel_crtc(crtc)->pipe;
+
+	if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR)
+		intel_dp_set_drrs_state(dev_priv->dev,
+			dev_priv->drrs.dp->attached_connector->panel.
+			fixed_mode->vrefresh);
+
+	frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
+
+	dev_priv->drrs.busy_frontbuffer_bits |= frontbuffer_bits;
+	mutex_unlock(&dev_priv->drrs.mutex);
+}
+
+void intel_edp_drrs_flush(struct drm_device *dev,
+			 unsigned frontbuffer_bits)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	enum pipe pipe;
+
+	mutex_lock(&dev_priv->drrs.mutex);
+	if (!dev_priv->drrs.dp) {
+		mutex_unlock(&dev_priv->drrs.mutex);
+		return;
+	}
+
+	crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;
+	pipe = to_intel_crtc(crtc)->pipe;
+	dev_priv->drrs.busy_frontbuffer_bits &= ~frontbuffer_bits;
+
+	if (dev_priv->drrs.refresh_rate_type != DRRS_LOW_RR &&
+		!dev_priv->psr.busy_frontbuffer_bits)
+			schedule_delayed_work(&dev_priv->drrs.work,
+				      msecs_to_jiffies(100));
+	mutex_unlock(&dev_priv->drrs.mutex);
+}
+
 static struct drm_display_mode *
 intel_dp_drrs_init(struct intel_connector *intel_connector,
 		   struct drm_display_mode *fixed_mode)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a728cc8..380bfe4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -929,6 +929,9 @@ void intel_edp_psr_init(struct drm_device *dev);
 
 void intel_edp_drrs_enable(struct intel_dp *intel_dp);
 void intel_edp_drrs_disable(struct intel_dp *intel_dp);
+void intel_edp_drrs_invalidate(struct drm_device *dev,
+				unsigned frontbuffer_bits);
+void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
 
 int intel_dp_handle_hpd_irq(struct intel_digital_port *digport, bool long_hpd);
 void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector);
-- 
2.0.1

Re: [RFC 4/6] drm/i915: DRRS calls based on frontbuffer

[Daniel Vetter]

On Fri, Sep 12, 2014 at 11:13:07PM +0530, Vandana Kannan wrote:
> Calls to switch between high and low refresh rates based on calls made to
> fb_invalidate and fb_flush.
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>

A few comments below.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |  5 ++++
>  drivers/gpu/drm/i915/intel_dp.c      | 51 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  3 +++
>  3 files changed, 59 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6d6214a..3bf1732 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9146,6 +9146,11 @@ static void intel_mark_fb_busy(struct drm_device *dev,
>  		intel_increase_pllclock(dev, pipe);
>  		if (ring && intel_fbc_enabled(dev))
>  			ring->fbc_dirty = true;
> +
> +		if (ring)
> +			intel_edp_drrs_invalidate(dev, frontbuffer_bits);
> +		else
> +			intel_edp_drrs_flush(dev, frontbuffer_bits);

I think the integration here isn't quite right. I've written a small patch
to add a bit more documentation about how the frontbuffer tracking should
work. See

http://people.freedesktop.org/~danvet/drm/drmI915.html#idp72543008

So I think we need two cases here:
- Mark DRRS as busy with intel_edp_drrs_busy. You can place it into
  intel_mark_fb_busy since that will be called for all 3 events
  (invalidate, flush, flip) as needed. But the place right now is the
  wrong spot since it's in the pipe loop so means you're function gets
  called 3 times. And with the below item I think it's better to move this
  call out of fb_mark_busy anyway.
- Rearm the idle timer. This should _only_ be done for flush&flip, with a
  delay (100ms like in your patch sounds good) and _after_ you've cranked
  up the refresh rate to high. So checking for "ring" here isn't the right
  thing, we instead need to add an additional call at the end of
  intel_frontbuffer_flush. Or just add an "arm-idle-work" boolean to the
  _busy function and call it from intel_fb_obj_invalidate with false and
  intel_frontbuffer_flush with true.

>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d50249e..bd0415b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4949,6 +4949,57 @@ unlock:
>  	mutex_unlock(&dev_priv->drrs.mutex);
>  }
>  
> +void intel_edp_drrs_invalidate(struct drm_device *dev,
> +			      unsigned frontbuffer_bits)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
> +	enum pipe pipe;
> +
> +	mutex_lock(&dev_priv->drrs.mutex);
> +	if (!dev_priv->drrs.dp) {
> +		mutex_unlock(&dev_priv->drrs.mutex);
> +		return;
> +	}
> +
> +	crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;

Dereferencing this pointer chain is a bit risk since this function gets
called without modeset locks held. It should work due to ordering, but I'd
prefer if we compute the DRRS pipe in drrs_enable and store it in
dev_prive->drrs.pipe.

> +	pipe = to_intel_crtc(crtc)->pipe;
> +
> +	if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR)
> +		intel_dp_set_drrs_state(dev_priv->dev,
> +			dev_priv->drrs.dp->attached_connector->panel.
> +			fixed_mode->vrefresh);
> +
> +	frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
> +
> +	dev_priv->drrs.busy_frontbuffer_bits |= frontbuffer_bits;
> +	mutex_unlock(&dev_priv->drrs.mutex);
> +}
> +
> +void intel_edp_drrs_flush(struct drm_device *dev,
> +			 unsigned frontbuffer_bits)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
> +	enum pipe pipe;
> +
> +	mutex_lock(&dev_priv->drrs.mutex);
> +	if (!dev_priv->drrs.dp) {
> +		mutex_unlock(&dev_priv->drrs.mutex);
> +		return;
> +	}
> +
> +	crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;
> +	pipe = to_intel_crtc(crtc)->pipe;
> +	dev_priv->drrs.busy_frontbuffer_bits &= ~frontbuffer_bits;
> +
> +	if (dev_priv->drrs.refresh_rate_type != DRRS_LOW_RR &&
> +		!dev_priv->psr.busy_frontbuffer_bits)
> +			schedule_delayed_work(&dev_priv->drrs.work,
> +				      msecs_to_jiffies(100));
> +	mutex_unlock(&dev_priv->drrs.mutex);
> +}
> +
>  static struct drm_display_mode *
>  intel_dp_drrs_init(struct intel_connector *intel_connector,
>  		   struct drm_display_mode *fixed_mode)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a728cc8..380bfe4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -929,6 +929,9 @@ void intel_edp_psr_init(struct drm_device *dev);
>  
>  void intel_edp_drrs_enable(struct intel_dp *intel_dp);
>  void intel_edp_drrs_disable(struct intel_dp *intel_dp);
> +void intel_edp_drrs_invalidate(struct drm_device *dev,
> +				unsigned frontbuffer_bits);
> +void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
>  
>  int intel_dp_handle_hpd_irq(struct intel_digital_port *digport, bool long_hpd);
>  void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector);
> -- 
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[RFC 5/6] drm/i915/bdw: Add support for DRRS to switch RR

[Vandana Kannan]

For Broadwell, there is one instance of Transcoder MN values per transcoder.
For dynamic switching between multiple refreshr rates, M/N values may be
reprogrammed on the fly. Link N programming triggers update of all data and
link M & N registers and the new M/N values will be used in the next frame
that is output.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  9 +++------
 drivers/gpu/drm/i915/intel_dp.c      | 15 ++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  3 +++
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3bf1732..db43022 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -93,9 +93,6 @@ static int intel_framebuffer_init(struct drm_device *dev,
 				  struct drm_i915_gem_object *obj);
 static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc);
 static void intel_set_pipe_timings(struct intel_crtc *intel_crtc);
-static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
-					 struct intel_link_m_n *m_n,
-					 struct intel_link_m_n *m2_n2);
 static void ironlake_set_pipeconf(struct drm_crtc *crtc);
 static void haswell_set_pipeconf(struct drm_crtc *crtc);
 static void intel_set_pipe_csc(struct drm_crtc *crtc);
@@ -5643,9 +5640,9 @@ static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
 	I915_WRITE(PCH_TRANS_LINK_N1(pipe), m_n->link_n);
 }
 
-static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
-					 struct intel_link_m_n *m_n,
-					 struct intel_link_m_n *m2_n2)
+void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+				 struct intel_link_m_n *m_n,
+				 struct intel_link_m_n *m2_n2)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bd0415b..a46279f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4856,7 +4856,20 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 		return;
 	}
 
-	if (INTEL_INFO(dev)->gen > 6 && INTEL_INFO(dev)->gen < 8) {
+	if (INTEL_INFO(dev)->gen >= 8) {
+		switch(index) {
+		case DRRS_HIGH_RR:
+			intel_dp_set_m_n(intel_crtc);
+			break;
+		case DRRS_LOW_RR:
+			intel_cpu_transcoder_set_m_n(intel_crtc,
+					&intel_crtc->config.dp_m2_n2, NULL);
+			break;
+		case DRRS_MAX_RR:
+		default:
+			break;
+		}
+	} else if (INTEL_INFO(dev)->gen > 6) {
 		reg = PIPECONF(intel_crtc->config.cpu_transcoder);
 		val = I915_READ(reg);
 		if (index > DRRS_HIGH_RR) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 380bfe4..a605273 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -881,6 +881,9 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv);
 void intel_dp_get_m_n(struct intel_crtc *crtc,
 		      struct intel_crtc_config *pipe_config);
 void intel_dp_set_m_n(struct intel_crtc *crtc);
+void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
+				 struct intel_link_m_n *m_n,
+				 struct intel_link_m_n *m2_n2);
 int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);
 void
 ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
-- 
2.0.1

[RFC 6/6] drm/i915: Support for RR switching on VLV

[Vandana Kannan]

Definition of VLV RR switch bit and corresponding toggling in
set_drrs function.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  1 +
 drivers/gpu/drm/i915/intel_dp.c | 10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 15c0eaa..a8da1c5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3721,6 +3721,7 @@ enum punit_power_well {
 #define   PIPECONF_INTERLACE_MODE_MASK		(7 << 21)
 #define   PIPECONF_EDP_RR_MODE_SWITCH		(1 << 20)
 #define   PIPECONF_CXSR_DOWNCLOCK	(1<<16)
+#define   PIPECONF_EDP_RR_MODE_SWITCH_VLV	(1 << 14)
 #define   PIPECONF_COLOR_RANGE_SELECT	(1 << 13)
 #define   PIPECONF_BPC_MASK	(0x7 << 5)
 #define   PIPECONF_8BPC		(0<<5)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a46279f..a6dee2e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4873,10 +4873,16 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 		reg = PIPECONF(intel_crtc->config.cpu_transcoder);
 		val = I915_READ(reg);
 		if (index > DRRS_HIGH_RR) {
-			val |= PIPECONF_EDP_RR_MODE_SWITCH;
+			if (IS_VALLEYVIEW(dev))
+				val |= PIPECONF_EDP_RR_MODE_SWITCH_VLV;
+			else
+				val |= PIPECONF_EDP_RR_MODE_SWITCH;
 			intel_dp_set_m_n(intel_crtc);
 		} else {
-			val &= ~PIPECONF_EDP_RR_MODE_SWITCH;
+			if (IS_VALLEYVIEW(dev))
+				val &= ~PIPECONF_EDP_RR_MODE_SWITCH_VLV;
+			else
+				val &= ~PIPECONF_EDP_RR_MODE_SWITCH;
 		}
 		I915_WRITE(reg, val);
 	}
-- 
2.0.1

Re: [RFC 0/6] eDP DRRS based on frontbuffer tracking

[Daniel Vetter]

On Fri, Sep 12, 2014 at 11:13:03PM +0530, Vandana Kannan wrote:
> As part of implementing DRRS based on frontbuffer tracking, this patch series
> includes some modifications related to the data for DRRS, code to enable/
> disable DRRS during init or uninit, calls to switch between high and low
> refresh rates based on calls to mark_fb_busy.
> Instead of having a module param to specify delay before scheduling DRRS
> work, (as there was in the old patch series), a delay of 100ms is used.
> Additional patches including changes specific to bdw and vlv have been
> included.

Sorry for the delay in answering, been a bit busy. Looks good overall,
just spotted a few issue with the integration into the frontbuffer
tracking. That's probably just because the documentation we have is awful,
so I've improved that and replied with what I think we need.

It unfortunately means that you need to rebase the integration patch since
the frontbuffer functions moved. But since that needs to be redone anyway
I hope that's not too much fuzz.

And if you think the documentation is still lacking please raise this - we
need to get much better at documenting i915 internals and that will only
work if people critize and improve what's there.

> TODO:-
> Need more analysis into clone mode scenario

Hm, I don't really understand what the issue is here? Currently i915
doesn't support hw cloning for (e)DP ports, so you always have a 1:1 link
between eDP panel and pipe. And if there's more than one pipe scanning out
the same frontbuffer gem object then the frontbuffer tracking keeps them
separate since it tracks frontbuffer attachments per-pipe.

So can you please explain what you thing might be an issue? Very well
possible that I'm just blind ;-)

Thanks, Daniel


> 
> Vandana Kannan (6):
>   drm/i915: Modifying structures related to DRRS
>   drm/i915: Initialize DRRS delayed work
>   drm/i915: Enable/disable DRRS
>   drm/i915: DRRS calls based on frontbuffer
>   drm/i915/bdw: Add support for DRRS to switch RR
>   drm/i915: Support for RR switching on VLV
> 
>  drivers/gpu/drm/i915/i915_drv.h      |  32 ++++--
>  drivers/gpu/drm/i915/i915_reg.h      |   1 +
>  drivers/gpu/drm/i915/intel_ddi.c     |   2 +
>  drivers/gpu/drm/i915/intel_display.c |  14 +--
>  drivers/gpu/drm/i915/intel_dp.c      | 201 +++++++++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h     |  27 ++---
>  6 files changed, 211 insertions(+), 66 deletions(-)
> 
> -- 
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC 0/6] eDP DRRS based on frontbuffer tracking

[Kannan, Vandana]

On 23-Sep-14 2:31 PM, Daniel Vetter wrote:
> On Fri, Sep 12, 2014 at 11:13:03PM +0530, Vandana Kannan wrote:
>> As part of implementing DRRS based on frontbuffer tracking, this patch series
>> includes some modifications related to the data for DRRS, code to enable/
>> disable DRRS during init or uninit, calls to switch between high and low
>> refresh rates based on calls to mark_fb_busy.
>> Instead of having a module param to specify delay before scheduling DRRS
>> work, (as there was in the old patch series), a delay of 100ms is used.
>> Additional patches including changes specific to bdw and vlv have been
>> included.
>
> Sorry for the delay in answering, been a bit busy. Looks good overall,
> just spotted a few issue with the integration into the frontbuffer
> tracking. That's probably just because the documentation we have is awful,
> so I've improved that and replied with what I think we need.
>
Hi Daniel,

Sorry for the delay on this. I've been working on and off on it.
I went through the comment added in intel_frontbuffer.c, and just wanted 
to confirm if my understanding is correct.

In intel_frontbuffer.c, we have the comment
"
  * The other type of display power saving feature only cares about busyness
  * (e.g. DRRS). In that case all three (invalidate, flush and flip) 
indicate
  * busyness. There is no direct way to detect idleness.
"
Here, flip refers to intel_crtc_page_flip?
Since a flip also indicates busyness, it would mean that a switch back 
to high RR (invalidate DRRS) would be required at this point ? (apart 
from the GEM tracking).

Please let me know.
Thanks,
Vandana

> It unfortunately means that you need to rebase the integration patch since
> the frontbuffer functions moved. But since that needs to be redone anyway
> I hope that's not too much fuzz.
>
> And if you think the documentation is still lacking please raise this - we
> need to get much better at documenting i915 internals and that will only
> work if people critize and improve what's there.
>
>> TODO:-
>> Need more analysis into clone mode scenario
>
> Hm, I don't really understand what the issue is here? Currently i915
> doesn't support hw cloning for (e)DP ports, so you always have a 1:1 link
> between eDP panel and pipe. And if there's more than one pipe scanning out
> the same frontbuffer gem object then the frontbuffer tracking keeps them
> separate since it tracks frontbuffer attachments per-pipe.
>
> So can you please explain what you thing might be an issue? Very well
> possible that I'm just blind ;-)
>
> Thanks, Daniel
>
>
>>
>> Vandana Kannan (6):
>>    drm/i915: Modifying structures related to DRRS
>>    drm/i915: Initialize DRRS delayed work
>>    drm/i915: Enable/disable DRRS
>>    drm/i915: DRRS calls based on frontbuffer
>>    drm/i915/bdw: Add support for DRRS to switch RR
>>    drm/i915: Support for RR switching on VLV
>>
>>   drivers/gpu/drm/i915/i915_drv.h      |  32 ++++--
>>   drivers/gpu/drm/i915/i915_reg.h      |   1 +
>>   drivers/gpu/drm/i915/intel_ddi.c     |   2 +
>>   drivers/gpu/drm/i915/intel_display.c |  14 +--
>>   drivers/gpu/drm/i915/intel_dp.c      | 201 +++++++++++++++++++++++++++++------
>>   drivers/gpu/drm/i915/intel_drv.h     |  27 ++---
>>   6 files changed, 211 insertions(+), 66 deletions(-)
>>
>> --
>> 2.0.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/6] eDP DRRS based on frontbuffer tracking

[Daniel Vetter]

On Fri, Nov 28, 2014 at 04:14:18PM +0530, Kannan, Vandana wrote:
> 
> 
> On 23-Sep-14 2:31 PM, Daniel Vetter wrote:
> >On Fri, Sep 12, 2014 at 11:13:03PM +0530, Vandana Kannan wrote:
> >>As part of implementing DRRS based on frontbuffer tracking, this patch series
> >>includes some modifications related to the data for DRRS, code to enable/
> >>disable DRRS during init or uninit, calls to switch between high and low
> >>refresh rates based on calls to mark_fb_busy.
> >>Instead of having a module param to specify delay before scheduling DRRS
> >>work, (as there was in the old patch series), a delay of 100ms is used.
> >>Additional patches including changes specific to bdw and vlv have been
> >>included.
> >
> >Sorry for the delay in answering, been a bit busy. Looks good overall,
> >just spotted a few issue with the integration into the frontbuffer
> >tracking. That's probably just because the documentation we have is awful,
> >so I've improved that and replied with what I think we need.
> >
> Hi Daniel,
> 
> Sorry for the delay on this. I've been working on and off on it.
> I went through the comment added in intel_frontbuffer.c, and just wanted to
> confirm if my understanding is correct.
> 
> In intel_frontbuffer.c, we have the comment
> "
>  * The other type of display power saving feature only cares about busyness
>  * (e.g. DRRS). In that case all three (invalidate, flush and flip) indicate
>  * busyness. There is no direct way to detect idleness.
> "
> Here, flip refers to intel_crtc_page_flip?
> Since a flip also indicates busyness, it would mean that a switch back to
> high RR (invalidate DRRS) would be required at this point ? (apart from the
> GEM tracking).
> 
> Please let me know.

Yes - the comment says that all 3 indicate busyness for stuff like DRRS.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx