Re: [PATCH v1.2 02/13] drm/atomic: Update legacy DPMS state during modesets, v3.

Daniel Vetter
2015-07-27T13:53:44Z

On Mon, Jul 27, 2015 at 01:24:29PM +0200, Maarten Lankhorst wrote:
> This is required for DPMS to work correctly, during a modeset
> the DPMS property should be turned off, unless the state is
> crtc is made active in which case it should be set to DPMS on.
>
> Changes since v1:
> - Set DPMS to off when a connector is removed from a crtc too.
> - Update the legacy dpms property too.
> - Add an exception for the legacy dpms paths, it updates its own state.
> Changes since v2:
> - Do not preserve dpms property.
>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Yeah I think that's the one, applied to drm-misc.

Thanks, Daniel

> ---
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 57847ae8ce8c..0b475fae067d 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -660,15 +660,29 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
> struct drm_crtc_state *old_crtc_state;
> int i;
>
> - /* clear out existing links */
> + /* clear out existing links and update dpms */
> for_each_connector_in_state(old_state, connector, old_conn_state, i) {
> - if (!connector->encoder)
> - continue;
> + if (connector->encoder) {
> + WARN_ON(!connector->encoder->crtc);
> +
> + connector->encoder->crtc = NULL;
> + connector->encoder = NULL;
> + }
>
> - WARN_ON(!connector->encoder->crtc);
> + crtc = connector->state->crtc;
> + if ((!crtc && old_conn_state->crtc) ||
> + (crtc && drm_atomic_crtc_needs_modeset(crtc->state))) {
> + struct drm_property *dpms_prop =
> + dev->mode_config.dpms_property;
> + int mode = DRM_MODE_DPMS_OFF;
>
> - connector->encoder->crtc = NULL;
> - connector->encoder = NULL;
> + if (crtc && crtc->state->active)
> + mode = DRM_MODE_DPMS_ON;
> +
> + connector->dpms = mode;
> + drm_object_property_set_value(&connector->base,
> + dpms_prop, mode);
> + }
> }
>
> /* set new links */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 13a6608be689..43b0f17ad1fa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12349,16 +12349,9 @@ intel_modeset_update_state(struct drm_atomic_state *state)
> continue;
>
> if (crtc->state->active) {
> - struct drm_property *dpms_property =
> - dev->mode_config.dpms_property;
> -
> - connector->dpms = DRM_MODE_DPMS_ON;
> - drm_object_property_set_value(&connector->base, dpms_property, DRM_MODE_DPMS_ON);
> -
> intel_encoder = to_intel_encoder(connector->encoder);
> intel_encoder->connectors_active = true;
> - } else
> - connector->dpms = DRM_MODE_DPMS_OFF;
> + }
> }
> }
>
>

--
Daniel Vetter
Software Engineer, Intel Corporation