We need to look at crtc->state->active for the legacy crc to match the drm crc api. Even though the drm api explictily checks it, there's no harm in doing the same there so keep it for both. Introduce a intel_crtc->ips_disabled flag for CRC, which is protected by crtc->mutex and set it to true while collecting. This way CRC will not interfere with ips, regardless of visibility. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101664 Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++-------------- drivers/gpu/drm/i915/intel_drv.h | 3 ++ drivers/gpu/drm/i915/intel_pipe_crc.c | 58 +++++++++++++++++++++++++++-------- 3 files changed, 71 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a8775ed817d1..88dfb4bd8db0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -117,7 +117,8 @@ static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force); static void ironlake_pfit_enable(struct intel_crtc *crtc); static void intel_modeset_setup_hw_state(struct drm_device *dev, struct drm_modeset_acquire_ctx *ctx); -static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc); +static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc, + struct intel_crtc_state *pipe_config); struct intel_limit { struct { @@ -2723,7 +2724,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, intel_set_plane_visible(to_intel_crtc_state(crtc_state), to_intel_plane_state(plane_state), false); - intel_pre_disable_primary_noatomic(&intel_crtc->base); + intel_pre_disable_primary_noatomic(&intel_crtc->base, + to_intel_crtc_state(crtc_state)); trace_intel_disable_plane(primary, intel_crtc); intel_plane->disable_plane(intel_plane, intel_crtc); @@ -4668,9 +4670,6 @@ void hsw_enable_ips(struct intel_crtc *crtc) struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); - if (!crtc->config->ips_enabled) - return; - /* * We can only enable IPS after we enable a plane and wait for a vblank * This function is called from post_plane_update, which is run after @@ -4706,9 +4705,6 @@ void hsw_disable_ips(struct intel_crtc *crtc) struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); - if (!crtc->config->ips_enabled) - return; - assert_plane_enabled(dev_priv, crtc->plane); if (IS_BROADWELL(dev_priv)) { mutex_lock(&dev_priv->rps.hw_lock); @@ -4754,7 +4750,7 @@ static void intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc) * completely hide the primary plane. */ static void -intel_post_enable_primary(struct drm_crtc *crtc) +intel_post_enable_primary(struct drm_crtc *crtc, struct intel_crtc_state *pipe_config) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -4767,7 +4763,8 @@ intel_post_enable_primary(struct drm_crtc *crtc) * when going from primary only to sprite only and vice * versa. */ - hsw_enable_ips(intel_crtc); + if (pipe_config->ips_enabled && !intel_crtc->ips_disabled) + hsw_enable_ips(intel_crtc); /* * Gen2 reports pipe underruns whenever all planes are disabled. @@ -4786,7 +4783,7 @@ intel_post_enable_primary(struct drm_crtc *crtc) /* FIXME move all this to pre_plane_update() with proper state tracking */ static void -intel_pre_disable_primary(struct drm_crtc *crtc) +intel_pre_disable_primary(struct drm_crtc *crtc, struct intel_crtc_state *pipe_config) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -4808,19 +4805,20 @@ intel_pre_disable_primary(struct drm_crtc *crtc) * when going from primary only to sprite only and vice * versa. */ - hsw_disable_ips(intel_crtc); + if (pipe_config->ips_enabled) + hsw_disable_ips(intel_crtc); } /* FIXME get rid of this and use pre_plane_update */ static void -intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) +intel_pre_disable_primary_noatomic(struct drm_crtc *crtc, struct intel_crtc_state *pipe_config) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int pipe = intel_crtc->pipe; - intel_pre_disable_primary(crtc); + intel_pre_disable_primary(crtc, pipe_config); /* * Vblank time updates from the shadow to live plane control register @@ -4858,7 +4856,7 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) if (new_primary_state->visible && (needs_modeset(&pipe_config->base) || !old_primary_state->visible)) - intel_post_enable_primary(&crtc->base); + intel_post_enable_primary(&crtc->base, pipe_config); } } @@ -4885,7 +4883,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, if (old_primary_state->visible && (modeset || !new_primary_state->visible)) - intel_pre_disable_primary(&crtc->base); + intel_pre_disable_primary(&crtc->base, pipe_config); } /* @@ -5700,13 +5698,6 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc, if (!intel_crtc->active) return; - if (crtc->primary->state->visible) { - intel_pre_disable_primary_noatomic(crtc); - - intel_crtc_disable_planes(crtc, 1 << drm_plane_index(crtc->primary)); - crtc->primary->state->visible = false; - } - state = drm_atomic_state_alloc(crtc->dev); if (!state) { DRM_DEBUG_KMS("failed to disable [CRTC:%d:%s], out of memory", @@ -5722,6 +5713,14 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc, WARN_ON(IS_ERR(crtc_state) || ret); + if (crtc->primary->state->visible) { + intel_pre_disable_primary_noatomic(crtc, crtc_state); + + intel_crtc_disable_planes(crtc, 1 << drm_plane_index(crtc->primary)); + crtc->primary->state->visible = false; + } + + dev_priv->display.crtc_disable(crtc_state, state); drm_atomic_state_put(state); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a07d06fbc4b4..872b1fce1d0e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -828,6 +828,9 @@ struct intel_crtc { bool cpu_fifo_underrun_disabled; bool pch_fifo_underrun_disabled; + /* Whether ips is force-disabled for collecting CRC */ + bool ips_disabled; + /* per-pipe watermark state */ struct { /* watermarks currently being used */ diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c index 8fbd2bd0877f..fc3682c4c055 100644 --- a/drivers/gpu/drm/i915/intel_pipe_crc.c +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c @@ -601,6 +601,33 @@ static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv, return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val); } +static int toggle_ips_and_test_active(struct intel_crtc *crtc, + bool has_source) +{ + bool has_ips = false; + int ret; + + + ret = drm_modeset_lock_interruptible(&crtc->base.mutex, NULL); + if (ret) + return ret; + + if (to_intel_crtc_state(crtc->base.state)->ips_enabled && + crtc->base.state->plane_mask & BIT(drm_plane_index(crtc->base.primary))) + has_ips = crtc->base.primary->state->visible; + + if (!crtc->base.state->active && has_source) { + DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n"); + ret = -EIO; + } + + crtc->ips_disabled = has_source; + + drm_modeset_unlock(&crtc->base.mutex); + + return ret ?: has_ips; +} + static int pipe_crc_set_source(struct drm_i915_private *dev_priv, enum pipe pipe, enum intel_pipe_crc_source source) @@ -610,6 +637,7 @@ static int pipe_crc_set_source(struct drm_i915_private *dev_priv, enum intel_display_power_domain power_domain; u32 val = 0; /* shut up gcc */ int ret; + int has_ips; if (pipe_crc->source == source) return 0; @@ -618,11 +646,12 @@ static int pipe_crc_set_source(struct drm_i915_private *dev_priv, if (pipe_crc->source && source) return -EINVAL; + has_ips = toggle_ips_and_test_active(crtc, source); + if (has_ips < 0) + return has_ips; + power_domain = POWER_DOMAIN_PIPE(pipe); - if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) { - DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n"); - return -EIO; - } + intel_display_power_get(dev_priv, power_domain); ret = get_new_crc_ctl_reg(dev_priv, pipe, &source, &val); if (ret != 0) @@ -649,7 +678,8 @@ static int pipe_crc_set_source(struct drm_i915_private *dev_priv, * user space can't make reliable use of the CRCs, so let's just * completely disable it. */ - hsw_disable_ips(crtc); + if (has_ips) + hsw_disable_ips(crtc); spin_lock_irq(&pipe_crc->lock); kfree(pipe_crc->entries); @@ -694,7 +724,8 @@ static int pipe_crc_set_source(struct drm_i915_private *dev_priv, else if (IS_HASWELL(dev_priv) && pipe == PIPE_A) hsw_trans_edp_pipe_A_crc_wa(dev_priv, false); - hsw_enable_ips(crtc); + if (has_ips) + hsw_enable_ips(crtc); } ret = 0; @@ -919,23 +950,25 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, enum intel_pipe_crc_source source; u32 val = 0; /* shut up gcc */ int ret = 0; + int has_ips; if (display_crc_ctl_parse_source(source_name, &source) < 0) { DRM_DEBUG_DRIVER("unknown source %s\n", source_name); return -EINVAL; } + has_ips = toggle_ips_and_test_active(intel_crtc, source); + if (has_ips < 0) + return has_ips; + power_domain = POWER_DOMAIN_PIPE(crtc->index); - if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) { - DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n"); - return -EIO; - } + intel_display_power_get(dev_priv, power_domain); ret = get_new_crc_ctl_reg(dev_priv, crtc->index, &source, &val); if (ret != 0) goto out; - if (source) { + if (source && has_ips) { /* * When IPS gets enabled, the pipe CRC changes. Since IPS gets * enabled and disabled dynamically based on package C states, @@ -956,7 +989,8 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, else if (IS_HASWELL(dev_priv) && crtc->index == PIPE_A) hsw_trans_edp_pipe_A_crc_wa(dev_priv, false); - hsw_enable_ips(intel_crtc); + if (has_ips) + hsw_enable_ips(intel_crtc); } pipe_crc->skipped = 0; -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
