diff mbox

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

Message ID 1387785153-5329-4-git-send-email-vandana.kannan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

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

v2: Daniel's review comments
Computing M2/N2 in compute_config and storing it in crtc_config

v3: Modified reference to edp_downclock and edp_downclock_avail based on the
changes made to move them from dev_private to intel_panel.

v4: Modified references to is_drrs_supported based on the changes made to
rename it to drrs_support.

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  |  106 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |    6 ++-
 3 files changed, 112 insertions(+), 1 deletion(-)

Comments

Jani Nikula Jan. 22, 2014, 2:14 p.m. UTC | #1
On Mon, 23 Dec 2013, Vandana Kannan <vandana.kannan@intel.com> wrote:
> 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.
>
> v2: Daniel's review comments
> Computing M2/N2 in compute_config and storing it in crtc_config
>
> v3: Modified reference to edp_downclock and edp_downclock_avail based on the
> changes made to move them from dev_private to intel_panel.
>
> v4: Modified references to is_drrs_supported based on the changes made to
> rename it to drrs_support.
>
> 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  |  106 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |    6 ++-
>  3 files changed, 112 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f1eece4..57d2b64 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3206,6 +3206,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 079b53f..d1e1d6e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -791,6 +791,21 @@ 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;
> +
> +	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;

Superfluous return statement.

> +}
> +
>  bool
>  intel_dp_compute_config(struct intel_encoder *encoder,
>  			struct intel_crtc_config *pipe_config)
> @@ -894,6 +909,14 @@ found:
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n);
>  
> +	if (intel_connector->panel.edp_downclock_avail &&
> +	intel_dp->drrs_state.drrs_support == SEAMLESS_DRRS_SUPPORT) {
> +		intel_link_compute_m_n(bpp, lane_count,
> +				intel_connector->panel.edp_downclock,
> +				pipe_config->port_clock,
> +				&pipe_config->dp_m2_n2);
> +	}

Indentation in the above block.

> +
>  	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
>  
>  	return true;
> @@ -3522,6 +3545,87 @@ 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) {

The opening brace belongs on a new line.

> +	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;
> +	}

Under what conditions do you expect this to happen?

> +
> +	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");

DRM_INFO is probably a bit verbose.

> +				goto out;
> +			}
> +			config = &intel_crtc->config;
> +			intel_connector = intel_dp->attached_connector;
> +			found_edp = true;
> +			break;
> +		}
> +	}

I'm confused. You go through all the trouble of saving the crtc and the
connector (although only in the next patch) but here you iterate all the
encoders to find the one. Why?

> +
> +	if (!found_edp) {

There's no need to add an extra variable for this. You already have
intel_dp, intel_crtc, config and intel_connector you can check!

> +		DRM_INFO("DRRS supported for eDP only.\n");
> +		goto out;
> +	}
> +
> +	if (intel_dp->drrs_state.drrs_support < 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;

Please check bspec for workarounds you need to do wrt frame start delay
before enabling the pipe/transcoder.

> +			intel_dp_set_m2_n2(intel_crtc, &config->dp_m2_n2);
> +		} else
> +			val &= ~PIPECONF_EDP_RR_MODE_SWITCH;

Per coding style, if one branch needs braces, then all do.

> +		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;

Superfluous return statement.

