| Message ID | 1387258107-19232-3-git-send-email-vandana.kannan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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
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.
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
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 >
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 {