Re: [PATCH 12/20] drm/i915: Use atomic state in intel_ddi_crtc_get_new_encoder()

Daniel Vetter
2015-03-26T16:56:02Z

On Fri, Mar 20, 2015 at 04:18:12PM +0200, Ander Conselvan de Oliveira wrote:
> Instead of using connector->new_encoder, get the same information from
> the pipe_config, thus making the function ready for the atomic
> conversion.
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++++---------
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 8aee7d7..47b9307 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -492,17 +492,23 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
> }
>
> static struct intel_encoder *
> -intel_ddi_get_crtc_new_encoder(struct intel_crtc *crtc)
> +intel_ddi_get_crtc_new_encoder(struct intel_crtc_state *crtc_state)
> {
> - struct drm_device *dev = crtc->base.dev;
> - struct intel_encoder *intel_encoder, *ret = NULL;
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct intel_encoder *ret = NULL;
> + struct drm_atomic_state *state;
> int num_encoders = 0;
> + int i;
>
> - for_each_intel_encoder(dev, intel_encoder) {
> - if (intel_encoder->new_crtc == crtc) {
> - ret = intel_encoder;
> - num_encoders++;
> - }
> + state = crtc_state->base.state;
> +
> + for (i = 0; i < state->num_connector; i++) {
> + if (!state->connectors[i] ||
> + state->connector_states[i]->crtc != crtc_state->base.crtc)
> + continue;
> +
> + ret = to_intel_encoder(state->connector_states[i]->best_encoder);
> + num_encoders++;
> }
>
> WARN(num_encoders != 1, "%d encoders on crtc for pipe %c\n", num_encoders,
> @@ -1216,7 +1222,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
> {
> struct drm_device *dev = intel_crtc->base.dev;
> struct intel_encoder *intel_encoder =
> - intel_ddi_get_crtc_new_encoder(intel_crtc);
> + intel_ddi_get_crtc_new_encoder(crtc_state);

I think for atomic we need to change this further - the only reason we
want to look at the encoder is to figure out the type. And that can
actually change at runtime (hooray). What we really need is a
ddi_personality enum in the pipe_config which is set once in the encoder
compute_config stuff and then used everywhere in the ddi modeset code
where we currently use at encoder->type.

But that's an entire different project, this patch here is a suitable
direct conversion. Not sure whether we have a jira for this part already,
certainly should update the one that talks about modesets vs. probe races
(which this kinda is). There's also been multiple different patchsets
floating around that all got bikeshedded a bit somehow.
-Daniel

> int clock = crtc_state->port_clock;
>
> if (IS_SKYLAKE(dev))
> --
> 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