On Mon, 2019-12-16 at 15:52 -0800, Lucas De Marchi wrote: > On Mon, Dec 16, 2019 at 02:07:37PM -0800, Jose Souza wrote: > > intel_connector_needs_modeset() will be used outside of > > intel_display.c in a future patch so it would only be necessary to > > remove the state and add the prototype to the header file. > > > > But while at it, I simplified the arguments and moved it to a > > better > > place intel_atomic.c. > > > > That allowed us to convert the whole > > intel_encoders_update_prepare/complete to intel types too. > > > > No behavior changes intended here. > > > > v3: > > - removed digital from exported version of > > intel_connector_needs_modeset > > - rollback connector to drm type > > > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/display/intel_atomic.c | 18 +++++++ > > drivers/gpu/drm/i915/display/intel_atomic.h | 2 + > > drivers/gpu/drm/i915/display/intel_display.c | 53 ++++++----------- > > --- > > 3 files changed, 36 insertions(+), 37 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c > > b/drivers/gpu/drm/i915/display/intel_atomic.c > > index fd0026fc3618..b7dda18b6f29 100644 > > --- a/drivers/gpu/drm/i915/display/intel_atomic.c > > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > > @@ -174,6 +174,24 @@ intel_digital_connector_duplicate_state(struct > > drm_connector *connector) > > return &state->base; > > } > > > > +/** > > + * intel_connector_needs_modeset - check if connector needs a > > modeset > > + */ > > +bool > > +intel_connector_needs_modeset(struct intel_atomic_state *state, > > + struct drm_connector *connector) > > +{ > > + const struct drm_connector_state *old_conn_state, > > *new_conn_state; > > + > > + old_conn_state = drm_atomic_get_old_connector_state(&state- > > >base, connector); > > + new_conn_state = drm_atomic_get_new_connector_state(&state- > > >base, connector); > > + > > + return old_conn_state->crtc != new_conn_state->crtc || > > + (new_conn_state->crtc && > > + drm_atomic_crtc_needs_modeset(drm_atomic_get_new_crtc_s > > tate(&state->base, > > + > > new_conn_state->crtc))); > > +} > > + > > /** > > * intel_crtc_duplicate_state - duplicate crtc state > > * @crtc: drm crtc > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h > > b/drivers/gpu/drm/i915/display/intel_atomic.h > > index 7b49623419ba..a7d1a8576c48 100644 > > --- a/drivers/gpu/drm/i915/display/intel_atomic.h > > +++ b/drivers/gpu/drm/i915/display/intel_atomic.h > > @@ -32,6 +32,8 @@ int intel_digital_connector_atomic_check(struct > > drm_connector *conn, > > struct drm_atomic_state > > *state); > > struct drm_connector_state * > > intel_digital_connector_duplicate_state(struct drm_connector > > *connector); > > +bool intel_connector_needs_modeset(struct intel_atomic_state > > *state, > > + struct drm_connector *connector); > > > > struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc > > *crtc); > > void intel_crtc_destroy_state(struct drm_crtc *crtc, > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 8e3e05cfcb27..0ee2e86a8826 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -6185,71 +6185,50 @@ intel_connector_primary_encoder(struct > > intel_connector *connector) > > return encoder; > > } > > > > -static bool > > -intel_connector_needs_modeset(struct intel_atomic_state *state, > > - const struct drm_connector_state > > *old_conn_state, > > - const struct drm_connector_state > > *new_conn_state) > > -{ > > - struct intel_crtc *old_crtc = old_conn_state->crtc ? > > - to_intel_crtc(old_conn_state- > > >crtc) : NULL; > > - struct intel_crtc *new_crtc = new_conn_state->crtc ? > > - to_intel_crtc(new_conn_state- > > >crtc) : NULL; > > - > > - return new_crtc != old_crtc || > > - (new_crtc && > > - needs_modeset(intel_atomic_get_new_crtc_state(state, > > new_crtc))); > > -} > > - > > static void intel_encoders_update_prepare(struct intel_atomic_state > > *state) > > { > > - struct drm_connector_state *old_conn_state; > > - struct drm_connector_state *new_conn_state; > > - struct drm_connector *conn; > > + struct intel_digital_connector_state *new_connector_state; > > what's up with this fight new_conn_state vs new_connector_state? It' > s > the first time in the driver we would actually drive such conversion. > I'd say to just keep the old name. Ville asked me to rename conn to connector in the new functions that I'm adding as this function was heavily modified I did the same here. I will rollback if agreed by you and Ville(the ones helping with TGL MST) > > I'm not sure I like the conversion to use the connector rather than > the > connector_state because we are likely to have the state around in the > caller. Anyway, I'm ok with it. > > Lucas De Marchi > > > + struct intel_connector *connector; > > int i; > > > > - for_each_oldnew_connector_in_state(&state->base, conn, > > - old_conn_state, > > new_conn_state, i) { > > + for_each_new_intel_connector_in_state(state, connector, > > + new_connector_state, i) { > > struct intel_encoder *encoder; > > struct intel_crtc *crtc; > > > > - if (!intel_connector_needs_modeset(state, > > - old_conn_state, > > - new_conn_state)) > > + if (!intel_connector_needs_modeset(state, &connector- > > >base)) > > continue; > > > > - encoder = > > intel_connector_primary_encoder(to_intel_connector(conn)); > > + encoder = intel_connector_primary_encoder(connector); > > if (!encoder->update_prepare) > > continue; > > > > - crtc = new_conn_state->crtc ? > > - to_intel_crtc(new_conn_state->crtc) : NULL; > > + crtc = new_connector_state->base.crtc ? > > + to_intel_crtc(new_connector_state->base.crtc) : > > NULL; > > encoder->update_prepare(state, encoder, crtc); > > } > > } > > > > static void intel_encoders_update_complete(struct > > intel_atomic_state *state) > > { > > - struct drm_connector_state *old_conn_state; > > - struct drm_connector_state *new_conn_state; > > - struct drm_connector *conn; > > + struct intel_digital_connector_state *new_connector_state; > > + struct intel_connector *connector; > > int i; > > > > - for_each_oldnew_connector_in_state(&state->base, conn, > > - old_conn_state, > > new_conn_state, i) { > > + for_each_new_intel_connector_in_state(state, connector, > > + new_connector_state, i) { > > struct intel_encoder *encoder; > > struct intel_crtc *crtc; > > > > - if (!intel_connector_needs_modeset(state, > > - old_conn_state, > > - new_conn_state)) > > + if (!intel_connector_needs_modeset(state, &connector- > > >base)) > > continue; > > > > - encoder = > > intel_connector_primary_encoder(to_intel_connector(conn)); > > + encoder = intel_connector_primary_encoder(connector); > > if (!encoder->update_complete) > > continue; > > > > - crtc = new_conn_state->crtc ? > > - to_intel_crtc(new_conn_state->crtc) : NULL; > > + crtc = new_connector_state->base.crtc ? > > + to_intel_crtc(new_connector_state->base.crtc) : > > NULL; > > encoder->update_complete(state, encoder, crtc); > > } > > } > > -- > > 2.24.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx