[Intel-gfx] [PATCH v3 06/11] drm/i915/display: Share intel_connector_needs_modeset()
Lucas De Marchi
lucas.demarchi at intel.com
Tue Dec 17 17:37:03 UTC 2019
On Tue, Dec 17, 2019 at 08:39:15AM -0800, Jose Souza wrote:
>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 at linux.intel.com>
>> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
>> > ---
>> > 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 think that was about conn -> connector, not *conn_state ->
*connector_state, wasn't it?
Lucas De Marchi
>
>>
>> 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 at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list