[Intel-gfx] [PATCH 2/4] drm/atomic: Add drm_crtc_state->active

[Rob Clark]

Looks basically sane..  you have a branch on top of drm-next w/ this
patchset somewhere?  There is enough to it that I probably need to
actually try it out and see what explodes ;-)

couple minor comments below

On Tue, Jan 20, 2015 at 5:09 PM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
[snip]
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 541ba833ed36..a48f5c9690d5 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -404,12 +404,28 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>                 crtc = state->crtcs[i];
>                 crtc_state = state->crtc_states[i];
>
> -               if (!crtc || !crtc_state->mode_changed)
> +               if (!crtc)
>                         continue;
>
> -               DRM_DEBUG_KMS("[CRTC:%d] needs full modeset, enable: %c\n",
> +               /*
> +                * We must set ->active_changed after walking connectors for
> +                * otherwise and an update that only changes active would result

s/and// (I think?)

> +                * in a full modeset because update_connector_routing force
> +                * that.
> +                */
> +               if (crtc->state->active != crtc_state->active) {
> +                       DRM_DEBUG_KMS("[CRTC:%d] active changed\n",
> +                                     crtc->base.id);
> +                       crtc_state->active_changed = true;
> +               }
> +
> +               if (!(crtc->state->mode_changed || crtc->state->active_changed))
> +                       continue;

maybe a bit nit-picky, or matter of personal preference, but there are
a few mode_changed || active_changed, so maybe static inline
full_modeset(state) helper?