| Message ID | 1420836965-10068-2-git-send-email-vandana.kannan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I believe we could start this re-org by moving it out to intel_drrs.c renaming functions and adding entry docbook entry. But anyway this patch is right and doesn't seem to change anything that is already working so free free to use: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> I'll continue the reviews tomorrow and intend to finish by Thursday. I'll also resend 2 patches that had conflicts with latest -nightly... On Fri, Jan 9, 2015 at 12:55 PM, Vandana Kannan <vandana.kannan@intel.com> wrote: > Earlier, DRRS structures were specific to eDP (used only in intel_dp). > Since DRRS can be extended to other internal display types > (if the panel supports multiple RR), modifying structures > to be part of drm_i915_private and have a provision to add display related > structs like intel_dp. > Also, aligning with frontbuffer tracking mechanism, the new structure > contains data for busy frontbuffer bits. > > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 32 ++++++++++++++++++------- > drivers/gpu/drm/i915/intel_dp.c | 50 ++++++++++++++++++---------------------- > drivers/gpu/drm/i915/intel_drv.h | 18 --------------- > 3 files changed, 47 insertions(+), 53 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8f771af..ed368ca 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -771,11 +771,33 @@ struct i915_fbc { > } no_fbc_reason; > }; > > -struct i915_drrs { > - struct intel_connector *connector; > +/** > + * 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 drrs_refresh_rate_type { > + DRRS_HIGH_RR, > + DRRS_LOW_RR, > + DRRS_MAX_RR, /* RR count */ > +}; > + > +enum drrs_support_type { > + DRRS_NOT_SUPPORTED = 0, > + STATIC_DRRS_SUPPORT = 1, > + SEAMLESS_DRRS_SUPPORT = 2 > }; > > struct intel_dp; > +struct i915_drrs { > + struct mutex mutex; > + struct delayed_work work; > + struct intel_dp *dp; > + unsigned busy_frontbuffer_bits; > + enum drrs_refresh_rate_type refresh_rate_type; > + enum drrs_support_type type; > +}; > + > struct i915_psr { > struct mutex lock; > bool sink_support; > @@ -1354,12 +1376,6 @@ struct ddi_vbt_port_info { > uint8_t supports_dp:1; > }; > > -enum drrs_support_type { > - DRRS_NOT_SUPPORTED = 0, > - STATIC_DRRS_SUPPORT = 1, > - SEAMLESS_DRRS_SUPPORT = 2 > -}; > - > enum psr_lines_to_wait { > PSR_0_LINES_TO_WAIT = 0, > PSR_1_LINE_TO_WAIT, > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 88d81a8..778dcd0 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1269,7 +1269,7 @@ found: > &pipe_config->dp_m_n); > > if (intel_connector->panel.downclock_mode != NULL && > - intel_dp->drrs_state.type == SEAMLESS_DRRS_SUPPORT) { > + dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) { > pipe_config->has_drrs = true; > intel_link_compute_m_n(bpp, lane_count, > intel_connector->panel.downclock_mode->clock, > @@ -4745,24 +4745,24 @@ 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) > +static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_encoder *encoder; > - struct intel_dp *intel_dp = NULL; > + struct intel_digital_port *dig_port = NULL; > + struct intel_dp *intel_dp = dev_priv->drrs.dp; > struct intel_crtc_config *config = NULL; > struct intel_crtc *intel_crtc = NULL; > - struct intel_connector *intel_connector = dev_priv->drrs.connector; > u32 reg, val; > - enum edp_drrs_refresh_rate_type index = DRRS_HIGH_RR; > + enum drrs_refresh_rate_type index = DRRS_HIGH_RR; > > if (refresh_rate <= 0) { > DRM_DEBUG_KMS("Refresh rate should be positive non-zero.\n"); > return; > } > > - if (intel_connector == NULL) { > - DRM_DEBUG_KMS("DRRS supported for eDP only.\n"); > + if (intel_dp == NULL) { > + DRM_DEBUG_KMS("DRRS not supported.\n"); > return; > } > > @@ -4771,8 +4771,8 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate) > * platforms that cannot have PSR and DRRS enabled at the same time. > */ > > - encoder = intel_attached_encoder(&intel_connector->base); > - intel_dp = enc_to_intel_dp(&encoder->base); > + dig_port = dp_to_dig_port(intel_dp); > + encoder = &dig_port->base; > intel_crtc = encoder->new_crtc; > > if (!intel_crtc) { > @@ -4782,15 +4782,16 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate) > > config = &intel_crtc->config; > > - if (intel_dp->drrs_state.type < SEAMLESS_DRRS_SUPPORT) { > + if (dev_priv->drrs.type < SEAMLESS_DRRS_SUPPORT) { > DRM_DEBUG_KMS("Only Seamless DRRS supported.\n"); > return; > } > > - if (intel_connector->panel.downclock_mode->vrefresh == refresh_rate) > + if (intel_dp->attached_connector->panel.downclock_mode->vrefresh == > + refresh_rate) > index = DRRS_LOW_RR; > > - if (index == intel_dp->drrs_state.refresh_rate_type) { > + if (index == dev_priv->drrs.refresh_rate_type) { > DRM_DEBUG_KMS( > "DRRS requested for previously set RR...ignoring\n"); > return; > @@ -4820,23 +4821,21 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate) > * possible calls from user space to set differnt RR are made. > */ > > - mutex_lock(&intel_dp->drrs_state.mutex); > + mutex_lock(&dev_priv->drrs.mutex); > > - intel_dp->drrs_state.refresh_rate_type = index; > + dev_priv->drrs.refresh_rate_type = index; > > - mutex_unlock(&intel_dp->drrs_state.mutex); > + mutex_unlock(&dev_priv->drrs.mutex); > > DRM_DEBUG_KMS("eDP Refresh Rate set to : %dHz\n", refresh_rate); > } > > 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) > +intel_dp_drrs_init(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_device *dev = connector->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_display_mode *downclock_mode = NULL; > > @@ -4858,13 +4857,11 @@ intel_dp_drrs_init(struct intel_digital_port *intel_dig_port, > return NULL; > } > > - dev_priv->drrs.connector = intel_connector; > - > - mutex_init(&intel_dp->drrs_state.mutex); > + mutex_init(&dev_priv->drrs.mutex); > > - intel_dp->drrs_state.type = dev_priv->vbt.drrs_type; > + dev_priv->drrs.type = dev_priv->vbt.drrs_type; > > - intel_dp->drrs_state.refresh_rate_type = DRRS_HIGH_RR; > + dev_priv->drrs.refresh_rate_type = DRRS_HIGH_RR; > DRM_DEBUG_KMS("seamless DRRS supported for eDP panel.\n"); > return downclock_mode; > } > @@ -4884,7 +4881,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > struct edid *edid; > enum pipe pipe = INVALID_PIPE; > > - intel_dp->drrs_state.type = DRRS_NOT_SUPPORTED; > + dev_priv->drrs.type = DRRS_NOT_SUPPORTED; > > if (!is_edp(intel_dp)) > return true; > @@ -4933,7 +4930,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > 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; > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index bb871f3..2ba045d 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -566,17 +566,6 @@ struct intel_hdmi { > struct intel_dp_mst_encoder; > #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; > @@ -632,12 +621,6 @@ 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; > - struct mutex mutex; > - } drrs_state; > - > }; > > struct intel_digital_port { > @@ -1006,7 +989,6 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp); > void intel_edp_panel_vdd_on(struct intel_dp *intel_dp); > void intel_edp_panel_on(struct intel_dp *intel_dp); > void intel_edp_panel_off(struct intel_dp *intel_dp); > -void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate); > void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector); > void intel_dp_mst_suspend(struct drm_device *dev); > void intel_dp_mst_resume(struct drm_device *dev); > -- > 2.0.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jan 13, 2015 at 05:27:01PM -0800, Rodrigo Vivi wrote: > I believe we could start this re-org by moving it out to intel_drrs.c > renaming functions and adding entry docbook entry. > > But anyway this patch is right and doesn't seem to change anything > that is already working so free free to use: > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > I'll continue the reviews tomorrow and intend to finish by Thursday. > I'll also resend 2 patches that had conflicts with latest -nightly... I merged this, but it causes a compiler warning. Which means I need the revised patches to get this all into shape asap or need to drop the patch again. -Daniel
On Thursday 22 January 2015 12:18 PM, Daniel Vetter wrote: > On Tue, Jan 13, 2015 at 05:27:01PM -0800, Rodrigo Vivi wrote: >> I believe we could start this re-org by moving it out to intel_drrs.c >> renaming functions and adding entry docbook entry. >> >> But anyway this patch is right and doesn't seem to change anything >> that is already working so free free to use: >> >> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> >> >> I'll continue the reviews tomorrow and intend to finish by Thursday. >> I'll also resend 2 patches that had conflicts with latest -nightly... > I merged this, but it causes a compiler warning. Which means I need the > revised patches to get this all into shape asap or need to drop the patch > again. I have just shared a new patches for the changes suggested except for igt and related debugfs entry patches [ 9 and 10]. Working on the required changes for igt and the debugfs interface. But I hope the changes required for igt need not stop other patches [1 to 8 of 10 patches]. > -Daniel --Ram
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8f771af..ed368ca 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -771,11 +771,33 @@ struct i915_fbc { } no_fbc_reason; }; -struct i915_drrs { - struct intel_connector *connector; +/** + * 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 drrs_refresh_rate_type { + DRRS_HIGH_RR, + DRRS_LOW_RR, + DRRS_MAX_RR, /* RR count */ +}; + +enum drrs_support_type { + DRRS_NOT_SUPPORTED = 0, + STATIC_DRRS_SUPPORT = 1, + SEAMLESS_DRRS_SUPPORT = 2 }; struct intel_dp; +struct i915_drrs { + struct mutex mutex; + struct delayed_work work; + struct intel_dp *dp; + unsigned busy_frontbuffer_bits; + enum drrs_refresh_rate_type refresh_rate_type; + enum drrs_support_type type; +}; + struct i915_psr { struct mutex lock; bool sink_support; @@ -1354,12 +1376,6 @@ struct ddi_vbt_port_info { uint8_t supports_dp:1; }; -enum drrs_support_type { - DRRS_NOT_SUPPORTED = 0, - STATIC_DRRS_SUPPORT = 1, - SEAMLESS_DRRS_SUPPORT = 2 -}; - enum psr_lines_to_wait { PSR_0_LINES_TO_WAIT = 0, PSR_1_LINE_TO_WAIT, diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 88d81a8..778dcd0 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1269,7 +1269,7 @@ found: &pipe_config->dp_m_n); if (intel_connector->panel.downclock_mode != NULL && - intel_dp->drrs_state.type == SEAMLESS_DRRS_SUPPORT) { + dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) { pipe_config->has_drrs = true; intel_link_compute_m_n(bpp, lane_count, intel_connector->panel.downclock_mode->clock, @@ -4745,24 +4745,24 @@ 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) +static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_encoder *encoder; - struct intel_dp *intel_dp = NULL; + struct intel_digital_port *dig_port = NULL; + struct intel_dp *intel_dp = dev_priv->drrs.dp; struct intel_crtc_config *config = NULL; struct intel_crtc *intel_crtc = NULL; - struct intel_connector *intel_connector = dev_priv->drrs.connector; u32 reg, val; - enum edp_drrs_refresh_rate_type index = DRRS_HIGH_RR; + enum drrs_refresh_rate_type index = DRRS_HIGH_RR; if (refresh_rate <= 0) { DRM_DEBUG_KMS("Refresh rate should be positive non-zero.\n"); return; } - if (intel_connector == NULL) { - DRM_DEBUG_KMS("DRRS supported for eDP only.\n"); + if (intel_dp == NULL) { + DRM_DEBUG_KMS("DRRS not supported.\n"); return; } @@ -4771,8 +4771,8 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate) * platforms that cannot have PSR and DRRS enabled at the same time. */ - encoder = intel_attached_encoder(&intel_connector->base); - intel_dp = enc_to_intel_dp(&encoder->base); + dig_port = dp_to_dig_port(intel_dp); + encoder = &dig_port->base; intel_crtc = encoder->new_crtc; if (!intel_crtc) { @@ -4782,15 +4782,16 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate) config = &intel_crtc->config; - if (intel_dp->drrs_state.type < SEAMLESS_DRRS_SUPPORT) { + if (dev_priv->drrs.type < SEAMLESS_DRRS_SUPPORT) { DRM_DEBUG_KMS("Only Seamless DRRS supported.\n"); return; } - if (intel_connector->panel.downclock_mode->vrefresh == refresh_rate) + if (intel_dp->attached_connector->panel.downclock_mode->vrefresh == + refresh_rate) index = DRRS_LOW_RR; - if (index == intel_dp->drrs_state.refresh_rate_type) { + if (index == dev_priv->drrs.refresh_rate_type) { DRM_DEBUG_KMS( "DRRS requested for previously set RR...ignoring\n"); return; @@ -4820,23 +4821,21 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate) * possible calls from user space to set differnt RR are made. */ - mutex_lock(&intel_dp->drrs_state.mutex); + mutex_lock(&dev_priv->drrs.mutex); - intel_dp->drrs_state.refresh_rate_type = index; + dev_priv->drrs.refresh_rate_type = index; - mutex_unlock(&intel_dp->drrs_state.mutex); + mutex_unlock(&dev_priv->drrs.mutex); DRM_DEBUG_KMS("eDP Refresh Rate set to : %dHz\n", refresh_rate); } 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) +intel_dp_drrs_init(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_device *dev = connector->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct drm_display_mode *downclock_mode = NULL; @@ -4858,13 +4857,11 @@ intel_dp_drrs_init(struct intel_digital_port *intel_dig_port, return NULL; } - dev_priv->drrs.connector = intel_connector; - - mutex_init(&intel_dp->drrs_state.mutex); + mutex_init(&dev_priv->drrs.mutex); - intel_dp->drrs_state.type = dev_priv->vbt.drrs_type; + dev_priv->drrs.type = dev_priv->vbt.drrs_type; - intel_dp->drrs_state.refresh_rate_type = DRRS_HIGH_RR; + dev_priv->drrs.refresh_rate_type = DRRS_HIGH_RR; DRM_DEBUG_KMS("seamless DRRS supported for eDP panel.\n"); return downclock_mode; } @@ -4884,7 +4881,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, struct edid *edid; enum pipe pipe = INVALID_PIPE; - intel_dp->drrs_state.type = DRRS_NOT_SUPPORTED; + dev_priv->drrs.type = DRRS_NOT_SUPPORTED; if (!is_edp(intel_dp)) return true; @@ -4933,7 +4930,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, 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; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index bb871f3..2ba045d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -566,17 +566,6 @@ struct intel_hdmi { struct intel_dp_mst_encoder; #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; @@ -632,12 +621,6 @@ 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; - struct mutex mutex; - } drrs_state; - }; struct intel_digital_port { @@ -1006,7 +989,6 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp); void intel_edp_panel_vdd_on(struct intel_dp *intel_dp); void intel_edp_panel_on(struct intel_dp *intel_dp); void intel_edp_panel_off(struct intel_dp *intel_dp); -void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate); void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector); void intel_dp_mst_suspend(struct drm_device *dev); void intel_dp_mst_resume(struct drm_device *dev);
Earlier, DRRS structures were specific to eDP (used only in intel_dp). Since DRRS can be extended to other internal display types (if the panel supports multiple RR), modifying structures to be part of drm_i915_private and have a provision to add display related structs like intel_dp. Also, aligning with frontbuffer tracking mechanism, the new structure contains data for busy frontbuffer bits. Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 32 ++++++++++++++++++------- drivers/gpu/drm/i915/intel_dp.c | 50 ++++++++++++++++++---------------------- drivers/gpu/drm/i915/intel_drv.h | 18 --------------- 3 files changed, 47 insertions(+), 53 deletions(-)