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

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

 



On Apr-04-2014 4:25 PM, Jani Nikula wrote:
> On Fri, 04 Apr 2014, Vandana Kannan <vandana.kannan@xxxxxxxxx> wrote:
>> From: Pradeep Bhat <pradeep.bhat@xxxxxxxxx>
>>
>> This patch and finds out the lowest refresh rate supported for the resolution
>> same as the fixed_mode.
>> 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.
>>
>> v2: Daniel's review comments
>> Modified downclock deduction based on intel_find_panel_downclock
>>
>> v3: Chris's review comments
>> Moved edp_downclock_avail and edp_downclock to intel_panel
>>
>> v4: Jani's review comments.
>> Changed name of the enum edp_panel_type to drrs_support type.
>> Change is_drrs_supported to drrs_support of type enum drrs_support_type.
>>
>> v5: Incorporated Jani's review comments
>> Modify intel_dp_drrs_initialize to return downclock mode. Support for Gen7
>> and above.
>>
>> v6: Incorporated Chris's review comments.
>> Changed initialize to init in intel_drrs_initialize
>>
>> v7: Incorporated Jani's review comments.
>> Removed edp_downclock and edp_downclock_avail. Return NULL explicitly.
>> Make drrs_state and unnamed struct. Move Gen based check inside drrs_init.
>>
>> v8: Made changes to track PSR enable/disable throughout system use (instead
>> of just in the init sequence) for disabling/enabling DRRS. Jani's review
>> comments.
>>
>> v9: PSR tracking will be done as part of idleness detection patch. Removed
>> PSR state tracker in i915_drrs. Jani's review comments.
>>
>> v10: Added log for DRRS not supported in drrs_init
>>
>> Signed-off-by: Pradeep Bhat <pradeep.bhat@xxxxxxxxx>
>> Signed-off-by: Vandana Kannan <vandana.kannan@xxxxxxxxx>
>> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c  | 45 +++++++++++++++++++++++++++++++++++++++-
>>  drivers/gpu/drm/i915/intel_drv.h | 16 ++++++++++++++
>>  2 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index e48d47c..9f110bd 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -3632,6 +3632,43 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>>  		      I915_READ(pp_div_reg));
>>  }
>>  
>> +static struct drm_display_mode *
>> +intel_dp_drrs_init(struct intel_digital_port *intel_dig_port,
>> +			struct intel_connector *intel_connector,
>> +			struct drm_display_mode *fixed_mode)
>> +{
>> +	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;
>> +	struct drm_display_mode *downclock_mode = NULL;
>> +
>> +	if (INTEL_INFO(dev)->gen <= 6) {
>> +		DRM_DEBUG_KMS("DRRS supported for Gen7 and above\n");
>> +		return NULL;
>> +	}
>> +
>> +	if (dev_priv->vbt.drrs_type == DRRS_NOT_SUPPORTED) {
>> +		DRM_INFO("VBT doesn't support DRRS\n");
>> +		return NULL;
>> +	}
>> +
>> +	downclock_mode = intel_find_panel_downclock
>> +					(dev, fixed_mode, connector);
>> +
>> +	if (downclock_mode != NULL &&
>> +		dev_priv->vbt.drrs_type == SEAMLESS_DRRS_SUPPORT) {
>> +		intel_dp->drrs_state.type = dev_priv->vbt.drrs_type;
>> +
>> +		intel_dp->drrs_state.refresh_rate_type = DRRS_HIGH_RR;
>> +		DRM_INFO("seamless DRRS supported for eDP panel.\n");
>> +		return downclock_mode;
>> +	} else {
>> +		DRM_INFO("DRRS not supported\n");
>> +		return NULL;
>> +	}
> 
> The above now leaks downclock_mode on the return NULL path. Please try
> to be more careful.
> 
> Please let's try to bring this patchset and review into
> conclusion. We'll both end up frustrated otherwise.
> 
> How about this instead?
> 
> 	if (dev_priv->vbt.drrs_type != SEAMLESS_DRRS_SUPPORT) {
> 		DRM_INFO("VBT doesn't support DRRS\n");
> 		return NULL;
> 	}
> 
> 	downclock_mode = intel_find_panel_downclock(dev, fixed_mode, connector);
> 	if (!downclock_mode) {
> 		DRM_INFO("DRRS not supported\n");
> 		return NULL;
> 	}
> 
> 	intel_dp->drrs_state.type = dev_priv->vbt.drrs_type;
> 	intel_dp->drrs_state.refresh_rate_type = DRRS_HIGH_RR;
> 	DRM_INFO("seamless DRRS supported for eDP panel.\n");
> 
> 	return downclock_mode;
> 
> 
> BR,
> Jani.
> 
> 
Thanks.
Will make this change and resend.
-Vandana
>> +}
>> +
>>  static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  				     struct intel_connector *intel_connector,
>>  				     struct edp_power_seq *power_seq)
>> @@ -3641,10 +3678,13 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct drm_display_mode *fixed_mode = NULL;
>> +	struct drm_display_mode *downclock_mode = NULL;
>>  	bool has_dpcd;
>>  	struct drm_display_mode *scan;
>>  	struct edid *edid;
>>  
>> +	intel_dp->drrs_state.type = DRRS_NOT_SUPPORTED;
>> +
>>  	if (!is_edp(intel_dp))
>>  		return true;
>>  
>> @@ -3687,6 +3727,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);
>> +			downclock_mode = intel_dp_drrs_init(
>> +						intel_dig_port,
>> +						intel_connector, fixed_mode);
>>  			break;
>>  		}
>>  	}
>> @@ -3700,7 +3743,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  	}
>>  	mutex_unlock(&dev->mode_config.mutex);
>>  
>> -	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>> +	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
>>  	intel_panel_setup_backlight(connector);
>>  
>>  	return true;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 42762b7..286d4fb 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -482,6 +482,17 @@ struct intel_hdmi {
>>  
>>  #define DP_MAX_DOWNSTREAM_PORTS		0x10
>>  
>> +/**
>> + * HIGH_RR is the highest eDP panel refresh rate read from EDID
>> + * LOW_RR is the lowest eDP panel refresh rate found from EDID
>> + * parsing for same resolution.
>> + */
>> +enum edp_drrs_refresh_rate_type {
>> +	DRRS_HIGH_RR,
>> +	DRRS_LOW_RR,
>> +	DRRS_MAX_RR, /* RR count */
>> +};
>> +
>>  struct intel_dp {
>>  	uint32_t output_reg;
>>  	uint32_t aux_ch_ctl_reg;
>> @@ -520,6 +531,11 @@ struct intel_dp {
>>  				     bool has_aux_irq,
>>  				     int send_bytes,
>>  				     uint32_t aux_clock_divider);
>> +	struct {
>> +		enum drrs_support_type type;
>> +		enum edp_drrs_refresh_rate_type refresh_rate_type;
>> +	} drrs_state;
>> +
>>  };
>>  
>>  struct intel_digital_port {
>> -- 
>> 1.9.1
>>
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux