[Intel-gfx] [PATCH] drm/i915: use for_each_intel_connector_iter in intel_display.c
Daniel Vetter
daniel at ffwll.ch
Mon Dec 19 09:22:19 UTC 2016
On Mon, Dec 19, 2016 at 09:58:28AM +0100, Daniel Vetter wrote:
> This gets rid of the last users of for_each_intel_connector(), remove
> that too.
>
> The one exception is the loop in verify_encoder_state - that needs to
> switch over to for_each_connector_in_state.
>
> v2: Maarten corrected me that in the state verifier we indeed must
> only walk connectors in the atomic commit, not all of them!
Ok, CI just told me this was a stupid idea. Since we don't walk all
connectors, but still walk all encoders there's not plenty of state
mismatches (e.g. if you have 2 crtc with different connectors enabled and
you're doing a modeset on just one).
Not exactly sure how to best fix this, since replacing the encoder
walking with a connnector walking and then dereferencing
connector->state->best_encoder to get at only the encoders relevant for us
defeats the point of the cross check.
-Daniel
>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 -----
> drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++++++++---------
> 2 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 57914f008ed8..fe2b37fe0b91 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -488,11 +488,6 @@ struct i915_hotplug {
> &(dev)->mode_config.encoder_list, \
> base.head)
>
> -#define for_each_intel_connector(dev, intel_connector) \
> - list_for_each_entry(intel_connector, \
> - &(dev)->mode_config.connector_list, \
> - base.head)
> -
> #define for_each_intel_connector_iter(intel_connector, iter) \
> while ((intel_connector = to_intel_connector(drm_connector_list_iter_next(iter))))
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 438d27f93aca..9715c64eeb08 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12661,8 +12661,10 @@ static const struct drm_crtc_helper_funcs intel_helper_funcs = {
> static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
> {
> struct intel_connector *connector;
> + struct drm_connector_list_iter conn_iter;
>
> - for_each_intel_connector(dev, connector) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + for_each_intel_connector_iter(connector, &conn_iter) {
> if (connector->base.state->crtc)
> drm_connector_unreference(&connector->base);
>
> @@ -12678,6 +12680,7 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
> connector->base.state->crtc = NULL;
> }
> }
> + drm_connector_list_iter_put(&conn_iter);
> }
>
> static void
> @@ -13606,10 +13609,12 @@ verify_connector_state(struct drm_device *dev,
> }
>
> static void
> -verify_encoder_state(struct drm_device *dev)
> +verify_encoder_state(struct drm_device *dev, struct drm_atomic_state *state)
> {
> struct intel_encoder *encoder;
> - struct intel_connector *connector;
> + struct drm_connector *connector;
> + struct drm_connector_state *old_conn_state;
> + int i;
>
> for_each_intel_encoder(dev, encoder) {
> bool enabled = false;
> @@ -13619,12 +13624,12 @@ verify_encoder_state(struct drm_device *dev)
> encoder->base.base.id,
> encoder->base.name);
>
> - for_each_intel_connector(dev, connector) {
> - if (connector->base.state->best_encoder != &encoder->base)
> + for_each_connector_in_state(state, connector, old_conn_state, i) {
> + if (connector->state->best_encoder != &encoder->base)
> continue;
> enabled = true;
>
> - I915_STATE_WARN(connector->base.state->crtc !=
> + I915_STATE_WARN(connector->state->crtc !=
> encoder->base.crtc,
> "connector's crtc doesn't match encoder crtc\n");
> }
> @@ -13827,7 +13832,7 @@ static void
> intel_modeset_verify_disabled(struct drm_device *dev,
> struct drm_atomic_state *state)
> {
> - verify_encoder_state(dev);
> + verify_encoder_state(dev, state);
> verify_connector_state(dev, state, NULL);
> verify_disabled_dpll_state(dev);
> }
> @@ -16638,6 +16643,7 @@ int intel_modeset_init(struct drm_device *dev)
> static void intel_enable_pipe_a(struct drm_device *dev)
> {
> struct intel_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> struct drm_connector *crt = NULL;
> struct intel_load_detect_pipe load_detect_temp;
> struct drm_modeset_acquire_ctx *ctx = dev->mode_config.acquire_ctx;
> @@ -16645,12 +16651,14 @@ static void intel_enable_pipe_a(struct drm_device *dev)
> /* We can't just switch on the pipe A, we need to set things up with a
> * proper mode and output configuration. As a gross hack, enable pipe A
> * by enabling the load detect pipe once. */
> - for_each_intel_connector(dev, connector) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + for_each_intel_connector_iter(connector, &conn_iter) {
> if (connector->encoder->type == INTEL_OUTPUT_ANALOG) {
> crt = &connector->base;
> break;
> }
> }
> + drm_connector_list_iter_put(&conn_iter);
>
> if (!crt)
> return;
> @@ -16896,6 +16904,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> struct intel_crtc *crtc;
> struct intel_encoder *encoder;
> struct intel_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> int i;
>
> dev_priv->active_crtcs = 0;
> @@ -16973,7 +16982,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> pipe_name(pipe));
> }
>
> - for_each_intel_connector(dev, connector) {
> + drm_connector_list_iter_get(dev, &conn_iter);
> + for_each_intel_connector_iter(connector, &conn_iter) {
> if (connector->get_hw_state(connector)) {
> connector->base.dpms = DRM_MODE_DPMS_ON;
>
> @@ -17001,6 +17011,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> connector->base.base.id, connector->base.name,
> enableddisabled(connector->base.encoder));
> }
> + drm_connector_list_iter_put(&conn_iter);
>
> for_each_intel_crtc(dev, crtc) {
> crtc->base.hwmode = crtc->config->base.adjusted_mode;
> --
> 2.11.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list