[PATCH 0/5] Enabling DRRS support in the kernel

[PATCH 0/5] Enabling DRRS support in the kernel

[Vandana Kannan]

Dynamic Refresh Rate Switching (DRRS) is a power conservation feature which     
enables switching between low and high refresh rates based on the usage         
scenario. This feature is applicable for internal eDP panel. Indication that    
the panel can support DRRS is given by the panel EDID, which would list         
multiple refresh rates for one resolution.                                      
The patch series supports idleness detection in display i915 driver and switch  
to low refresh rate.

Pradeep Bhat (3):
  [Intel-gfx] drm/i915: Adding VBT fields to support eDP DRRS feature
  [Intel-gfx] drm/i915: Parse EDID probed modes for DRRS support
  [Intel-gfx] drm/i915: Add support for DRRS to switch RR

Vandana Kannan (2):
  [Intel-gfx] drm/i915: Idleness detection for DRRS
  [Intel-gfx] drm/i915/bdw: Add support for DRRS to switch RR

 drivers/gpu/drm/i915/i915_drv.h      |   30 ++++++
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_bios.c    |   23 +++++
 drivers/gpu/drm/i915/intel_bios.h    |   29 ++++++
 drivers/gpu/drm/i915/intel_display.c |   13 +++
 drivers/gpu/drm/i915/intel_dp.c      |  180 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |   35 ++++++-
 drivers/gpu/drm/i915/intel_pm.c      |  112 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_sprite.c  |    3 +
 9 files changed, 425 insertions(+), 1 deletion(-)

-- 
1.7.9.5

[PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature

[Vandana Kannan]

From: Pradeep Bhat <pradeep.bhat@intel.com>

This patch reads the DRRS support and Mode type from VBT fields.
The read information will be stored in VBT struct during BIOS
parsing. The above functionality is needed for decision making
whether DRRS feature is supported in i915 driver for eDP panels.
This information helps us decide if seamless DRRS can be done
at runtime to support certain power saving features. This patch
was tested by setting necessary bit in VBT struct and merging
the new VBT with system BIOS so that we can read the value.

Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |    9 +++++++++
 drivers/gpu/drm/i915/intel_bios.c |   23 +++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_bios.h |   29 +++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 64ed8f4..02e11dc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1175,6 +1175,15 @@ struct intel_vbt_data {
 	int lvds_ssc_freq;
 	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
 
+	/**
+	 * DRRS mode type (Seamless OR Static DRRS)
+	 * drrs_mode Val 0x2 is Seamless DRRS and 0 is Static DRRS.
+	 * These values correspond to the VBT values for drrs mode.
+	 */
+	int drrs_mode;
+	/* DRRS enabled or disabled in VBT */
+	bool intel_drrs_enabled;
+
 	/* eDP */
 	int edp_rate;
 	int edp_lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 6dd622d..15ee0b8 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -195,6 +195,21 @@ get_lvds_fp_timing(const struct bdb_header *bdb,
 	return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
 }
 
+/**
+ * This function returns the 2 bit information pertaining to a panel type
+ * present in a 32 bit field in VBT blocks. There are 16 panel types in VBT
+ * each occupying 2 bits of information in some 32 bit fields of VBT blocks.
+ */
+static int
+get_mode_by_paneltype(unsigned int word)
+{
+	/**
+	 * The caller of this API should interpret the 2 bits
+	 * based on VBT description for that field.
+	 */
+	return (word >> ((panel_type - 1) * 2)) & MODE_MASK;
+}
+
 /* Try to find integrated panel data */
 static void
 parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -218,6 +233,11 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
 
 	panel_type = lvds_options->panel_type;
 
+	dev_priv->vbt.drrs_mode =
+		get_mode_by_paneltype(lvds_options->dps_panel_type_bits);
+	DRM_DEBUG_KMS("DRRS supported mode is : %s\n",
+			(dev_priv->vbt.drrs_mode == 0) ? "STATIC" : "SEAMLESS");
+
 	lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
 	if (!lvds_lfp_data)
 		return;
@@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
 
 	if (driver->dual_frequency)
 		dev_priv->render_reclock_avail = true;
+
+	dev_priv->vbt.intel_drrs_enabled = driver->intel_drrs_state;
+	DRM_DEBUG_KMS("DRRS State Enabled : %d\n", driver->intel_drrs_state);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index f580a2b..da6685b 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -202,6 +202,9 @@ struct bdb_general_features {
 #define DEVICE_PORT_DVOB	0x01
 #define DEVICE_PORT_DVOC	0x02
 
+/* Mask for DRRS / Panel Channel / SSC / BLT control bits extraction */
+#define MODE_MASK		0x3
+
 /* We used to keep this struct but without any version control. We should avoid
  * using it in the future, but it should be safe to keep using it in the old
  * code. */
@@ -293,6 +296,18 @@ struct bdb_lvds_options {
 	u8 lvds_edid:1;
 	u8 rsvd2:1;
 	u8 rsvd4;
+	/* LVDS Panel channel bits stored here */
+	u32 lvds_panel_channel_bits;
+	/* LVDS SSC (Spread Spectrum Clock) bits stored here. */
+	u16 ssc_bits;
+	u16 ssc_freq;
+	u16 ssc_ddt;
+	/* Panel color depth defined here */
+	u16 panel_color_depth;
+	/* LVDS panel type bits stored here */
+	u32 dps_panel_type_bits;
+	/* LVDS backlight control type bits stored here */
+	u32 blt_control_type_bits;
 } __attribute__((packed));
 
 /* LFP pointer table contains entries to the struct below */
@@ -462,6 +477,20 @@ struct bdb_driver_features {
 
 	u8 hdmi_termination;
 	u8 custom_vbt_version;
+	/* Driver features data block */
+	u16 intel_rmpm_state:1;
+	u16 intel_s2ddt_state:1;
+	u16 intel_dpst_state:1;
+	u16 intel_bltclt_state:1;
+	u16 intel_adb_state:1;
+	u16 intel_drrs_state:1;
+	u16 intel_grs_state:1;
+	u16 intel_gpmt_state:1;
+	u16 intel_tbt_state:1;
+	u16 psr_state:1;
+	u16 ips_state:1;
+	u16 reserved3:4;
+	u16 pc_feature_validity:1;
 } __attribute__((packed));
 
 #define EDP_18BPP	0
-- 
1.7.9.5

Re: [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature

[Chris Wilson]

On Tue, Dec 17, 2013 at 10:58:23AM +0530, Vandana Kannan wrote:
> From: Pradeep Bhat <pradeep.bhat@intel.com>
> 
> This patch reads the DRRS support and Mode type from VBT fields.
> The read information will be stored in VBT struct during BIOS
> parsing. The above functionality is needed for decision making
> whether DRRS feature is supported in i915 driver for eDP panels.
> This information helps us decide if seamless DRRS can be done
> at runtime to support certain power saving features. This patch
> was tested by setting necessary bit in VBT struct and merging
> the new VBT with system BIOS so that we can read the value.

What's the reason for the inconsistent intel_ tautology?

> @@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
>  
>  	if (driver->dual_frequency)
>  		dev_priv->render_reclock_avail = true;
> +
> +	dev_priv->vbt.intel_drrs_enabled = driver->intel_drrs_state;
> +	DRM_DEBUG_KMS("DRRS State Enabled : %d\n", driver->intel_drrs_state);

Now this oddity needs a big explanation. Writing that will hopefully
reveal a better strategy.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature

[Vandana Kannan]

On Dec-17-2013 5:56 PM, Chris Wilson wrote:
> On Tue, Dec 17, 2013 at 10:58:23AM +0530, Vandana Kannan wrote:
>> From: Pradeep Bhat <pradeep.bhat@intel.com>
>>
>> This patch reads the DRRS support and Mode type from VBT fields.
>> The read information will be stored in VBT struct during BIOS
>> parsing. The above functionality is needed for decision making
>> whether DRRS feature is supported in i915 driver for eDP panels.
>> This information helps us decide if seamless DRRS can be done
>> at runtime to support certain power saving features. This patch
>> was tested by setting necessary bit in VBT struct and merging
>> the new VBT with system BIOS so that we can read the value.
> 
> What's the reason for the inconsistent intel_ tautology?
> 
If you are referring to the names of members under bdb_driver_features,
which start with intel_, then this is something which can be modified to
remove the "intel_". Is it ok?

>> @@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
>>  
>>  	if (driver->dual_frequency)
>>  		dev_priv->render_reclock_avail = true;
>> +
>> +	dev_priv->vbt.intel_drrs_enabled = driver->intel_drrs_state;
>> +	DRM_DEBUG_KMS("DRRS State Enabled : %d\n", driver->intel_drrs_state);
> 
> Now this oddity needs a big explanation. Writing that will hopefully
> reveal a better strategy.
> -Chris
> 
Panel vendors specify panel capabilities via the VBT. Following this,
the panel's capability to support seamless DRRS has to be read from the
VBT. The same is being done in the piece of code above. Without this we
cannot assume that the panel supports seamless DRRS.

Re: [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature

[Chris Wilson]

On Wed, Dec 18, 2013 at 01:38:44PM +0530, Vandana Kannan wrote:
> On Dec-17-2013 5:56 PM, Chris Wilson wrote:
> > On Tue, Dec 17, 2013 at 10:58:23AM +0530, Vandana Kannan wrote:
> >> From: Pradeep Bhat <pradeep.bhat@intel.com>
> >>
> >> This patch reads the DRRS support and Mode type from VBT fields.
> >> The read information will be stored in VBT struct during BIOS
> >> parsing. The above functionality is needed for decision making
> >> whether DRRS feature is supported in i915 driver for eDP panels.
> >> This information helps us decide if seamless DRRS can be done
> >> at runtime to support certain power saving features. This patch
> >> was tested by setting necessary bit in VBT struct and merging
> >> the new VBT with system BIOS so that we can read the value.
> > 
> > What's the reason for the inconsistent intel_ tautology?
> > 
> If you are referring to the names of members under bdb_driver_features,
> which start with intel_, then this is something which can be modified to
> remove the "intel_". Is it ok?

Yes, we use intel_ as a function prefix for generic routines that apply
to almost all display engines. We expect that all of our hardware specific
structure are used for Intel and so don't need reminding, least
of all, inconsistently.

And s/drrs_state/drrs/.
 
> >> @@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
> >>  
> >>  	if (driver->dual_frequency)
> >>  		dev_priv->render_reclock_avail = true;
> >> +
> >> +	dev_priv->vbt.intel_drrs_enabled = driver->intel_drrs_state;
> >> +	DRM_DEBUG_KMS("DRRS State Enabled : %d\n", driver->intel_drrs_state);
> > 
> > Now this oddity needs a big explanation. Writing that will hopefully
> > reveal a better strategy.
> > -Chris
> > 
> Panel vendors specify panel capabilities via the VBT. Following this,
> the panel's capability to support seamless DRRS has to be read from the
> VBT. The same is being done in the piece of code above. Without this we
> cannot assume that the panel supports seamless DRRS.

Nevermind, I misread driver-> as dev_priv->.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature

[Vandana Kannan]

On Dec-18-2013 2:41 PM, Chris Wilson wrote:
> On Wed, Dec 18, 2013 at 01:38:44PM +0530, Vandana Kannan wrote:
>> On Dec-17-2013 5:56 PM, Chris Wilson wrote:
>>> On Tue, Dec 17, 2013 at 10:58:23AM +0530, Vandana Kannan wrote:
>>>> From: Pradeep Bhat <pradeep.bhat@intel.com>
>>>>
>>>> This patch reads the DRRS support and Mode type from VBT fields.
>>>> The read information will be stored in VBT struct during BIOS
>>>> parsing. The above functionality is needed for decision making
>>>> whether DRRS feature is supported in i915 driver for eDP panels.
>>>> This information helps us decide if seamless DRRS can be done
>>>> at runtime to support certain power saving features. This patch
>>>> was tested by setting necessary bit in VBT struct and merging
>>>> the new VBT with system BIOS so that we can read the value.
>>>
>>> What's the reason for the inconsistent intel_ tautology?
>>>
>> If you are referring to the names of members under bdb_driver_features,
>> which start with intel_, then this is something which can be modified to
>> remove the "intel_". Is it ok?
> 
> Yes, we use intel_ as a function prefix for generic routines that apply
> to almost all display engines. We expect that all of our hardware specific
> structure are used for Intel and so don't need reminding, least
> of all, inconsistently.
> 
> And s/drrs_state/drrs/.
>  
I will make these changes.
- Vandana
>>>> @@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
>>>>  
>>>>  	if (driver->dual_frequency)
>>>>  		dev_priv->render_reclock_avail = true;
>>>> +
>>>> +	dev_priv->vbt.intel_drrs_enabled = driver->intel_drrs_state;
>>>> +	DRM_DEBUG_KMS("DRRS State Enabled : %d\n", driver->intel_drrs_state);
>>>
>>> Now this oddity needs a big explanation. Writing that will hopefully
>>> reveal a better strategy.
>>> -Chris
>>>
>> Panel vendors specify panel capabilities via the VBT. Following this,
>> the panel's capability to support seamless DRRS has to be read from the
>> VBT. The same is being done in the piece of code above. Without this we
>> cannot assume that the panel supports seamless DRRS.
> 
> Nevermind, I misread driver-> as dev_priv->.
> -Chris
> 

[PATCH 2/5] drm/i915: Parse EDID probed modes for DRRS support

[Vandana Kannan]

From: Pradeep Bhat <pradeep.bhat@intel.com>

This patch and finds out the lowest refresh rate supported for the resolution
same as the fixed_mode, based on the implementaion find_panel_downclock.
It also checks the VBT fields to see if panel supports seamless DRRS or not.
Based on above data it marks whether eDP panel supports seamless DRRS or not.
This information is needed for supporting seamless DRRS switch for
certain power saving usecases. This patch is tested by enabling the DRM logs
and user should see whether Seamless DRRS is supported or not.

Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |    2 ++
 drivers/gpu/drm/i915/intel_dp.c  |   47 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |   29 +++++++++++++++++++++++
 3 files changed, 78 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 02e11dc..c9bca16 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1462,8 +1462,10 @@ typedef struct drm_i915_private {
 	/* Reclocking support */
 	bool render_reclock_avail;
 	bool lvds_downclock_avail;
+	bool edp_downclock_avail;
 	/* indicates the reduced downclock for LVDS*/
 	int lvds_downclock;
+	int edp_downclock;
 	u16 orig_clock;
 
 	bool mchbar_need_disable;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 82de200..935b46e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3524,6 +3524,48 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 		      I915_READ(pp_div_reg));
 }
 
+static void
+intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
+			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_i915_private *dev_priv = dev->dev_private;
+
+	/**
+	 * Check if PSR is supported by panel and enabled
+	 * if so then DRRS is reported as not supported for Haswell.
+	 */
+	if (INTEL_INFO(dev)->gen < 8 &&	intel_edp_is_psr_enabled(dev)) {
+		DRM_INFO("eDP panel has PSR enabled. Cannot support DRRS\n");
+		return;
+	}
+
+	/* First check if DRRS is enabled from VBT struct */
+	if (!dev_priv->vbt.intel_drrs_enabled) {
+		DRM_INFO("VBT doesn't support DRRS\n");
+		return;
+	}
+
+	intel_connector->panel.downclock_mode =	intel_find_panel_downclock(dev,
+					fixed_mode, connector);
+
+	if (intel_connector->panel.downclock_mode != NULL &&
+		dev_priv->vbt.drrs_mode == SEAMLESS_DRRS_SUPPORT) {
+		dev_priv->edp_downclock_avail = true;
+		dev_priv->edp_downclock =
+			intel_connector->panel.downclock_mode->clock;
+
+		intel_dp->drrs_state.is_drrs_supported
+				= dev_priv->vbt.drrs_mode;
+
+		intel_dp->drrs_state.drrs_refresh_rate_type = DRRS_HIGH_RR;
+		DRM_INFO("SEAMLESS DRRS supported for eDP panel.\n");
+	}
+}
+
 static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 				     struct intel_connector *intel_connector)
 {
@@ -3537,6 +3579,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	struct drm_display_mode *scan;
 	struct edid *edid;
 
+	intel_dp->drrs_state.is_drrs_supported = DRRS_NOT_SUPPORTED;
+
 	if (!is_edp(intel_dp))
 		return true;
 
@@ -3581,6 +3625,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	list_for_each_entry(scan, &connector->probed_modes, head) {
 		if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
 			fixed_mode = drm_mode_duplicate(dev, scan);
+			if (INTEL_INFO(dev)->gen >= 5)
+				intel_dp_drrs_initialize(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 9f8b465..6ec5f4e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -462,6 +462,34 @@ struct intel_hdmi {
 
 #define DP_MAX_DOWNSTREAM_PORTS		0x10
 
+/**
+ * This enum is used to indicate the DRRS support type.
+ * The values of the enum map 1-to-1 with the values from VBT.
+ */
+enum edp_panel_type {
+	DRRS_NOT_SUPPORTED = -1,
+	STATIC_DRRS_SUPPORT = 0,
+	SEAMLESS_DRRS_SUPPORT = 2
+};
+/**
+ * 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 */
+};
+/**
+ * The drrs_info struct will represent the DRRS feature for eDP
+ * panel.
+ */
+struct drrs_info {
+	int is_drrs_supported;
+	int drrs_refresh_rate_type;
+};
+
 struct intel_dp {
 	uint32_t output_reg;
 	uint32_t aux_ch_ctl_reg;
@@ -487,6 +515,7 @@ struct intel_dp {
 	bool want_panel_vdd;
 	bool psr_setup_done;
 	struct intel_connector *attached_connector;
+	struct drrs_info drrs_state;
 };
 
 struct intel_digital_port {
-- 
1.7.9.5

Re: [PATCH 2/5] drm/i915: Parse EDID probed modes for DRRS support

[Chris Wilson]

On Tue, Dec 17, 2013 at 10:58:24AM +0530, Vandana Kannan wrote:
> From: Pradeep Bhat <pradeep.bhat@intel.com>
> 
> This patch and finds out the lowest refresh rate supported for the resolution
> same as the fixed_mode, based on the implementaion find_panel_downclock.
> It also checks the VBT fields to see if panel supports seamless DRRS or not.
> Based on above data it marks whether eDP panel supports seamless DRRS or not.
> This information is needed for supporting seamless DRRS switch for
> certain power saving usecases. This patch is tested by enabling the DRM logs
> and user should see whether Seamless DRRS is supported or not.
> 
> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |    2 ++
>  drivers/gpu/drm/i915/intel_dp.c  |   47 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |   29 +++++++++++++++++++++++
>  3 files changed, 78 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 02e11dc..c9bca16 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1462,8 +1462,10 @@ typedef struct drm_i915_private {
>  	/* Reclocking support */
>  	bool render_reclock_avail;
>  	bool lvds_downclock_avail;
> +	bool edp_downclock_avail;
>  	/* indicates the reduced downclock for LVDS*/
>  	int lvds_downclock;
> +	int edp_downclock;
>  	u16 orig_clock;

Do any machines have both edp and lvds? Shouldn't this be a part of the
panel state?

>  
> +/**
> + * This enum is used to indicate the DRRS support type.
> + * The values of the enum map 1-to-1 with the values from VBT.
> + */
> +enum edp_panel_type {
> +	DRRS_NOT_SUPPORTED = -1,
> +	STATIC_DRRS_SUPPORT = 0,
> +	SEAMLESS_DRRS_SUPPORT = 2
> +};
> +/**
> + * 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 */
> +};
> +/**
> + * The drrs_info struct will represent the DRRS feature for eDP
> + * panel.
> + */
> +struct drrs_info {
> +	int is_drrs_supported;
> +	int drrs_refresh_rate_type;

So what was the point of the enums again? Are you purposely trying to
disable gcc and sparse's type-safety?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/5] drm/i915: Parse EDID probed modes for DRRS support

[Vandana Kannan]

On Dec-17-2013 5:58 PM, Chris Wilson wrote:
> On Tue, Dec 17, 2013 at 10:58:24AM +0530, Vandana Kannan wrote:
>> From: Pradeep Bhat <pradeep.bhat@intel.com>
>>
>> This patch and finds out the lowest refresh rate supported for the resolution
>> same as the fixed_mode, based on the implementaion find_panel_downclock.
>> It also checks the VBT fields to see if panel supports seamless DRRS or not.
>> Based on above data it marks whether eDP panel supports seamless DRRS or not.
>> This information is needed for supporting seamless DRRS switch for
>> certain power saving usecases. This patch is tested by enabling the DRM logs
>> and user should see whether Seamless DRRS is supported or not.
>>
>> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |    2 ++
>>  drivers/gpu/drm/i915/intel_dp.c  |   47 ++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_drv.h |   29 +++++++++++++++++++++++
>>  3 files changed, 78 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 02e11dc..c9bca16 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1462,8 +1462,10 @@ typedef struct drm_i915_private {
>>  	/* Reclocking support */
>>  	bool render_reclock_avail;
>>  	bool lvds_downclock_avail;
>> +	bool edp_downclock_avail;
>>  	/* indicates the reduced downclock for LVDS*/
>>  	int lvds_downclock;
>> +	int edp_downclock;
>>  	u16 orig_clock;
> 
> Do any machines have both edp and lvds? Shouldn't this be a part of the
> panel state?
>
If there is a machine having both edp and lvds, then edp takes higher
priority. edp_downclock_avail and edp_downclock were added here
following the existing code having lvds_downclock_avail and
lvds_downclock here. If required, edp_downclock_avail and edp_downclock
can be moved to intel_panel structure. Kindly let us know.

>>  
>> +/**
>> + * This enum is used to indicate the DRRS support type.
>> + * The values of the enum map 1-to-1 with the values from VBT.
>> + */
>> +enum edp_panel_type {
>> +	DRRS_NOT_SUPPORTED = -1,
>> +	STATIC_DRRS_SUPPORT = 0,
>> +	SEAMLESS_DRRS_SUPPORT = 2
>> +};
>> +/**
>> + * 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 */
>> +};
>> +/**
>> + * The drrs_info struct will represent the DRRS feature for eDP
>> + * panel.
>> + */
>> +struct drrs_info {
>> +	int is_drrs_supported;
>> +	int drrs_refresh_rate_type;
> 
> So what was the point of the enums again? Are you purposely trying to
> disable gcc and sparse's type-safety?
> -Chris
> 
The enum edp_panel_type is required to check DRRS capability of the
panel before performing any enabling. We will look into an
implementation which can do without edp_drrs_refresh_rate_type.

Re: [PATCH 2/5] drm/i915: Parse EDID probed modes for DRRS support

[Chris Wilson]

On Wed, Dec 18, 2013 at 01:41:21PM +0530, Vandana Kannan wrote:
> On Dec-17-2013 5:58 PM, Chris Wilson wrote:
> > On Tue, Dec 17, 2013 at 10:58:24AM +0530, Vandana Kannan wrote:
> >> From: Pradeep Bhat <pradeep.bhat@intel.com>
> >>
> >> This patch and finds out the lowest refresh rate supported for the resolution
> >> same as the fixed_mode, based on the implementaion find_panel_downclock.
> >> It also checks the VBT fields to see if panel supports seamless DRRS or not.
> >> Based on above data it marks whether eDP panel supports seamless DRRS or not.
> >> This information is needed for supporting seamless DRRS switch for
> >> certain power saving usecases. This patch is tested by enabling the DRM logs
> >> and user should see whether Seamless DRRS is supported or not.
> >>
> >> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_drv.h  |    2 ++
> >>  drivers/gpu/drm/i915/intel_dp.c  |   47 ++++++++++++++++++++++++++++++++++++++
> >>  drivers/gpu/drm/i915/intel_drv.h |   29 +++++++++++++++++++++++
> >>  3 files changed, 78 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 02e11dc..c9bca16 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -1462,8 +1462,10 @@ typedef struct drm_i915_private {
> >>  	/* Reclocking support */
> >>  	bool render_reclock_avail;
> >>  	bool lvds_downclock_avail;
> >> +	bool edp_downclock_avail;
> >>  	/* indicates the reduced downclock for LVDS*/
> >>  	int lvds_downclock;
> >> +	int edp_downclock;
> >>  	u16 orig_clock;
> > 
> > Do any machines have both edp and lvds? Shouldn't this be a part of the
> > panel state?
> >
> If there is a machine having both edp and lvds, then edp takes higher
> priority. edp_downclock_avail and edp_downclock were added here
> following the existing code having lvds_downclock_avail and
> lvds_downclock here. If required, edp_downclock_avail and edp_downclock
> can be moved to intel_panel structure. Kindly let us know.

And we can consolidate both into intel_panel.
 
> >>  
> >> +/**
> >> + * This enum is used to indicate the DRRS support type.
> >> + * The values of the enum map 1-to-1 with the values from VBT.
> >> + */
> >> +enum edp_panel_type {
> >> +	DRRS_NOT_SUPPORTED = -1,
> >> +	STATIC_DRRS_SUPPORT = 0,
> >> +	SEAMLESS_DRRS_SUPPORT = 2
> >> +};
> >> +/**
> >> + * 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 */
> >> +};
> >> +/**
> >> + * The drrs_info struct will represent the DRRS feature for eDP
> >> + * panel.
> >> + */
> >> +struct drrs_info {
> >> +	int is_drrs_supported;
> >> +	int drrs_refresh_rate_type;
> > 
> > So what was the point of the enums again? Are you purposely trying to
> > disable gcc and sparse's type-safety?
> > 
> The enum edp_panel_type is required to check DRRS capability of the
> panel before performing any enabling. We will look into an
> implementation which can do without edp_drrs_refresh_rate_type.

All I am saying is have enum, use enum. It improves type safety.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/5] drm/i915: Parse EDID probed modes for DRRS support

[Vandana Kannan]

On Dec-18-2013 2:36 PM, Chris Wilson wrote:
> On Wed, Dec 18, 2013 at 01:41:21PM +0530, Vandana Kannan wrote:
>> On Dec-17-2013 5:58 PM, Chris Wilson wrote:
>>> On Tue, Dec 17, 2013 at 10:58:24AM +0530, Vandana Kannan wrote:
>>>> From: Pradeep Bhat <pradeep.bhat@intel.com>
>>>>
>>>> This patch and finds out the lowest refresh rate supported for the resolution
>>>> same as the fixed_mode, based on the implementaion find_panel_downclock.
>>>> It also checks the VBT fields to see if panel supports seamless DRRS or not.
>>>> Based on above data it marks whether eDP panel supports seamless DRRS or not.
>>>> This information is needed for supporting seamless DRRS switch for
>>>> certain power saving usecases. This patch is tested by enabling the DRM logs
>>>> and user should see whether Seamless DRRS is supported or not.
>>>>
>>>> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
>>>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/i915_drv.h  |    2 ++
>>>>  drivers/gpu/drm/i915/intel_dp.c  |   47 ++++++++++++++++++++++++++++++++++++++
>>>>  drivers/gpu/drm/i915/intel_drv.h |   29 +++++++++++++++++++++++
>>>>  3 files changed, 78 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>> index 02e11dc..c9bca16 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -1462,8 +1462,10 @@ typedef struct drm_i915_private {
>>>>  	/* Reclocking support */
>>>>  	bool render_reclock_avail;
>>>>  	bool lvds_downclock_avail;
>>>> +	bool edp_downclock_avail;
>>>>  	/* indicates the reduced downclock for LVDS*/
>>>>  	int lvds_downclock;
>>>> +	int edp_downclock;
>>>>  	u16 orig_clock;
>>>
>>> Do any machines have both edp and lvds? Shouldn't this be a part of the
>>> panel state?
>>>
>> If there is a machine having both edp and lvds, then edp takes higher
>> priority. edp_downclock_avail and edp_downclock were added here
>> following the existing code having lvds_downclock_avail and
>> lvds_downclock here. If required, edp_downclock_avail and edp_downclock
>> can be moved to intel_panel structure. Kindly let us know.
> 
> And we can consolidate both into intel_panel.
>  
For this patch series, I will move edp_downclock_avail and edp_downclock
to intel_panel, and following that, work on a separate patch to move
lvds_downclock_avail and lvds_downclock to intel_panel.
>>>>  
>>>> +/**
>>>> + * This enum is used to indicate the DRRS support type.
>>>> + * The values of the enum map 1-to-1 with the values from VBT.
>>>> + */
>>>> +enum edp_panel_type {
>>>> +	DRRS_NOT_SUPPORTED = -1,
>>>> +	STATIC_DRRS_SUPPORT = 0,
>>>> +	SEAMLESS_DRRS_SUPPORT = 2
>>>> +};
>>>> +/**
>>>> + * 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 */
>>>> +};
>>>> +/**
>>>> + * The drrs_info struct will represent the DRRS feature for eDP
>>>> + * panel.
>>>> + */
>>>> +struct drrs_info {
>>>> +	int is_drrs_supported;
>>>> +	int drrs_refresh_rate_type;
>>>
>>> So what was the point of the enums again? Are you purposely trying to
>>> disable gcc and sparse's type-safety?
>>>
>> The enum edp_panel_type is required to check DRRS capability of the
>> panel before performing any enabling. We will look into an
>> implementation which can do without edp_drrs_refresh_rate_type.
> 
> All I am saying is have enum, use enum. It improves type safety.
> -Chris
> 
Ok. I will not be making any change related to this.

Re: [PATCH 2/5] drm/i915: Parse EDID probed modes for DRRS support

[Jani Nikula]

On Tue, 17 Dec 2013, Vandana Kannan <vandana.kannan@intel.com> wrote:
> From: Pradeep Bhat <pradeep.bhat@intel.com>
>
> This patch and finds out the lowest refresh rate supported for the resolution
> same as the fixed_mode, based on the implementaion find_panel_downclock.
> It also checks the VBT fields to see if panel supports seamless DRRS or not.
> Based on above data it marks whether eDP panel supports seamless DRRS or not.
> This information is needed for supporting seamless DRRS switch for
> certain power saving usecases. This patch is tested by enabling the DRM logs
> and user should see whether Seamless DRRS is supported or not.
>
> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |    2 ++
>  drivers/gpu/drm/i915/intel_dp.c  |   47 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |   29 +++++++++++++++++++++++
>  3 files changed, 78 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 02e11dc..c9bca16 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1462,8 +1462,10 @@ typedef struct drm_i915_private {
>  	/* Reclocking support */
>  	bool render_reclock_avail;
>  	bool lvds_downclock_avail;
> +	bool edp_downclock_avail;
>  	/* indicates the reduced downclock for LVDS*/
>  	int lvds_downclock;
> +	int edp_downclock;
>  	u16 orig_clock;
>  
>  	bool mchbar_need_disable;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 82de200..935b46e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3524,6 +3524,48 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  		      I915_READ(pp_div_reg));
>  }
>  
> +static void
> +intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
> +			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_i915_private *dev_priv = dev->dev_private;
> +
> +	/**
> +	 * Check if PSR is supported by panel and enabled
> +	 * if so then DRRS is reported as not supported for Haswell.
> +	 */
> +	if (INTEL_INFO(dev)->gen < 8 &&	intel_edp_is_psr_enabled(dev)) {
> +		DRM_INFO("eDP panel has PSR enabled. Cannot support DRRS\n");
> +		return;
> +	}
> +
> +	/* First check if DRRS is enabled from VBT struct */
> +	if (!dev_priv->vbt.intel_drrs_enabled) {
> +		DRM_INFO("VBT doesn't support DRRS\n");
> +		return;
> +	}
> +
> +	intel_connector->panel.downclock_mode =	intel_find_panel_downclock(dev,
> +					fixed_mode, connector);
> +
> +	if (intel_connector->panel.downclock_mode != NULL &&
> +		dev_priv->vbt.drrs_mode == SEAMLESS_DRRS_SUPPORT) {
> +		dev_priv->edp_downclock_avail = true;
> +		dev_priv->edp_downclock =
> +			intel_connector->panel.downclock_mode->clock;
> +
> +		intel_dp->drrs_state.is_drrs_supported
> +				= dev_priv->vbt.drrs_mode;
> +
> +		intel_dp->drrs_state.drrs_refresh_rate_type = DRRS_HIGH_RR;
> +		DRM_INFO("SEAMLESS DRRS supported for eDP panel.\n");
> +	}
> +}
> +
>  static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  				     struct intel_connector *intel_connector)
>  {
> @@ -3537,6 +3579,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	struct drm_display_mode *scan;
>  	struct edid *edid;
>  
> +	intel_dp->drrs_state.is_drrs_supported = DRRS_NOT_SUPPORTED;
> +
>  	if (!is_edp(intel_dp))
>  		return true;
>  
> @@ -3581,6 +3625,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	list_for_each_entry(scan, &connector->probed_modes, head) {
>  		if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
>  			fixed_mode = drm_mode_duplicate(dev, scan);
> +			if (INTEL_INFO(dev)->gen >= 5)
> +				intel_dp_drrs_initialize(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 9f8b465..6ec5f4e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -462,6 +462,34 @@ struct intel_hdmi {
>  
>  #define DP_MAX_DOWNSTREAM_PORTS		0x10
>  
> +/**
> + * This enum is used to indicate the DRRS support type.
> + * The values of the enum map 1-to-1 with the values from VBT.
> + */
> +enum edp_panel_type {

This is about DRRS, not some fundamental "eDP panel type". The comment
above explains what this is, but why not make the enum self explanatory,
for example panel_drrs_support or something.

As to the 1-to-1 map mention, the vbt data is unsigned, but not
supported is -1 below...

> +	DRRS_NOT_SUPPORTED = -1,
> +	STATIC_DRRS_SUPPORT = 0,
> +	SEAMLESS_DRRS_SUPPORT = 2
> +};
> +/**
> + * 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 */
> +};
> +/**
> + * The drrs_info struct will represent the DRRS feature for eDP
> + * panel.
> + */
> +struct drrs_info {
> +	int is_drrs_supported;

IMHO anything that begins with "is" should be strictly boolean. When you
make this an enum per Chris' request, please change this as well. For
exampel drrs_support or something.

> +	int drrs_refresh_rate_type;
> +};
> +
>  struct intel_dp {
>  	uint32_t output_reg;
>  	uint32_t aux_ch_ctl_reg;
> @@ -487,6 +515,7 @@ struct intel_dp {
>  	bool want_panel_vdd;
>  	bool psr_setup_done;
>  	struct intel_connector *attached_connector;
> +	struct drrs_info drrs_state;
>  };
>  
>  struct intel_digital_port {
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 2/5] drm/i915: Parse EDID probed modes for DRRS support

[Vandana Kannan]

On Dec-19-2013 5:21 PM, Jani Nikula wrote:
> On Tue, 17 Dec 2013, Vandana Kannan <vandana.kannan@intel.com> wrote:
>> From: Pradeep Bhat <pradeep.bhat@intel.com>
>>
>> This patch and finds out the lowest refresh rate supported for the resolution
>> same as the fixed_mode, based on the implementaion find_panel_downclock.
>> It also checks the VBT fields to see if panel supports seamless DRRS or not.
>> Based on above data it marks whether eDP panel supports seamless DRRS or not.
>> This information is needed for supporting seamless DRRS switch for
>> certain power saving usecases. This patch is tested by enabling the DRM logs
>> and user should see whether Seamless DRRS is supported or not.
>>
>> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |    2 ++
>>  drivers/gpu/drm/i915/intel_dp.c  |   47 ++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_drv.h |   29 +++++++++++++++++++++++
>>  3 files changed, 78 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 02e11dc..c9bca16 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1462,8 +1462,10 @@ typedef struct drm_i915_private {
>>  	/* Reclocking support */
>>  	bool render_reclock_avail;
>>  	bool lvds_downclock_avail;
>> +	bool edp_downclock_avail;
>>  	/* indicates the reduced downclock for LVDS*/
>>  	int lvds_downclock;
>> +	int edp_downclock;
>>  	u16 orig_clock;
>>  
>>  	bool mchbar_need_disable;
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 82de200..935b46e 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -3524,6 +3524,48 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>>  		      I915_READ(pp_div_reg));
>>  }
>>  
>> +static void
>> +intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
>> +			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_i915_private *dev_priv = dev->dev_private;
>> +
>> +	/**
>> +	 * Check if PSR is supported by panel and enabled
>> +	 * if so then DRRS is reported as not supported for Haswell.
>> +	 */
>> +	if (INTEL_INFO(dev)->gen < 8 &&	intel_edp_is_psr_enabled(dev)) {
>> +		DRM_INFO("eDP panel has PSR enabled. Cannot support DRRS\n");
>> +		return;
>> +	}
>> +
>> +	/* First check if DRRS is enabled from VBT struct */
>> +	if (!dev_priv->vbt.intel_drrs_enabled) {
>> +		DRM_INFO("VBT doesn't support DRRS\n");
>> +		return;
>> +	}
>> +
>> +	intel_connector->panel.downclock_mode =	intel_find_panel_downclock(dev,
>> +					fixed_mode, connector);
>> +
>> +	if (intel_connector->panel.downclock_mode != NULL &&
>> +		dev_priv->vbt.drrs_mode == SEAMLESS_DRRS_SUPPORT) {
>> +		dev_priv->edp_downclock_avail = true;
>> +		dev_priv->edp_downclock =
>> +			intel_connector->panel.downclock_mode->clock;
>> +
>> +		intel_dp->drrs_state.is_drrs_supported
>> +				= dev_priv->vbt.drrs_mode;
>> +
>> +		intel_dp->drrs_state.drrs_refresh_rate_type = DRRS_HIGH_RR;
>> +		DRM_INFO("SEAMLESS DRRS supported for eDP panel.\n");
>> +	}
>> +}
>> +
>>  static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  				     struct intel_connector *intel_connector)
>>  {
>> @@ -3537,6 +3579,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  	struct drm_display_mode *scan;
>>  	struct edid *edid;
>>  
>> +	intel_dp->drrs_state.is_drrs_supported = DRRS_NOT_SUPPORTED;
>> +
>>  	if (!is_edp(intel_dp))
>>  		return true;
>>  
>> @@ -3581,6 +3625,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  	list_for_each_entry(scan, &connector->probed_modes, head) {
>>  		if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
>>  			fixed_mode = drm_mode_duplicate(dev, scan);
>> +			if (INTEL_INFO(dev)->gen >= 5)
>> +				intel_dp_drrs_initialize(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 9f8b465..6ec5f4e 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -462,6 +462,34 @@ struct intel_hdmi {
>>  
>>  #define DP_MAX_DOWNSTREAM_PORTS		0x10
>>  
>> +/**
>> + * This enum is used to indicate the DRRS support type.
>> + * The values of the enum map 1-to-1 with the values from VBT.
>> + */
>> +enum edp_panel_type {
> 
> This is about DRRS, not some fundamental "eDP panel type". The comment
> above explains what this is, but why not make the enum self explanatory,
> for example panel_drrs_support or something.
I will change this to drrs_support_type
> 
> As to the 1-to-1 map mention, the vbt data is unsigned, but not
> supported is -1 below...
> 
>> +	DRRS_NOT_SUPPORTED = -1,
>> +	STATIC_DRRS_SUPPORT = 0,
>> +	SEAMLESS_DRRS_SUPPORT = 2
>> +};
VBT would return 0 for static DRRS and 2 for seamless DRRS.
DRRS_NOT_SUPPORTED is for internal usage - not 1:1 mapped with VBT.
>> +/**
>> + * 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 */
>> +};
>> +/**
>> + * The drrs_info struct will represent the DRRS feature for eDP
>> + * panel.
>> + */
>> +struct drrs_info {
>> +	int is_drrs_supported;
> 
> IMHO anything that begins with "is" should be strictly boolean. When you
> make this an enum per Chris' request, please change this as well. For
> exampel drrs_support or something.
> 
I will change is_drrs_supported to enum type and change the name.
>> +	int drrs_refresh_rate_type;
>> +};
>> +
>>  struct intel_dp {
>>  	uint32_t output_reg;
>>  	uint32_t aux_ch_ctl_reg;
>> @@ -487,6 +515,7 @@ struct intel_dp {
>>  	bool want_panel_vdd;
>>  	bool psr_setup_done;
>>  	struct intel_connector *attached_connector;
>> +	struct drrs_info drrs_state;
>>  };
>>  
>>  struct intel_digital_port {
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

[PATCH 3/5] drm/i915: Add support for DRRS to switch RR

[Vandana Kannan]

From: Pradeep Bhat <pradeep.bhat@intel.com>

This patch computes and stored 2nd M/N/TU for switching to different
refresh rate dynamically. PIPECONF_EDP_RR_MODE_SWITCH bit helps toggle
between alternate refresh rates programmed in 2nd M/N/TU registers.

Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |    1 +
 drivers/gpu/drm/i915/intel_dp.c  |  109 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |    6 ++-
 3 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3be449d..a52c663 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3203,6 +3203,7 @@
 #define   PIPECONF_INTERLACED_DBL_ILK		(4 << 21) /* ilk/snb only */
 #define   PIPECONF_PFIT_PF_INTERLACED_DBL_ILK	(5 << 21) /* ilk/snb only */
 #define   PIPECONF_INTERLACE_MODE_MASK		(7 << 21)
+#define   PIPECONF_EDP_RR_MODE_SWITCH		(1 << 20)
 #define   PIPECONF_CXSR_DOWNCLOCK	(1<<16)
 #define   PIPECONF_COLOR_RANGE_SELECT	(1 << 13)
 #define   PIPECONF_BPC_MASK	(0x7 << 5)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 935b46e..fbf71ed 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -791,6 +791,23 @@ intel_dp_set_clock(struct intel_encoder *encoder,
 	}
 }
 
+static void
+intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum transcoder transcoder = crtc->config.cpu_transcoder;
+
+	if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
+		I915_WRITE(PIPE_DATA_M2(transcoder),
+			   TU_SIZE(m_n->tu) | m_n->gmch_m);
+		I915_WRITE(PIPE_DATA_N2(transcoder), m_n->gmch_n);
+		I915_WRITE(PIPE_LINK_M2(transcoder), m_n->link_m);
+		I915_WRITE(PIPE_LINK_N2(transcoder), m_n->link_n);
+	}
+	return;
+}
+
 bool
 intel_dp_compute_config(struct intel_encoder *encoder,
 			struct intel_crtc_config *pipe_config)
@@ -894,6 +911,14 @@ found:
 			       pipe_config->port_clock,
 			       &pipe_config->dp_m_n);
 
+	if (dev_priv->edp_downclock_avail &&
+	intel_dp->drrs_state.is_drrs_supported == SEAMLESS_DRRS_SUPPORT) {
+		intel_link_compute_m_n(bpp, lane_count,
+				dev_priv->edp_downclock,
+				pipe_config->port_clock,
+				&pipe_config->dp_m2_n2);
+	}
+
 	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
 
 	return true;
@@ -3524,6 +3549,88 @@ 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)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_mode_config *mode_config = &dev->mode_config;
+	struct intel_encoder *encoder;
+	struct intel_dp *intel_dp = NULL;
+	struct intel_crtc_config *config = NULL;
+	struct intel_crtc *intel_crtc = NULL;
+	struct intel_connector *intel_connector = NULL;
+	bool found_edp = false;
+	u32 reg, val;
+	int index = 0;
+
+	if (refresh_rate <= 0) {
+		DRM_INFO("Refresh rate should be positive non-zero.\n");
+		goto out;
+	}
+
+	list_for_each_entry(encoder, &mode_config->encoder_list, base.head) {
+		if (encoder->type == INTEL_OUTPUT_EDP) {
+			intel_dp = enc_to_intel_dp(&encoder->base);
+			intel_crtc = encoder->new_crtc;
+			if (!intel_crtc) {
+			DRM_INFO("DRRS: intel_crtc not initialized\n");
+				goto out;
+			}
+			config = &intel_crtc->config;
+			intel_connector = intel_dp->attached_connector;
+			found_edp = true;
+			break;
+		}
+	}
+
+	if (!found_edp) {
+		DRM_INFO("DRRS supported for eDP only.\n");
+		goto out;
+	}
+
+	if (intel_dp->drrs_state.is_drrs_supported < SEAMLESS_DRRS_SUPPORT) {
+		DRM_INFO("Seamless DRRS not supported.\n");
+		goto out;
+	}
+
+	if (intel_connector->panel.fixed_mode->vrefresh == refresh_rate)
+		index = DRRS_HIGH_RR;
+	else
+		index = DRRS_LOW_RR;
+
+	if (index == intel_dp->drrs_state.drrs_refresh_rate_type) {
+		DRM_INFO("DRRS requested for previously set RR...ignoring\n");
+		goto out;
+	}
+
+	if (!intel_crtc->active) {
+		DRM_INFO("eDP encoder has been disabled. CRTC not Active\n");
+		goto out;
+	}
+
+	mutex_lock(&intel_dp->drrs_state.mutex);
+
+	/* Haswell and below */
+	if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
+		reg = PIPECONF(intel_crtc->config.cpu_transcoder);
+		val = I915_READ(reg);
+		if (index > DRRS_HIGH_RR) {
+			val |= PIPECONF_EDP_RR_MODE_SWITCH;
+			intel_dp_set_m2_n2(intel_crtc, &config->dp_m2_n2);
+		} else
+			val &= ~PIPECONF_EDP_RR_MODE_SWITCH;
+		I915_WRITE(reg, val);
+	}
+
+	intel_dp->drrs_state.drrs_refresh_rate_type = index;
+	DRM_INFO("eDP Refresh Rate set to : %dHz\n", refresh_rate);
+
+	mutex_unlock(&intel_dp->drrs_state.mutex);
+
+out:
+	return;
+}
+
 static void
 intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
 			struct intel_connector *intel_connector,
@@ -3558,6 +3665,8 @@ intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
 		dev_priv->edp_downclock =
 			intel_connector->panel.downclock_mode->clock;
 
+		mutex_init(&intel_dp->drrs_state.mutex);
+
 		intel_dp->drrs_state.is_drrs_supported
 				= dev_priv->vbt.drrs_mode;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6ec5f4e..3e4306b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -288,6 +288,9 @@ struct intel_crtc_config {
 	int pipe_bpp;
 	struct intel_link_m_n dp_m_n;
 
+	/* m2_n2 for eDP downclock */
+	struct intel_link_m_n dp_m2_n2;
+
 	/*
 	 * Frequence the dpll for the port should run at. Differs from the
 	 * adjusted dotclock e.g. for DP or 12bpc hdmi mode. This is also
@@ -488,6 +491,7 @@ enum edp_drrs_refresh_rate_type {
 struct drrs_info {
 	int is_drrs_supported;
 	int drrs_refresh_rate_type;
+	struct mutex mutex;
 };
 
 struct intel_dp {
@@ -760,7 +764,7 @@ void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
 void intel_edp_psr_enable(struct intel_dp *intel_dp);
 void intel_edp_psr_disable(struct intel_dp *intel_dp);
 void intel_edp_psr_update(struct drm_device *dev);
-
+extern void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate);
 
 /* intel_dsi.c */
 bool intel_dsi_init(struct drm_device *dev);
-- 
1.7.9.5

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

[Vandana Kannan]

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(+)

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);
 
 	/*
-- 
1.7.9.5

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

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Vandana Kannan]

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?

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

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Vandana Kannan]

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?

[PATCH 5/5] 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_dp.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 209be3c..183cfd7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -798,9 +798,15 @@ intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum transcoder transcoder = crtc->config.cpu_transcoder;
 
-	if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
+	if (INTEL_INFO(dev)->gen >= 8) {
+		I915_WRITE(PIPE_DATA_M1(transcoder),
+			TU_SIZE(m_n->tu) | m_n->gmch_m);
+		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
+		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
+		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
+	} else if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
 		I915_WRITE(PIPE_DATA_M2(transcoder),
-			   TU_SIZE(m_n->tu) | m_n->gmch_m);
+			TU_SIZE(m_n->tu) | m_n->gmch_m);
 		I915_WRITE(PIPE_DATA_N2(transcoder), m_n->gmch_n);
 		I915_WRITE(PIPE_LINK_M2(transcoder), m_n->link_m);
 		I915_WRITE(PIPE_LINK_N2(transcoder), m_n->link_n);
@@ -3617,8 +3623,17 @@ intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 
 	mutex_lock(&intel_dp->drrs_state.mutex);
 
-	/* Haswell and below */
-	if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
+	if (INTEL_INFO(dev)->gen >= 8) {
+		switch (index) {
+		case DRRS_HIGH_RR:
+			intel_dp_set_m2_n2(intel_crtc, &config->dp_m_n);
+			break;
+		case DRRS_LOW_RR:
+			intel_dp_set_m2_n2(intel_crtc, &config->dp_m2_n2);
+			break;
+		};
+	} else if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
+		/* Haswell and below */
 		reg = PIPECONF(intel_crtc->config.cpu_transcoder);
 		val = I915_READ(reg);
 		if (index > DRRS_HIGH_RR) {
-- 
1.7.9.5

Re: [PATCH 5/5] drm/i915/bdw: Add support for DRRS to switch RR

[Chris Wilson]

On Tue, Dec 17, 2013 at 10:58:27AM +0530, Vandana Kannan wrote:
> 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_dp.c |   23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 209be3c..183cfd7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -798,9 +798,15 @@ intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum transcoder transcoder = crtc->config.cpu_transcoder;
>  
> -	if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
> +	if (INTEL_INFO(dev)->gen >= 8) {
> +		I915_WRITE(PIPE_DATA_M1(transcoder),
> +			TU_SIZE(m_n->tu) | m_n->gmch_m);
> +		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
> +		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
> +		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
> +	} else if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {

Ouch. Double ouch later.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 5/5] drm/i915/bdw: Add support for DRRS to switch RR

[Vandana Kannan]

On Dec-17-2013 6:00 PM, Chris Wilson wrote:
> On Tue, Dec 17, 2013 at 10:58:27AM +0530, Vandana Kannan wrote:
>> 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_dp.c |   23 +++++++++++++++++++----
>>  1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 209be3c..183cfd7 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -798,9 +798,15 @@ intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	enum transcoder transcoder = crtc->config.cpu_transcoder;
>>  
>> -	if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
>> +	if (INTEL_INFO(dev)->gen >= 8) {
>> +		I915_WRITE(PIPE_DATA_M1(transcoder),
>> +			TU_SIZE(m_n->tu) | m_n->gmch_m);
>> +		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
>> +		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
>> +		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
>> +	} else if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
> 
> Ouch. Double ouch later.
> -Chris
> 
We are looking to write in M1/N1 registers for BDW and M2/N2 registers
for HSW and below. What is your suggestion on how to implement this ?

Re: [PATCH 5/5] drm/i915/bdw: Add support for DRRS to switch RR

[Chris Wilson]

On Wed, Dec 18, 2013 at 01:54:56PM +0530, Vandana Kannan wrote:
> On Dec-17-2013 6:00 PM, Chris Wilson wrote:
> > On Tue, Dec 17, 2013 at 10:58:27AM +0530, Vandana Kannan wrote:
> >> 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_dp.c |   23 +++++++++++++++++++----
> >>  1 file changed, 19 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 209be3c..183cfd7 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -798,9 +798,15 @@ intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>  	enum transcoder transcoder = crtc->config.cpu_transcoder;
> >>  
> >> -	if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
> >> +	if (INTEL_INFO(dev)->gen >= 8) {
> >> +		I915_WRITE(PIPE_DATA_M1(transcoder),
> >> +			TU_SIZE(m_n->tu) | m_n->gmch_m);
> >> +		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
> >> +		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
> >> +		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
> >> +	} else if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
> > 
> > Ouch. Double ouch later.
> > -Chris
> > 
> We are looking to write in M1/N1 registers for BDW and M2/N2 registers
> for HSW and below. What is your suggestion on how to implement this ?

  if (gen >= 8) {
  } else if (gen >= 5) {
  }

Or as you use gen >= 5 && gen < 8 elsewhere, a feature macro would be
even more sensible, HAS_DRRS().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 5/5] drm/i915/bdw: Add support for DRRS to switch RR

[Vandana Kannan]

On Dec-18-2013 2:31 PM, Chris Wilson wrote:
> On Wed, Dec 18, 2013 at 01:54:56PM +0530, Vandana Kannan wrote:
>> On Dec-17-2013 6:00 PM, Chris Wilson wrote:
>>> On Tue, Dec 17, 2013 at 10:58:27AM +0530, Vandana Kannan wrote:
>>>> 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_dp.c |   23 +++++++++++++++++++----
>>>>  1 file changed, 19 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>> index 209be3c..183cfd7 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -798,9 +798,15 @@ intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
>>>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>>>  	enum transcoder transcoder = crtc->config.cpu_transcoder;
>>>>  
>>>> -	if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
>>>> +	if (INTEL_INFO(dev)->gen >= 8) {
>>>> +		I915_WRITE(PIPE_DATA_M1(transcoder),
>>>> +			TU_SIZE(m_n->tu) | m_n->gmch_m);
>>>> +		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
>>>> +		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
>>>> +		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
>>>> +	} else if (INTEL_INFO(dev)->gen >= 5 && INTEL_INFO(dev)->gen < 8) {
>>>
>>> Ouch. Double ouch later.
>>> -Chris
>>>
>> We are looking to write in M1/N1 registers for BDW and M2/N2 registers
>> for HSW and below. What is your suggestion on how to implement this ?
> 
>   if (gen >= 8) {
>   } else if (gen >= 5) {
>   }
> 
> Or as you use gen >= 5 && gen < 8 elsewhere, a feature macro would be
> even more sensible, HAS_DRRS().
> -Chris
> 
I will make this change to
   if (gen >= 8) {
   } else if (gen >= 5) {
   }
- Vandana

Re: [PATCH 0/5] Enabling DRRS support in the kernel

[Daniel Vetter]

On Tue, Dec 17, 2013 at 10:58:22AM +0530, Vandana Kannan wrote:
> Dynamic Refresh Rate Switching (DRRS) is a power conservation feature which     
> enables switching between low and high refresh rates based on the usage         
> scenario. This feature is applicable for internal eDP panel. Indication that    
> the panel can support DRRS is given by the panel EDID, which would list         
> multiple refresh rates for one resolution.                                      
> The patch series supports idleness detection in display i915 driver and switch  
> to low refresh rate.

Just one overall comment on process:

When resending patches _always_ mention what you've changed compared to
the last version. Specifically call out which review comments you've
incorporated and even more important which one's you've decided to
implement differently or rejected.

The preferred form for that is with a per-patch changelog in the commit
message. Also mentioned the reviewer's name if the change is due to
review.

For bigger reworks that affect the entire patch series please also mention
them in the cover letter.

I'll update our internal BKMs with this information.

Thanks, Daniel
> 
> Pradeep Bhat (3):
>   [Intel-gfx] drm/i915: Adding VBT fields to support eDP DRRS feature
>   [Intel-gfx] drm/i915: Parse EDID probed modes for DRRS support
>   [Intel-gfx] drm/i915: Add support for DRRS to switch RR
> 
> Vandana Kannan (2):
>   [Intel-gfx] drm/i915: Idleness detection for DRRS
>   [Intel-gfx] drm/i915/bdw: Add support for DRRS to switch RR
> 
>  drivers/gpu/drm/i915/i915_drv.h      |   30 ++++++
>  drivers/gpu/drm/i915/i915_reg.h      |    1 +
>  drivers/gpu/drm/i915/intel_bios.c    |   23 +++++
>  drivers/gpu/drm/i915/intel_bios.h    |   29 ++++++
>  drivers/gpu/drm/i915/intel_display.c |   13 +++
>  drivers/gpu/drm/i915/intel_dp.c      |  180 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |   35 ++++++-
>  drivers/gpu/drm/i915/intel_pm.c      |  112 +++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_sprite.c  |    3 +
>  9 files changed, 425 insertions(+), 1 deletion(-)
> 
> -- 
> 1.7.9.5
> 
> _______________________________________________
> 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: [PATCH 0/5] Enabling DRRS support in the kernel

[Vandana Kannan]

On Dec-18-2013 2:54 PM, Daniel Vetter wrote:
> On Tue, Dec 17, 2013 at 10:58:22AM +0530, Vandana Kannan wrote:
>> Dynamic Refresh Rate Switching (DRRS) is a power conservation feature which     
>> enables switching between low and high refresh rates based on the usage         
>> scenario. This feature is applicable for internal eDP panel. Indication that    
>> the panel can support DRRS is given by the panel EDID, which would list         
>> multiple refresh rates for one resolution.                                      
>> The patch series supports idleness detection in display i915 driver and switch  
>> to low refresh rate.
> 
> Just one overall comment on process:
> 
> When resending patches _always_ mention what you've changed compared to
> the last version. Specifically call out which review comments you've
> incorporated and even more important which one's you've decided to
> implement differently or rejected.
> 
> The preferred form for that is with a per-patch changelog in the commit
> message. Also mentioned the reviewer's name if the change is due to
> review.
> 
> For bigger reworks that affect the entire patch series please also mention
> them in the cover letter.
> 
> I'll update our internal BKMs with this information.
> 
> Thanks, Daniel
Thanks Daniel.
I will incorporate these comments while resending the patches.
- Vandana
>>
>> Pradeep Bhat (3):
>>   [Intel-gfx] drm/i915: Adding VBT fields to support eDP DRRS feature
>>   [Intel-gfx] drm/i915: Parse EDID probed modes for DRRS support
>>   [Intel-gfx] drm/i915: Add support for DRRS to switch RR
>>
>> Vandana Kannan (2):
>>   [Intel-gfx] drm/i915: Idleness detection for DRRS
>>   [Intel-gfx] drm/i915/bdw: Add support for DRRS to switch RR
>>
>>  drivers/gpu/drm/i915/i915_drv.h      |   30 ++++++
>>  drivers/gpu/drm/i915/i915_reg.h      |    1 +
>>  drivers/gpu/drm/i915/intel_bios.c    |   23 +++++
>>  drivers/gpu/drm/i915/intel_bios.h    |   29 ++++++
>>  drivers/gpu/drm/i915/intel_display.c |   13 +++
>>  drivers/gpu/drm/i915/intel_dp.c      |  180 ++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_drv.h     |   35 ++++++-
>>  drivers/gpu/drm/i915/intel_pm.c      |  112 +++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_sprite.c  |    3 +
>>  9 files changed, 425 insertions(+), 1 deletion(-)
>>
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>