Re: [PATCH 19/20] drm/i915: Remove usage of encoder->new_crtc from clock computations

Daniel Vetter
2015-03-27T09:38:28Z

On Fri, Mar 20, 2015 at 04:18:19PM +0200, Ander Conselvan de Oliveira wrote:
> Some of the crtc_compute_clock() still depended on encoder->new_crtc
> since they didn't use intel_pipe_will_have_type() and used an open
> coded version of that function instead. This patch replaces those with
> the appropriate code that checks the atomic state intead.
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++++++++++++-----------
> 1 file changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 26f4d30..89299b6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6695,11 +6695,18 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
> bool is_lvds = false, is_dsi = false;
> struct intel_encoder *encoder;
> const intel_limit_t *limit;
> + struct drm_atomic_state *state = crtc_state->base.state;
> + struct drm_connector_state *connector_state;
> + int i;
>
> - for_each_intel_encoder(dev, encoder) {
> - if (encoder->new_crtc != crtc)
> + for (i = 0; i < state->num_connector; i++) {
> + connector_state = state->connector_states[i];
> + if (!state->connectors[i] ||

Another case for a for_each_connector_in_state macro. I'll do the same
bikeshed as with the others.
-Daniel

> + connector_state->crtc != &crtc->base)
> continue;
>
> + encoder = to_intel_encoder(connector_state->best_encoder);
> +
> switch (encoder->type) {
> case INTEL_OUTPUT_LVDS:
> is_lvds = true;
> @@ -7373,18 +7380,24 @@ void intel_init_pch_refclk(struct drm_device *dev)
> lpt_init_pch_refclk(dev);
> }
>
> -static int ironlake_get_refclk(struct drm_crtc *crtc)
> +static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
> {
> - struct drm_device *dev = crtc->dev;
> + 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_state *connector_state;
> struct intel_encoder *encoder;
> - int num_connectors = 0;
> + int num_connectors = 0, i;
> bool is_lvds = false;
>
> - for_each_intel_encoder(dev, encoder) {
> - if (encoder->new_crtc != to_intel_crtc(crtc))
> + for (i = 0; i < state->num_connector; i++) {
> + connector_state = state->connector_states[i];
> + if (!state->connectors[i] ||
> + 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;
> @@ -7577,7 +7590,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>
> is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
>
> - refclk = ironlake_get_refclk(crtc);
> + refclk = ironlake_get_refclk(crtc_state);
>
> /*
> * Returns a set of divisors for the desired target clock with the given
> @@ -7632,16 +7645,22 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
> struct drm_crtc *crtc = &intel_crtc->base;
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_encoder *intel_encoder;
> + struct drm_atomic_state *state = crtc_state->base.state;
> + struct drm_connector_state *connector_state;
> + struct intel_encoder *encoder;
> uint32_t dpll;
> - int factor, num_connectors = 0;
> + int factor, num_connectors = 0, i;
> bool is_lvds = false, is_sdvo = false;
>
> - for_each_intel_encoder(dev, intel_encoder) {
> - if (intel_encoder->new_crtc != to_intel_crtc(crtc))
> + for (i = 0; i < state->num_connector; i++) {
> + connector_state = state->connector_states[i];
> + if (!state->connectors[i] ||
> + connector_state->crtc != crtc_state->base.crtc)
> continue;
>
> - switch (intel_encoder->type) {
> + encoder = to_intel_encoder(connector_state->best_encoder);
> +
> + switch (encoder->type) {
> case INTEL_OUTPUT_LVDS:
> is_lvds = true;
> break;
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation