On Dec-19-2013 5:21 PM, Jani Nikula wrote:
> On Tue, 17 Dec 2013, Vandana Kannan <vandana.kan...@intel.com> wrote:
>> From: Pradeep Bhat <pradeep.b...@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.b...@intel.com>
>> Signed-off-by: Vandana Kannan <vandana.kan...@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
> 

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

Reply via email to