| Message ID | 1457945747-2161-2-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 14, 2016 at 10:55:40AM +0200, Ander Conselvan de Oliveira wrote: > LVDS is not cloneable, so the check is unnecessary. Removing it makes > the surrouding code a bit simpler. > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 31 ++++--------------------------- > 1 file changed, 4 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2d151ad..e7d6584 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8593,30 +8593,9 @@ static int ironlake_get_refclk(struct intel_crtc_state *crtc_state) > { > struct drm_device *dev = crtc_state->base.crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - struct drm_atomic_state *state = crtc_state->base.state; > - struct drm_connector *connector; > - struct drm_connector_state *connector_state; > - struct intel_encoder *encoder; > - int num_connectors = 0, i; > - bool is_lvds = false; > - > - for_each_connector_in_state(state, connector, connector_state, i) { > - if (connector_state->crtc != crtc_state->base.crtc) > - continue; > - > - encoder = to_intel_encoder(connector_state->best_encoder); > - > - switch (encoder->type) { > - case INTEL_OUTPUT_LVDS: > - is_lvds = true; > - break; > - default: > - break; > - } > - num_connectors++; > - } > > - if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) { We have the exact same checks in the gmch code as well. For consistency you should change those as well. > + if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) && > + intel_panel_use_ssc(dev_priv)) { > DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", > dev_priv->vbt.lvds_ssc_freq); > return dev_priv->vbt.lvds_ssc_freq; > @@ -8842,7 +8821,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, > struct drm_connector_state *connector_state; > struct intel_encoder *encoder; > uint32_t dpll; > - int factor, num_connectors = 0, i; > + int factor, i; > bool is_lvds = false, is_sdvo = false; > > for_each_connector_in_state(state, connector, connector_state, i) { > @@ -8862,8 +8841,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, > default: > break; > } > - > - num_connectors++; > } > > /* Enable autotuning of the PLL clock (if permissible) */ > @@ -8917,7 +8894,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, > break; > } > > - if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) > + if (is_lvds && intel_panel_use_ssc(dev_priv)) > dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN; > else > dpll |= PLL_REF_INPUT_DREFCLK; > -- > 2.4.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 2016-03-14 at 15:51 +0200, Ville Syrjälä wrote: > On Mon, Mar 14, 2016 at 10:55:40AM +0200, Ander Conselvan de Oliveira wrote: > > LVDS is not cloneable, so the check is unnecessary. Removing it makes > > the surrouding code a bit simpler. > > > > Signed-off-by: Ander Conselvan de Oliveira < > > ander.conselvan.de.oliveira@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 31 ++++--------------------------- > > 1 file changed, 4 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 2d151ad..e7d6584 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -8593,30 +8593,9 @@ static int ironlake_get_refclk(struct > > intel_crtc_state *crtc_state) > > { > > struct drm_device *dev = crtc_state->base.crtc->dev; > > struct drm_i915_private *dev_priv = dev->dev_private; > > - struct drm_atomic_state *state = crtc_state->base.state; > > - struct drm_connector *connector; > > - struct drm_connector_state *connector_state; > > - struct intel_encoder *encoder; > > - int num_connectors = 0, i; > > - bool is_lvds = false; > > - > > - for_each_connector_in_state(state, connector, connector_state, i) { > > - if (connector_state->crtc != crtc_state->base.crtc) > > - continue; > > - > > - encoder = to_intel_encoder(connector_state->best_encoder); > > - > > - switch (encoder->type) { > > - case INTEL_OUTPUT_LVDS: > > - is_lvds = true; > > - break; > > - default: > > - break; > > - } > > - num_connectors++; > > - } > > > > - if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) > > { > > We have the exact same checks in the gmch code as well. For consistency > you should change those as well. True. Would it be ok in a follow-up patch? I did that today now that I started doing some clean ups for i9xx_crtc_compute_clock(). If not, I can resend. Thanks, Ander > > > + if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) && > > + intel_panel_use_ssc(dev_priv)) { > > DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", > > dev_priv->vbt.lvds_ssc_freq); > > return dev_priv->vbt.lvds_ssc_freq; > > @@ -8842,7 +8821,7 @@ static uint32_t ironlake_compute_dpll(struct > > intel_crtc *intel_crtc, > > struct drm_connector_state *connector_state; > > struct intel_encoder *encoder; > > uint32_t dpll; > > - int factor, num_connectors = 0, i; > > + int factor, i; > > bool is_lvds = false, is_sdvo = false; > > > > for_each_connector_in_state(state, connector, connector_state, i) { > > @@ -8862,8 +8841,6 @@ static uint32_t ironlake_compute_dpll(struct > > intel_crtc *intel_crtc, > > default: > > break; > > } > > - > > - num_connectors++; > > } > > > > /* Enable autotuning of the PLL clock (if permissible) */ > > @@ -8917,7 +8894,7 @@ static uint32_t ironlake_compute_dpll(struct > > intel_crtc *intel_crtc, > > break; > > } > > > > - if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) > > + if (is_lvds && intel_panel_use_ssc(dev_priv)) > > dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN; > > else > > dpll |= PLL_REF_INPUT_DREFCLK; > > -- > > 2.4.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
On Mon, Mar 14, 2016 at 01:55:49PM +0000, Conselvan De Oliveira, Ander wrote: > On Mon, 2016-03-14 at 15:51 +0200, Ville Syrjälä wrote: > > On Mon, Mar 14, 2016 at 10:55:40AM +0200, Ander Conselvan de Oliveira wrote: > > > LVDS is not cloneable, so the check is unnecessary. Removing it makes > > > the surrouding code a bit simpler. > > > > > > Signed-off-by: Ander Conselvan de Oliveira < > > > ander.conselvan.de.oliveira@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 31 ++++--------------------------- > > > 1 file changed, 4 insertions(+), 27 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index 2d151ad..e7d6584 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -8593,30 +8593,9 @@ static int ironlake_get_refclk(struct > > > intel_crtc_state *crtc_state) > > > { > > > struct drm_device *dev = crtc_state->base.crtc->dev; > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > - struct drm_atomic_state *state = crtc_state->base.state; > > > - struct drm_connector *connector; > > > - struct drm_connector_state *connector_state; > > > - struct intel_encoder *encoder; > > > - int num_connectors = 0, i; > > > - bool is_lvds = false; > > > - > > > - for_each_connector_in_state(state, connector, connector_state, i) { > > > - if (connector_state->crtc != crtc_state->base.crtc) > > > - continue; > > > - > > > - encoder = to_intel_encoder(connector_state->best_encoder); > > > - > > > - switch (encoder->type) { > > > - case INTEL_OUTPUT_LVDS: > > > - is_lvds = true; > > > - break; > > > - default: > > > - break; > > > - } > > > - num_connectors++; > > > - } > > > > > > - if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) > > > { > > > > We have the exact same checks in the gmch code as well. For consistency > > you should change those as well. > > True. Would it be ok in a follow-up patch? I did that today now that I started > doing some clean ups for i9xx_crtc_compute_clock(). If not, I can resend. Followup is fine by me. I had some dpll code unification patches in my lvds_downclock branch too, feel free to steal any if you think they are helpful. Hoping I can reduce that branch as much as possuble before I start bombing the list with it :P > > Thanks, > Ander > > > > > > + if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) && > > > + intel_panel_use_ssc(dev_priv)) { > > > DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", > > > dev_priv->vbt.lvds_ssc_freq); > > > return dev_priv->vbt.lvds_ssc_freq; > > > @@ -8842,7 +8821,7 @@ static uint32_t ironlake_compute_dpll(struct > > > intel_crtc *intel_crtc, > > > struct drm_connector_state *connector_state; > > > struct intel_encoder *encoder; > > > uint32_t dpll; > > > - int factor, num_connectors = 0, i; > > > + int factor, i; > > > bool is_lvds = false, is_sdvo = false; > > > > > > for_each_connector_in_state(state, connector, connector_state, i) { > > > @@ -8862,8 +8841,6 @@ static uint32_t ironlake_compute_dpll(struct > > > intel_crtc *intel_crtc, > > > default: > > > break; > > > } > > > - > > > - num_connectors++; > > > } > > > > > > /* Enable autotuning of the PLL clock (if permissible) */ > > > @@ -8917,7 +8894,7 @@ static uint32_t ironlake_compute_dpll(struct > > > intel_crtc *intel_crtc, > > > break; > > > } > > > > > > - if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) > > > + if (is_lvds && intel_panel_use_ssc(dev_priv)) > > > dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN; > > > else > > > dpll |= PLL_REF_INPUT_DREFCLK; > > > -- > > > 2.4.3 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2d151ad..e7d6584 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8593,30 +8593,9 @@ static int ironlake_get_refclk(struct intel_crtc_state *crtc_state) { struct drm_device *dev = crtc_state->base.crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_atomic_state *state = crtc_state->base.state; - struct drm_connector *connector; - struct drm_connector_state *connector_state; - struct intel_encoder *encoder; - int num_connectors = 0, i; - bool is_lvds = false; - - for_each_connector_in_state(state, connector, connector_state, i) { - if (connector_state->crtc != crtc_state->base.crtc) - continue; - - encoder = to_intel_encoder(connector_state->best_encoder); - - switch (encoder->type) { - case INTEL_OUTPUT_LVDS: - is_lvds = true; - break; - default: - break; - } - num_connectors++; - } - if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) { + if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) && + intel_panel_use_ssc(dev_priv)) { DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", dev_priv->vbt.lvds_ssc_freq); return dev_priv->vbt.lvds_ssc_freq; @@ -8842,7 +8821,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, struct drm_connector_state *connector_state; struct intel_encoder *encoder; uint32_t dpll; - int factor, num_connectors = 0, i; + int factor, i; bool is_lvds = false, is_sdvo = false; for_each_connector_in_state(state, connector, connector_state, i) { @@ -8862,8 +8841,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, default: break; } - - num_connectors++; } /* Enable autotuning of the PLL clock (if permissible) */ @@ -8917,7 +8894,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, break; } - if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) + if (is_lvds && intel_panel_use_ssc(dev_priv)) dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN; else dpll |= PLL_REF_INPUT_DREFCLK;
LVDS is not cloneable, so the check is unnecessary. Removing it makes the surrouding code a bit simpler. Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-)