Re: [PATCH v3 06/11] drm/i915/display: Share intel_connector_needs_modeset()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux