Re: [PATCH RFC 5/5] drm/i915: Implement intel_crtc_toggle using atomic state

Maarten Lankhorst
2015-04-22T14:18:48Z

Op 22-04-15 om 13:24 schreef maarten.lankhorst@linux.intel.com:
> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> Assume the function is locked with drm_modeset_lock_all for now.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 131 ++++++++++-------------------------
> 1 file changed, 38 insertions(+), 93 deletions(-)
> <snip>
> static void ironlake_crtc_enable(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> @@ -5728,10 +5652,13 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
> void intel_crtc_control(struct drm_crtc *crtc, bool enable)
> {
> struct drm_device *dev = crtc->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - enum intel_display_power_domain domain;
> - unsigned long domains;
> + struct intel_crtc_state *pipe_config;
> + struct drm_plane_state *plane_state;
> + struct drm_atomic_state *state;
> + int ret;
>
> if (enable == intel_crtc->active)
> return;
> @@ -5740,23 +5667,41 @@ void intel_crtc_control(struct drm_crtc *crtc, bool enable)
> return;
>
> crtc->state->active = enable;
^Oops, this line should be removed now or this patch will glitch badly.

And a lot of places that use state->enable should use state->active instead.

Still leaving this here as it's a RFC. :-)
> - if (enable) {
> - domains = get_crtc_power_domains(crtc);
> - for_each_power_domain(domain, domains)
> - intel_display_power_get(dev_priv, domain);
> - intel_crtc->enabled_power_domains = domains;
>
> - dev_priv->display.crtc_enable(crtc);
> - intel_crtc_enable_planes(crtc);
> - } else {
> - intel_crtc_disable_planes(crtc);
> - dev_priv->display.crtc_disable(crtc);
> + /* this function should be called with drm_modeset_lock_all for now */
> + if (WARN_ON(!ctx))
> + return;
> + lockdep_assert_held(&ctx->ww_ctx);
>
> - domains = intel_crtc->enabled_power_domains;
> - for_each_power_domain(domain, domains)
> - intel_display_power_put(dev_priv, domain);
> - intel_crtc->enabled_power_domains = 0;
> + state = drm_atomic_state_alloc(dev);
> + if (WARN_ON(!state))
> + return;
> +
> + state->acquire_ctx = ctx;
> + state->allow_modeset = true;
> +
> + pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
> + if (IS_ERR(pipe_config)) {
> + ret = PTR_ERR(pipe_config);
> + goto err;
> + }
> + pipe_config->base.active = enable;
> +
> + plane_state = drm_atomic_get_plane_state(state, crtc->primary);
> + if (IS_ERR(plane_state)) {
> + ret = PTR_ERR(plane_state);
> + goto err;
> }
> +
> + ret = intel_set_mode(crtc, state);
> + if (!ret)
> + return;
> +
> + DRM_ERROR("Failed to toggle crtc!\n");
> +
> +err:
> + DRM_ERROR("Updating crtc active failed with %i\n", ret);
> + drm_atomic_state_free(state);
> }
>
> /**