On Mon, Jun 13, 2016 at 04:25:55PM +0200, Daniel Vetter wrote:
> On Wed, Jun 08, 2016 at 01:41:38PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Rather than looping through encoders to see which encoder types
> > are being driven by the pipe, add an output_types bitmask into
> > the crtc state and populate it prior to compute_config and during
> > state readout.
> >
> > v2: Determine output_types before .compute_config() hooks are called
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Not sure whether this will mesh with LSPCON perfectly, but the atomic way
> is to pass connector_state and crtc_state to all encoder functions. That's
> at least how the atomic helpers in the core work, and it imo works well.
> i915 isn't there yet, but we want that anyway.
>
> Once you have the connector_state, the connector is just a pointer away.
> And the functions which care can look up the connector type.
>
> I'd like to go that direction since it would align i915 more with core
> atomic. And that misalignment is imo a big reason for why our transition
> is so super painful.
We want the bitmask anyway since we want to figure things out about
the output type in places where we have no connector state (eg. DPLL
code , or when when we'd have to deal with multiple connector states
(cloning).
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++---------------------
> > drivers/gpu/drm/i915/intel_drv.h | 5 ++++
> > 2 files changed, 26 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 12ff79594bc1..eabace447b1c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -535,14 +535,7 @@ needs_modeset(struct drm_crtc_state *state)
> > */
> > bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
> > {
> > - struct drm_device *dev = crtc->base.dev;
> > - struct intel_encoder *encoder;
> > -
> > - for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> > - if (encoder->type == type)
> > - return true;
> > -
> > - return false;
> > + return crtc->config->output_types & (1 << type);
> > }
> >
> > /**
> > @@ -552,28 +545,9 @@ bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
> > * encoder->crtc.
> > */
> > static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
> > - int type)
> > + enum intel_output_type type)
> > {
> > - struct drm_atomic_state *state = crtc_state->base.state;
> > - struct drm_connector *connector;
> > - struct drm_connector_state *connector_state;
> > - struct intel_encoder *encoder;
> > - int i, num_connectors = 0;
> > -
> > - for_each_connector_in_state(state, connector, connector_state, i) {
> > - if (connector_state->crtc != crtc_state->base.crtc)
> > - continue;
> > -
> > - num_connectors++;
> > -
> > - encoder = to_intel_encoder(connector_state->best_encoder);
> > - if (encoder->type == type)
> > - return true;
> > - }
> > -
> > - WARN_ON(num_connectors == 0);
> > -
> > - return false;
> > + return crtc_state->output_types & (1 << type);
> > }
> >
> > /*
> > @@ -12529,6 +12503,19 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> > &pipe_config->pipe_src_w,
> > &pipe_config->pipe_src_h);
> >
> > + for_each_connector_in_state(state, connector, connector_state, i) {
> > + if (connector_state->crtc != crtc)
> > + continue;
> > +
> > + encoder = to_intel_encoder(connector_state->best_encoder);
> > +
> > + /*
> > + * Determine output_types before calling the .compute_config()
> > + * hooks so that the hooks can use this information safely.
> > + */
> > + pipe_config->output_types |= 1 << encoder->type;
> > + }
> > +
> > encoder_retry:
> > /* Ensure the port clock defaults are reset when retrying. */
> > pipe_config->port_clock = 0;
> > @@ -12826,6 +12813,7 @@ intel_pipe_config_compare(struct drm_device *dev,
> > PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
> >
> > PIPE_CONF_CHECK_I(has_dsi_encoder);
> > + PIPE_CONF_CHECK_X(output_types);
> >
> > PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
> > PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal);
> > @@ -13093,8 +13081,10 @@ verify_crtc_state(struct drm_crtc *crtc,
> > "Encoder connected to wrong pipe %c\n",
> > pipe_name(pipe));
> >
> > - if (active)
> > + if (active) {
> > + pipe_config->output_types |= 1 << encoder->type;
> > encoder->get_config(encoder, pipe_config);
> > + }
> > }
> >
> > if (!new_crtc_state->active)
> > @@ -15985,6 +15975,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> > if (encoder->get_hw_state(encoder, &pipe)) {
> > crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> > encoder->base.crtc = &crtc->base;
> > + crtc->config->output_types |= 1 << encoder->type;
> > encoder->get_config(encoder, crtc->config);
> > } else {
> > encoder->base.crtc = NULL;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index ebe7b3427e2e..3dae05e6d95b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -512,6 +512,11 @@ struct intel_crtc_state {
> > /* DSI has special cases */
> > bool has_dsi_encoder;
> >
> > + /* Bitmask of encoder types (enum intel_output_type)
> > + * driven by the pipe.
> > + */
> > + unsigned int output_types;
> > +
> > /* Whether we should send NULL infoframes. Required for audio. */
> > bool has_hdmi_sink;
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
