| Message ID | 1387534258-5283-3-git-send-email-vandana.kannan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 20 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. > > 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. > > Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com> > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com> I'm sorry, this is not true. Only I get to say when you can add my Reviewed-by. Sometimes when the issues to be addressed are trivial, people say, "fix this and you have my Reviewed-by", and adding it is okay. Here, I was merely commenting on a few specific issues that I spotted on a cursory reading of the patch, but I did not review the patch, and I want my Reviewed-by carry some weight. Thank you for your understanding, Jani. > --- > drivers/gpu/drm/i915/intel_dp.c | 45 ++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 30 +++++++++++++++++++++++++ > 2 files changed, 75 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 8f17f8f..079b53f 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3522,6 +3522,46 @@ 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.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) { > + intel_connector->panel.edp_downclock_avail = true; > + intel_connector->panel.edp_downclock = > + intel_connector->panel.downclock_mode->clock; > + > + intel_dp->drrs_state.drrs_support = 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) > { > @@ -3535,6 +3575,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > struct drm_display_mode *scan; > struct edid *edid; > > + intel_dp->drrs_state.drrs_support = DRRS_NOT_SUPPORTED; > + > if (!is_edp(intel_dp)) > return true; > > @@ -3579,6 +3621,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 e903432..d208bf5 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -168,6 +168,9 @@ struct intel_panel { > bool active_low_pwm; > struct backlight_device *device; > } backlight; > + > + bool edp_downclock_avail; > + int edp_downclock; > }; > > struct intel_connector { > @@ -462,6 +465,32 @@ struct intel_hdmi { > > #define DP_MAX_DOWNSTREAM_PORTS 0x10 > > +/** > + * This enum is used to indicate the DRRS support type. > + */ > +enum drrs_support_type { > + DRRS_NOT_SUPPORTED = -1, > + STATIC_DRRS_SUPPORT = 0, /* 1:1 mapping with VBT */ > + SEAMLESS_DRRS_SUPPORT = 2 /* 1:1 mapping 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 { > + enum drrs_support_type drrs_support; > + enum edp_drrs_refresh_rate_type drrs_refresh_rate_type; > +}; > + > struct intel_dp { > uint32_t output_reg; > uint32_t aux_ch_ctl_reg; > @@ -487,6 +516,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 Fri, Dec 20, 2013 at 1:29 PM, Jani Nikula <jani.nikula@linux.intel.com> wrote: >> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> >> Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com> > > I'm sorry, this is not true. Only I get to say when you can add my > Reviewed-by. Yeah, you can't just add review-by comments. In the linux kernel this has a very specific meaning and can't just get handed out for "has looked at the patches". For details of what a reviewed-by tag means please see the "Reviewer's statement of oversight" in Documentation/SubmittingPatches. I've noticed that you've also done this with Ville's r-b tag for the picture ratio patch, I'll send out a note that this r-b isn't legit. -Daniel
On Dec-20-2013 7:35 PM, Daniel Vetter wrote: > On Fri, Dec 20, 2013 at 1:29 PM, Jani Nikula > <jani.nikula@linux.intel.com> wrote: >>> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com> >>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> >>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> >>> Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com> >> >> I'm sorry, this is not true. Only I get to say when you can add my >> Reviewed-by. > > Yeah, you can't just add review-by comments. In the linux kernel this > has a very specific meaning and can't just get handed out for "has > looked at the patches". For details of what a reviewed-by tag means > please see the "Reviewer's statement of oversight" in > Documentation/SubmittingPatches. I've noticed that you've also done > this with Ville's r-b tag for the picture ratio patch, I'll send out a > note that this r-b isn't legit. > -Daniel > Thanks for your inputs. I will remove the r-b tag from all DRRS related patches, keeping the details of versions/review comments incorporated in the commit message. Also, I will follow the same and resend the patch on picture aspect ratio. - Vandana
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 8f17f8f..079b53f 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3522,6 +3522,46 @@ 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.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) { + intel_connector->panel.edp_downclock_avail = true; + intel_connector->panel.edp_downclock = + intel_connector->panel.downclock_mode->clock; + + intel_dp->drrs_state.drrs_support = 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) { @@ -3535,6 +3575,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, struct drm_display_mode *scan; struct edid *edid; + intel_dp->drrs_state.drrs_support = DRRS_NOT_SUPPORTED; + if (!is_edp(intel_dp)) return true; @@ -3579,6 +3621,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 e903432..d208bf5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -168,6 +168,9 @@ struct intel_panel { bool active_low_pwm; struct backlight_device *device; } backlight; + + bool edp_downclock_avail; + int edp_downclock; }; struct intel_connector { @@ -462,6 +465,32 @@ struct intel_hdmi { #define DP_MAX_DOWNSTREAM_PORTS 0x10 +/** + * This enum is used to indicate the DRRS support type. + */ +enum drrs_support_type { + DRRS_NOT_SUPPORTED = -1, + STATIC_DRRS_SUPPORT = 0, /* 1:1 mapping with VBT */ + SEAMLESS_DRRS_SUPPORT = 2 /* 1:1 mapping 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 { + enum drrs_support_type drrs_support; + enum edp_drrs_refresh_rate_type drrs_refresh_rate_type; +}; + struct intel_dp { uint32_t output_reg; uint32_t aux_ch_ctl_reg; @@ -487,6 +516,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 {