> +}
> +
>  static void
>  intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
>  			struct intel_connector *intel_connector,
> @@ -3555,6 +3659,8 @@ intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
>  		intel_connector->panel.edp_downclock =
>  			intel_connector->panel.downclock_mode->clock;
>  
> +		mutex_init(&intel_dp->drrs_state.mutex);
> +
>  		intel_dp->drrs_state.drrs_support = dev_priv->vbt.drrs_mode;
>  
>  		intel_dp->drrs_state.drrs_refresh_rate_type = DRRS_HIGH_RR;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d208bf5..d1c60fa 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -291,6 +291,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
> @@ -489,6 +492,7 @@ enum edp_drrs_refresh_rate_type {
>  struct drrs_info {
>  	enum drrs_support_type drrs_support;
>  	enum edp_drrs_refresh_rate_type drrs_refresh_rate_type;
> +	struct mutex mutex;
>  };
>  
>  struct intel_dp {
> @@ -761,7 +765,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);

No need for the "extern".

>  
>  /* intel_dsi.c */
>  bool intel_dsi_init(struct drm_device *dev);
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
vandana.kannan@intel.com Jan. 30, 2014, 3:37 a.m. UTC | #2
On Jan-22-2014 7:44 PM, Jani Nikula wrote:
> On Mon, 23 Dec 2013, Vandana Kannan <vandana.kannan@intel.com> wrote:
>> 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.
>>
>> v2: Daniel's review comments
>> Computing M2/N2 in compute_config and storing it in crtc_config
>>
>> v3: Modified reference to edp_downclock and edp_downclock_avail based on the
>> changes made to move them from dev_private to intel_panel.
>>
>> v4: Modified references to is_drrs_supported based on the changes made to
>> rename it to drrs_support.
>>
>> 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  |  106 ++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_drv.h |    6 ++-
>>  3 files changed, 112 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index f1eece4..57d2b64 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3206,6 +3206,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 079b53f..d1e1d6e 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -791,6 +791,21 @@ 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;
>> +
>> +	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;
> 
> Superfluous return statement.
> 
Ok.
>> +}
>> +
>>  bool
>>  intel_dp_compute_config(struct intel_encoder *encoder,
>>  			struct intel_crtc_config *pipe_config)
>> @@ -894,6 +909,14 @@ found:
>>  			       pipe_config->port_clock,
>>  			       &pipe_config->dp_m_n);
>>  
>> +	if (intel_connector->panel.edp_downclock_avail &&
>> +	intel_dp->drrs_state.drrs_support == SEAMLESS_DRRS_SUPPORT) {
>> +		intel_link_compute_m_n(bpp, lane_count,
>> +				intel_connector->panel.edp_downclock,
>> +				pipe_config->port_clock,
>> +				&pipe_config->dp_m2_n2);
>> +	}
> 
> Indentation in the above block.
> 
Ok.
>> +
>>  	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
>>  
>>  	return true;
>> @@ -3522,6 +3545,87 @@ 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) {
> 
> The opening brace belongs on a new line.
> 
Ok.
>> +	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;
>> +	}
> 
> Under what conditions do you expect this to happen?
> 
In future, when refresh rate switching based on user space requests will
be implemented, intel_dp_set_drrs_state() will be used. This check here
is to safeguard calls from user space.
>> +
>> +	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");
> 
> DRM_INFO is probably a bit verbose.
> 
>> +				goto out;
>> +			}
>> +			config = &intel_crtc->config;
>> +			intel_connector = intel_dp->attached_connector;
>> +			found_edp = true;
>> +			break;
>> +		}
>> +	}
> 
> I'm confused. You go through all the trouble of saving the crtc and the
> connector (although only in the next patch) but here you iterate all the
> encoders to find the one. Why?
> 
>> +
>> +	if (!found_edp) {
> 
> There's no need to add an extra variable for this. You already have
> intel_dp, intel_crtc, config and intel_connector you can check!
> 
Ok. I will make necessary changes.
>> +		DRM_INFO("DRRS supported for eDP only.\n");
>> +		goto out;
>> +	}
>> +
>> +	if (intel_dp->drrs_state.drrs_support < 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;
> 
> Please check bspec for workarounds you need to do wrt frame start delay
> before enabling the pipe/transcoder.
> 
We checked the BSpec and there was no workaround mentioned for this part.
>> +			intel_dp_set_m2_n2(intel_crtc, &config->dp_m2_n2);
>> +		} else
>> +			val &= ~PIPECONF_EDP_RR_MODE_SWITCH;
> 
> Per coding style, if one branch needs braces, then all do.
> 
Ok.
>> +		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;
> 
> Superfluous return statement.
> 
Ok.
>> +}
>> +
>>  static void
>>  intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
>>  			struct intel_connector *intel_connector,
>> @@ -3555,6 +3659,8 @@ intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
>>  		intel_connector->panel.edp_downclock =
>>  			intel_connector->panel.downclock_mode->clock;
>>  
>> +		mutex_init(&intel_dp->drrs_state.mutex);
>> +
>>  		intel_dp->drrs_state.drrs_support = dev_priv->vbt.drrs_mode;
>>  
>>  		intel_dp->drrs_state.drrs_refresh_rate_type = DRRS_HIGH_RR;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index d208bf5..d1c60fa 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -291,6 +291,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
>> @@ -489,6 +492,7 @@ enum edp_drrs_refresh_rate_type {
>>  struct drrs_info {
>>  	enum drrs_support_type drrs_support;
>>  	enum edp_drrs_refresh_rate_type drrs_refresh_rate_type;
>> +	struct mutex mutex;
>>  };
>>  
>>  struct intel_dp {
>> @@ -761,7 +765,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);
> 
> No need for the "extern".
> 
Ok.
>>  
>>  /* intel_dsi.c */
>>  bool intel_dsi_init(struct drm_device *dev);
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Jani Nikula Jan. 30, 2014, 6:52 a.m. UTC | #3
On Thu, 30 Jan 2014, Vandana Kannan <vandana.kannan@intel.com> wrote:
> On Jan-22-2014 7:44 PM, Jani Nikula wrote:
>> On Mon, 23 Dec 2013, Vandana Kannan <vandana.kannan@intel.com> wrote:
>>> 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.
>>>
>>> v2: Daniel's review comments
>>> Computing M2/N2 in compute_config and storing it in crtc_config
>>>
>>> v3: Modified reference to edp_downclock and edp_downclock_avail based on the
>>> changes made to move them from dev_private to intel_panel.
>>>
>>> v4: Modified references to is_drrs_supported based on the changes made to
>>> rename it to drrs_support.
>>>
>>> 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  |  106 ++++++++++++++++++++++++++++++++++++++
>>>  drivers/gpu/drm/i915/intel_drv.h |    6 ++-
>>>  3 files changed, 112 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index f1eece4..57d2b64 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -3206,6 +3206,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 079b53f..d1e1d6e 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -791,6 +791,21 @@ 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;
>>> +
>>> +	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;
>> 
>> Superfluous return statement.
>> 
> Ok.
>>> +}
>>> +
>>>  bool
>>>  intel_dp_compute_config(struct intel_encoder *encoder,
>>>  			struct intel_crtc_config *pipe_config)
>>> @@ -894,6 +909,14 @@ found:
>>>  			       pipe_config->port_clock,
>>>  			       &pipe_config->dp_m_n);
>>>  
>>> +	if (intel_connector->panel.edp_downclock_avail &&
>>> +	intel_dp->drrs_state.drrs_support == SEAMLESS_DRRS_SUPPORT) {
>>> +		intel_link_compute_m_n(bpp, lane_count,
>>> +				intel_connector->panel.edp_downclock,
>>> +				pipe_config->port_clock,
>>> +				&pipe_config->dp_m2_n2);
>>> +	}
>> 
>> Indentation in the above block.
>> 
> Ok.
>>> +
>>>  	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
>>>  
>>>  	return true;
>>> @@ -3522,6 +3545,87 @@ 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) {
>> 
>> The opening brace belongs on a new line.
>> 
> Ok.
>>> +	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;
>>> +	}
>> 
>> Under what conditions do you expect this to happen?
>> 
> In future, when refresh rate switching based on user space requests will
> be implemented, intel_dp_set_drrs_state() will be used. This check here
> is to safeguard calls from user space.
>>> +
>>> +	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");
>> 
>> DRM_INFO is probably a bit verbose.
>> 
>>> +				goto out;
>>> +			}
>>> +			config = &intel_crtc->config;
>>> +			intel_connector = intel_dp->attached_connector;
>>> +			found_edp = true;
>>> +			break;
>>> +		}
>>> +	}
>> 
>> I'm confused. You go through all the trouble of saving the crtc and the
>> connector (although only in the next patch) but here you iterate all the
>> encoders to find the one. Why?
>> 
>>> +
>>> +	if (!found_edp) {
>> 
>> There's no need to add an extra variable for this. You already have
>> intel_dp, intel_crtc, config and intel_connector you can check!
>> 
> Ok. I will make necessary changes.
>>> +		DRM_INFO("DRRS supported for eDP only.\n");
>>> +		goto out;
>>> +	}
>>> +
>>> +	if (intel_dp->drrs_state.drrs_support < 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;
>> 
>> Please check bspec for workarounds you need to do wrt frame start delay
>> before enabling the pipe/transcoder.
>> 
> We checked the BSpec and there was no workaround mentioned for this part.

Oh, I'm sorry, I should have been more specific. This was in the SNB
specs: "Workaround : If this pipe is connected to a port on the PCH and
this power savings mode will be used, then before the pipe and
transcoder are enabled, the frame start delay in the pipe and transcoder
must be set to 11b. When these conditions are no longer true, the frame
start delay must be returned to the previous value after the pipe and
transcoder are disabled."

Going forward, I think it's okay to enable drrs on ivb+ at first, and
enable snb with the workarounds in a follow-up patch. Others may
disagree...

>>> +			intel_dp_set_m2_n2(intel_crtc, &config->dp_m2_n2);
>>> +		} else
>>> +			val &= ~PIPECONF_EDP_RR_MODE_SWITCH;
>> 
>> Per coding style, if one branch needs braces, then all do.
>> 
> Ok.
>>> +		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;
>> 
>> Superfluous return statement.
>> 
> Ok.
>>> +}
>>> +
>>>  static void
>>>  intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
>>>  			struct intel_connector *intel_connector,
>>> @@ -3555,6 +3659,8 @@ intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
>>>  		intel_connector->panel.edp_downclock =
>>>  			intel_connector->panel.downclock_mode->clock;
>>>  
>>> +		mutex_init(&intel_dp->drrs_state.mutex);
>>> +
>>>  		intel_dp->drrs_state.drrs_support = dev_priv->vbt.drrs_mode;
>>>  
>>>  		intel_dp->drrs_state.drrs_refresh_rate_type = DRRS_HIGH_RR;
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index d208bf5..d1c60fa 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -291,6 +291,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
>>> @@ -489,6 +492,7 @@ enum edp_drrs_refresh_rate_type {
>>>  struct drrs_info {
>>>  	enum drrs_support_type drrs_support;
>>>  	enum edp_drrs_refresh_rate_type drrs_refresh_rate_type;
>>> +	struct mutex mutex;
>>>  };
>>>  
>>>  struct intel_dp {
>>> @@ -761,7 +765,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);
>> 
>> No need for the "extern".
>> 
> Ok.
>>>  
>>>  /* intel_dsi.c */
>>>  bool intel_dsi_init(struct drm_device *dev);
>>> -- 
>>> 1.7.9.5
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>
vandana.kannan@intel.com Feb. 3, 2014, 3:46 a.m. UTC | #4
On Jan-30-2014 12:22 PM, Jani Nikula wrote:
> On Thu, 30 Jan 2014, Vandana Kannan <vandana.kannan@intel.com> wrote:
>> On Jan-22-2014 7:44 PM, Jani Nikula wrote:
>>> On Mon, 23 Dec 2013, Vandana Kannan <vandana.kannan@intel.com> wrote:
>>>> 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.
>>>>
>>>> v2: Daniel's review comments
>>>> Computing M2/N2 in compute_config and storing it in crtc_config
>>>>
>>>> v3: Modified reference to edp_downclock and edp_downclock_avail based on the
>>>> changes made to move them from dev_private to intel_panel.
>>>>
>>>> v4: Modified references to is_drrs_supported based on the changes made to
>>>> rename it to drrs_support.
>>>>
>>>> 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  |  106 ++++++++++++++++++++++++++++++++++++++
>>>>  drivers/gpu/drm/i915/intel_drv.h |    6 ++-
>>>>  3 files changed, 112 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>>> index f1eece4..57d2b64 100644
>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>> @@ -3206,6 +3206,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 079b53f..d1e1d6e 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -791,6 +791,21 @@ 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;
>>>> +
>>>> +	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;
>>>
>>> Superfluous return statement.
>>>
>> Ok.
>>>> +}
>>>> +
>>>>  bool
>>>>  intel_dp_compute_config(struct intel_encoder *encoder,
>>>>  			struct intel_crtc_config *pipe_config)
>>>> @@ -894,6 +909,14 @@ found:
>>>>  			       pipe_config->port_clock,
>>>>  			       &pipe_config->dp_m_n);
>>>>  
>>>> +	if (intel_connector->panel.edp_downclock_avail &&
>>>> +	intel_dp->drrs_state.drrs_support == SEAMLESS_DRRS_SUPPORT) {
>>>> +		intel_link_compute_m_n(bpp, lane_count,
>>>> +				intel_connector->panel.edp_downclock,
>>>> +				pipe_config->port_clock,
>>>> +				&pipe_config->dp_m2_n2);
>>>> +	}
>>>
>>> Indentation in the above block.
>>>
>> Ok.
>>>> +
>>>>  	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
>>>>  
>>>>  	return true;
>>>> @@ -3522,6 +3545,87 @@ 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) {
>>>
>>> The opening brace belongs on a new line.
>>>
>> Ok.
>>>> +	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;
>>>> +	}
>>>
>>> Under what conditions do you expect this to happen?
>>>
>> In future, when refresh rate switching based on user space requests will
>> be implemented, intel_dp_set_drrs_state() will be used. This check here
>> is to safeguard calls from user space.
>>>> +
>>>> +	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");
>>>
>>> DRM_INFO is probably a bit verbose.
>>>
>>>> +				goto out;
>>>> +			}
>>>> +			config = &intel_crtc->config;
>>>> +			intel_connector = intel_dp->attached_connector;
>>>> +			found_edp = true;
>>>> +			break;
>>>> +		}
>>>> +	}
>>>
>>> I'm confused. You go through all the trouble of saving the crtc and the
>>> connector (although only in the next patch) but here you iterate all the
>>> encoders to find the one. Why?
>>>
>>>> +
>>>> +	if (!found_edp) {
>>>
>>> There's no need to add an extra variable for this. You already have
>>> intel_dp, intel_crtc, config and intel_connector you can check!
>>>
>> Ok. I will make necessary changes.
>>>> +		DRM_INFO("DRRS supported for eDP only.\n");
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +	if (intel_dp->drrs_state.drrs_support < 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;
>>>
>>> Please check bspec for workarounds you need to do wrt frame start delay
>>> before enabling the pipe/transcoder.
>>>
>> We checked the BSpec and there was no workaround mentioned for this part.
> 
> Oh, I'm sorry, I should have been more specific. This was in the SNB
> specs: "Workaround : If this pipe is connected to a port on the PCH and
> this power savings mode will be used, then before the pipe and
> transcoder are enabled, the frame start delay in the pipe and transcoder
> must be set to 11b. When these conditions are no longer true, the frame
> start delay must be returned to the previous value after the pipe and
> transcoder are disabled."
> 
> Going forward, I think it's okay to enable drrs on ivb+ at first, and
> enable snb with the workarounds in a follow-up patch. Others may
> disagree...
> 
SNB support will be added as a separate patch. I will modify the checks
wherever applicable to include Gen6+
>>>> +			intel_dp_set_m2_n2(intel_crtc, &config->dp_m2_n2);
>>>> +		} else
>>>> +			val &= ~PIPECONF_EDP_RR_MODE_SWITCH;
>>>
>>> Per coding style, if one branch needs braces, then all do.
>>>
>> Ok.
>>>> +		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;
>>>
>>> Superfluous return statement.
>>>
>> Ok.
>>>> +}
>>>> +
>>>>  static void
>>>>  intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
>>>>  			struct intel_connector *intel_connector,
>>>> @@ -3555,6 +3659,8 @@ intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
>>>>  		intel_connector->panel.edp_downclock =
>>>>  			intel_connector->panel.downclock_mode->clock;
>>>>  
>>>> +		mutex_init(&intel_dp->drrs_state.mutex);
>>>> +
>>>>  		intel_dp->drrs_state.drrs_support = dev_priv->vbt.drrs_mode;
>>>>  
>>>>  		intel_dp->drrs_state.drrs_refresh_rate_type = DRRS_HIGH_RR;
>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>>> index d208bf5..d1c60fa 100644
>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>>> @@ -291,6 +291,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
>>>> @@ -489,6 +492,7 @@ enum edp_drrs_refresh_rate_type {
>>>>  struct drrs_info {
>>>>  	enum drrs_support_type drrs_support;
>>>>  	enum edp_drrs_refresh_rate_type drrs_refresh_rate_type;
>>>> +	struct mutex mutex;
>>>>  };
>>>>  
>>>>  struct intel_dp {
>>>> @@ -761,7 +765,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);
>>>
>>> No need for the "extern".
>>>
>> Ok.
>>>>  
>>>>  /* intel_dsi.c */
>>>>  bool intel_dsi_init(struct drm_device *dev);
>>>> -- 
>>>> 1.7.9.5
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f1eece4..57d2b64 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3206,6 +3206,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 079b53f..d1e1d6e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -791,6 +791,21 @@  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;
+
+	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 +909,14 @@  found:
 			       pipe_config->port_clock,
 			       &pipe_config->dp_m_n);
 
