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 <[email protected]>
> > ---
> >  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 
> <[email protected]>

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
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to