| Message ID | 20190416103603.8266-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,1/2] drm/i915: start moving state checker to intel_verify.c | expand |
On Tue, Apr 16, 2019 at 01:36:02PM +0300, Jani Nikula wrote: > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/intel_display.c | 474 +-------------------------- > drivers/gpu/drm/i915/intel_drv.h | 12 + > drivers/gpu/drm/i915/intel_verify.c | 464 ++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_verify.h | 22 ++ > 5 files changed, 510 insertions(+), 463 deletions(-) > create mode 100644 drivers/gpu/drm/i915/intel_verify.c > create mode 100644 drivers/gpu/drm/i915/intel_verify.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index fbcb0904..a000fad 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -166,6 +166,7 @@ i915-y += dvo_ch7017.o \ > intel_panel.o \ > intel_sdvo.o \ > intel_tv.o \ > + intel_verify.o \ > vlv_dsi.o \ > vlv_dsi_pll.o \ > intel_vdsc.o > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3bd40a..31a931 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -69,6 +69,7 @@ > #include "intel_sdvo.h" > #include "intel_sprite.h" > #include "intel_tv.h" > +#include "intel_verify.h" > > /* Primary plane formats for gen <= 3 */ > static const u32 i8xx_primary_formats[] = { > @@ -1243,7 +1244,7 @@ void assert_pipe(struct drm_i915_private *dev_priv, > pipe_name(pipe), onoff(state), onoff(cur_state)); > } > > -static void assert_plane(struct intel_plane *plane, bool state) > +void assert_plane(struct intel_plane *plane, bool state) > { > enum pipe pipe; > bool cur_state; > @@ -6607,45 +6608,6 @@ void intel_encoder_destroy(struct drm_encoder *encoder) > kfree(intel_encoder); > } > > -/* Cross check the actual hw state with our own modeset state tracking (and it's > - * internal consistency). */ > -static void intel_connector_verify_state(struct drm_crtc_state *crtc_state, > - struct drm_connector_state *conn_state) > -{ > - struct intel_connector *connector = to_intel_connector(conn_state->connector); > - > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > - connector->base.base.id, > - connector->base.name); > - > - if (connector->get_hw_state(connector)) { > - struct intel_encoder *encoder = connector->encoder; > - > - I915_STATE_WARN(!crtc_state, > - "connector enabled without attached crtc\n"); > - > - if (!crtc_state) > - return; > - > - I915_STATE_WARN(!crtc_state->active, > - "connector is active, but attached crtc isn't\n"); > - > - if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST) > - return; > - > - I915_STATE_WARN(conn_state->best_encoder != &encoder->base, > - "atomic encoder doesn't match attached encoder\n"); > - > - I915_STATE_WARN(conn_state->crtc != encoder->base.crtc, > - "attached encoder crtc differs from connector crtc\n"); > - } else { > - I915_STATE_WARN(crtc_state && crtc_state->active, > - "attached crtc is active, but connector isn't\n"); > - I915_STATE_WARN(!crtc_state && conn_state->best_encoder, > - "best encoder set without crtc!\n"); > - } > -} > - > static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state) > { > if (crtc_state->base.enable && crtc_state->has_pch_encoder) > @@ -6879,7 +6841,7 @@ static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config) > return pixel_rate; > } > > -static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state) > +void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); > > @@ -11584,9 +11546,9 @@ static const char *output_formats(enum intel_output_format format) > return output_format_str[format]; > } > > -static void intel_dump_pipe_config(struct intel_crtc *crtc, > - struct intel_crtc_state *pipe_config, > - const char *context) > +void intel_dump_pipe_config(struct intel_crtc *crtc, > + struct intel_crtc_state *pipe_config, > + const char *context) > { > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > @@ -12076,7 +12038,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv) > return false; > } > > -static bool > +bool > intel_pipe_config_compare(struct drm_i915_private *dev_priv, > struct intel_crtc_state *current_config, > struct intel_crtc_state *pipe_config, > @@ -12389,8 +12351,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv, > return ret; > } > > -static void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, > - const struct intel_crtc_state *pipe_config) > +void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, > + const struct intel_crtc_state *pipe_config) > { > if (pipe_config->has_pch_encoder) { > int fdi_dotclock = intel_dotclock_calculate(intel_fdi_link_freq(dev_priv, pipe_config), > @@ -12407,420 +12369,6 @@ static void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, > } > } > > -static void verify_wm_state(struct drm_crtc *crtc, > - struct drm_crtc_state *new_state) > -{ > - struct drm_i915_private *dev_priv = to_i915(crtc->dev); > - struct skl_hw_state { > - struct skl_ddb_entry ddb_y[I915_MAX_PLANES]; > - struct skl_ddb_entry ddb_uv[I915_MAX_PLANES]; > - struct skl_ddb_allocation ddb; > - struct skl_pipe_wm wm; > - } *hw; > - struct skl_ddb_allocation *sw_ddb; > - struct skl_pipe_wm *sw_wm; > - struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - const enum pipe pipe = intel_crtc->pipe; > - int plane, level, max_level = ilk_wm_max_level(dev_priv); > - > - if (INTEL_GEN(dev_priv) < 9 || !new_state->active) > - return; > - > - hw = kzalloc(sizeof(*hw), GFP_KERNEL); > - if (!hw) > - return; > - > - skl_pipe_wm_get_hw_state(intel_crtc, &hw->wm); > - sw_wm = &to_intel_crtc_state(new_state)->wm.skl.optimal; > - > - skl_pipe_ddb_get_hw_state(intel_crtc, hw->ddb_y, hw->ddb_uv); > - > - skl_ddb_get_hw_state(dev_priv, &hw->ddb); > - sw_ddb = &dev_priv->wm.skl_hw.ddb; > - > - if (INTEL_GEN(dev_priv) >= 11 && > - hw->ddb.enabled_slices != sw_ddb->enabled_slices) > - DRM_ERROR("mismatch in DBUF Slices (expected %u, got %u)\n", > - sw_ddb->enabled_slices, > - hw->ddb.enabled_slices); > - > - /* planes */ > - for_each_universal_plane(dev_priv, pipe, plane) { > - struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; > - > - hw_plane_wm = &hw->wm.planes[plane]; > - sw_plane_wm = &sw_wm->planes[plane]; > - > - /* Watermarks */ > - for (level = 0; level <= max_level; level++) { > - if (skl_wm_level_equals(&hw_plane_wm->wm[level], > - &sw_plane_wm->wm[level])) > - continue; > - > - DRM_ERROR("mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > - pipe_name(pipe), plane + 1, level, > - sw_plane_wm->wm[level].plane_en, > - sw_plane_wm->wm[level].plane_res_b, > - sw_plane_wm->wm[level].plane_res_l, > - hw_plane_wm->wm[level].plane_en, > - hw_plane_wm->wm[level].plane_res_b, > - hw_plane_wm->wm[level].plane_res_l); > - } > - > - if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, > - &sw_plane_wm->trans_wm)) { > - DRM_ERROR("mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > - pipe_name(pipe), plane + 1, > - sw_plane_wm->trans_wm.plane_en, > - sw_plane_wm->trans_wm.plane_res_b, > - sw_plane_wm->trans_wm.plane_res_l, > - hw_plane_wm->trans_wm.plane_en, > - hw_plane_wm->trans_wm.plane_res_b, > - hw_plane_wm->trans_wm.plane_res_l); > - } > - > - /* DDB */ > - hw_ddb_entry = &hw->ddb_y[plane]; > - sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[plane]; > - > - if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { > - DRM_ERROR("mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n", > - pipe_name(pipe), plane + 1, > - sw_ddb_entry->start, sw_ddb_entry->end, > - hw_ddb_entry->start, hw_ddb_entry->end); > - } > - } > - > - /* > - * cursor > - * If the cursor plane isn't active, we may not have updated it's ddb > - * allocation. In that case since the ddb allocation will be updated > - * once the plane becomes visible, we can skip this check > - */ > - if (1) { > - struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; > - > - hw_plane_wm = &hw->wm.planes[PLANE_CURSOR]; > - sw_plane_wm = &sw_wm->planes[PLANE_CURSOR]; > - > - /* Watermarks */ > - for (level = 0; level <= max_level; level++) { > - if (skl_wm_level_equals(&hw_plane_wm->wm[level], > - &sw_plane_wm->wm[level])) > - continue; > - > - DRM_ERROR("mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > - pipe_name(pipe), level, > - sw_plane_wm->wm[level].plane_en, > - sw_plane_wm->wm[level].plane_res_b, > - sw_plane_wm->wm[level].plane_res_l, > - hw_plane_wm->wm[level].plane_en, > - hw_plane_wm->wm[level].plane_res_b, > - hw_plane_wm->wm[level].plane_res_l); > - } > - > - if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, > - &sw_plane_wm->trans_wm)) { > - DRM_ERROR("mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > - pipe_name(pipe), > - sw_plane_wm->trans_wm.plane_en, > - sw_plane_wm->trans_wm.plane_res_b, > - sw_plane_wm->trans_wm.plane_res_l, > - hw_plane_wm->trans_wm.plane_en, > - hw_plane_wm->trans_wm.plane_res_b, > - hw_plane_wm->trans_wm.plane_res_l); > - } > - > - /* DDB */ > - hw_ddb_entry = &hw->ddb_y[PLANE_CURSOR]; > - sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[PLANE_CURSOR]; > - > - if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { > - DRM_ERROR("mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n", > - pipe_name(pipe), > - sw_ddb_entry->start, sw_ddb_entry->end, > - hw_ddb_entry->start, hw_ddb_entry->end); > - } > - } > - > - kfree(hw); > -} > - > -static void > -verify_connector_state(struct drm_device *dev, > - struct drm_atomic_state *state, > - struct drm_crtc *crtc) > -{ > - struct drm_connector *connector; > - struct drm_connector_state *new_conn_state; > - int i; > - > - for_each_new_connector_in_state(state, connector, new_conn_state, i) { > - struct drm_encoder *encoder = connector->encoder; > - struct drm_crtc_state *crtc_state = NULL; > - > - if (new_conn_state->crtc != crtc) > - continue; > - > - if (crtc) > - crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc); > - > - intel_connector_verify_state(crtc_state, new_conn_state); > - > - I915_STATE_WARN(new_conn_state->best_encoder != encoder, > - "connector's atomic encoder doesn't match legacy encoder\n"); > - } > -} > - > -static void > -verify_encoder_state(struct drm_device *dev, struct drm_atomic_state *state) > -{ > - struct intel_encoder *encoder; > - struct drm_connector *connector; > - struct drm_connector_state *old_conn_state, *new_conn_state; > - int i; > - > - for_each_intel_encoder(dev, encoder) { > - bool enabled = false, found = false; > - enum pipe pipe; > - > - DRM_DEBUG_KMS("[ENCODER:%d:%s]\n", > - encoder->base.base.id, > - encoder->base.name); > - > - for_each_oldnew_connector_in_state(state, connector, old_conn_state, > - new_conn_state, i) { > - if (old_conn_state->best_encoder == &encoder->base) > - found = true; > - > - if (new_conn_state->best_encoder != &encoder->base) > - continue; > - found = enabled = true; > - > - I915_STATE_WARN(new_conn_state->crtc != > - encoder->base.crtc, > - "connector's crtc doesn't match encoder crtc\n"); > - } > - > - if (!found) > - continue; > - > - I915_STATE_WARN(!!encoder->base.crtc != enabled, > - "encoder's enabled state mismatch " > - "(expected %i, found %i)\n", > - !!encoder->base.crtc, enabled); > - > - if (!encoder->base.crtc) { > - bool active; > - > - active = encoder->get_hw_state(encoder, &pipe); > - I915_STATE_WARN(active, > - "encoder detached but still enabled on pipe %c.\n", > - pipe_name(pipe)); > - } > - } > -} > - > -static void > -verify_crtc_state(struct drm_crtc *crtc, > - struct drm_crtc_state *old_crtc_state, > - struct drm_crtc_state *new_crtc_state) > -{ > - struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_encoder *encoder; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_crtc_state *pipe_config, *sw_config; > - struct drm_atomic_state *old_state; > - bool active; > - > - old_state = old_crtc_state->state; > - __drm_atomic_helper_crtc_destroy_state(old_crtc_state); > - pipe_config = to_intel_crtc_state(old_crtc_state); > - memset(pipe_config, 0, sizeof(*pipe_config)); > - pipe_config->base.crtc = crtc; > - pipe_config->base.state = old_state; > - > - DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); > - > - active = dev_priv->display.get_pipe_config(intel_crtc, pipe_config); > - > - /* we keep both pipes enabled on 830 */ > - if (IS_I830(dev_priv)) > - active = new_crtc_state->active; > - > - I915_STATE_WARN(new_crtc_state->active != active, > - "crtc active state doesn't match with hw state " > - "(expected %i, found %i)\n", new_crtc_state->active, active); > - > - I915_STATE_WARN(intel_crtc->active != new_crtc_state->active, > - "transitional active state does not match atomic hw state " > - "(expected %i, found %i)\n", new_crtc_state->active, intel_crtc->active); > - > - for_each_encoder_on_crtc(dev, crtc, encoder) { > - enum pipe pipe; > - > - active = encoder->get_hw_state(encoder, &pipe); > - I915_STATE_WARN(active != new_crtc_state->active, > - "[ENCODER:%i] active %i with crtc active %i\n", > - encoder->base.base.id, active, new_crtc_state->active); > - > - I915_STATE_WARN(active && intel_crtc->pipe != pipe, > - "Encoder connected to wrong pipe %c\n", > - pipe_name(pipe)); > - > - if (active) > - encoder->get_config(encoder, pipe_config); > - } > - > - intel_crtc_compute_pixel_rate(pipe_config); > - > - if (!new_crtc_state->active) > - return; > - > - intel_pipe_config_sanity_check(dev_priv, pipe_config); > - > - sw_config = to_intel_crtc_state(new_crtc_state); > - if (!intel_pipe_config_compare(dev_priv, sw_config, > - pipe_config, false)) { > - I915_STATE_WARN(1, "pipe state doesn't match!\n"); > - intel_dump_pipe_config(intel_crtc, pipe_config, > - "[hw state]"); > - intel_dump_pipe_config(intel_crtc, sw_config, > - "[sw state]"); > - } > -} > - > -static void > -intel_verify_planes(struct intel_atomic_state *state) > -{ > - struct intel_plane *plane; > - const struct intel_plane_state *plane_state; > - int i; > - > - for_each_new_intel_plane_in_state(state, plane, > - plane_state, i) > - assert_plane(plane, plane_state->slave || > - plane_state->base.visible); > -} > - > -static void > -verify_single_dpll_state(struct drm_i915_private *dev_priv, > - struct intel_shared_dpll *pll, > - struct drm_crtc *crtc, > - struct drm_crtc_state *new_state) > -{ > - struct intel_dpll_hw_state dpll_hw_state; > - unsigned int crtc_mask; > - bool active; > - > - memset(&dpll_hw_state, 0, sizeof(dpll_hw_state)); > - > - DRM_DEBUG_KMS("%s\n", pll->info->name); > - > - active = pll->info->funcs->get_hw_state(dev_priv, pll, &dpll_hw_state); > - > - if (!(pll->info->flags & INTEL_DPLL_ALWAYS_ON)) { > - I915_STATE_WARN(!pll->on && pll->active_mask, > - "pll in active use but not on in sw tracking\n"); > - I915_STATE_WARN(pll->on && !pll->active_mask, > - "pll is on but not used by any active crtc\n"); > - I915_STATE_WARN(pll->on != active, > - "pll on state mismatch (expected %i, found %i)\n", > - pll->on, active); > - } > - > - if (!crtc) { > - I915_STATE_WARN(pll->active_mask & ~pll->state.crtc_mask, > - "more active pll users than references: %x vs %x\n", > - pll->active_mask, pll->state.crtc_mask); > - > - return; > - } > - > - crtc_mask = drm_crtc_mask(crtc); > - > - if (new_state->active) > - I915_STATE_WARN(!(pll->active_mask & crtc_mask), > - "pll active mismatch (expected pipe %c in active mask 0x%02x)\n", > - pipe_name(drm_crtc_index(crtc)), pll->active_mask); > - else > - I915_STATE_WARN(pll->active_mask & crtc_mask, > - "pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n", > - pipe_name(drm_crtc_index(crtc)), pll->active_mask); > - > - I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask), > - "pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n", > - crtc_mask, pll->state.crtc_mask); > - > - I915_STATE_WARN(pll->on && memcmp(&pll->state.hw_state, > - &dpll_hw_state, > - sizeof(dpll_hw_state)), > - "pll hw state mismatch\n"); > -} > - > -static void > -verify_shared_dpll_state(struct drm_device *dev, struct drm_crtc *crtc, > - struct drm_crtc_state *old_crtc_state, > - struct drm_crtc_state *new_crtc_state) > -{ > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_crtc_state *old_state = to_intel_crtc_state(old_crtc_state); > - struct intel_crtc_state *new_state = to_intel_crtc_state(new_crtc_state); > - > - if (new_state->shared_dpll) > - verify_single_dpll_state(dev_priv, new_state->shared_dpll, crtc, new_crtc_state); > - > - if (old_state->shared_dpll && > - old_state->shared_dpll != new_state->shared_dpll) { > - unsigned int crtc_mask = drm_crtc_mask(crtc); > - struct intel_shared_dpll *pll = old_state->shared_dpll; > - > - I915_STATE_WARN(pll->active_mask & crtc_mask, > - "pll active mismatch (didn't expect pipe %c in active mask)\n", > - pipe_name(drm_crtc_index(crtc))); > - I915_STATE_WARN(pll->state.crtc_mask & crtc_mask, > - "pll enabled crtcs mismatch (found %x in enabled mask)\n", > - pipe_name(drm_crtc_index(crtc))); > - } > -} > - > -static void > -intel_modeset_verify_crtc(struct drm_crtc *crtc, > - struct drm_atomic_state *state, > - struct drm_crtc_state *old_state, > - struct drm_crtc_state *new_state) > -{ > - if (!needs_modeset(new_state) && > - !to_intel_crtc_state(new_state)->update_pipe) > - return; > - > - verify_wm_state(crtc, new_state); > - verify_connector_state(crtc->dev, state, crtc); > - verify_crtc_state(crtc, old_state, new_state); > - verify_shared_dpll_state(crtc->dev, crtc, old_state, new_state); > -} > - > -static void > -verify_disabled_dpll_state(struct drm_device *dev) > -{ > - struct drm_i915_private *dev_priv = to_i915(dev); > - int i; > - > - for (i = 0; i < dev_priv->num_shared_dpll; i++) > - verify_single_dpll_state(dev_priv, &dev_priv->shared_dplls[i], NULL, NULL); > -} > - > -static void > -intel_modeset_verify_disabled(struct drm_device *dev, > - struct drm_atomic_state *state) > -{ > - verify_encoder_state(dev, state); > - verify_connector_state(dev, state, NULL); > - verify_disabled_dpll_state(dev); > -} > - > static void update_scanline_offset(const struct intel_crtc_state *crtc_state) > { > struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > @@ -13521,7 +13069,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > if (!intel_can_enable_sagv(state)) > intel_disable_sagv(dev_priv); > > - intel_modeset_verify_disabled(dev, state); > + intel_verify_modeset_disabled(dev, state); > } I think we should take this opportunity to slightly clean up the interface here, and align a bit with drm_atomic_helper_commit_modeset_enables/disables from helpers. So rename this function here to intel_verify_modeset_disables(dev, state) so the naming is a bit clearer - it took me a while what exactly _disabled here means: Is the verifier disabled, or the modeset is disabled or what exactly. I think the idea of "modeset_disables" as a phase of an atomic commit is better established. > > /* Complete the events for pipes that have now been disabled */ > @@ -13589,7 +13137,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > if (put_domains[i]) > modeset_put_power_domains(dev_priv, put_domains[i]); > > - intel_modeset_verify_crtc(crtc, state, old_crtc_state, new_crtc_state); > + intel_verify_modeset_crtc(crtc, state, old_crtc_state, new_crtc_state); > } Same here, create a new intel_verify_modeset_enables(dev, state) which combines the crtc and plan checks (the additional loop wont matter), and neatly aligns with the idea of a "modeset_enables" phase from atomic helpers. With that I think we'll have some really tidy interface into intel_verify.c, and your patch here has my Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cheers, Daniel > > if (intel_state->modeset) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index a38b9c..77767c 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1668,6 +1668,17 @@ int intel_get_pipe_from_crtc_id_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv); > enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv, > enum pipe pipe); > +void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, > + const struct intel_crtc_state *pipe_config); > +bool > +intel_pipe_config_compare(struct drm_i915_private *dev_priv, > + struct intel_crtc_state *current_config, > + struct intel_crtc_state *pipe_config, > + bool adjust); > +void intel_dump_pipe_config(struct intel_crtc *crtc, > + struct intel_crtc_state *pipe_config, > + const char *context); > +void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state); > static inline bool > intel_crtc_has_type(const struct intel_crtc_state *crtc_state, > enum intel_output_type type) > @@ -1760,6 +1771,7 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv, > void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, bool state); > #define assert_pipe_enabled(d, p) assert_pipe(d, p, true) > #define assert_pipe_disabled(d, p) assert_pipe(d, p, false) > +void assert_plane(struct intel_plane *plane, bool state); > void intel_prepare_reset(struct drm_i915_private *dev_priv); > void intel_finish_reset(struct drm_i915_private *dev_priv); > void hsw_enable_pc8(struct drm_i915_private *dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_verify.c b/drivers/gpu/drm/i915/intel_verify.c > new file mode 100644 > index 000000..4c8990 > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_verify.c > @@ -0,0 +1,464 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#include <drm/drm_atomic_state_helper.h> > + > +#include "i915_drv.h" > +#include "intel_drv.h" > +#include "intel_pm.h" > +#include "intel_verify.h" > + > +static void verify_wm_state(struct drm_crtc *crtc, > + struct drm_crtc_state *new_state) > +{ > + struct drm_i915_private *dev_priv = to_i915(crtc->dev); > + struct skl_hw_state { > + struct skl_ddb_entry ddb_y[I915_MAX_PLANES]; > + struct skl_ddb_entry ddb_uv[I915_MAX_PLANES]; > + struct skl_ddb_allocation ddb; > + struct skl_pipe_wm wm; > + } *hw; > + struct skl_ddb_allocation *sw_ddb; > + struct skl_pipe_wm *sw_wm; > + struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + const enum pipe pipe = intel_crtc->pipe; > + int plane, level, max_level = ilk_wm_max_level(dev_priv); > + > + if (INTEL_GEN(dev_priv) < 9 || !new_state->active) > + return; > + > + hw = kzalloc(sizeof(*hw), GFP_KERNEL); > + if (!hw) > + return; > + > + skl_pipe_wm_get_hw_state(intel_crtc, &hw->wm); > + sw_wm = &to_intel_crtc_state(new_state)->wm.skl.optimal; > + > + skl_pipe_ddb_get_hw_state(intel_crtc, hw->ddb_y, hw->ddb_uv); > + > + skl_ddb_get_hw_state(dev_priv, &hw->ddb); > + sw_ddb = &dev_priv->wm.skl_hw.ddb; > + > + if (INTEL_GEN(dev_priv) >= 11 && > + hw->ddb.enabled_slices != sw_ddb->enabled_slices) > + DRM_ERROR("mismatch in DBUF Slices (expected %u, got %u)\n", > + sw_ddb->enabled_slices, > + hw->ddb.enabled_slices); > + > + /* planes */ > + for_each_universal_plane(dev_priv, pipe, plane) { > + struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; > + > + hw_plane_wm = &hw->wm.planes[plane]; > + sw_plane_wm = &sw_wm->planes[plane]; > + > + /* Watermarks */ > + for (level = 0; level <= max_level; level++) { > + if (skl_wm_level_equals(&hw_plane_wm->wm[level], > + &sw_plane_wm->wm[level])) > + continue; > + > + DRM_ERROR("mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > + pipe_name(pipe), plane + 1, level, > + sw_plane_wm->wm[level].plane_en, > + sw_plane_wm->wm[level].plane_res_b, > + sw_plane_wm->wm[level].plane_res_l, > + hw_plane_wm->wm[level].plane_en, > + hw_plane_wm->wm[level].plane_res_b, > + hw_plane_wm->wm[level].plane_res_l); > + } > + > + if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, > + &sw_plane_wm->trans_wm)) { > + DRM_ERROR("mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > + pipe_name(pipe), plane + 1, > + sw_plane_wm->trans_wm.plane_en, > + sw_plane_wm->trans_wm.plane_res_b, > + sw_plane_wm->trans_wm.plane_res_l, > + hw_plane_wm->trans_wm.plane_en, > + hw_plane_wm->trans_wm.plane_res_b, > + hw_plane_wm->trans_wm.plane_res_l); > + } > + > + /* DDB */ > + hw_ddb_entry = &hw->ddb_y[plane]; > + sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[plane]; > + > + if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { > + DRM_ERROR("mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n", > + pipe_name(pipe), plane + 1, > + sw_ddb_entry->start, sw_ddb_entry->end, > + hw_ddb_entry->start, hw_ddb_entry->end); > + } > + } > + > + /* > + * cursor > + * If the cursor plane isn't active, we may not have updated it's ddb > + * allocation. In that case since the ddb allocation will be updated > + * once the plane becomes visible, we can skip this check > + */ > + if (1) { > + struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; > + > + hw_plane_wm = &hw->wm.planes[PLANE_CURSOR]; > + sw_plane_wm = &sw_wm->planes[PLANE_CURSOR]; > + > + /* Watermarks */ > + for (level = 0; level <= max_level; level++) { > + if (skl_wm_level_equals(&hw_plane_wm->wm[level], > + &sw_plane_wm->wm[level])) > + continue; > + > + DRM_ERROR("mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > + pipe_name(pipe), level, > + sw_plane_wm->wm[level].plane_en, > + sw_plane_wm->wm[level].plane_res_b, > + sw_plane_wm->wm[level].plane_res_l, > + hw_plane_wm->wm[level].plane_en, > + hw_plane_wm->wm[level].plane_res_b, > + hw_plane_wm->wm[level].plane_res_l); > + } > + > + if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, > + &sw_plane_wm->trans_wm)) { > + DRM_ERROR("mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > + pipe_name(pipe), > + sw_plane_wm->trans_wm.plane_en, > + sw_plane_wm->trans_wm.plane_res_b, > + sw_plane_wm->trans_wm.plane_res_l, > + hw_plane_wm->trans_wm.plane_en, > + hw_plane_wm->trans_wm.plane_res_b, > + hw_plane_wm->trans_wm.plane_res_l); > + } > + > + /* DDB */ > + hw_ddb_entry = &hw->ddb_y[PLANE_CURSOR]; > + sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[PLANE_CURSOR]; > + > + if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { > + DRM_ERROR("mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n", > + pipe_name(pipe), > + sw_ddb_entry->start, sw_ddb_entry->end, > + hw_ddb_entry->start, hw_ddb_entry->end); > + } > + } > + > + kfree(hw); > +} > + > +/* Cross check the actual hw state with our own modeset state tracking (and it's > + * internal consistency). */ > +static void intel_connector_verify_state(struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + struct intel_connector *connector = to_intel_connector(conn_state->connector); > + > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > + connector->base.base.id, > + connector->base.name); > + > + if (connector->get_hw_state(connector)) { > + struct intel_encoder *encoder = connector->encoder; > + > + I915_STATE_WARN(!crtc_state, > + "connector enabled without attached crtc\n"); > + > + if (!crtc_state) > + return; > + > + I915_STATE_WARN(!crtc_state->active, > + "connector is active, but attached crtc isn't\n"); > + > + if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST) > + return; > + > + I915_STATE_WARN(conn_state->best_encoder != &encoder->base, > + "atomic encoder doesn't match attached encoder\n"); > + > + I915_STATE_WARN(conn_state->crtc != encoder->base.crtc, > + "attached encoder crtc differs from connector crtc\n"); > + } else { > + I915_STATE_WARN(crtc_state && crtc_state->active, > + "attached crtc is active, but connector isn't\n"); > + I915_STATE_WARN(!crtc_state && conn_state->best_encoder, > + "best encoder set without crtc!\n"); > + } > +} > + > +static void > +verify_connector_state(struct drm_device *dev, > + struct drm_atomic_state *state, > + struct drm_crtc *crtc) > +{ > + struct drm_connector *connector; > + struct drm_connector_state *new_conn_state; > + int i; > + > + for_each_new_connector_in_state(state, connector, new_conn_state, i) { > + struct drm_encoder *encoder = connector->encoder; > + struct drm_crtc_state *crtc_state = NULL; > + > + if (new_conn_state->crtc != crtc) > + continue; > + > + if (crtc) > + crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc); > + > + intel_connector_verify_state(crtc_state, new_conn_state); > + > + I915_STATE_WARN(new_conn_state->best_encoder != encoder, > + "connector's atomic encoder doesn't match legacy encoder\n"); > + } > +} > + > +static void > +verify_encoder_state(struct drm_device *dev, struct drm_atomic_state *state) > +{ > + struct intel_encoder *encoder; > + struct drm_connector *connector; > + struct drm_connector_state *old_conn_state, *new_conn_state; > + int i; > + > + for_each_intel_encoder(dev, encoder) { > + bool enabled = false, found = false; > + enum pipe pipe; > + > + DRM_DEBUG_KMS("[ENCODER:%d:%s]\n", > + encoder->base.base.id, > + encoder->base.name); > + > + for_each_oldnew_connector_in_state(state, connector, old_conn_state, > + new_conn_state, i) { > + if (old_conn_state->best_encoder == &encoder->base) > + found = true; > + > + if (new_conn_state->best_encoder != &encoder->base) > + continue; > + found = enabled = true; > + > + I915_STATE_WARN(new_conn_state->crtc != > + encoder->base.crtc, > + "connector's crtc doesn't match encoder crtc\n"); > + } > + > + if (!found) > + continue; > + > + I915_STATE_WARN(!!encoder->base.crtc != enabled, > + "encoder's enabled state mismatch " > + "(expected %i, found %i)\n", > + !!encoder->base.crtc, enabled); > + > + if (!encoder->base.crtc) { > + bool active; > + > + active = encoder->get_hw_state(encoder, &pipe); > + I915_STATE_WARN(active, > + "encoder detached but still enabled on pipe %c.\n", > + pipe_name(pipe)); > + } > + } > +} > + > +static void > +verify_crtc_state(struct drm_crtc *crtc, > + struct drm_crtc_state *old_crtc_state, > + struct drm_crtc_state *new_crtc_state) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + struct intel_encoder *encoder; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc_state *pipe_config, *sw_config; > + struct drm_atomic_state *old_state; > + bool active; > + > + old_state = old_crtc_state->state; > + __drm_atomic_helper_crtc_destroy_state(old_crtc_state); > + pipe_config = to_intel_crtc_state(old_crtc_state); > + memset(pipe_config, 0, sizeof(*pipe_config)); > + pipe_config->base.crtc = crtc; > + pipe_config->base.state = old_state; > + > + DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); > + > + active = dev_priv->display.get_pipe_config(intel_crtc, pipe_config); > + > + /* we keep both pipes enabled on 830 */ > + if (IS_I830(dev_priv)) > + active = new_crtc_state->active; > + > + I915_STATE_WARN(new_crtc_state->active != active, > + "crtc active state doesn't match with hw state " > + "(expected %i, found %i)\n", new_crtc_state->active, active); > + > + I915_STATE_WARN(intel_crtc->active != new_crtc_state->active, > + "transitional active state does not match atomic hw state " > + "(expected %i, found %i)\n", new_crtc_state->active, intel_crtc->active); > + > + for_each_encoder_on_crtc(dev, crtc, encoder) { > + enum pipe pipe; > + > + active = encoder->get_hw_state(encoder, &pipe); > + I915_STATE_WARN(active != new_crtc_state->active, > + "[ENCODER:%i] active %i with crtc active %i\n", > + encoder->base.base.id, active, new_crtc_state->active); > + > + I915_STATE_WARN(active && intel_crtc->pipe != pipe, > + "Encoder connected to wrong pipe %c\n", > + pipe_name(pipe)); > + > + if (active) > + encoder->get_config(encoder, pipe_config); > + } > + > + intel_crtc_compute_pixel_rate(pipe_config); > + > + if (!new_crtc_state->active) > + return; > + > + intel_pipe_config_sanity_check(dev_priv, pipe_config); > + > + sw_config = to_intel_crtc_state(new_crtc_state); > + if (!intel_pipe_config_compare(dev_priv, sw_config, > + pipe_config, false)) { > + I915_STATE_WARN(1, "pipe state doesn't match!\n"); > + intel_dump_pipe_config(intel_crtc, pipe_config, > + "[hw state]"); > + intel_dump_pipe_config(intel_crtc, sw_config, > + "[sw state]"); > + } > +} > + > +void > +intel_verify_planes(struct intel_atomic_state *state) > +{ > + struct intel_plane *plane; > + const struct intel_plane_state *plane_state; > + int i; > + > + for_each_new_intel_plane_in_state(state, plane, > + plane_state, i) > + assert_plane(plane, plane_state->slave || > + plane_state->base.visible); > +} > + > +static void > +verify_single_dpll_state(struct drm_i915_private *dev_priv, > + struct intel_shared_dpll *pll, > + struct drm_crtc *crtc, > + struct drm_crtc_state *new_state) > +{ > + struct intel_dpll_hw_state dpll_hw_state; > + unsigned int crtc_mask; > + bool active; > + > + memset(&dpll_hw_state, 0, sizeof(dpll_hw_state)); > + > + DRM_DEBUG_KMS("%s\n", pll->info->name); > + > + active = pll->info->funcs->get_hw_state(dev_priv, pll, &dpll_hw_state); > + > + if (!(pll->info->flags & INTEL_DPLL_ALWAYS_ON)) { > + I915_STATE_WARN(!pll->on && pll->active_mask, > + "pll in active use but not on in sw tracking\n"); > + I915_STATE_WARN(pll->on && !pll->active_mask, > + "pll is on but not used by any active crtc\n"); > + I915_STATE_WARN(pll->on != active, > + "pll on state mismatch (expected %i, found %i)\n", > + pll->on, active); > + } > + > + if (!crtc) { > + I915_STATE_WARN(pll->active_mask & ~pll->state.crtc_mask, > + "more active pll users than references: %x vs %x\n", > + pll->active_mask, pll->state.crtc_mask); > + > + return; > + } > + > + crtc_mask = drm_crtc_mask(crtc); > + > + if (new_state->active) > + I915_STATE_WARN(!(pll->active_mask & crtc_mask), > + "pll active mismatch (expected pipe %c in active mask 0x%02x)\n", > + pipe_name(drm_crtc_index(crtc)), pll->active_mask); > + else > + I915_STATE_WARN(pll->active_mask & crtc_mask, > + "pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n", > + pipe_name(drm_crtc_index(crtc)), pll->active_mask); > + > + I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask), > + "pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n", > + crtc_mask, pll->state.crtc_mask); > + > + I915_STATE_WARN(pll->on && memcmp(&pll->state.hw_state, > + &dpll_hw_state, > + sizeof(dpll_hw_state)), > + "pll hw state mismatch\n"); > +} > + > +static void > +verify_shared_dpll_state(struct drm_device *dev, struct drm_crtc *crtc, > + struct drm_crtc_state *old_crtc_state, > + struct drm_crtc_state *new_crtc_state) > +{ > + struct drm_i915_private *dev_priv = to_i915(dev); > + struct intel_crtc_state *old_state = to_intel_crtc_state(old_crtc_state); > + struct intel_crtc_state *new_state = to_intel_crtc_state(new_crtc_state); > + > + if (new_state->shared_dpll) > + verify_single_dpll_state(dev_priv, new_state->shared_dpll, crtc, new_crtc_state); > + > + if (old_state->shared_dpll && > + old_state->shared_dpll != new_state->shared_dpll) { > + unsigned int crtc_mask = drm_crtc_mask(crtc); > + struct intel_shared_dpll *pll = old_state->shared_dpll; > + > + I915_STATE_WARN(pll->active_mask & crtc_mask, > + "pll active mismatch (didn't expect pipe %c in active mask)\n", > + pipe_name(drm_crtc_index(crtc))); > + I915_STATE_WARN(pll->state.crtc_mask & crtc_mask, > + "pll enabled crtcs mismatch (found %x in enabled mask)\n", > + pipe_name(drm_crtc_index(crtc))); > + } > +} > + > +void > +intel_verify_modeset_crtc(struct drm_crtc *crtc, > + struct drm_atomic_state *state, > + struct drm_crtc_state *old_state, > + struct drm_crtc_state *new_state) > +{ > + if (!drm_atomic_crtc_needs_modeset(new_state) && > + !to_intel_crtc_state(new_state)->update_pipe) > + return; > + > + verify_wm_state(crtc, new_state); > + verify_connector_state(crtc->dev, state, crtc); > + verify_crtc_state(crtc, old_state, new_state); > + verify_shared_dpll_state(crtc->dev, crtc, old_state, new_state); > +} > + > +static void > +verify_disabled_dpll_state(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = to_i915(dev); > + int i; > + > + for (i = 0; i < dev_priv->num_shared_dpll; i++) > + verify_single_dpll_state(dev_priv, &dev_priv->shared_dplls[i], NULL, NULL); > +} > + > +void > +intel_verify_modeset_disabled(struct drm_device *dev, > + struct drm_atomic_state *state) > +{ > + verify_encoder_state(dev, state); > + verify_connector_state(dev, state, NULL); > + verify_disabled_dpll_state(dev); > +} > diff --git a/drivers/gpu/drm/i915/intel_verify.h b/drivers/gpu/drm/i915/intel_verify.h > new file mode 100644 > index 000000..4b751ea > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_verify.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: MIT */ > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#ifndef __INTEL_VERIFY_H__ > +#define __INTEL_VERIFY_H__ > + > +struct drm_atomic_state; > +struct drm_crtc; > +struct drm_crtc_state; > +struct drm_device; > + > +void intel_verify_modeset_crtc(struct drm_crtc *crtc, > + struct drm_atomic_state *state, > + struct drm_crtc_state *old_state, > + struct drm_crtc_state *new_state); > +void intel_verify_modeset_disabled(struct drm_device *dev, > + struct drm_atomic_state *state); > +void intel_verify_planes(struct intel_atomic_state *state); > + > +#endif /* __INTEL_VERIFY_H__ */ > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, May 15, 2019 at 11:33:31AM +0200, Daniel Vetter wrote: > On Tue, Apr 16, 2019 at 01:36:02PM +0300, Jani Nikula wrote: > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > drivers/gpu/drm/i915/Makefile | 1 + > > drivers/gpu/drm/i915/intel_display.c | 474 +-------------------------- > > drivers/gpu/drm/i915/intel_drv.h | 12 + > > drivers/gpu/drm/i915/intel_verify.c | 464 ++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_verify.h | 22 ++ > > 5 files changed, 510 insertions(+), 463 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/intel_verify.c > > create mode 100644 drivers/gpu/drm/i915/intel_verify.h > > > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > > index fbcb0904..a000fad 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -166,6 +166,7 @@ i915-y += dvo_ch7017.o \ > > intel_panel.o \ > > intel_sdvo.o \ > > intel_tv.o \ > > + intel_verify.o \ > > vlv_dsi.o \ > > vlv_dsi_pll.o \ > > intel_vdsc.o > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 3bd40a..31a931 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -69,6 +69,7 @@ > > #include "intel_sdvo.h" > > #include "intel_sprite.h" > > #include "intel_tv.h" > > +#include "intel_verify.h" > > > > /* Primary plane formats for gen <= 3 */ > > static const u32 i8xx_primary_formats[] = { > > @@ -1243,7 +1244,7 @@ void assert_pipe(struct drm_i915_private *dev_priv, > > pipe_name(pipe), onoff(state), onoff(cur_state)); > > } > > > > -static void assert_plane(struct intel_plane *plane, bool state) > > +void assert_plane(struct intel_plane *plane, bool state) > > { > > enum pipe pipe; > > bool cur_state; > > @@ -6607,45 +6608,6 @@ void intel_encoder_destroy(struct drm_encoder *encoder) > > kfree(intel_encoder); > > } > > > > -/* Cross check the actual hw state with our own modeset state tracking (and it's > > - * internal consistency). */ > > -static void intel_connector_verify_state(struct drm_crtc_state *crtc_state, > > - struct drm_connector_state *conn_state) > > -{ > > - struct intel_connector *connector = to_intel_connector(conn_state->connector); > > - > > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > > - connector->base.base.id, > > - connector->base.name); > > - > > - if (connector->get_hw_state(connector)) { > > - struct intel_encoder *encoder = connector->encoder; > > - > > - I915_STATE_WARN(!crtc_state, > > - "connector enabled without attached crtc\n"); > > - > > - if (!crtc_state) > > - return; > > - > > - I915_STATE_WARN(!crtc_state->active, > > - "connector is active, but attached crtc isn't\n"); > > - > > - if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST) > > - return; > > - > > - I915_STATE_WARN(conn_state->best_encoder != &encoder->base, > > - "atomic encoder doesn't match attached encoder\n"); > > - > > - I915_STATE_WARN(conn_state->crtc != encoder->base.crtc, > > - "attached encoder crtc differs from connector crtc\n"); > > - } else { > > - I915_STATE_WARN(crtc_state && crtc_state->active, > > - "attached crtc is active, but connector isn't\n"); > > - I915_STATE_WARN(!crtc_state && conn_state->best_encoder, > > - "best encoder set without crtc!\n"); > > - } > > -} > > - > > static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state) > > { > > if (crtc_state->base.enable && crtc_state->has_pch_encoder) > > @@ -6879,7 +6841,7 @@ static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config) > > return pixel_rate; > > } > > > > -static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state) > > +void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state) > > { > > struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); > > > > @@ -11584,9 +11546,9 @@ static const char *output_formats(enum intel_output_format format) > > return output_format_str[format]; > > } > > > > -static void intel_dump_pipe_config(struct intel_crtc *crtc, > > - struct intel_crtc_state *pipe_config, > > - const char *context) > > +void intel_dump_pipe_config(struct intel_crtc *crtc, > > + struct intel_crtc_state *pipe_config, > > + const char *context) > > { > > struct drm_device *dev = crtc->base.dev; > > struct drm_i915_private *dev_priv = to_i915(dev); > > @@ -12076,7 +12038,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv) > > return false; > > } > > > > -static bool > > +bool > > intel_pipe_config_compare(struct drm_i915_private *dev_priv, > > struct intel_crtc_state *current_config, > > struct intel_crtc_state *pipe_config, > > @@ -12389,8 +12351,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv, > > return ret; > > } > > > > -static void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, > > - const struct intel_crtc_state *pipe_config) > > +void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, > > + const struct intel_crtc_state *pipe_config) > > { > > if (pipe_config->has_pch_encoder) { > > int fdi_dotclock = intel_dotclock_calculate(intel_fdi_link_freq(dev_priv, pipe_config), > > @@ -12407,420 +12369,6 @@ static void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, > > } > > } > > > > -static void verify_wm_state(struct drm_crtc *crtc, > > - struct drm_crtc_state *new_state) > > -{ > > - struct drm_i915_private *dev_priv = to_i915(crtc->dev); > > - struct skl_hw_state { > > - struct skl_ddb_entry ddb_y[I915_MAX_PLANES]; > > - struct skl_ddb_entry ddb_uv[I915_MAX_PLANES]; > > - struct skl_ddb_allocation ddb; > > - struct skl_pipe_wm wm; > > - } *hw; > > - struct skl_ddb_allocation *sw_ddb; > > - struct skl_pipe_wm *sw_wm; > > - struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry; > > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > - const enum pipe pipe = intel_crtc->pipe; > > - int plane, level, max_level = ilk_wm_max_level(dev_priv); > > - > > - if (INTEL_GEN(dev_priv) < 9 || !new_state->active) > > - return; > > - > > - hw = kzalloc(sizeof(*hw), GFP_KERNEL); > > - if (!hw) > > - return; > > - > > - skl_pipe_wm_get_hw_state(intel_crtc, &hw->wm); > > - sw_wm = &to_intel_crtc_state(new_state)->wm.skl.optimal; > > - > > - skl_pipe_ddb_get_hw_state(intel_crtc, hw->ddb_y, hw->ddb_uv); > > - > > - skl_ddb_get_hw_state(dev_priv, &hw->ddb); > > - sw_ddb = &dev_priv->wm.skl_hw.ddb; > > - > > - if (INTEL_GEN(dev_priv) >= 11 && > > - hw->ddb.enabled_slices != sw_ddb->enabled_slices) > > - DRM_ERROR("mismatch in DBUF Slices (expected %u, got %u)\n", > > - sw_ddb->enabled_slices, > > - hw->ddb.enabled_slices); > > - > > - /* planes */ > > - for_each_universal_plane(dev_priv, pipe, plane) { > > - struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; > > - > > - hw_plane_wm = &hw->wm.planes[plane]; > > - sw_plane_wm = &sw_wm->planes[plane]; > > - > > - /* Watermarks */ > > - for (level = 0; level <= max_level; level++) { > > - if (skl_wm_level_equals(&hw_plane_wm->wm[level], > > - &sw_plane_wm->wm[level])) > > - continue; > > - > > - DRM_ERROR("mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > > - pipe_name(pipe), plane + 1, level, > > - sw_plane_wm->wm[level].plane_en, > > - sw_plane_wm->wm[level].plane_res_b, > > - sw_plane_wm->wm[level].plane_res_l, > > - hw_plane_wm->wm[level].plane_en, > > - hw_plane_wm->wm[level].plane_res_b, > > - hw_plane_wm->wm[level].plane_res_l); > > - } > > - > > - if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, > > - &sw_plane_wm->trans_wm)) { > > - DRM_ERROR("mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > > - pipe_name(pipe), plane + 1, > > - sw_plane_wm->trans_wm.plane_en, > > - sw_plane_wm->trans_wm.plane_res_b, > > - sw_plane_wm->trans_wm.plane_res_l, > > - hw_plane_wm->trans_wm.plane_en, > > - hw_plane_wm->trans_wm.plane_res_b, > > - hw_plane_wm->trans_wm.plane_res_l); > > - } > > - > > - /* DDB */ > > - hw_ddb_entry = &hw->ddb_y[plane]; > > - sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[plane]; > > - > > - if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { > > - DRM_ERROR("mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n", > > - pipe_name(pipe), plane + 1, > > - sw_ddb_entry->start, sw_ddb_entry->end, > > - hw_ddb_entry->start, hw_ddb_entry->end); > > - } > > - } > > - > > - /* > > - * cursor > > - * If the cursor plane isn't active, we may not have updated it's ddb > > - * allocation. In that case since the ddb allocation will be updated > > - * once the plane becomes visible, we can skip this check > > - */ > > - if (1) { > > - struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; > > - > > - hw_plane_wm = &hw->wm.planes[PLANE_CURSOR]; > > - sw_plane_wm = &sw_wm->planes[PLANE_CURSOR]; > > - > > - /* Watermarks */ > > - for (level = 0; level <= max_level; level++) { > > - if (skl_wm_level_equals(&hw_plane_wm->wm[level], > > - &sw_plane_wm->wm[level])) > > - continue; > > - > > - DRM_ERROR("mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > > - pipe_name(pipe), level, > > - sw_plane_wm->wm[level].plane_en, > > - sw_plane_wm->wm[level].plane_res_b, > > - sw_plane_wm->wm[level].plane_res_l, > > - hw_plane_wm->wm[level].plane_en, > > - hw_plane_wm->wm[level].plane_res_b, > > - hw_plane_wm->wm[level].plane_res_l); > > - } > > - > > - if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, > > - &sw_plane_wm->trans_wm)) { > > - DRM_ERROR("mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > > - pipe_name(pipe), > > - sw_plane_wm->trans_wm.plane_en, > > - sw_plane_wm->trans_wm.plane_res_b, > > - sw_plane_wm->trans_wm.plane_res_l, > > - hw_plane_wm->trans_wm.plane_en, > > - hw_plane_wm->trans_wm.plane_res_b, > > - hw_plane_wm->trans_wm.plane_res_l); > > - } > > - > > - /* DDB */ > > - hw_ddb_entry = &hw->ddb_y[PLANE_CURSOR]; > > - sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[PLANE_CURSOR]; > > - > > - if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { > > - DRM_ERROR("mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n", > > - pipe_name(pipe), > > - sw_ddb_entry->start, sw_ddb_entry->end, > > - hw_ddb_entry->start, hw_ddb_entry->end); > > - } > > - } > > - > > - kfree(hw); > > -} > > - > > -static void > > -verify_connector_state(struct drm_device *dev, > > - struct drm_atomic_state *state, > > - struct drm_crtc *crtc) > > -{ > > - struct drm_connector *connector; > > - struct drm_connector_state *new_conn_state; > > - int i; > > - > > - for_each_new_connector_in_state(state, connector, new_conn_state, i) { > > - struct drm_encoder *encoder = connector->encoder; > > - struct drm_crtc_state *crtc_state = NULL; > > - > > - if (new_conn_state->crtc != crtc) > > - continue; > > - > > - if (crtc) > > - crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc); > > - > > - intel_connector_verify_state(crtc_state, new_conn_state); > > - > > - I915_STATE_WARN(new_conn_state->best_encoder != encoder, > > - "connector's atomic encoder doesn't match legacy encoder\n"); > > - } > > -} > > - > > -static void > > -verify_encoder_state(struct drm_device *dev, struct drm_atomic_state *state) > > -{ > > - struct intel_encoder *encoder; > > - struct drm_connector *connector; > > - struct drm_connector_state *old_conn_state, *new_conn_state; > > - int i; > > - > > - for_each_intel_encoder(dev, encoder) { > > - bool enabled = false, found = false; > > - enum pipe pipe; > > - > > - DRM_DEBUG_KMS("[ENCODER:%d:%s]\n", > > - encoder->base.base.id, > > - encoder->base.name); > > - > > - for_each_oldnew_connector_in_state(state, connector, old_conn_state, > > - new_conn_state, i) { > > - if (old_conn_state->best_encoder == &encoder->base) > > - found = true; > > - > > - if (new_conn_state->best_encoder != &encoder->base) > > - continue; > > - found = enabled = true; > > - > > - I915_STATE_WARN(new_conn_state->crtc != > > - encoder->base.crtc, > > - "connector's crtc doesn't match encoder crtc\n"); > > - } > > - > > - if (!found) > > - continue; > > - > > - I915_STATE_WARN(!!encoder->base.crtc != enabled, > > - "encoder's enabled state mismatch " > > - "(expected %i, found %i)\n", > > - !!encoder->base.crtc, enabled); > > - > > - if (!encoder->base.crtc) { > > - bool active; > > - > > - active = encoder->get_hw_state(encoder, &pipe); > > - I915_STATE_WARN(active, > > - "encoder detached but still enabled on pipe %c.\n", > > - pipe_name(pipe)); > > - } > > - } > > -} > > - > > -static void > > -verify_crtc_state(struct drm_crtc *crtc, > > - struct drm_crtc_state *old_crtc_state, > > - struct drm_crtc_state *new_crtc_state) > > -{ > > - struct drm_device *dev = crtc->dev; > > - struct drm_i915_private *dev_priv = to_i915(dev); > > - struct intel_encoder *encoder; > > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > - struct intel_crtc_state *pipe_config, *sw_config; > > - struct drm_atomic_state *old_state; > > - bool active; > > - > > - old_state = old_crtc_state->state; > > - __drm_atomic_helper_crtc_destroy_state(old_crtc_state); > > - pipe_config = to_intel_crtc_state(old_crtc_state); > > - memset(pipe_config, 0, sizeof(*pipe_config)); > > - pipe_config->base.crtc = crtc; > > - pipe_config->base.state = old_state; > > - > > - DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); > > - > > - active = dev_priv->display.get_pipe_config(intel_crtc, pipe_config); > > - > > - /* we keep both pipes enabled on 830 */ > > - if (IS_I830(dev_priv)) > > - active = new_crtc_state->active; > > - > > - I915_STATE_WARN(new_crtc_state->active != active, > > - "crtc active state doesn't match with hw state " > > - "(expected %i, found %i)\n", new_crtc_state->active, active); > > - > > - I915_STATE_WARN(intel_crtc->active != new_crtc_state->active, > > - "transitional active state does not match atomic hw state " > > - "(expected %i, found %i)\n", new_crtc_state->active, intel_crtc->active); > > - > > - for_each_encoder_on_crtc(dev, crtc, encoder) { > > - enum pipe pipe; > > - > > - active = encoder->get_hw_state(encoder, &pipe); > > - I915_STATE_WARN(active != new_crtc_state->active, > > - "[ENCODER:%i] active %i with crtc active %i\n", > > - encoder->base.base.id, active, new_crtc_state->active); > > - > > - I915_STATE_WARN(active && intel_crtc->pipe != pipe, > > - "Encoder connected to wrong pipe %c\n", > > - pipe_name(pipe)); > > - > > - if (active) > > - encoder->get_config(encoder, pipe_config); > > - } > > - > > - intel_crtc_compute_pixel_rate(pipe_config); > > - > > - if (!new_crtc_state->active) > > - return; > > - > > - intel_pipe_config_sanity_check(dev_priv, pipe_config); > > - > > - sw_config = to_intel_crtc_state(new_crtc_state); > > - if (!intel_pipe_config_compare(dev_priv, sw_config, > > - pipe_config, false)) { > > - I915_STATE_WARN(1, "pipe state doesn't match!\n"); > > - intel_dump_pipe_config(intel_crtc, pipe_config, > > - "[hw state]"); > > - intel_dump_pipe_config(intel_crtc, sw_config, > > - "[sw state]"); > > - } > > -} > > - > > -static void > > -intel_verify_planes(struct intel_atomic_state *state) > > -{ > > - struct intel_plane *plane; > > - const struct intel_plane_state *plane_state; > > - int i; > > - > > - for_each_new_intel_plane_in_state(state, plane, > > - plane_state, i) > > - assert_plane(plane, plane_state->slave || > > - plane_state->base.visible); > > -} > > - > > -static void > > -verify_single_dpll_state(struct drm_i915_private *dev_priv, > > - struct intel_shared_dpll *pll, > > - struct drm_crtc *crtc, > > - struct drm_crtc_state *new_state) > > -{ > > - struct intel_dpll_hw_state dpll_hw_state; > > - unsigned int crtc_mask; > > - bool active; > > - > > - memset(&dpll_hw_state, 0, sizeof(dpll_hw_state)); > > - > > - DRM_DEBUG_KMS("%s\n", pll->info->name); > > - > > - active = pll->info->funcs->get_hw_state(dev_priv, pll, &dpll_hw_state); > > - > > - if (!(pll->info->flags & INTEL_DPLL_ALWAYS_ON)) { > > - I915_STATE_WARN(!pll->on && pll->active_mask, > > - "pll in active use but not on in sw tracking\n"); > > - I915_STATE_WARN(pll->on && !pll->active_mask, > > - "pll is on but not used by any active crtc\n"); > > - I915_STATE_WARN(pll->on != active, > > - "pll on state mismatch (expected %i, found %i)\n", > > - pll->on, active); > > - } > > - > > - if (!crtc) { > > - I915_STATE_WARN(pll->active_mask & ~pll->state.crtc_mask, > > - "more active pll users than references: %x vs %x\n", > > - pll->active_mask, pll->state.crtc_mask); > > - > > - return; > > - } > > - > > - crtc_mask = drm_crtc_mask(crtc); > > - > > - if (new_state->active) > > - I915_STATE_WARN(!(pll->active_mask & crtc_mask), > > - "pll active mismatch (expected pipe %c in active mask 0x%02x)\n", > > - pipe_name(drm_crtc_index(crtc)), pll->active_mask); > > - else > > - I915_STATE_WARN(pll->active_mask & crtc_mask, > > - "pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n", > > - pipe_name(drm_crtc_index(crtc)), pll->active_mask); > > - > > - I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask), > > - "pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n", > > - crtc_mask, pll->state.crtc_mask); > > - > > - I915_STATE_WARN(pll->on && memcmp(&pll->state.hw_state, > > - &dpll_hw_state, > > - sizeof(dpll_hw_state)), > > - "pll hw state mismatch\n"); > > -} > > - > > -static void > > -verify_shared_dpll_state(struct drm_device *dev, struct drm_crtc *crtc, > > - struct drm_crtc_state *old_crtc_state, > > - struct drm_crtc_state *new_crtc_state) > > -{ > > - struct drm_i915_private *dev_priv = to_i915(dev); > > - struct intel_crtc_state *old_state = to_intel_crtc_state(old_crtc_state); > > - struct intel_crtc_state *new_state = to_intel_crtc_state(new_crtc_state); > > - > > - if (new_state->shared_dpll) > > - verify_single_dpll_state(dev_priv, new_state->shared_dpll, crtc, new_crtc_state); > > - > > - if (old_state->shared_dpll && > > - old_state->shared_dpll != new_state->shared_dpll) { > > - unsigned int crtc_mask = drm_crtc_mask(crtc); > > - struct intel_shared_dpll *pll = old_state->shared_dpll; > > - > > - I915_STATE_WARN(pll->active_mask & crtc_mask, > > - "pll active mismatch (didn't expect pipe %c in active mask)\n", > > - pipe_name(drm_crtc_index(crtc))); > > - I915_STATE_WARN(pll->state.crtc_mask & crtc_mask, > > - "pll enabled crtcs mismatch (found %x in enabled mask)\n", > > - pipe_name(drm_crtc_index(crtc))); > > - } > > -} > > - > > -static void > > -intel_modeset_verify_crtc(struct drm_crtc *crtc, > > - struct drm_atomic_state *state, > > - struct drm_crtc_state *old_state, > > - struct drm_crtc_state *new_state) > > -{ > > - if (!needs_modeset(new_state) && > > - !to_intel_crtc_state(new_state)->update_pipe) > > - return; > > - > > - verify_wm_state(crtc, new_state); > > - verify_connector_state(crtc->dev, state, crtc); > > - verify_crtc_state(crtc, old_state, new_state); > > - verify_shared_dpll_state(crtc->dev, crtc, old_state, new_state); > > -} > > - > > -static void > > -verify_disabled_dpll_state(struct drm_device *dev) > > -{ > > - struct drm_i915_private *dev_priv = to_i915(dev); > > - int i; > > - > > - for (i = 0; i < dev_priv->num_shared_dpll; i++) > > - verify_single_dpll_state(dev_priv, &dev_priv->shared_dplls[i], NULL, NULL); > > -} > > - > > -static void > > -intel_modeset_verify_disabled(struct drm_device *dev, > > - struct drm_atomic_state *state) > > -{ > > - verify_encoder_state(dev, state); > > - verify_connector_state(dev, state, NULL); > > - verify_disabled_dpll_state(dev); > > -} > > - > > static void update_scanline_offset(const struct intel_crtc_state *crtc_state) > > { > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > @@ -13521,7 +13069,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > > if (!intel_can_enable_sagv(state)) > > intel_disable_sagv(dev_priv); > > > > - intel_modeset_verify_disabled(dev, state); > > + intel_verify_modeset_disabled(dev, state); > > } > > I think we should take this opportunity to slightly clean up the interface > here, and align a bit with > drm_atomic_helper_commit_modeset_enables/disables from helpers. So > rename this function here to > > intel_verify_modeset_disables(dev, state) > > so the naming is a bit clearer - it took me a while what exactly _disabled > here means: Is the verifier disabled, or the modeset is disabled or what > exactly. I think the idea of "modeset_disables" as a phase of an atomic > commit is better established. > > > > /* Complete the events for pipes that have now been disabled */ > > @@ -13589,7 +13137,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > > if (put_domains[i]) > > modeset_put_power_domains(dev_priv, put_domains[i]); > > > > - intel_modeset_verify_crtc(crtc, state, old_crtc_state, new_crtc_state); > > + intel_verify_modeset_crtc(crtc, state, old_crtc_state, new_crtc_state); > > } > > Same here, create a new > > intel_verify_modeset_enables(dev, state) > > which combines the crtc and plan checks (the additional loop wont matter), > and neatly aligns with the idea of a "modeset_enables" phase from atomic > helpers. > > With that I think we'll have some really tidy interface into > intel_verify.c, and your patch here has my Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Bonus if you can throw some kerneldoc for these on top, explaining the link with core helper concepts for the actual functions, plus a short intro on wtf this is. Because the idea behind our state verifier seems rather elusive to newbies. But that would be a 2nd patch I think. I can also help with the docs if you want. -Daniel > > Cheers, Daniel > > > > > if (intel_state->modeset) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index a38b9c..77767c 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1668,6 +1668,17 @@ int intel_get_pipe_from_crtc_id_ioctl(struct drm_device *dev, void *data, > > struct drm_file *file_priv); > > enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv, > > enum pipe pipe); > > +void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, > > + const struct intel_crtc_state *pipe_config); > > +bool > > +intel_pipe_config_compare(struct drm_i915_private *dev_priv, > > + struct intel_crtc_state *current_config, > > + struct intel_crtc_state *pipe_config, > > + bool adjust); > > +void intel_dump_pipe_config(struct intel_crtc *crtc, > > + struct intel_crtc_state *pipe_config, > > + const char *context); > > +void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state); > > static inline bool > > intel_crtc_has_type(const struct intel_crtc_state *crtc_state, > > enum intel_output_type type) > > @@ -1760,6 +1771,7 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv, > > void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, bool state); > > #define assert_pipe_enabled(d, p) assert_pipe(d, p, true) > > #define assert_pipe_disabled(d, p) assert_pipe(d, p, false) > > +void assert_plane(struct intel_plane *plane, bool state); > > void intel_prepare_reset(struct drm_i915_private *dev_priv); > > void intel_finish_reset(struct drm_i915_private *dev_priv); > > void hsw_enable_pc8(struct drm_i915_private *dev_priv); > > diff --git a/drivers/gpu/drm/i915/intel_verify.c b/drivers/gpu/drm/i915/intel_verify.c > > new file mode 100644 > > index 000000..4c8990 > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/intel_verify.c > > @@ -0,0 +1,464 @@ > > +// SPDX-License-Identifier: MIT > > +/* > > + * Copyright © 2019 Intel Corporation > > + */ > > + > > +#include <drm/drm_atomic_state_helper.h> > > + > > +#include "i915_drv.h" > > +#include "intel_drv.h" > > +#include "intel_pm.h" > > +#include "intel_verify.h" > > + > > +static void verify_wm_state(struct drm_crtc *crtc, > > + struct drm_crtc_state *new_state) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(crtc->dev); > > + struct skl_hw_state { > > + struct skl_ddb_entry ddb_y[I915_MAX_PLANES]; > > + struct skl_ddb_entry ddb_uv[I915_MAX_PLANES]; > > + struct skl_ddb_allocation ddb; > > + struct skl_pipe_wm wm; > > + } *hw; > > + struct skl_ddb_allocation *sw_ddb; > > + struct skl_pipe_wm *sw_wm; > > + struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry; > > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > + const enum pipe pipe = intel_crtc->pipe; > > + int plane, level, max_level = ilk_wm_max_level(dev_priv); > > + > > + if (INTEL_GEN(dev_priv) < 9 || !new_state->active) > > + return; > > + > > + hw = kzalloc(sizeof(*hw), GFP_KERNEL); > > + if (!hw) > > + return; > > + > > + skl_pipe_wm_get_hw_state(intel_crtc, &hw->wm); > > + sw_wm = &to_intel_crtc_state(new_state)->wm.skl.optimal; > > + > > + skl_pipe_ddb_get_hw_state(intel_crtc, hw->ddb_y, hw->ddb_uv); > > + > > + skl_ddb_get_hw_state(dev_priv, &hw->ddb); > > + sw_ddb = &dev_priv->wm.skl_hw.ddb; > > + > > + if (INTEL_GEN(dev_priv) >= 11 && > > + hw->ddb.enabled_slices != sw_ddb->enabled_slices) > > + DRM_ERROR("mismatch in DBUF Slices (expected %u, got %u)\n", > > + sw_ddb->enabled_slices, > > + hw->ddb.enabled_slices); > > + > > + /* planes */ > > + for_each_universal_plane(dev_priv, pipe, plane) { > > + struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; > > + > > + hw_plane_wm = &hw->wm.planes[plane]; > > + sw_plane_wm = &sw_wm->planes[plane]; > > + > > + /* Watermarks */ > > + for (level = 0; level <= max_level; level++) { > > + if (skl_wm_level_equals(&hw_plane_wm->wm[level], > > + &sw_plane_wm->wm[level])) > > + continue; > > + > > + DRM_ERROR("mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > > + pipe_name(pipe), plane + 1, level, > > + sw_plane_wm->wm[level].plane_en, > > + sw_plane_wm->wm[level].plane_res_b, > > + sw_plane_wm->wm[level].plane_res_l, > > + hw_plane_wm->wm[level].plane_en, > > + hw_plane_wm->wm[level].plane_res_b, > > + hw_plane_wm->wm[level].plane_res_l); > > + } > > + > > + if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, > > + &sw_plane_wm->trans_wm)) { > > + DRM_ERROR("mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > > + pipe_name(pipe), plane + 1, > > + sw_plane_wm->trans_wm.plane_en, > > + sw_plane_wm->trans_wm.plane_res_b, > > + sw_plane_wm->trans_wm.plane_res_l, > > + hw_plane_wm->trans_wm.plane_en, > > + hw_plane_wm->trans_wm.plane_res_b, > > + hw_plane_wm->trans_wm.plane_res_l); > > + } > > + > > + /* DDB */ > > + hw_ddb_entry = &hw->ddb_y[plane]; > > + sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[plane]; > > + > > + if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { > > + DRM_ERROR("mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n", > > + pipe_name(pipe), plane + 1, > > + sw_ddb_entry->start, sw_ddb_entry->end, > > + hw_ddb_entry->start, hw_ddb_entry->end); > > + } > > + } > > + > > + /* > > + * cursor > > + * If the cursor plane isn't active, we may not have updated it's ddb > > + * allocation. In that case since the ddb allocation will be updated > > + * once the plane becomes visible, we can skip this check > > + */ > > + if (1) { > > + struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; > > + > > + hw_plane_wm = &hw->wm.planes[PLANE_CURSOR]; > > + sw_plane_wm = &sw_wm->planes[PLANE_CURSOR]; > > + > > + /* Watermarks */ > > + for (level = 0; level <= max_level; level++) { > > + if (skl_wm_level_equals(&hw_plane_wm->wm[level], > > + &sw_plane_wm->wm[level])) > > + continue; > > + > > + DRM_ERROR("mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > > + pipe_name(pipe), level, > > + sw_plane_wm->wm[level].plane_en, > > + sw_plane_wm->wm[level].plane_res_b, > > + sw_plane_wm->wm[level].plane_res_l, > > + hw_plane_wm->wm[level].plane_en, > > + hw_plane_wm->wm[level].plane_res_b, > > + hw_plane_wm->wm[level].plane_res_l); > > + } > > + > > + if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, > > + &sw_plane_wm->trans_wm)) { > > + DRM_ERROR("mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", > > + pipe_name(pipe), > > + sw_plane_wm->trans_wm.plane_en, > > + sw_plane_wm->trans_wm.plane_res_b, > > + sw_plane_wm->trans_wm.plane_res_l, > > + hw_plane_wm->trans_wm.plane_en, > > + hw_plane_wm->trans_wm.plane_res_b, > > + hw_plane_wm->trans_wm.plane_res_l); > > + } > > + > > + /* DDB */ > > + hw_ddb_entry = &hw->ddb_y[PLANE_CURSOR]; > > + sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[PLANE_CURSOR]; > > + > > + if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { > > + DRM_ERROR("mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n", > > + pipe_name(pipe), > > + sw_ddb_entry->start, sw_ddb_entry->end, > > + hw_ddb_entry->start, hw_ddb_entry->end); > > + } > > + } > > + > > + kfree(hw); > > +} > > + > > +/* Cross check the actual hw state with our own modeset state tracking (and it's > > + * internal consistency). */ > > +static void intel_connector_verify_state(struct drm_crtc_state *crtc_state, > > + struct drm_connector_state *conn_state) > > +{ > > + struct intel_connector *connector = to_intel_connector(conn_state->connector); > > + > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > > + connector->base.base.id, > > + connector->base.name); > > + > > + if (connector->get_hw_state(connector)) { > > + struct intel_encoder *encoder = connector->encoder; > > + > > + I915_STATE_WARN(!crtc_state, > > + "connector enabled without attached crtc\n"); > > + > > + if (!crtc_state) > > + return; > > + > > + I915_STATE_WARN(!crtc_state->active, > > + "connector is active, but attached crtc isn't\n"); > > + > > + if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST) > > + return; > > + > > + I915_STATE_WARN(conn_state->best_encoder != &encoder->base, > > + "atomic encoder doesn't match attached encoder\n"); > > + > > + I915_STATE_WARN(conn_state->crtc != encoder->base.crtc, > > + "attached encoder crtc differs from connector crtc\n"); > > + } else { > > + I915_STATE_WARN(crtc_state && crtc_state->active, > > + "attached crtc is active, but connector isn't\n"); > > + I915_STATE_WARN(!crtc_state && conn_state->best_encoder, > > + "best encoder set without crtc!\n"); > > + } > > +} > > + > > +static void > > +verify_connector_state(struct drm_device *dev, > > + struct drm_atomic_state *state, > > + struct drm_crtc *crtc) > > +{ > > + struct drm_connector *connector; > > + struct drm_connector_state *new_conn_state; > > + int i; > > + > > + for_each_new_connector_in_state(state, connector, new_conn_state, i) { > > + struct drm_encoder *encoder = connector->encoder; > > + struct drm_crtc_state *crtc_state = NULL; > > + > > + if (new_conn_state->crtc != crtc) > > + continue; > > + > > + if (crtc) > > + crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc); > > + > > + intel_connector_verify_state(crtc_state, new_conn_state); > > + > > + I915_STATE_WARN(new_conn_state->best_encoder != encoder, > > + "connector's atomic encoder doesn't match legacy encoder\n"); > > + } > > +} > > + > > +static void > > +verify_encoder_state(struct drm_device *dev, struct drm_atomic_state *state) > > +{ > > + struct intel_encoder *encoder; > > + struct drm_connector *connector; > > + struct drm_connector_state *old_conn_state, *new_conn_state; > > + int i; > > + > > + for_each_intel_encoder(dev, encoder) { > > + bool enabled = false, found = false; > > + enum pipe pipe; > > + > > + DRM_DEBUG_KMS("[ENCODER:%d:%s]\n", > > + encoder->base.base.id, > > + encoder->base.name); > > + > > + for_each_oldnew_connector_in_state(state, connector, old_conn_state, > > + new_conn_state, i) { > > + if (old_conn_state->best_encoder == &encoder->base) > > + found = true; > > + > > + if (new_conn_state->best_encoder != &encoder->base) > > + continue; > > + found = enabled = true; > > + > > + I915_STATE_WARN(new_conn_state->crtc != > > + encoder->base.crtc, > > + "connector's crtc doesn't match encoder crtc\n"); > > + } > > + > > + if (!found) > > + continue; > > + > > + I915_STATE_WARN(!!encoder->base.crtc != enabled, > > + "encoder's enabled state mismatch " > > + "(expected %i, found %i)\n", > > + !!encoder->base.crtc, enabled); > > + > > + if (!encoder->base.crtc) { > > + bool active; > > + > > + active = encoder->get_hw_state(encoder, &pipe); > > + I915_STATE_WARN(active, > > + "encoder detached but still enabled on pipe %c.\n", > > + pipe_name(pipe)); > > + } > > + } > > +} > > + > > +static void > > +verify_crtc_state(struct drm_crtc *crtc, > > + struct drm_crtc_state *old_crtc_state, > > + struct drm_crtc_state *new_crtc_state) > > +{ > > + struct drm_device *dev = crtc->dev; > > + struct drm_i915_private *dev_priv = to_i915(dev); > > + struct intel_encoder *encoder; > > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > + struct intel_crtc_state *pipe_config, *sw_config; > > + struct drm_atomic_state *old_state; > > + bool active; > > + > > + old_state = old_crtc_state->state; > > + __drm_atomic_helper_crtc_destroy_state(old_crtc_state); > > + pipe_config = to_intel_crtc_state(old_crtc_state); > > + memset(pipe_config, 0, sizeof(*pipe_config)); > > + pipe_config->base.crtc = crtc; > > + pipe_config->base.state = old_state; > > + > > + DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); > > + > > + active = dev_priv->display.get_pipe_config(intel_crtc, pipe_config); > > + > > + /* we keep both pipes enabled on 830 */ > > + if (IS_I830(dev_priv)) > > + active = new_crtc_state->active; > > + > > + I915_STATE_WARN(new_crtc_state->active != active, > > + "crtc active state doesn't match with hw state " > > + "(expected %i, found %i)\n", new_crtc_state->active, active); > > + > > + I915_STATE_WARN(intel_crtc->active != new_crtc_state->active, > > + "transitional active state does not match atomic hw state " > > + "(expected %i, found %i)\n", new_crtc_state->active, intel_crtc->active); > > + > > + for_each_encoder_on_crtc(dev, crtc, encoder) { > > + enum pipe pipe; > > + > > + active = encoder->get_hw_state(encoder, &pipe); > > + I915_STATE_WARN(active != new_crtc_state->active, > > + "[ENCODER:%i] active %i with crtc active %i\n", > > + encoder->base.base.id, active, new_crtc_state->active); > > + > > + I915_STATE_WARN(active && intel_crtc->pipe != pipe, > > + "Encoder connected to wrong pipe %c\n", > > + pipe_name(pipe)); > > + > > + if (active) > > + encoder->get_config(encoder, pipe_config); > > + } > > + > > + intel_crtc_compute_pixel_rate(pipe_config); > > + > > + if (!new_crtc_state->active) > > + return; > > + > > + intel_pipe_config_sanity_check(dev_priv, pipe_config); > > + > > + sw_config = to_intel_crtc_state(new_crtc_state); > > + if (!intel_pipe_config_compare(dev_priv, sw_config, > > + pipe_config, false)) { > > + I915_STATE_WARN(1, "pipe state doesn't match!\n"); > > + intel_dump_pipe_config(intel_crtc, pipe_config, > > + "[hw state]"); > > + intel_dump_pipe_config(intel_crtc, sw_config, > > + "[sw state]"); > > + } > > +} > > + > > +void > > +intel_verify_planes(struct intel_atomic_state *state) > > +{ > > + struct intel_plane *plane; > > + const struct intel_plane_state *plane_state; > > + int i; > > + > > + for_each_new_intel_plane_in_state(state, plane, > > + plane_state, i) > > + assert_plane(plane, plane_state->slave || > > + plane_state->base.visible); > > +} > > + > > +static void > > +verify_single_dpll_state(struct drm_i915_private *dev_priv, > > + struct intel_shared_dpll *pll, > > + struct drm_crtc *crtc, > > + struct drm_crtc_state *new_state) > > +{ > > + struct intel_dpll_hw_state dpll_hw_state; > > + unsigned int crtc_mask; > > + bool active; > > + > > + memset(&dpll_hw_state, 0, sizeof(dpll_hw_state)); > > + > > + DRM_DEBUG_KMS("%s\n", pll->info->name); > > + > > + active = pll->info->funcs->get_hw_state(dev_priv, pll, &dpll_hw_state); > > + > > + if (!(pll->info->flags & INTEL_DPLL_ALWAYS_ON)) { > > + I915_STATE_WARN(!pll->on && pll->active_mask, > > + "pll in active use but not on in sw tracking\n"); > > + I915_STATE_WARN(pll->on && !pll->active_mask, > > + "pll is on but not used by any active crtc\n"); > > + I915_STATE_WARN(pll->on != active, > > + "pll on state mismatch (expected %i, found %i)\n", > > + pll->on, active); > > + } > > + > > + if (!crtc) { > > + I915_STATE_WARN(pll->active_mask & ~pll->state.crtc_mask, > > + "more active pll users than references: %x vs %x\n", > > + pll->active_mask, pll->state.crtc_mask); > > + > > + return; > > + } > > + > > + crtc_mask = drm_crtc_mask(crtc); > > + > > + if (new_state->active) > > + I915_STATE_WARN(!(pll->active_mask & crtc_mask), > > + "pll active mismatch (expected pipe %c in active mask 0x%02x)\n", > > + pipe_name(drm_crtc_index(crtc)), pll->active_mask); > > + else > > + I915_STATE_WARN(pll->active_mask & crtc_mask, > > + "pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n", > > + pipe_name(drm_crtc_index(crtc)), pll->active_mask); > > + > > + I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask), > > + "pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n", > > + crtc_mask, pll->state.crtc_mask); > > + > > + I915_STATE_WARN(pll->on && memcmp(&pll->state.hw_state, > > + &dpll_hw_state, > > + sizeof(dpll_hw_state)), > > + "pll hw state mismatch\n"); > > +} > > + > > +static void > > +verify_shared_dpll_state(struct drm_device *dev, struct drm_crtc *crtc, > > + struct drm_crtc_state *old_crtc_state, > > + struct drm_crtc_state *new_crtc_state) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(dev); > > + struct intel_crtc_state *old_state = to_intel_crtc_state(old_crtc_state); > > + struct intel_crtc_state *new_state = to_intel_crtc_state(new_crtc_state); > > + > > + if (new_state->shared_dpll) > > + verify_single_dpll_state(dev_priv, new_state->shared_dpll, crtc, new_crtc_state); > > + > > + if (old_state->shared_dpll && > > + old_state->shared_dpll != new_state->shared_dpll) { > > + unsigned int crtc_mask = drm_crtc_mask(crtc); > > + struct intel_shared_dpll *pll = old_state->shared_dpll; > > + > > + I915_STATE_WARN(pll->active_mask & crtc_mask, > > + "pll active mismatch (didn't expect pipe %c in active mask)\n", > > + pipe_name(drm_crtc_index(crtc))); > > + I915_STATE_WARN(pll->state.crtc_mask & crtc_mask, > > + "pll enabled crtcs mismatch (found %x in enabled mask)\n", > > + pipe_name(drm_crtc_index(crtc))); > > + } > > +} > > + > > +void > > +intel_verify_modeset_crtc(struct drm_crtc *crtc, > > + struct drm_atomic_state *state, > > + struct drm_crtc_state *old_state, > > + struct drm_crtc_state *new_state) > > +{ > > + if (!drm_atomic_crtc_needs_modeset(new_state) && > > + !to_intel_crtc_state(new_state)->update_pipe) > > + return; > > + > > + verify_wm_state(crtc, new_state); > > + verify_connector_state(crtc->dev, state, crtc); > > + verify_crtc_state(crtc, old_state, new_state); > > + verify_shared_dpll_state(crtc->dev, crtc, old_state, new_state); > > +} > > + > > +static void > > +verify_disabled_dpll_state(struct drm_device *dev) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(dev); > > + int i; > > + > > + for (i = 0; i < dev_priv->num_shared_dpll; i++) > > + verify_single_dpll_state(dev_priv, &dev_priv->shared_dplls[i], NULL, NULL); > > +} > > + > > +void > > +intel_verify_modeset_disabled(struct drm_device *dev, > > + struct drm_atomic_state *state) > > +{ > > + verify_encoder_state(dev, state); > > + verify_connector_state(dev, state, NULL); > > + verify_disabled_dpll_state(dev); > > +} > > diff --git a/drivers/gpu/drm/i915/intel_verify.h b/drivers/gpu/drm/i915/intel_verify.h > > new file mode 100644 > > index 000000..4b751ea > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/intel_verify.h > > @@ -0,0 +1,22 @@ > > +/* SPDX-License-Identifier: MIT */ > > +/* > > + * Copyright © 2019 Intel Corporation > > + */ > > + > > +#ifndef __INTEL_VERIFY_H__ > > +#define __INTEL_VERIFY_H__ > > + > > +struct drm_atomic_state; > > +struct drm_crtc; > > +struct drm_crtc_state; > > +struct drm_device; > > + > > +void intel_verify_modeset_crtc(struct drm_crtc *crtc, > > + struct drm_atomic_state *state, > > + struct drm_crtc_state *old_state, > > + struct drm_crtc_state *new_state); > > +void intel_verify_modeset_disabled(struct drm_device *dev, > > + struct drm_atomic_state *state); > > +void intel_verify_planes(struct intel_atomic_state *state); > > + > > +#endif /* __INTEL_VERIFY_H__ */ > > -- > > 2.20.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index fbcb0904..a000fad 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -166,6 +166,7 @@ i915-y += dvo_ch7017.o \ intel_panel.o \ intel_sdvo.o \ intel_tv.o \ + intel_verify.o \ vlv_dsi.o \ vlv_dsi_pll.o \ intel_vdsc.o diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3bd40a..31a931 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -69,6 +69,7 @@ #include "intel_sdvo.h" #include "intel_sprite.h" #include "intel_tv.h" +#include "intel_verify.h" /* Primary plane formats for gen <= 3 */ static const u32 i8xx_primary_formats[] = { @@ -1243,7 +1244,7 @@ void assert_pipe(struct drm_i915_private *dev_priv, pipe_name(pipe), onoff(state), onoff(cur_state)); } -static void assert_plane(struct intel_plane *plane, bool state) +void assert_plane(struct intel_plane *plane, bool state) { enum pipe pipe; bool cur_state; @@ -6607,45 +6608,6 @@ void intel_encoder_destroy(struct drm_encoder *encoder) kfree(intel_encoder); } -/* Cross check the actual hw state with our own modeset state tracking (and it's - * internal consistency). */ -static void intel_connector_verify_state(struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state) -{ - struct intel_connector *connector = to_intel_connector(conn_state->connector); - - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", - connector->base.base.id, - connector->base.name); - - if (connector->get_hw_state(connector)) { - struct intel_encoder *encoder = connector->encoder; - - I915_STATE_WARN(!crtc_state, - "connector enabled without attached crtc\n"); - - if (!crtc_state) - return; - - I915_STATE_WARN(!crtc_state->active, - "connector is active, but attached crtc isn't\n"); - - if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST) - return; - - I915_STATE_WARN(conn_state->best_encoder != &encoder->base, - "atomic encoder doesn't match attached encoder\n"); - - I915_STATE_WARN(conn_state->crtc != encoder->base.crtc, - "attached encoder crtc differs from connector crtc\n"); - } else { - I915_STATE_WARN(crtc_state && crtc_state->active, - "attached crtc is active, but connector isn't\n"); - I915_STATE_WARN(!crtc_state && conn_state->best_encoder, - "best encoder set without crtc!\n"); - } -} - static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state) { if (crtc_state->base.enable && crtc_state->has_pch_encoder) @@ -6879,7 +6841,7 @@ static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config) return pixel_rate; } -static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state) +void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); @@ -11584,9 +11546,9 @@ static const char *output_formats(enum intel_output_format format) return output_format_str[format]; } -static void intel_dump_pipe_config(struct intel_crtc *crtc, - struct intel_crtc_state *pipe_config, - const char *context) +void intel_dump_pipe_config(struct intel_crtc *crtc, + struct intel_crtc_state *pipe_config, + const char *context) { struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -12076,7 +12038,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv) return false; } -static bool +bool intel_pipe_config_compare(struct drm_i915_private *dev_priv, struct intel_crtc_state *current_config, struct intel_crtc_state *pipe_config, @@ -12389,8 +12351,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv, return ret; } -static void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, - const struct intel_crtc_state *pipe_config) +void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, + const struct intel_crtc_state *pipe_config) { if (pipe_config->has_pch_encoder) { int fdi_dotclock = intel_dotclock_calculate(intel_fdi_link_freq(dev_priv, pipe_config), @@ -12407,420 +12369,6 @@ static void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, } } -static void verify_wm_state(struct drm_crtc *crtc, - struct drm_crtc_state *new_state) -{ - struct drm_i915_private *dev_priv = to_i915(crtc->dev); - struct skl_hw_state { - struct skl_ddb_entry ddb_y[I915_MAX_PLANES]; - struct skl_ddb_entry ddb_uv[I915_MAX_PLANES]; - struct skl_ddb_allocation ddb; - struct skl_pipe_wm wm; - } *hw; - struct skl_ddb_allocation *sw_ddb; - struct skl_pipe_wm *sw_wm; - struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - const enum pipe pipe = intel_crtc->pipe; - int plane, level, max_level = ilk_wm_max_level(dev_priv); - - if (INTEL_GEN(dev_priv) < 9 || !new_state->active) - return; - - hw = kzalloc(sizeof(*hw), GFP_KERNEL); - if (!hw) - return; - - skl_pipe_wm_get_hw_state(intel_crtc, &hw->wm); - sw_wm = &to_intel_crtc_state(new_state)->wm.skl.optimal; - - skl_pipe_ddb_get_hw_state(intel_crtc, hw->ddb_y, hw->ddb_uv); - - skl_ddb_get_hw_state(dev_priv, &hw->ddb); - sw_ddb = &dev_priv->wm.skl_hw.ddb; - - if (INTEL_GEN(dev_priv) >= 11 && - hw->ddb.enabled_slices != sw_ddb->enabled_slices) - DRM_ERROR("mismatch in DBUF Slices (expected %u, got %u)\n", - sw_ddb->enabled_slices, - hw->ddb.enabled_slices); - - /* planes */ - for_each_universal_plane(dev_priv, pipe, plane) { - struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; - - hw_plane_wm = &hw->wm.planes[plane]; - sw_plane_wm = &sw_wm->planes[plane]; - - /* Watermarks */ - for (level = 0; level <= max_level; level++) { - if (skl_wm_level_equals(&hw_plane_wm->wm[level], - &sw_plane_wm->wm[level])) - continue; - - DRM_ERROR("mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", - pipe_name(pipe), plane + 1, level, - sw_plane_wm->wm[level].plane_en, - sw_plane_wm->wm[level].plane_res_b, - sw_plane_wm->wm[level].plane_res_l, - hw_plane_wm->wm[level].plane_en, - hw_plane_wm->wm[level].plane_res_b, - hw_plane_wm->wm[level].plane_res_l); - } - - if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, - &sw_plane_wm->trans_wm)) { - DRM_ERROR("mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", - pipe_name(pipe), plane + 1, - sw_plane_wm->trans_wm.plane_en, - sw_plane_wm->trans_wm.plane_res_b, - sw_plane_wm->trans_wm.plane_res_l, - hw_plane_wm->trans_wm.plane_en, - hw_plane_wm->trans_wm.plane_res_b, - hw_plane_wm->trans_wm.plane_res_l); - } - - /* DDB */ - hw_ddb_entry = &hw->ddb_y[plane]; - sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[plane]; - - if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { - DRM_ERROR("mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n", - pipe_name(pipe), plane + 1, - sw_ddb_entry->start, sw_ddb_entry->end, - hw_ddb_entry->start, hw_ddb_entry->end); - } - } - - /* - * cursor - * If the cursor plane isn't active, we may not have updated it's ddb - * allocation. In that case since the ddb allocation will be updated - * once the plane becomes visible, we can skip this check - */ - if (1) { - struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; - - hw_plane_wm = &hw->wm.planes[PLANE_CURSOR]; - sw_plane_wm = &sw_wm->planes[PLANE_CURSOR]; - - /* Watermarks */ - for (level = 0; level <= max_level; level++) { - if (skl_wm_level_equals(&hw_plane_wm->wm[level], - &sw_plane_wm->wm[level])) - continue; - - DRM_ERROR("mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", - pipe_name(pipe), level, - sw_plane_wm->wm[level].plane_en, - sw_plane_wm->wm[level].plane_res_b, - sw_plane_wm->wm[level].plane_res_l, - hw_plane_wm->wm[level].plane_en, - hw_plane_wm->wm[level].plane_res_b, - hw_plane_wm->wm[level].plane_res_l); - } - - if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, - &sw_plane_wm->trans_wm)) { - DRM_ERROR("mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", - pipe_name(pipe), - sw_plane_wm->trans_wm.plane_en, - sw_plane_wm->trans_wm.plane_res_b, - sw_plane_wm->trans_wm.plane_res_l, - hw_plane_wm->trans_wm.plane_en, - hw_plane_wm->trans_wm.plane_res_b, - hw_plane_wm->trans_wm.plane_res_l); - } - - /* DDB */ - hw_ddb_entry = &hw->ddb_y[PLANE_CURSOR]; - sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[PLANE_CURSOR]; - - if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { - DRM_ERROR("mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n", - pipe_name(pipe), - sw_ddb_entry->start, sw_ddb_entry->end, - hw_ddb_entry->start, hw_ddb_entry->end); - } - } - - kfree(hw); -} - -static void -verify_connector_state(struct drm_device *dev, - struct drm_atomic_state *state, - struct drm_crtc *crtc) -{ - struct drm_connector *connector; - struct drm_connector_state *new_conn_state; - int i; - - for_each_new_connector_in_state(state, connector, new_conn_state, i) { - struct drm_encoder *encoder = connector->encoder; - struct drm_crtc_state *crtc_state = NULL; - - if (new_conn_state->crtc != crtc) - continue; - - if (crtc) - crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc); - - intel_connector_verify_state(crtc_state, new_conn_state); - - I915_STATE_WARN(new_conn_state->best_encoder != encoder, - "connector's atomic encoder doesn't match legacy encoder\n"); - } -} - -static void -verify_encoder_state(struct drm_device *dev, struct drm_atomic_state *state) -{ - struct intel_encoder *encoder; - struct drm_connector *connector; - struct drm_connector_state *old_conn_state, *new_conn_state; - int i; - - for_each_intel_encoder(dev, encoder) { - bool enabled = false, found = false; - enum pipe pipe; - - DRM_DEBUG_KMS("[ENCODER:%d:%s]\n", - encoder->base.base.id, - encoder->base.name); - - for_each_oldnew_connector_in_state(state, connector, old_conn_state, - new_conn_state, i) { - if (old_conn_state->best_encoder == &encoder->base) - found = true; - - if (new_conn_state->best_encoder != &encoder->base) - continue; - found = enabled = true; - - I915_STATE_WARN(new_conn_state->crtc != - encoder->base.crtc, - "connector's crtc doesn't match encoder crtc\n"); - } - - if (!found) - continue; - - I915_STATE_WARN(!!encoder->base.crtc != enabled, - "encoder's enabled state mismatch " - "(expected %i, found %i)\n", - !!encoder->base.crtc, enabled); - - if (!encoder->base.crtc) { - bool active; - - active = encoder->get_hw_state(encoder, &pipe); - I915_STATE_WARN(active, - "encoder detached but still enabled on pipe %c.\n", - pipe_name(pipe)); - } - } -} - -static void -verify_crtc_state(struct drm_crtc *crtc, - struct drm_crtc_state *old_crtc_state, - struct drm_crtc_state *new_crtc_state) -{ - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_encoder *encoder; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_crtc_state *pipe_config, *sw_config; - struct drm_atomic_state *old_state; - bool active; - - old_state = old_crtc_state->state; - __drm_atomic_helper_crtc_destroy_state(old_crtc_state); - pipe_config = to_intel_crtc_state(old_crtc_state); - memset(pipe_config, 0, sizeof(*pipe_config)); - pipe_config->base.crtc = crtc; - pipe_config->base.state = old_state; - - DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); - - active = dev_priv->display.get_pipe_config(intel_crtc, pipe_config); - - /* we keep both pipes enabled on 830 */ - if (IS_I830(dev_priv)) - active = new_crtc_state->active; - - I915_STATE_WARN(new_crtc_state->active != active, - "crtc active state doesn't match with hw state " - "(expected %i, found %i)\n", new_crtc_state->active, active); - - I915_STATE_WARN(intel_crtc->active != new_crtc_state->active, - "transitional active state does not match atomic hw state " - "(expected %i, found %i)\n", new_crtc_state->active, intel_crtc->active); - - for_each_encoder_on_crtc(dev, crtc, encoder) { - enum pipe pipe; - - active = encoder->get_hw_state(encoder, &pipe); - I915_STATE_WARN(active != new_crtc_state->active, - "[ENCODER:%i] active %i with crtc active %i\n", - encoder->base.base.id, active, new_crtc_state->active); - - I915_STATE_WARN(active && intel_crtc->pipe != pipe, - "Encoder connected to wrong pipe %c\n", - pipe_name(pipe)); - - if (active) - encoder->get_config(encoder, pipe_config); - } - - intel_crtc_compute_pixel_rate(pipe_config); - - if (!new_crtc_state->active) - return; - - intel_pipe_config_sanity_check(dev_priv, pipe_config); - - sw_config = to_intel_crtc_state(new_crtc_state); - if (!intel_pipe_config_compare(dev_priv, sw_config, - pipe_config, false)) { - I915_STATE_WARN(1, "pipe state doesn't match!\n"); - intel_dump_pipe_config(intel_crtc, pipe_config, - "[hw state]"); - intel_dump_pipe_config(intel_crtc, sw_config, - "[sw state]"); - } -} - -static void -intel_verify_planes(struct intel_atomic_state *state) -{ - struct intel_plane *plane; - const struct intel_plane_state *plane_state; - int i; - - for_each_new_intel_plane_in_state(state, plane, - plane_state, i) - assert_plane(plane, plane_state->slave || - plane_state->base.visible); -} - -static void -verify_single_dpll_state(struct drm_i915_private *dev_priv, - struct intel_shared_dpll *pll, - struct drm_crtc *crtc, - struct drm_crtc_state *new_state) -{ - struct intel_dpll_hw_state dpll_hw_state; - unsigned int crtc_mask; - bool active; - - memset(&dpll_hw_state, 0, sizeof(dpll_hw_state)); - - DRM_DEBUG_KMS("%s\n", pll->info->name); - - active = pll->info->funcs->get_hw_state(dev_priv, pll, &dpll_hw_state); - - if (!(pll->info->flags & INTEL_DPLL_ALWAYS_ON)) { - I915_STATE_WARN(!pll->on && pll->active_mask, - "pll in active use but not on in sw tracking\n"); - I915_STATE_WARN(pll->on && !pll->active_mask, - "pll is on but not used by any active crtc\n"); - I915_STATE_WARN(pll->on != active, - "pll on state mismatch (expected %i, found %i)\n", - pll->on, active); - } - - if (!crtc) { - I915_STATE_WARN(pll->active_mask & ~pll->state.crtc_mask, - "more active pll users than references: %x vs %x\n", - pll->active_mask, pll->state.crtc_mask); - - return; - } - - crtc_mask = drm_crtc_mask(crtc); - - if (new_state->active) - I915_STATE_WARN(!(pll->active_mask & crtc_mask), - "pll active mismatch (expected pipe %c in active mask 0x%02x)\n", - pipe_name(drm_crtc_index(crtc)), pll->active_mask); - else - I915_STATE_WARN(pll->active_mask & crtc_mask, - "pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n", - pipe_name(drm_crtc_index(crtc)), pll->active_mask); - - I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask), - "pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n", - crtc_mask, pll->state.crtc_mask); - - I915_STATE_WARN(pll->on && memcmp(&pll->state.hw_state, - &dpll_hw_state, - sizeof(dpll_hw_state)), - "pll hw state mismatch\n"); -} - -static void -verify_shared_dpll_state(struct drm_device *dev, struct drm_crtc *crtc, - struct drm_crtc_state *old_crtc_state, - struct drm_crtc_state *new_crtc_state) -{ - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_crtc_state *old_state = to_intel_crtc_state(old_crtc_state); - struct intel_crtc_state *new_state = to_intel_crtc_state(new_crtc_state); - - if (new_state->shared_dpll) - verify_single_dpll_state(dev_priv, new_state->shared_dpll, crtc, new_crtc_state); - - if (old_state->shared_dpll && - old_state->shared_dpll != new_state->shared_dpll) { - unsigned int crtc_mask = drm_crtc_mask(crtc); - struct intel_shared_dpll *pll = old_state->shared_dpll; - - I915_STATE_WARN(pll->active_mask & crtc_mask, - "pll active mismatch (didn't expect pipe %c in active mask)\n", - pipe_name(drm_crtc_index(crtc))); - I915_STATE_WARN(pll->state.crtc_mask & crtc_mask, - "pll enabled crtcs mismatch (found %x in enabled mask)\n", - pipe_name(drm_crtc_index(crtc))); - } -} - -static void -intel_modeset_verify_crtc(struct drm_crtc *crtc, - struct drm_atomic_state *state, - struct drm_crtc_state *old_state, - struct drm_crtc_state *new_state) -{ - if (!needs_modeset(new_state) && - !to_intel_crtc_state(new_state)->update_pipe) - return; - - verify_wm_state(crtc, new_state); - verify_connector_state(crtc->dev, state, crtc); - verify_crtc_state(crtc, old_state, new_state); - verify_shared_dpll_state(crtc->dev, crtc, old_state, new_state); -} - -static void -verify_disabled_dpll_state(struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = to_i915(dev); - int i; - - for (i = 0; i < dev_priv->num_shared_dpll; i++) - verify_single_dpll_state(dev_priv, &dev_priv->shared_dplls[i], NULL, NULL); -} - -static void -intel_modeset_verify_disabled(struct drm_device *dev, - struct drm_atomic_state *state) -{ - verify_encoder_state(dev, state); - verify_connector_state(dev, state, NULL); - verify_disabled_dpll_state(dev); -} - static void update_scanline_offset(const struct intel_crtc_state *crtc_state) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); @@ -13521,7 +13069,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) if (!intel_can_enable_sagv(state)) intel_disable_sagv(dev_priv); - intel_modeset_verify_disabled(dev, state); + intel_verify_modeset_disabled(dev, state); } /* Complete the events for pipes that have now been disabled */ @@ -13589,7 +13137,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) if (put_domains[i]) modeset_put_power_domains(dev_priv, put_domains[i]); - intel_modeset_verify_crtc(crtc, state, old_crtc_state, new_crtc_state); + intel_verify_modeset_crtc(crtc, state, old_crtc_state, new_crtc_state); } if (intel_state->modeset) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a38b9c..77767c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1668,6 +1668,17 @@ int intel_get_pipe_from_crtc_id_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv, enum pipe pipe); +void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv, + const struct intel_crtc_state *pipe_config); +bool +intel_pipe_config_compare(struct drm_i915_private *dev_priv, + struct intel_crtc_state *current_config, + struct intel_crtc_state *pipe_config, + bool adjust); +void intel_dump_pipe_config(struct intel_crtc *crtc, + struct intel_crtc_state *pipe_config, + const char *context); +void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state); static inline bool intel_crtc_has_type(const struct intel_crtc_state *crtc_state, enum intel_output_type type) @@ -1760,6 +1771,7 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv, void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, bool state); #define assert_pipe_enabled(d, p) assert_pipe(d, p, true) #define assert_pipe_disabled(d, p) assert_pipe(d, p, false) +void assert_plane(struct intel_plane *plane, bool state); void intel_prepare_reset(struct drm_i915_private *dev_priv); void intel_finish_reset(struct drm_i915_private *dev_priv); void hsw_enable_pc8(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_verify.c b/drivers/gpu/drm/i915/intel_verify.c new file mode 100644 index 000000..4c8990 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_verify.c @@ -0,0 +1,464 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2019 Intel Corporation + */ + +#include <drm/drm_atomic_state_helper.h> + +#include "i915_drv.h" +#include "intel_drv.h" +#include "intel_pm.h" +#include "intel_verify.h" + +static void verify_wm_state(struct drm_crtc *crtc, + struct drm_crtc_state *new_state) +{ + struct drm_i915_private *dev_priv = to_i915(crtc->dev); + struct skl_hw_state { + struct skl_ddb_entry ddb_y[I915_MAX_PLANES]; + struct skl_ddb_entry ddb_uv[I915_MAX_PLANES]; + struct skl_ddb_allocation ddb; + struct skl_pipe_wm wm; + } *hw; + struct skl_ddb_allocation *sw_ddb; + struct skl_pipe_wm *sw_wm; + struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + const enum pipe pipe = intel_crtc->pipe; + int plane, level, max_level = ilk_wm_max_level(dev_priv); + + if (INTEL_GEN(dev_priv) < 9 || !new_state->active) + return; + + hw = kzalloc(sizeof(*hw), GFP_KERNEL); + if (!hw) + return; + + skl_pipe_wm_get_hw_state(intel_crtc, &hw->wm); + sw_wm = &to_intel_crtc_state(new_state)->wm.skl.optimal; + + skl_pipe_ddb_get_hw_state(intel_crtc, hw->ddb_y, hw->ddb_uv); + + skl_ddb_get_hw_state(dev_priv, &hw->ddb); + sw_ddb = &dev_priv->wm.skl_hw.ddb; + + if (INTEL_GEN(dev_priv) >= 11 && + hw->ddb.enabled_slices != sw_ddb->enabled_slices) + DRM_ERROR("mismatch in DBUF Slices (expected %u, got %u)\n", + sw_ddb->enabled_slices, + hw->ddb.enabled_slices); + + /* planes */ + for_each_universal_plane(dev_priv, pipe, plane) { + struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; + + hw_plane_wm = &hw->wm.planes[plane]; + sw_plane_wm = &sw_wm->planes[plane]; + + /* Watermarks */ + for (level = 0; level <= max_level; level++) { + if (skl_wm_level_equals(&hw_plane_wm->wm[level], + &sw_plane_wm->wm[level])) + continue; + + DRM_ERROR("mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", + pipe_name(pipe), plane + 1, level, + sw_plane_wm->wm[level].plane_en, + sw_plane_wm->wm[level].plane_res_b, + sw_plane_wm->wm[level].plane_res_l, + hw_plane_wm->wm[level].plane_en, + hw_plane_wm->wm[level].plane_res_b, + hw_plane_wm->wm[level].plane_res_l); + } + + if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, + &sw_plane_wm->trans_wm)) { + DRM_ERROR("mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", + pipe_name(pipe), plane + 1, + sw_plane_wm->trans_wm.plane_en, + sw_plane_wm->trans_wm.plane_res_b, + sw_plane_wm->trans_wm.plane_res_l, + hw_plane_wm->trans_wm.plane_en, + hw_plane_wm->trans_wm.plane_res_b, + hw_plane_wm->trans_wm.plane_res_l); + } + + /* DDB */ + hw_ddb_entry = &hw->ddb_y[plane]; + sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[plane]; + + if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { + DRM_ERROR("mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n", + pipe_name(pipe), plane + 1, + sw_ddb_entry->start, sw_ddb_entry->end, + hw_ddb_entry->start, hw_ddb_entry->end); + } + } + + /* + * cursor + * If the cursor plane isn't active, we may not have updated it's ddb + * allocation. In that case since the ddb allocation will be updated + * once the plane becomes visible, we can skip this check + */ + if (1) { + struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; + + hw_plane_wm = &hw->wm.planes[PLANE_CURSOR]; + sw_plane_wm = &sw_wm->planes[PLANE_CURSOR]; + + /* Watermarks */ + for (level = 0; level <= max_level; level++) { + if (skl_wm_level_equals(&hw_plane_wm->wm[level], + &sw_plane_wm->wm[level])) + continue; + + DRM_ERROR("mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", + pipe_name(pipe), level, + sw_plane_wm->wm[level].plane_en, + sw_plane_wm->wm[level].plane_res_b, + sw_plane_wm->wm[level].plane_res_l, + hw_plane_wm->wm[level].plane_en, + hw_plane_wm->wm[level].plane_res_b, + hw_plane_wm->wm[level].plane_res_l); + } + + if (!skl_wm_level_equals(&hw_plane_wm->trans_wm, + &sw_plane_wm->trans_wm)) { + DRM_ERROR("mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n", + pipe_name(pipe), + sw_plane_wm->trans_wm.plane_en, + sw_plane_wm->trans_wm.plane_res_b, + sw_plane_wm->trans_wm.plane_res_l, + hw_plane_wm->trans_wm.plane_en, + hw_plane_wm->trans_wm.plane_res_b, + hw_plane_wm->trans_wm.plane_res_l); + } + + /* DDB */ + hw_ddb_entry = &hw->ddb_y[PLANE_CURSOR]; + sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[PLANE_CURSOR]; + + if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { + DRM_ERROR("mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n", + pipe_name(pipe), + sw_ddb_entry->start, sw_ddb_entry->end, + hw_ddb_entry->start, hw_ddb_entry->end); + } + } + + kfree(hw); +} + +/* Cross check the actual hw state with our own modeset state tracking (and it's + * internal consistency). */ +static void intel_connector_verify_state(struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct intel_connector *connector = to_intel_connector(conn_state->connector); + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", + connector->base.base.id, + connector->base.name); + + if (connector->get_hw_state(connector)) { + struct intel_encoder *encoder = connector->encoder; + + I915_STATE_WARN(!crtc_state, + "connector enabled without attached crtc\n"); + + if (!crtc_state) + return; + + I915_STATE_WARN(!crtc_state->active, + "connector is active, but attached crtc isn't\n"); + + if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST) + return; + + I915_STATE_WARN(conn_state->best_encoder != &encoder->base, + "atomic encoder doesn't match attached encoder\n"); + + I915_STATE_WARN(conn_state->crtc != encoder->base.crtc, + "attached encoder crtc differs from connector crtc\n"); + } else { + I915_STATE_WARN(crtc_state && crtc_state->active, + "attached crtc is active, but connector isn't\n"); + I915_STATE_WARN(!crtc_state && conn_state->best_encoder, + "best encoder set without crtc!\n"); + } +} + +static void +verify_connector_state(struct drm_device *dev, + struct drm_atomic_state *state, + struct drm_crtc *crtc) +{ + struct drm_connector *connector; + struct drm_connector_state *new_conn_state; + int i; + + for_each_new_connector_in_state(state, connector, new_conn_state, i) { + struct drm_encoder *encoder = connector->encoder; + struct drm_crtc_state *crtc_state = NULL; + + if (new_conn_state->crtc != crtc) + continue; + + if (crtc) + crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc); + + intel_connector_verify_state(crtc_state, new_conn_state); + + I915_STATE_WARN(new_conn_state->best_encoder != encoder, + "connector's atomic encoder doesn't match legacy encoder\n"); + } +} + +static void +verify_encoder_state(struct drm_device *dev, struct drm_atomic_state *state) +{ + struct intel_encoder *encoder; + struct drm_connector *connector; + struct drm_connector_state *old_conn_state, *new_conn_state; + int i; + + for_each_intel_encoder(dev, encoder) { + bool enabled = false, found = false; + enum pipe pipe; + + DRM_DEBUG_KMS("[ENCODER:%d:%s]\n", + encoder->base.base.id, + encoder->base.name); + + for_each_oldnew_connector_in_state(state, connector, old_conn_state, + new_conn_state, i) { + if (old_conn_state->best_encoder == &encoder->base) + found = true; + + if (new_conn_state->best_encoder != &encoder->base) + continue; + found = enabled = true; + + I915_STATE_WARN(new_conn_state->crtc != + encoder->base.crtc, + "connector's crtc doesn't match encoder crtc\n"); + } + + if (!found) + continue; + + I915_STATE_WARN(!!encoder->base.crtc != enabled, + "encoder's enabled state mismatch " + "(expected %i, found %i)\n", + !!encoder->base.crtc, enabled); + + if (!encoder->base.crtc) { + bool active; + + active = encoder->get_hw_state(encoder, &pipe); + I915_STATE_WARN(active, + "encoder detached but still enabled on pipe %c.\n", + pipe_name(pipe)); + } + } +} + +static void +verify_crtc_state(struct drm_crtc *crtc, + struct drm_crtc_state *old_crtc_state, + struct drm_crtc_state *new_crtc_state) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = to_i915(dev); + struct intel_encoder *encoder; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc_state *pipe_config, *sw_config; + struct drm_atomic_state *old_state; + bool active; + + old_state = old_crtc_state->state; + __drm_atomic_helper_crtc_destroy_state(old_crtc_state); + pipe_config = to_intel_crtc_state(old_crtc_state); + memset(pipe_config, 0, sizeof(*pipe_config)); + pipe_config->base.crtc = crtc; + pipe_config->base.state = old_state; + + DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); + + active = dev_priv->display.get_pipe_config(intel_crtc, pipe_config); + + /* we keep both pipes enabled on 830 */ + if (IS_I830(dev_priv)) + active = new_crtc_state->active; + + I915_STATE_WARN(new_crtc_state->active != active, + "crtc active state doesn't match with hw state " + "(expected %i, found %i)\n", new_crtc_state->active, active); + + I915_STATE_WARN(intel_crtc->active != new_crtc_state->active, + "transitional active state does not match atomic hw state " + "(expected %i, found %i)\n", new_crtc_state->active, intel_crtc->active); + + for_each_encoder_on_crtc(dev, crtc, encoder) { + enum pipe pipe; + + active = encoder->get_hw_state(encoder, &pipe); + I915_STATE_WARN(active != new_crtc_state->active, + "[ENCODER:%i] active %i with crtc active %i\n", + encoder->base.base.id, active, new_crtc_state->active); + + I915_STATE_WARN(active && intel_crtc->pipe != pipe, + "Encoder connected to wrong pipe %c\n", + pipe_name(pipe)); + + if (active) + encoder->get_config(encoder, pipe_config); + } + + intel_crtc_compute_pixel_rate(pipe_config); + + if (!new_crtc_state->active) + return; + + intel_pipe_config_sanity_check(dev_priv, pipe_config); + + sw_config = to_intel_crtc_state(new_crtc_state); + if (!intel_pipe_config_compare(dev_priv, sw_config, + pipe_config, false)) { + I915_STATE_WARN(1, "pipe state doesn't match!\n"); + intel_dump_pipe_config(intel_crtc, pipe_config, + "[hw state]"); + intel_dump_pipe_config(intel_crtc, sw_config, + "[sw state]"); + } +} + +void +intel_verify_planes(struct intel_atomic_state *state) +{ + struct intel_plane *plane; + const struct intel_plane_state *plane_state; + int i; + + for_each_new_intel_plane_in_state(state, plane, + plane_state, i) + assert_plane(plane, plane_state->slave || + plane_state->base.visible); +} + +static void +verify_single_dpll_state(struct drm_i915_private *dev_priv, + struct intel_shared_dpll *pll, + struct drm_crtc *crtc, + struct drm_crtc_state *new_state) +{ + struct intel_dpll_hw_state dpll_hw_state; + unsigned int crtc_mask; + bool active; + + memset(&dpll_hw_state, 0, sizeof(dpll_hw_state)); + + DRM_DEBUG_KMS("%s\n", pll->info->name); + + active = pll->info->funcs->get_hw_state(dev_priv, pll, &dpll_hw_state); + + if (!(pll->info->flags & INTEL_DPLL_ALWAYS_ON)) { + I915_STATE_WARN(!pll->on && pll->active_mask, + "pll in active use but not on in sw tracking\n"); + I915_STATE_WARN(pll->on && !pll->active_mask, + "pll is on but not used by any active crtc\n"); + I915_STATE_WARN(pll->on != active, + "pll on state mismatch (expected %i, found %i)\n", + pll->on, active); + } + + if (!crtc) { + I915_STATE_WARN(pll->active_mask & ~pll->state.crtc_mask, + "more active pll users than references: %x vs %x\n", + pll->active_mask, pll->state.crtc_mask); + + return; + } + + crtc_mask = drm_crtc_mask(crtc); + + if (new_state->active) + I915_STATE_WARN(!(pll->active_mask & crtc_mask), + "pll active mismatch (expected pipe %c in active mask 0x%02x)\n", + pipe_name(drm_crtc_index(crtc)), pll->active_mask); + else + I915_STATE_WARN(pll->active_mask & crtc_mask, + "pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n", + pipe_name(drm_crtc_index(crtc)), pll->active_mask); + + I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask), + "pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n", + crtc_mask, pll->state.crtc_mask); + + I915_STATE_WARN(pll->on && memcmp(&pll->state.hw_state, + &dpll_hw_state, + sizeof(dpll_hw_state)), + "pll hw state mismatch\n"); +} + +static void +verify_shared_dpll_state(struct drm_device *dev, struct drm_crtc *crtc, + struct drm_crtc_state *old_crtc_state, + struct drm_crtc_state *new_crtc_state) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct intel_crtc_state *old_state = to_intel_crtc_state(old_crtc_state); + struct intel_crtc_state *new_state = to_intel_crtc_state(new_crtc_state); + + if (new_state->shared_dpll) + verify_single_dpll_state(dev_priv, new_state->shared_dpll, crtc, new_crtc_state); + + if (old_state->shared_dpll && + old_state->shared_dpll != new_state->shared_dpll) { + unsigned int crtc_mask = drm_crtc_mask(crtc); + struct intel_shared_dpll *pll = old_state->shared_dpll; + + I915_STATE_WARN(pll->active_mask & crtc_mask, + "pll active mismatch (didn't expect pipe %c in active mask)\n", + pipe_name(drm_crtc_index(crtc))); + I915_STATE_WARN(pll->state.crtc_mask & crtc_mask, + "pll enabled crtcs mismatch (found %x in enabled mask)\n", + pipe_name(drm_crtc_index(crtc))); + } +} + +void +intel_verify_modeset_crtc(struct drm_crtc *crtc, + struct drm_atomic_state *state, + struct drm_crtc_state *old_state, + struct drm_crtc_state *new_state) +{ + if (!drm_atomic_crtc_needs_modeset(new_state) && + !to_intel_crtc_state(new_state)->update_pipe) + return; + + verify_wm_state(crtc, new_state); + verify_connector_state(crtc->dev, state, crtc); + verify_crtc_state(crtc, old_state, new_state); + verify_shared_dpll_state(crtc->dev, crtc, old_state, new_state); +} + +static void +verify_disabled_dpll_state(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + int i; + + for (i = 0; i < dev_priv->num_shared_dpll; i++) + verify_single_dpll_state(dev_priv, &dev_priv->shared_dplls[i], NULL, NULL); +} + +void +intel_verify_modeset_disabled(struct drm_device *dev, + struct drm_atomic_state *state) +{ + verify_encoder_state(dev, state); + verify_connector_state(dev, state, NULL); + verify_disabled_dpll_state(dev); +} diff --git a/drivers/gpu/drm/i915/intel_verify.h b/drivers/gpu/drm/i915/intel_verify.h new file mode 100644 index 000000..4b751ea --- /dev/null +++ b/drivers/gpu/drm/i915/intel_verify.h @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2019 Intel Corporation + */ + +#ifndef __INTEL_VERIFY_H__ +#define __INTEL_VERIFY_H__ + +struct drm_atomic_state; +struct drm_crtc; +struct drm_crtc_state; +struct drm_device; + +void intel_verify_modeset_crtc(struct drm_crtc *crtc, + struct drm_atomic_state *state, + struct drm_crtc_state *old_state, + struct drm_crtc_state *new_state); +void intel_verify_modeset_disabled(struct drm_device *dev, + struct drm_atomic_state *state); +void intel_verify_planes(struct intel_atomic_state *state); + +#endif /* __INTEL_VERIFY_H__ */
Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_display.c | 474 +-------------------------- drivers/gpu/drm/i915/intel_drv.h | 12 + drivers/gpu/drm/i915/intel_verify.c | 464 ++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_verify.h | 22 ++ 5 files changed, 510 insertions(+), 463 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_verify.c create mode 100644 drivers/gpu/drm/i915/intel_verify.h