+	if (intel_connector->panel.edp_downclock_avail &&
+	intel_dp->drrs_state.drrs_support == SEAMLESS_DRRS_SUPPORT) {
+		intel_link_compute_m_n(bpp, lane_count,
+				intel_connector->panel.edp_downclock,
+				pipe_config->port_clock,
+				&pipe_config->dp_m2_n2);
+	}
+
 	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
 
 	return true;
@@ -3522,6 +3545,87 @@  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.drrs_support < 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,
@@ -3555,6 +3659,8 @@  intel_dp_drrs_initialize(struct intel_digital_port *intel_dig_port,
 		intel_connector->panel.edp_downclock =
 			intel_connector->panel.downclock_mode->clock;
 
+		mutex_init(&intel_dp->drrs_state.mutex);
+
 		intel_dp->drrs_state.drrs_support = dev_priv->vbt.drrs_mode;
 
 		intel_dp->drrs_state.drrs_refresh_rate_type = DRRS_HIGH_RR;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d208bf5..d1c60fa 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -291,6 +291,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
@@ -489,6 +492,7 @@  enum edp_drrs_refresh_rate_type {
 struct drrs_info {
 	enum drrs_support_type drrs_support;
 	enum edp_drrs_refresh_rate_type drrs_refresh_rate_type;
+	struct mutex mutex;
 };
 
 struct intel_dp {
@@ -761,7 +765,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);