| Message ID | 20241019200530.270738-4-aradhya.bhatia@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: cdns-dsi: Fix the color-shift issue | expand |
On Sun, Oct 20, 2024 at 01:35:28AM +0530, Aradhya Bhatia wrote: > From: Aradhya Bhatia <a-bhatia1@ti.com> > > The way any singular display pipeline, in need of a modeset, gets > enabled is as follows - > > CRTC Enable > All Bridge Pre-Enable > Encoder Enable > All Bridge Enable > > - and the disable path is exactly the reverse of this. > > The CRTC enable/disable occurs by looping over the old and new CRTC > states, while the bridge pre-enable, encoder enable, and bridge enable > occur by looping through the new connector states of the display > pipelines. > > At the moment, the encoder and bridge operations are grouped together > and occur under the same loops. > > Separate out the enable/disable loops of the encoder and bridge > operations into a different function, to make way for the re-ordering of > the enable and disable sequence of all these display elements. > > This patch doesn't alter in any way the ordering of CRTC/encoder/bridge > enable and disable, but merely helps to cleanly set up for the next > patch and to make sure that the bisectibility remains intact. > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > Signed-off-by: Aradhya Bhatia <aradhya.bhatia@linux.dev> > --- > drivers/gpu/drm/drm_atomic_helper.c | 97 +++++++++++++++++------------ > 1 file changed, 57 insertions(+), 40 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 5186d2114a50..7741fbcc8fc7 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1122,11 +1122,10 @@ crtc_needs_disable(struct drm_crtc_state *old_state, > } > > static void > -disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) > +encoder_bridge_chain_disable(struct drm_device *dev, struct drm_atomic_state *old_state) > { > struct drm_connector *connector; > struct drm_connector_state *old_conn_state, *new_conn_state; > - struct drm_crtc *crtc; > struct drm_crtc_state *old_crtc_state, *new_crtc_state; > int i; > > @@ -1189,6 +1188,16 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) > > drm_atomic_bridge_chain_post_disable(bridge, old_state); > } > +} > + > +static void > +disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) > +{ > + struct drm_crtc *crtc; > + struct drm_crtc_state *old_crtc_state, *new_crtc_state; > + int i; > + > + encoder_bridge_chain_disable(dev, old_state); > > for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { > const struct drm_crtc_helper_funcs *funcs; > @@ -1445,6 +1454,51 @@ static void drm_atomic_helper_commit_writebacks(struct drm_device *dev, > } > } > > +static void > +encoder_bridge_chain_enable(struct drm_device *dev, struct drm_atomic_state *old_state) > +{ > + struct drm_connector *connector; > + struct drm_connector_state *new_conn_state; > + int i; > + > + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { > + const struct drm_encoder_helper_funcs *funcs; > + struct drm_encoder *encoder; > + struct drm_bridge *bridge; > + > + if (!new_conn_state->best_encoder) > + continue; > + > + if (!new_conn_state->crtc->state->active || > + !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state)) > + continue; > + > + encoder = new_conn_state->best_encoder; > + funcs = encoder->helper_private; > + > + drm_dbg_atomic(dev, "enabling [ENCODER:%d:%s]\n", > + encoder->base.id, encoder->name); > + > + /* > + * Each encoder has at most one connector (since we always steal > + * it away), so we won't call enable hooks twice. > + */ > + bridge = drm_bridge_chain_get_first_bridge(encoder); > + drm_atomic_bridge_chain_pre_enable(bridge, old_state); > + > + if (funcs) { > + if (funcs->atomic_enable) > + funcs->atomic_enable(encoder, old_state); > + else if (funcs->enable) > + funcs->enable(encoder); > + else if (funcs->commit) > + funcs->commit(encoder); > + } > + > + drm_atomic_bridge_chain_enable(bridge, old_state); > + } > +} > + > /** > * drm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs > * @dev: DRM device > @@ -1465,8 +1519,6 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, > struct drm_crtc *crtc; > struct drm_crtc_state *old_crtc_state; > struct drm_crtc_state *new_crtc_state; > - struct drm_connector *connector; > - struct drm_connector_state *new_conn_state; > int i; > > for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { > @@ -1491,42 +1543,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, > } > } I'd say, if you want for it to be cleaner, split both loops from drm_atomic_helper_commit_modeset_enables() and call them separately. This save you from the code move, which is harder to review (and also helps with the function complexity). > > - for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { > - const struct drm_encoder_helper_funcs *funcs; > - struct drm_encoder *encoder; > - struct drm_bridge *bridge; > - > - if (!new_conn_state->best_encoder) > - continue; > - > - if (!new_conn_state->crtc->state->active || > - !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state)) > - continue; > - > - encoder = new_conn_state->best_encoder; > - funcs = encoder->helper_private; > - > - drm_dbg_atomic(dev, "enabling [ENCODER:%d:%s]\n", > - encoder->base.id, encoder->name); > - > - /* > - * Each encoder has at most one connector (since we always steal > - * it away), so we won't call enable hooks twice. > - */ > - bridge = drm_bridge_chain_get_first_bridge(encoder); > - drm_atomic_bridge_chain_pre_enable(bridge, old_state); > - > - if (funcs) { > - if (funcs->atomic_enable) > - funcs->atomic_enable(encoder, old_state); > - else if (funcs->enable) > - funcs->enable(encoder); > - else if (funcs->commit) > - funcs->commit(encoder); > - } > - > - drm_atomic_bridge_chain_enable(bridge, old_state); > - } > + encoder_bridge_chain_enable(dev, old_state); > > drm_atomic_helper_commit_writebacks(dev, old_state); > } > -- > 2.34.1 >
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 5186d2114a50..7741fbcc8fc7 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1122,11 +1122,10 @@ crtc_needs_disable(struct drm_crtc_state *old_state, } static void -disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) +encoder_bridge_chain_disable(struct drm_device *dev, struct drm_atomic_state *old_state) { struct drm_connector *connector; struct drm_connector_state *old_conn_state, *new_conn_state; - struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state, *new_crtc_state; int i; @@ -1189,6 +1188,16 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) drm_atomic_bridge_chain_post_disable(bridge, old_state); } +} + +static void +disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; + int i; + + encoder_bridge_chain_disable(dev, old_state); for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; @@ -1445,6 +1454,51 @@ static void drm_atomic_helper_commit_writebacks(struct drm_device *dev, } } +static void +encoder_bridge_chain_enable(struct drm_device *dev, struct drm_atomic_state *old_state) +{ + struct drm_connector *connector; + struct drm_connector_state *new_conn_state; + int i; + + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { + const struct drm_encoder_helper_funcs *funcs; + struct drm_encoder *encoder; + struct drm_bridge *bridge; + + if (!new_conn_state->best_encoder) + continue; + + if (!new_conn_state->crtc->state->active || + !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state)) + continue; + + encoder = new_conn_state->best_encoder; + funcs = encoder->helper_private; + + drm_dbg_atomic(dev, "enabling [ENCODER:%d:%s]\n", + encoder->base.id, encoder->name); + + /* + * Each encoder has at most one connector (since we always steal + * it away), so we won't call enable hooks twice. + */ + bridge = drm_bridge_chain_get_first_bridge(encoder); + drm_atomic_bridge_chain_pre_enable(bridge, old_state); + + if (funcs) { + if (funcs->atomic_enable) + funcs->atomic_enable(encoder, old_state); + else if (funcs->enable) + funcs->enable(encoder); + else if (funcs->commit) + funcs->commit(encoder); + } + + drm_atomic_bridge_chain_enable(bridge, old_state); + } +} + /** * drm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs * @dev: DRM device @@ -1465,8 +1519,6 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state; struct drm_crtc_state *new_crtc_state; - struct drm_connector *connector; - struct drm_connector_state *new_conn_state; int i; for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { @@ -1491,42 +1543,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, } } - for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { - const struct drm_encoder_helper_funcs *funcs; - struct drm_encoder *encoder; - struct drm_bridge *bridge; - - if (!new_conn_state->best_encoder) - continue; - - if (!new_conn_state->crtc->state->active || - !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state)) - continue; - - encoder = new_conn_state->best_encoder; - funcs = encoder->helper_private; - - drm_dbg_atomic(dev, "enabling [ENCODER:%d:%s]\n", - encoder->base.id, encoder->name); - - /* - * Each encoder has at most one connector (since we always steal - * it away), so we won't call enable hooks twice. - */ - bridge = drm_bridge_chain_get_first_bridge(encoder); - drm_atomic_bridge_chain_pre_enable(bridge, old_state); - - if (funcs) { - if (funcs->atomic_enable) - funcs->atomic_enable(encoder, old_state); - else if (funcs->enable) - funcs->enable(encoder); - else if (funcs->commit) - funcs->commit(encoder); - } - - drm_atomic_bridge_chain_enable(bridge, old_state); - } + encoder_bridge_chain_enable(dev, old_state); drm_atomic_helper_commit_writebacks(dev, old_state); }