* [RFC 1/2] drm/i915: start moving state checker to intel_verify.c
@ 2019-04-16 10:36 Jani Nikula
2019-04-16 10:36 ` [RFC 2/2] drm/i915: move pipe config compare " Jani Nikula
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Jani Nikula @ 2019-04-16 10:36 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
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);
}
/* 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__ */
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC 2/2] drm/i915: move pipe config compare to intel_verify.c
2019-04-16 10:36 [RFC 1/2] drm/i915: start moving state checker to intel_verify.c Jani Nikula
@ 2019-04-16 10:36 ` Jani Nikula
2019-05-15 9:36 ` Daniel Vetter
2019-04-16 12:14 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [RFC,1/2] drm/i915: start moving state checker " Patchwork
` (4 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2019-04-16 10:36 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 471 +--------------------------
drivers/gpu/drm/i915/intel_drv.h | 9 +-
drivers/gpu/drm/i915/intel_verify.c | 465 ++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_verify.h | 8 +
4 files changed, 478 insertions(+), 475 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 31a931..14899f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -202,9 +202,9 @@ static void intel_update_czclk(struct drm_i915_private *dev_priv)
DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
}
-static inline u32 /* units of 100MHz */
-intel_fdi_link_freq(struct drm_i915_private *dev_priv,
- const struct intel_crtc_state *pipe_config)
+/* units of 100MHz */
+u32 intel_fdi_link_freq(struct drm_i915_private *dev_priv,
+ const struct intel_crtc_state *pipe_config)
{
if (HAS_DDI(dev_priv))
return pipe_config->port_clock; /* SPLL */
@@ -11904,471 +11904,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
return 0;
}
-static bool intel_fuzzy_clock_check(int clock1, int clock2)
-{
- int diff;
-
- if (clock1 == clock2)
- return true;
-
- if (!clock1 || !clock2)
- return false;
-
- diff = abs(clock1 - clock2);
-
- if (((((diff + clock1 + clock2) * 100)) / (clock1 + clock2)) < 105)
- return true;
-
- return false;
-}
-
-static bool
-intel_compare_m_n(unsigned int m, unsigned int n,
- unsigned int m2, unsigned int n2,
- bool exact)
-{
- if (m == m2 && n == n2)
- return true;
-
- if (exact || !m || !n || !m2 || !n2)
- return false;
-
- BUILD_BUG_ON(DATA_LINK_M_N_MASK > INT_MAX);
-
- if (n > n2) {
- while (n > n2) {
- m2 <<= 1;
- n2 <<= 1;
- }
- } else if (n < n2) {
- while (n < n2) {
- m <<= 1;
- n <<= 1;
- }
- }
-
- if (n != n2)
- return false;
-
- return intel_fuzzy_clock_check(m, m2);
-}
-
-static bool
-intel_compare_link_m_n(const struct intel_link_m_n *m_n,
- struct intel_link_m_n *m2_n2,
- bool adjust)
-{
- if (m_n->tu == m2_n2->tu &&
- intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
- m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
- intel_compare_m_n(m_n->link_m, m_n->link_n,
- m2_n2->link_m, m2_n2->link_n, !adjust)) {
- if (adjust)
- *m2_n2 = *m_n;
-
- return true;
- }
-
- return false;
-}
-
-static bool
-intel_compare_infoframe(const union hdmi_infoframe *a,
- const union hdmi_infoframe *b)
-{
- return memcmp(a, b, sizeof(*a)) == 0;
-}
-
-static void
-pipe_config_infoframe_err(struct drm_i915_private *dev_priv,
- bool adjust, const char *name,
- const union hdmi_infoframe *a,
- const union hdmi_infoframe *b)
-{
- if (adjust) {
- if ((drm_debug & DRM_UT_KMS) == 0)
- return;
-
- drm_dbg(DRM_UT_KMS, "mismatch in %s infoframe", name);
- drm_dbg(DRM_UT_KMS, "expected:");
- hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, a);
- drm_dbg(DRM_UT_KMS, "found");
- hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, b);
- } else {
- drm_err("mismatch in %s infoframe", name);
- drm_err("expected:");
- hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, a);
- drm_err("found");
- hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, b);
- }
-}
-
-static void __printf(3, 4)
-pipe_config_err(bool adjust, const char *name, const char *format, ...)
-{
- struct va_format vaf;
- va_list args;
-
- va_start(args, format);
- vaf.fmt = format;
- vaf.va = &args;
-
- if (adjust)
- drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
- else
- drm_err("mismatch in %s %pV", name, &vaf);
-
- va_end(args);
-}
-
-static bool fastboot_enabled(struct drm_i915_private *dev_priv)
-{
- if (i915_modparams.fastboot != -1)
- return i915_modparams.fastboot;
-
- /* Enable fastboot by default on Skylake and newer */
- if (INTEL_GEN(dev_priv) >= 9)
- return true;
-
- /* Enable fastboot by default on VLV and CHV */
- if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- return true;
-
- /* Disabled by default on all others */
- return false;
-}
-
-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)
-{
- bool ret = true;
- bool fixup_inherited = adjust &&
- (current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
- !(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
-
- if (fixup_inherited && !fastboot_enabled(dev_priv)) {
- DRM_DEBUG_KMS("initial modeset and fastboot not set\n");
- ret = false;
- }
-
-#define PIPE_CONF_CHECK_X(name) do { \
- if (current_config->name != pipe_config->name) { \
- pipe_config_err(adjust, __stringify(name), \
- "(expected 0x%08x, found 0x%08x)\n", \
- current_config->name, \
- pipe_config->name); \
- ret = false; \
- } \
-} while (0)
-
-#define PIPE_CONF_CHECK_I(name) do { \
- if (current_config->name != pipe_config->name) { \
- pipe_config_err(adjust, __stringify(name), \
- "(expected %i, found %i)\n", \
- current_config->name, \
- pipe_config->name); \
- ret = false; \
- } \
-} while (0)
-
-#define PIPE_CONF_CHECK_BOOL(name) do { \
- if (current_config->name != pipe_config->name) { \
- pipe_config_err(adjust, __stringify(name), \
- "(expected %s, found %s)\n", \
- yesno(current_config->name), \
- yesno(pipe_config->name)); \
- ret = false; \
- } \
-} while (0)
-
-/*
- * Checks state where we only read out the enabling, but not the entire
- * state itself (like full infoframes or ELD for audio). These states
- * require a full modeset on bootup to fix up.
- */
-#define PIPE_CONF_CHECK_BOOL_INCOMPLETE(name) do { \
- if (!fixup_inherited || (!current_config->name && !pipe_config->name)) { \
- PIPE_CONF_CHECK_BOOL(name); \
- } else { \
- pipe_config_err(adjust, __stringify(name), \
- "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
- yesno(current_config->name), \
- yesno(pipe_config->name)); \
- ret = false; \
- } \
-} while (0)
-
-#define PIPE_CONF_CHECK_P(name) do { \
- if (current_config->name != pipe_config->name) { \
- pipe_config_err(adjust, __stringify(name), \
- "(expected %p, found %p)\n", \
- current_config->name, \
- pipe_config->name); \
- ret = false; \
- } \
-} while (0)
-
-#define PIPE_CONF_CHECK_M_N(name) do { \
- if (!intel_compare_link_m_n(¤t_config->name, \
- &pipe_config->name,\
- adjust)) { \
- pipe_config_err(adjust, __stringify(name), \
- "(expected tu %i gmch %i/%i link %i/%i, " \
- "found tu %i, gmch %i/%i link %i/%i)\n", \
- current_config->name.tu, \
- current_config->name.gmch_m, \
- current_config->name.gmch_n, \
- current_config->name.link_m, \
- current_config->name.link_n, \
- pipe_config->name.tu, \
- pipe_config->name.gmch_m, \
- pipe_config->name.gmch_n, \
- pipe_config->name.link_m, \
- pipe_config->name.link_n); \
- ret = false; \
- } \
-} while (0)
-
-/* This is required for BDW+ where there is only one set of registers for
- * switching between high and low RR.
- * This macro can be used whenever a comparison has to be made between one
- * hw state and multiple sw state variables.
- */
-#define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
- if (!intel_compare_link_m_n(¤t_config->name, \
- &pipe_config->name, adjust) && \
- !intel_compare_link_m_n(¤t_config->alt_name, \
- &pipe_config->name, adjust)) { \
- pipe_config_err(adjust, __stringify(name), \
- "(expected tu %i gmch %i/%i link %i/%i, " \
- "or tu %i gmch %i/%i link %i/%i, " \
- "found tu %i, gmch %i/%i link %i/%i)\n", \
- current_config->name.tu, \
- current_config->name.gmch_m, \
- current_config->name.gmch_n, \
- current_config->name.link_m, \
- current_config->name.link_n, \
- current_config->alt_name.tu, \
- current_config->alt_name.gmch_m, \
- current_config->alt_name.gmch_n, \
- current_config->alt_name.link_m, \
- current_config->alt_name.link_n, \
- pipe_config->name.tu, \
- pipe_config->name.gmch_m, \
- pipe_config->name.gmch_n, \
- pipe_config->name.link_m, \
- pipe_config->name.link_n); \
- ret = false; \
- } \
-} while (0)
-
-#define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
- if ((current_config->name ^ pipe_config->name) & (mask)) { \
- pipe_config_err(adjust, __stringify(name), \
- "(%x) (expected %i, found %i)\n", \
- (mask), \
- current_config->name & (mask), \
- pipe_config->name & (mask)); \
- ret = false; \
- } \
-} while (0)
-
-#define PIPE_CONF_CHECK_CLOCK_FUZZY(name) do { \
- if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
- pipe_config_err(adjust, __stringify(name), \
- "(expected %i, found %i)\n", \
- current_config->name, \
- pipe_config->name); \
- ret = false; \
- } \
-} while (0)
-
-#define PIPE_CONF_CHECK_INFOFRAME(name) do { \
- if (!intel_compare_infoframe(¤t_config->infoframes.name, \
- &pipe_config->infoframes.name)) { \
- pipe_config_infoframe_err(dev_priv, adjust, __stringify(name), \
- ¤t_config->infoframes.name, \
- &pipe_config->infoframes.name); \
- ret = false; \
- } \
-} while (0)
-
-#define PIPE_CONF_QUIRK(quirk) \
- ((current_config->quirks | pipe_config->quirks) & (quirk))
-
- PIPE_CONF_CHECK_I(cpu_transcoder);
-
- PIPE_CONF_CHECK_BOOL(has_pch_encoder);
- PIPE_CONF_CHECK_I(fdi_lanes);
- PIPE_CONF_CHECK_M_N(fdi_m_n);
-
- PIPE_CONF_CHECK_I(lane_count);
- PIPE_CONF_CHECK_X(lane_lat_optim_mask);
-
- if (INTEL_GEN(dev_priv) < 8) {
- PIPE_CONF_CHECK_M_N(dp_m_n);
-
- if (current_config->has_drrs)
- PIPE_CONF_CHECK_M_N(dp_m2_n2);
- } else
- PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
-
- PIPE_CONF_CHECK_X(output_types);
-
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal);
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_start);
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_end);
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hsync_start);
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hsync_end);
-
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vdisplay);
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vtotal);
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vblank_start);
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vblank_end);
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_start);
- PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_end);
-
- PIPE_CONF_CHECK_I(pixel_multiplier);
- PIPE_CONF_CHECK_I(output_format);
- PIPE_CONF_CHECK_BOOL(has_hdmi_sink);
- if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
- IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- PIPE_CONF_CHECK_BOOL(limited_color_range);
-
- PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
- PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
- PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe);
-
- PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
-
- PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
- DRM_MODE_FLAG_INTERLACE);
-
- if (!PIPE_CONF_QUIRK(PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS)) {
- PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
- DRM_MODE_FLAG_PHSYNC);
- PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
- DRM_MODE_FLAG_NHSYNC);
- PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
- DRM_MODE_FLAG_PVSYNC);
- PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
- DRM_MODE_FLAG_NVSYNC);
- }
-
- PIPE_CONF_CHECK_X(gmch_pfit.control);
- /* pfit ratios are autocomputed by the hw on gen4+ */
- if (INTEL_GEN(dev_priv) < 4)
- PIPE_CONF_CHECK_X(gmch_pfit.pgm_ratios);
- PIPE_CONF_CHECK_X(gmch_pfit.lvds_border_bits);
-
- if (!adjust) {
- PIPE_CONF_CHECK_I(pipe_src_w);
- PIPE_CONF_CHECK_I(pipe_src_h);
-
- PIPE_CONF_CHECK_BOOL(pch_pfit.enabled);
- if (current_config->pch_pfit.enabled) {
- PIPE_CONF_CHECK_X(pch_pfit.pos);
- PIPE_CONF_CHECK_X(pch_pfit.size);
- }
-
- PIPE_CONF_CHECK_I(scaler_state.scaler_id);
- PIPE_CONF_CHECK_CLOCK_FUZZY(pixel_rate);
-
- PIPE_CONF_CHECK_X(gamma_mode);
- if (IS_CHERRYVIEW(dev_priv))
- PIPE_CONF_CHECK_X(cgm_mode);
- else
- PIPE_CONF_CHECK_X(csc_mode);
- PIPE_CONF_CHECK_BOOL(gamma_enable);
- PIPE_CONF_CHECK_BOOL(csc_enable);
- }
-
- PIPE_CONF_CHECK_BOOL(double_wide);
-
- PIPE_CONF_CHECK_P(shared_dpll);
- PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
- PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
- PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
- PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
- PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
- PIPE_CONF_CHECK_X(dpll_hw_state.spll);
- PIPE_CONF_CHECK_X(dpll_hw_state.ctrl1);
- PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr1);
- PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr2);
- PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr0);
- PIPE_CONF_CHECK_X(dpll_hw_state.ebb0);
- PIPE_CONF_CHECK_X(dpll_hw_state.ebb4);
- PIPE_CONF_CHECK_X(dpll_hw_state.pll0);
- PIPE_CONF_CHECK_X(dpll_hw_state.pll1);
- PIPE_CONF_CHECK_X(dpll_hw_state.pll2);
- PIPE_CONF_CHECK_X(dpll_hw_state.pll3);
- PIPE_CONF_CHECK_X(dpll_hw_state.pll6);
- PIPE_CONF_CHECK_X(dpll_hw_state.pll8);
- PIPE_CONF_CHECK_X(dpll_hw_state.pll9);
- PIPE_CONF_CHECK_X(dpll_hw_state.pll10);
- PIPE_CONF_CHECK_X(dpll_hw_state.pcsdw12);
- PIPE_CONF_CHECK_X(dpll_hw_state.mg_refclkin_ctl);
- PIPE_CONF_CHECK_X(dpll_hw_state.mg_clktop2_coreclkctl1);
- PIPE_CONF_CHECK_X(dpll_hw_state.mg_clktop2_hsclkctl);
- PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_div0);
- PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_div1);
- PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_lf);
- PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_frac_lock);
- PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_ssc);
- PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_bias);
- PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_tdc_coldst_bias);
-
- PIPE_CONF_CHECK_X(dsi_pll.ctrl);
- PIPE_CONF_CHECK_X(dsi_pll.div);
-
- if (IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5)
- PIPE_CONF_CHECK_I(pipe_bpp);
-
- PIPE_CONF_CHECK_CLOCK_FUZZY(base.adjusted_mode.crtc_clock);
- PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock);
-
- PIPE_CONF_CHECK_I(min_voltage_level);
-
- PIPE_CONF_CHECK_X(infoframes.enable);
- PIPE_CONF_CHECK_X(infoframes.gcp);
- PIPE_CONF_CHECK_INFOFRAME(avi);
- PIPE_CONF_CHECK_INFOFRAME(spd);
- PIPE_CONF_CHECK_INFOFRAME(hdmi);
-
-#undef PIPE_CONF_CHECK_X
-#undef PIPE_CONF_CHECK_I
-#undef PIPE_CONF_CHECK_BOOL
-#undef PIPE_CONF_CHECK_BOOL_INCOMPLETE
-#undef PIPE_CONF_CHECK_P
-#undef PIPE_CONF_CHECK_FLAGS
-#undef PIPE_CONF_CHECK_CLOCK_FUZZY
-#undef PIPE_CONF_QUIRK
-
- return ret;
-}
-
-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),
- &pipe_config->fdi_m_n);
- int dotclock = pipe_config->base.adjusted_mode.crtc_clock;
-
- /*
- * FDI already provided one idea for the dotclock.
- * Yell if the encoder disagrees.
- */
- WARN(!intel_fuzzy_clock_check(fdi_dotclock, dotclock),
- "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
- fdi_dotclock, dotclock);
- }
-}
-
static void update_scanline_offset(const struct intel_crtc_state *crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 77767c..ed853b2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1668,17 +1668,12 @@ 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);
+u32 intel_fdi_link_freq(struct drm_i915_private *dev_priv,
+ const struct intel_crtc_state *pipe_config);
static inline bool
intel_crtc_has_type(const struct intel_crtc_state *crtc_state,
enum intel_output_type type)
diff --git a/drivers/gpu/drm/i915/intel_verify.c b/drivers/gpu/drm/i915/intel_verify.c
index 4c8990..9785ad 100644
--- a/drivers/gpu/drm/i915/intel_verify.c
+++ b/drivers/gpu/drm/i915/intel_verify.c
@@ -10,6 +10,471 @@
#include "intel_pm.h"
#include "intel_verify.h"
+static bool intel_fuzzy_clock_check(int clock1, int clock2)
+{
+ int diff;
+
+ if (clock1 == clock2)
+ return true;
+
+ if (!clock1 || !clock2)
+ return false;
+
+ diff = abs(clock1 - clock2);
+
+ if (((((diff + clock1 + clock2) * 100)) / (clock1 + clock2)) < 105)
+ return true;
+
+ return false;
+}
+
+static bool
+intel_compare_m_n(unsigned int m, unsigned int n,
+ unsigned int m2, unsigned int n2,
+ bool exact)
+{
+ if (m == m2 && n == n2)
+ return true;
+
+ if (exact || !m || !n || !m2 || !n2)
+ return false;
+
+ BUILD_BUG_ON(DATA_LINK_M_N_MASK > INT_MAX);
+
+ if (n > n2) {
+ while (n > n2) {
+ m2 <<= 1;
+ n2 <<= 1;
+ }
+ } else if (n < n2) {
+ while (n < n2) {
+ m <<= 1;
+ n <<= 1;
+ }
+ }
+
+ if (n != n2)
+ return false;
+
+ return intel_fuzzy_clock_check(m, m2);
+}
+
+static bool
+intel_compare_link_m_n(const struct intel_link_m_n *m_n,
+ struct intel_link_m_n *m2_n2,
+ bool adjust)
+{
+ if (m_n->tu == m2_n2->tu &&
+ intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
+ m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
+ intel_compare_m_n(m_n->link_m, m_n->link_n,
+ m2_n2->link_m, m2_n2->link_n, !adjust)) {
+ if (adjust)
+ *m2_n2 = *m_n;
+
+ return true;
+ }
+
+ return false;
+}
+
+static bool
+intel_compare_infoframe(const union hdmi_infoframe *a,
+ const union hdmi_infoframe *b)
+{
+ return memcmp(a, b, sizeof(*a)) == 0;
+}
+
+static void
+pipe_config_infoframe_err(struct drm_i915_private *dev_priv,
+ bool adjust, const char *name,
+ const union hdmi_infoframe *a,
+ const union hdmi_infoframe *b)
+{
+ if (adjust) {
+ if ((drm_debug & DRM_UT_KMS) == 0)
+ return;
+
+ drm_dbg(DRM_UT_KMS, "mismatch in %s infoframe", name);
+ drm_dbg(DRM_UT_KMS, "expected:");
+ hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, a);
+ drm_dbg(DRM_UT_KMS, "found");
+ hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, b);
+ } else {
+ drm_err("mismatch in %s infoframe", name);
+ drm_err("expected:");
+ hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, a);
+ drm_err("found");
+ hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, b);
+ }
+}
+
+static void __printf(3, 4)
+pipe_config_err(bool adjust, const char *name, const char *format, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ va_start(args, format);
+ vaf.fmt = format;
+ vaf.va = &args;
+
+ if (adjust)
+ drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
+ else
+ drm_err("mismatch in %s %pV", name, &vaf);
+
+ va_end(args);
+}
+
+static bool fastboot_enabled(struct drm_i915_private *dev_priv)
+{
+ if (i915_modparams.fastboot != -1)
+ return i915_modparams.fastboot;
+
+ /* Enable fastboot by default on Skylake and newer */
+ if (INTEL_GEN(dev_priv) >= 9)
+ return true;
+
+ /* Enable fastboot by default on VLV and CHV */
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ return true;
+
+ /* Disabled by default on all others */
+ return false;
+}
+
+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)
+{
+ bool ret = true;
+ bool fixup_inherited = adjust &&
+ (current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
+ !(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
+
+ if (fixup_inherited && !fastboot_enabled(dev_priv)) {
+ DRM_DEBUG_KMS("initial modeset and fastboot not set\n");
+ ret = false;
+ }
+
+#define PIPE_CONF_CHECK_X(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected 0x%08x, found 0x%08x)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
+
+#define PIPE_CONF_CHECK_I(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %i, found %i)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
+
+#define PIPE_CONF_CHECK_BOOL(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %s, found %s)\n", \
+ yesno(current_config->name), \
+ yesno(pipe_config->name)); \
+ ret = false; \
+ } \
+} while (0)
+
+/*
+ * Checks state where we only read out the enabling, but not the entire
+ * state itself (like full infoframes or ELD for audio). These states
+ * require a full modeset on bootup to fix up.
+ */
+#define PIPE_CONF_CHECK_BOOL_INCOMPLETE(name) do { \
+ if (!fixup_inherited || (!current_config->name && !pipe_config->name)) { \
+ PIPE_CONF_CHECK_BOOL(name); \
+ } else { \
+ pipe_config_err(adjust, __stringify(name), \
+ "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
+ yesno(current_config->name), \
+ yesno(pipe_config->name)); \
+ ret = false; \
+ } \
+} while (0)
+
+#define PIPE_CONF_CHECK_P(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %p, found %p)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
+
+#define PIPE_CONF_CHECK_M_N(name) do { \
+ if (!intel_compare_link_m_n(¤t_config->name, \
+ &pipe_config->name,\
+ adjust)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected tu %i gmch %i/%i link %i/%i, " \
+ "found tu %i, gmch %i/%i link %i/%i)\n", \
+ current_config->name.tu, \
+ current_config->name.gmch_m, \
+ current_config->name.gmch_n, \
+ current_config->name.link_m, \
+ current_config->name.link_n, \
+ pipe_config->name.tu, \
+ pipe_config->name.gmch_m, \
+ pipe_config->name.gmch_n, \
+ pipe_config->name.link_m, \
+ pipe_config->name.link_n); \
+ ret = false; \
+ } \
+} while (0)
+
+/* This is required for BDW+ where there is only one set of registers for
+ * switching between high and low RR.
+ * This macro can be used whenever a comparison has to be made between one
+ * hw state and multiple sw state variables.
+ */
+#define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
+ if (!intel_compare_link_m_n(¤t_config->name, \
+ &pipe_config->name, adjust) && \
+ !intel_compare_link_m_n(¤t_config->alt_name, \
+ &pipe_config->name, adjust)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected tu %i gmch %i/%i link %i/%i, " \
+ "or tu %i gmch %i/%i link %i/%i, " \
+ "found tu %i, gmch %i/%i link %i/%i)\n", \
+ current_config->name.tu, \
+ current_config->name.gmch_m, \
+ current_config->name.gmch_n, \
+ current_config->name.link_m, \
+ current_config->name.link_n, \
+ current_config->alt_name.tu, \
+ current_config->alt_name.gmch_m, \
+ current_config->alt_name.gmch_n, \
+ current_config->alt_name.link_m, \
+ current_config->alt_name.link_n, \
+ pipe_config->name.tu, \
+ pipe_config->name.gmch_m, \
+ pipe_config->name.gmch_n, \
+ pipe_config->name.link_m, \
+ pipe_config->name.link_n); \
+ ret = false; \
+ } \
+} while (0)
+
+#define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
+ if ((current_config->name ^ pipe_config->name) & (mask)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(%x) (expected %i, found %i)\n", \
+ (mask), \
+ current_config->name & (mask), \
+ pipe_config->name & (mask)); \
+ ret = false; \
+ } \
+} while (0)
+
+#define PIPE_CONF_CHECK_CLOCK_FUZZY(name) do { \
+ if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %i, found %i)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
+
+#define PIPE_CONF_CHECK_INFOFRAME(name) do { \
+ if (!intel_compare_infoframe(¤t_config->infoframes.name, \
+ &pipe_config->infoframes.name)) { \
+ pipe_config_infoframe_err(dev_priv, adjust, __stringify(name), \
+ ¤t_config->infoframes.name, \
+ &pipe_config->infoframes.name); \
+ ret = false; \
+ } \
+} while (0)
+
+#define PIPE_CONF_QUIRK(quirk) \
+ ((current_config->quirks | pipe_config->quirks) & (quirk))
+
+ PIPE_CONF_CHECK_I(cpu_transcoder);
+
+ PIPE_CONF_CHECK_BOOL(has_pch_encoder);
+ PIPE_CONF_CHECK_I(fdi_lanes);
+ PIPE_CONF_CHECK_M_N(fdi_m_n);
+
+ PIPE_CONF_CHECK_I(lane_count);
+ PIPE_CONF_CHECK_X(lane_lat_optim_mask);
+
+ if (INTEL_GEN(dev_priv) < 8) {
+ PIPE_CONF_CHECK_M_N(dp_m_n);
+
+ if (current_config->has_drrs)
+ PIPE_CONF_CHECK_M_N(dp_m2_n2);
+ } else
+ PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
+
+ PIPE_CONF_CHECK_X(output_types);
+
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal);
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_start);
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_end);
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hsync_start);
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hsync_end);
+
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vdisplay);
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vtotal);
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vblank_start);
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vblank_end);
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_start);
+ PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_end);
+
+ PIPE_CONF_CHECK_I(pixel_multiplier);
+ PIPE_CONF_CHECK_I(output_format);
+ PIPE_CONF_CHECK_BOOL(has_hdmi_sink);
+ if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
+ IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ PIPE_CONF_CHECK_BOOL(limited_color_range);
+
+ PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
+ PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
+ PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe);
+
+ PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
+
+ PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
+ DRM_MODE_FLAG_INTERLACE);
+
+ if (!PIPE_CONF_QUIRK(PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS)) {
+ PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
+ DRM_MODE_FLAG_PHSYNC);
+ PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
+ DRM_MODE_FLAG_NHSYNC);
+ PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
+ DRM_MODE_FLAG_PVSYNC);
+ PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
+ DRM_MODE_FLAG_NVSYNC);
+ }
+
+ PIPE_CONF_CHECK_X(gmch_pfit.control);
+ /* pfit ratios are autocomputed by the hw on gen4+ */
+ if (INTEL_GEN(dev_priv) < 4)
+ PIPE_CONF_CHECK_X(gmch_pfit.pgm_ratios);
+ PIPE_CONF_CHECK_X(gmch_pfit.lvds_border_bits);
+
+ if (!adjust) {
+ PIPE_CONF_CHECK_I(pipe_src_w);
+ PIPE_CONF_CHECK_I(pipe_src_h);
+
+ PIPE_CONF_CHECK_BOOL(pch_pfit.enabled);
+ if (current_config->pch_pfit.enabled) {
+ PIPE_CONF_CHECK_X(pch_pfit.pos);
+ PIPE_CONF_CHECK_X(pch_pfit.size);
+ }
+
+ PIPE_CONF_CHECK_I(scaler_state.scaler_id);
+ PIPE_CONF_CHECK_CLOCK_FUZZY(pixel_rate);
+
+ PIPE_CONF_CHECK_X(gamma_mode);
+ if (IS_CHERRYVIEW(dev_priv))
+ PIPE_CONF_CHECK_X(cgm_mode);
+ else
+ PIPE_CONF_CHECK_X(csc_mode);
+ PIPE_CONF_CHECK_BOOL(gamma_enable);
+ PIPE_CONF_CHECK_BOOL(csc_enable);
+ }
+
+ PIPE_CONF_CHECK_BOOL(double_wide);
+
+ PIPE_CONF_CHECK_P(shared_dpll);
+ PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
+ PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
+ PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
+ PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
+ PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
+ PIPE_CONF_CHECK_X(dpll_hw_state.spll);
+ PIPE_CONF_CHECK_X(dpll_hw_state.ctrl1);
+ PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr1);
+ PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr2);
+ PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr0);
+ PIPE_CONF_CHECK_X(dpll_hw_state.ebb0);
+ PIPE_CONF_CHECK_X(dpll_hw_state.ebb4);
+ PIPE_CONF_CHECK_X(dpll_hw_state.pll0);
+ PIPE_CONF_CHECK_X(dpll_hw_state.pll1);
+ PIPE_CONF_CHECK_X(dpll_hw_state.pll2);
+ PIPE_CONF_CHECK_X(dpll_hw_state.pll3);
+ PIPE_CONF_CHECK_X(dpll_hw_state.pll6);
+ PIPE_CONF_CHECK_X(dpll_hw_state.pll8);
+ PIPE_CONF_CHECK_X(dpll_hw_state.pll9);
+ PIPE_CONF_CHECK_X(dpll_hw_state.pll10);
+ PIPE_CONF_CHECK_X(dpll_hw_state.pcsdw12);
+ PIPE_CONF_CHECK_X(dpll_hw_state.mg_refclkin_ctl);
+ PIPE_CONF_CHECK_X(dpll_hw_state.mg_clktop2_coreclkctl1);
+ PIPE_CONF_CHECK_X(dpll_hw_state.mg_clktop2_hsclkctl);
+ PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_div0);
+ PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_div1);
+ PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_lf);
+ PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_frac_lock);
+ PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_ssc);
+ PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_bias);
+ PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_tdc_coldst_bias);
+
+ PIPE_CONF_CHECK_X(dsi_pll.ctrl);
+ PIPE_CONF_CHECK_X(dsi_pll.div);
+
+ if (IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5)
+ PIPE_CONF_CHECK_I(pipe_bpp);
+
+ PIPE_CONF_CHECK_CLOCK_FUZZY(base.adjusted_mode.crtc_clock);
+ PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock);
+
+ PIPE_CONF_CHECK_I(min_voltage_level);
+
+ PIPE_CONF_CHECK_X(infoframes.enable);
+ PIPE_CONF_CHECK_X(infoframes.gcp);
+ PIPE_CONF_CHECK_INFOFRAME(avi);
+ PIPE_CONF_CHECK_INFOFRAME(spd);
+ PIPE_CONF_CHECK_INFOFRAME(hdmi);
+
+#undef PIPE_CONF_CHECK_X
+#undef PIPE_CONF_CHECK_I
+#undef PIPE_CONF_CHECK_BOOL
+#undef PIPE_CONF_CHECK_BOOL_INCOMPLETE
+#undef PIPE_CONF_CHECK_P
+#undef PIPE_CONF_CHECK_FLAGS
+#undef PIPE_CONF_CHECK_CLOCK_FUZZY
+#undef PIPE_CONF_QUIRK
+
+ return ret;
+}
+
+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),
+ &pipe_config->fdi_m_n);
+ int dotclock = pipe_config->base.adjusted_mode.crtc_clock;
+
+ /*
+ * FDI already provided one idea for the dotclock.
+ * Yell if the encoder disagrees.
+ */
+ WARN(!intel_fuzzy_clock_check(fdi_dotclock, dotclock),
+ "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
+ fdi_dotclock, dotclock);
+ }
+}
+
static void verify_wm_state(struct drm_crtc *crtc,
struct drm_crtc_state *new_state)
{
diff --git a/drivers/gpu/drm/i915/intel_verify.h b/drivers/gpu/drm/i915/intel_verify.h
index 4b751ea..faaf8f 100644
--- a/drivers/gpu/drm/i915/intel_verify.h
+++ b/drivers/gpu/drm/i915/intel_verify.h
@@ -10,7 +10,15 @@ struct drm_atomic_state;
struct drm_crtc;
struct drm_crtc_state;
struct drm_device;
+struct drm_i915_private;
+struct intel_crtc_state;
+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_verify_modeset_crtc(struct drm_crtc *crtc,
struct drm_atomic_state *state,
struct drm_crtc_state *old_state,
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [RFC,1/2] drm/i915: start moving state checker to intel_verify.c
2019-04-16 10:36 [RFC 1/2] drm/i915: start moving state checker to intel_verify.c Jani Nikula
2019-04-16 10:36 ` [RFC 2/2] drm/i915: move pipe config compare " Jani Nikula
@ 2019-04-16 12:14 ` Patchwork
2019-04-16 12:15 ` ✗ Fi.CI.SPARSE: " Patchwork
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-04-16 12:14 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: series starting with [RFC,1/2] drm/i915: start moving state checker to intel_verify.c
URL : https://patchwork.freedesktop.org/series/59569/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
a4e2e34c9de6 drm/i915: start moving state checker to intel_verify.c
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one
-:599: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#599:
new file mode 100644
-:757: WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line
#757: FILE: drivers/gpu/drm/i915/intel_verify.c:154:
+ * internal consistency). */
-:771: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#771: FILE: drivers/gpu/drm/i915/intel_verify.c:168:
+ I915_STATE_WARN(!crtc_state,
+ "connector enabled without attached crtc\n");
-:777: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#777: FILE: drivers/gpu/drm/i915/intel_verify.c:174:
+ I915_STATE_WARN(!crtc_state->active,
+ "connector is active, but attached crtc isn't\n");
-:783: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#783: FILE: drivers/gpu/drm/i915/intel_verify.c:180:
+ I915_STATE_WARN(conn_state->best_encoder != &encoder->base,
+ "atomic encoder doesn't match attached encoder\n");
-:786: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#786: FILE: drivers/gpu/drm/i915/intel_verify.c:183:
+ I915_STATE_WARN(conn_state->crtc != encoder->base.crtc,
+ "attached encoder crtc differs from connector crtc\n");
-:789: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#789: FILE: drivers/gpu/drm/i915/intel_verify.c:186:
+ I915_STATE_WARN(crtc_state && crtc_state->active,
+ "attached crtc is active, but connector isn't\n");
-:791: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#791: FILE: drivers/gpu/drm/i915/intel_verify.c:188:
+ I915_STATE_WARN(!crtc_state && conn_state->best_encoder,
+ "best encoder set without crtc!\n");
-:817: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#817: FILE: drivers/gpu/drm/i915/intel_verify.c:214:
+ I915_STATE_WARN(new_conn_state->best_encoder != encoder,
+ "connector's atomic encoder doesn't match legacy encoder\n");
-:844: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#844: FILE: drivers/gpu/drm/i915/intel_verify.c:241:
+ found = enabled = true;
-:855: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#855: FILE: drivers/gpu/drm/i915/intel_verify.c:252:
+ I915_STATE_WARN(!!encoder->base.crtc != enabled,
+ "encoder's enabled state mismatch "
-:864: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#864: FILE: drivers/gpu/drm/i915/intel_verify.c:261:
+ I915_STATE_WARN(active,
+ "encoder detached but still enabled on pipe %c.\n",
-:899: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#899: FILE: drivers/gpu/drm/i915/intel_verify.c:296:
+ I915_STATE_WARN(new_crtc_state->active != active,
+ "crtc active state doesn't match with hw state "
-:903: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#903: FILE: drivers/gpu/drm/i915/intel_verify.c:300:
+ I915_STATE_WARN(intel_crtc->active != new_crtc_state->active,
+ "transitional active state does not match atomic hw state "
-:911: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#911: FILE: drivers/gpu/drm/i915/intel_verify.c:308:
+ I915_STATE_WARN(active != new_crtc_state->active,
+ "[ENCODER:%i] active %i with crtc active %i\n",
-:971: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#971: FILE: drivers/gpu/drm/i915/intel_verify.c:368:
+ I915_STATE_WARN(!pll->on && pll->active_mask,
+ "pll in active use but not on in sw tracking\n");
-:973: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#973: FILE: drivers/gpu/drm/i915/intel_verify.c:370:
+ I915_STATE_WARN(pll->on && !pll->active_mask,
+ "pll is on but not used by any active crtc\n");
-:975: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#975: FILE: drivers/gpu/drm/i915/intel_verify.c:372:
+ I915_STATE_WARN(pll->on != active,
+ "pll on state mismatch (expected %i, found %i)\n",
total: 0 errors, 3 warnings, 16 checks, 1051 lines checked
73e020a244ec drm/i915: move pipe config compare to intel_verify.c
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one
-:679: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#679: FILE: drivers/gpu/drm/i915/intel_verify.c:163:
+#define PIPE_CONF_CHECK_X(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected 0x%08x, found 0x%08x)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
-:679: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#679: FILE: drivers/gpu/drm/i915/intel_verify.c:163:
+#define PIPE_CONF_CHECK_X(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected 0x%08x, found 0x%08x)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
-:689: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#689: FILE: drivers/gpu/drm/i915/intel_verify.c:173:
+#define PIPE_CONF_CHECK_I(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %i, found %i)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
-:689: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#689: FILE: drivers/gpu/drm/i915/intel_verify.c:173:
+#define PIPE_CONF_CHECK_I(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %i, found %i)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
-:699: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#699: FILE: drivers/gpu/drm/i915/intel_verify.c:183:
+#define PIPE_CONF_CHECK_BOOL(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %s, found %s)\n", \
+ yesno(current_config->name), \
+ yesno(pipe_config->name)); \
+ ret = false; \
+ } \
+} while (0)
-:699: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#699: FILE: drivers/gpu/drm/i915/intel_verify.c:183:
+#define PIPE_CONF_CHECK_BOOL(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %s, found %s)\n", \
+ yesno(current_config->name), \
+ yesno(pipe_config->name)); \
+ ret = false; \
+ } \
+} while (0)
-:714: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#714: FILE: drivers/gpu/drm/i915/intel_verify.c:198:
+#define PIPE_CONF_CHECK_BOOL_INCOMPLETE(name) do { \
+ if (!fixup_inherited || (!current_config->name && !pipe_config->name)) { \
+ PIPE_CONF_CHECK_BOOL(name); \
+ } else { \
+ pipe_config_err(adjust, __stringify(name), \
+ "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
+ yesno(current_config->name), \
+ yesno(pipe_config->name)); \
+ ret = false; \
+ } \
+} while (0)
-:714: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#714: FILE: drivers/gpu/drm/i915/intel_verify.c:198:
+#define PIPE_CONF_CHECK_BOOL_INCOMPLETE(name) do { \
+ if (!fixup_inherited || (!current_config->name && !pipe_config->name)) { \
+ PIPE_CONF_CHECK_BOOL(name); \
+ } else { \
+ pipe_config_err(adjust, __stringify(name), \
+ "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
+ yesno(current_config->name), \
+ yesno(pipe_config->name)); \
+ ret = false; \
+ } \
+} while (0)
-:719: WARNING:LONG_LINE: line over 100 characters
#719: FILE: drivers/gpu/drm/i915/intel_verify.c:203:
+ "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
-:726: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#726: FILE: drivers/gpu/drm/i915/intel_verify.c:210:
+#define PIPE_CONF_CHECK_P(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %p, found %p)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
-:726: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#726: FILE: drivers/gpu/drm/i915/intel_verify.c:210:
+#define PIPE_CONF_CHECK_P(name) do { \
+ if (current_config->name != pipe_config->name) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %p, found %p)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
-:736: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#736: FILE: drivers/gpu/drm/i915/intel_verify.c:220:
+#define PIPE_CONF_CHECK_M_N(name) do { \
+ if (!intel_compare_link_m_n(¤t_config->name, \
+ &pipe_config->name,\
+ adjust)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected tu %i gmch %i/%i link %i/%i, " \
+ "found tu %i, gmch %i/%i link %i/%i)\n", \
+ current_config->name.tu, \
+ current_config->name.gmch_m, \
+ current_config->name.gmch_n, \
+ current_config->name.link_m, \
+ current_config->name.link_n, \
+ pipe_config->name.tu, \
+ pipe_config->name.gmch_m, \
+ pipe_config->name.gmch_n, \
+ pipe_config->name.link_m, \
+ pipe_config->name.link_n); \
+ ret = false; \
+ } \
+} while (0)
-:736: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#736: FILE: drivers/gpu/drm/i915/intel_verify.c:220:
+#define PIPE_CONF_CHECK_M_N(name) do { \
+ if (!intel_compare_link_m_n(¤t_config->name, \
+ &pipe_config->name,\
+ adjust)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected tu %i gmch %i/%i link %i/%i, " \
+ "found tu %i, gmch %i/%i link %i/%i)\n", \
+ current_config->name.tu, \
+ current_config->name.gmch_m, \
+ current_config->name.gmch_n, \
+ current_config->name.link_m, \
+ current_config->name.link_n, \
+ pipe_config->name.tu, \
+ pipe_config->name.gmch_m, \
+ pipe_config->name.gmch_n, \
+ pipe_config->name.link_m, \
+ pipe_config->name.link_n); \
+ ret = false; \
+ } \
+} while (0)
-:762: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#762: FILE: drivers/gpu/drm/i915/intel_verify.c:246:
+#define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
+ if (!intel_compare_link_m_n(¤t_config->name, \
+ &pipe_config->name, adjust) && \
+ !intel_compare_link_m_n(¤t_config->alt_name, \
+ &pipe_config->name, adjust)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected tu %i gmch %i/%i link %i/%i, " \
+ "or tu %i gmch %i/%i link %i/%i, " \
+ "found tu %i, gmch %i/%i link %i/%i)\n", \
+ current_config->name.tu, \
+ current_config->name.gmch_m, \
+ current_config->name.gmch_n, \
+ current_config->name.link_m, \
+ current_config->name.link_n, \
+ current_config->alt_name.tu, \
+ current_config->alt_name.gmch_m, \
+ current_config->alt_name.gmch_n, \
+ current_config->alt_name.link_m, \
+ current_config->alt_name.link_n, \
+ pipe_config->name.tu, \
+ pipe_config->name.gmch_m, \
+ pipe_config->name.gmch_n, \
+ pipe_config->name.link_m, \
+ pipe_config->name.link_n); \
+ ret = false; \
+ } \
+} while (0)
-:762: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#762: FILE: drivers/gpu/drm/i915/intel_verify.c:246:
+#define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
+ if (!intel_compare_link_m_n(¤t_config->name, \
+ &pipe_config->name, adjust) && \
+ !intel_compare_link_m_n(¤t_config->alt_name, \
+ &pipe_config->name, adjust)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected tu %i gmch %i/%i link %i/%i, " \
+ "or tu %i gmch %i/%i link %i/%i, " \
+ "found tu %i, gmch %i/%i link %i/%i)\n", \
+ current_config->name.tu, \
+ current_config->name.gmch_m, \
+ current_config->name.gmch_n, \
+ current_config->name.link_m, \
+ current_config->name.link_n, \
+ current_config->alt_name.tu, \
+ current_config->alt_name.gmch_m, \
+ current_config->alt_name.gmch_n, \
+ current_config->alt_name.link_m, \
+ current_config->alt_name.link_n, \
+ pipe_config->name.tu, \
+ pipe_config->name.gmch_m, \
+ pipe_config->name.gmch_n, \
+ pipe_config->name.link_m, \
+ pipe_config->name.link_n); \
+ ret = false; \
+ } \
+} while (0)
-:762: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'alt_name' - possible side-effects?
#762: FILE: drivers/gpu/drm/i915/intel_verify.c:246:
+#define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
+ if (!intel_compare_link_m_n(¤t_config->name, \
+ &pipe_config->name, adjust) && \
+ !intel_compare_link_m_n(¤t_config->alt_name, \
+ &pipe_config->name, adjust)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected tu %i gmch %i/%i link %i/%i, " \
+ "or tu %i gmch %i/%i link %i/%i, " \
+ "found tu %i, gmch %i/%i link %i/%i)\n", \
+ current_config->name.tu, \
+ current_config->name.gmch_m, \
+ current_config->name.gmch_n, \
+ current_config->name.link_m, \
+ current_config->name.link_n, \
+ current_config->alt_name.tu, \
+ current_config->alt_name.gmch_m, \
+ current_config->alt_name.gmch_n, \
+ current_config->alt_name.link_m, \
+ current_config->alt_name.link_n, \
+ pipe_config->name.tu, \
+ pipe_config->name.gmch_m, \
+ pipe_config->name.gmch_n, \
+ pipe_config->name.link_m, \
+ pipe_config->name.link_n); \
+ ret = false; \
+ } \
+} while (0)
-:762: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'alt_name' may be better as '(alt_name)' to avoid precedence issues
#762: FILE: drivers/gpu/drm/i915/intel_verify.c:246:
+#define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
+ if (!intel_compare_link_m_n(¤t_config->name, \
+ &pipe_config->name, adjust) && \
+ !intel_compare_link_m_n(¤t_config->alt_name, \
+ &pipe_config->name, adjust)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected tu %i gmch %i/%i link %i/%i, " \
+ "or tu %i gmch %i/%i link %i/%i, " \
+ "found tu %i, gmch %i/%i link %i/%i)\n", \
+ current_config->name.tu, \
+ current_config->name.gmch_m, \
+ current_config->name.gmch_n, \
+ current_config->name.link_m, \
+ current_config->name.link_n, \
+ current_config->alt_name.tu, \
+ current_config->alt_name.gmch_m, \
+ current_config->alt_name.gmch_n, \
+ current_config->alt_name.link_m, \
+ current_config->alt_name.link_n, \
+ pipe_config->name.tu, \
+ pipe_config->name.gmch_m, \
+ pipe_config->name.gmch_n, \
+ pipe_config->name.link_m, \
+ pipe_config->name.link_n); \
+ ret = false; \
+ } \
+} while (0)
-:790: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#790: FILE: drivers/gpu/drm/i915/intel_verify.c:274:
+#define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
+ if ((current_config->name ^ pipe_config->name) & (mask)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(%x) (expected %i, found %i)\n", \
+ (mask), \
+ current_config->name & (mask), \
+ pipe_config->name & (mask)); \
+ ret = false; \
+ } \
+} while (0)
-:790: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#790: FILE: drivers/gpu/drm/i915/intel_verify.c:274:
+#define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
+ if ((current_config->name ^ pipe_config->name) & (mask)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(%x) (expected %i, found %i)\n", \
+ (mask), \
+ current_config->name & (mask), \
+ pipe_config->name & (mask)); \
+ ret = false; \
+ } \
+} while (0)
-:790: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'mask' - possible side-effects?
#790: FILE: drivers/gpu/drm/i915/intel_verify.c:274:
+#define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
+ if ((current_config->name ^ pipe_config->name) & (mask)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(%x) (expected %i, found %i)\n", \
+ (mask), \
+ current_config->name & (mask), \
+ pipe_config->name & (mask)); \
+ ret = false; \
+ } \
+} while (0)
-:801: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#801: FILE: drivers/gpu/drm/i915/intel_verify.c:285:
+#define PIPE_CONF_CHECK_CLOCK_FUZZY(name) do { \
+ if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %i, found %i)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
-:801: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#801: FILE: drivers/gpu/drm/i915/intel_verify.c:285:
+#define PIPE_CONF_CHECK_CLOCK_FUZZY(name) do { \
+ if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
+ pipe_config_err(adjust, __stringify(name), \
+ "(expected %i, found %i)\n", \
+ current_config->name, \
+ pipe_config->name); \
+ ret = false; \
+ } \
+} while (0)
-:811: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#811: FILE: drivers/gpu/drm/i915/intel_verify.c:295:
+#define PIPE_CONF_CHECK_INFOFRAME(name) do { \
+ if (!intel_compare_infoframe(¤t_config->infoframes.name, \
+ &pipe_config->infoframes.name)) { \
+ pipe_config_infoframe_err(dev_priv, adjust, __stringify(name), \
+ ¤t_config->infoframes.name, \
+ &pipe_config->infoframes.name); \
+ ret = false; \
+ } \
+} while (0)
-:833: CHECK:BRACES: braces {} should be used on all arms of this statement
#833: FILE: drivers/gpu/drm/i915/intel_verify.c:317:
+ if (INTEL_GEN(dev_priv) < 8) {
[...]
+ } else
[...]
-:838: CHECK:BRACES: Unbalanced braces around else statement
#838: FILE: drivers/gpu/drm/i915/intel_verify.c:322:
+ } else
-:980: WARNING:LONG_LINE: line over 100 characters
#980: FILE: drivers/gpu/drm/i915/intel_verify.c:464:
+ int fdi_dotclock = intel_dotclock_calculate(intel_fdi_link_freq(dev_priv, pipe_config),
total: 0 errors, 3 warnings, 24 checks, 988 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✗ Fi.CI.SPARSE: warning for series starting with [RFC,1/2] drm/i915: start moving state checker to intel_verify.c
2019-04-16 10:36 [RFC 1/2] drm/i915: start moving state checker to intel_verify.c Jani Nikula
2019-04-16 10:36 ` [RFC 2/2] drm/i915: move pipe config compare " Jani Nikula
2019-04-16 12:14 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [RFC,1/2] drm/i915: start moving state checker " Patchwork
@ 2019-04-16 12:15 ` Patchwork
2019-04-16 12:42 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-04-16 12:15 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: series starting with [RFC,1/2] drm/i915: start moving state checker to intel_verify.c
URL : https://patchwork.freedesktop.org/series/59569/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: start moving state checker to intel_verify.c
+./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)
Commit: drm/i915: move pipe config compare to intel_verify.c
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [RFC,1/2] drm/i915: start moving state checker to intel_verify.c
2019-04-16 10:36 [RFC 1/2] drm/i915: start moving state checker to intel_verify.c Jani Nikula
` (2 preceding siblings ...)
2019-04-16 12:15 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-04-16 12:42 ` Patchwork
2019-04-16 17:45 ` ✓ Fi.CI.IGT: " Patchwork
2019-05-15 9:33 ` [RFC 1/2] " Daniel Vetter
5 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-04-16 12:42 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: series starting with [RFC,1/2] drm/i915: start moving state checker to intel_verify.c
URL : https://patchwork.freedesktop.org/series/59569/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5937 -> Patchwork_12815
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/59569/revisions/1/mbox/
Known issues
------------
Here are the changes found in Patchwork_12815 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_basic@userptr:
- fi-kbl-8809g: PASS -> DMESG-WARN [fdo#108965]
* igt@gem_exec_basic@readonly-bsd2:
- fi-pnv-d510: NOTRUN -> SKIP [fdo#109271] +71
* igt@kms_busy@basic-flip-c:
- fi-pnv-d510: NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
* igt@kms_frontbuffer_tracking@basic:
- fi-byt-clapper: PASS -> FAIL [fdo#103167]
#### Possible fixes ####
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: FAIL [fdo#108511] -> PASS
* igt@i915_selftest@live_execlists:
- fi-apl-guc: INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS
* igt@kms_pipe_crc_basic@read-crc-pipe-a:
- fi-byt-clapper: FAIL [fdo#103191] -> PASS
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
[fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
Participating hosts (48 -> 42)
------------------------------
Additional (1): fi-pnv-d510
Missing (7): fi-kbl-soraka fi-ilk-m540 fi-bsw-n3050 fi-hsw-4200u fi-ctg-p8600 fi-icl-y fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_5937 -> Patchwork_12815
CI_DRM_5937: f65ca0674f94ab6a66bcec4d3a27b5e2c06f1813 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4951: cc9a60c029432b5843724e4f2c57f9f815f7adbb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12815: 73e020a244ecd62e3619f91b0d55aad5b28249fa @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
73e020a244ec drm/i915: move pipe config compare to intel_verify.c
a4e2e34c9de6 drm/i915: start moving state checker to intel_verify.c
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12815/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [RFC,1/2] drm/i915: start moving state checker to intel_verify.c
2019-04-16 10:36 [RFC 1/2] drm/i915: start moving state checker to intel_verify.c Jani Nikula
` (3 preceding siblings ...)
2019-04-16 12:42 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-04-16 17:45 ` Patchwork
2019-05-15 9:33 ` [RFC 1/2] " Daniel Vetter
5 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-04-16 17:45 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: series starting with [RFC,1/2] drm/i915: start moving state checker to intel_verify.c
URL : https://patchwork.freedesktop.org/series/59569/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5937_full -> Patchwork_12815_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Known issues
------------
Here are the changes found in Patchwork_12815_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_isolation@vcs1-reset:
- shard-iclb: NOTRUN -> SKIP [fdo#109276] +5
* igt@gem_ctx_sseu@invalid-args:
- shard-skl: NOTRUN -> SKIP [fdo#109271] +75
* igt@gem_exec_parse@basic-rejected:
- shard-iclb: NOTRUN -> SKIP [fdo#109289] +3
* igt@gem_softpin@noreloc-s3:
- shard-apl: PASS -> DMESG-WARN [fdo#108566] +1
* igt@gem_tiled_swapping@non-threaded:
- shard-snb: NOTRUN -> FAIL [fdo#108686]
* igt@gem_userptr_blits@coherency-sync:
- shard-iclb: NOTRUN -> SKIP [fdo#109290]
* igt@i915_pm_rpm@sysfs-read:
- shard-skl: PASS -> INCOMPLETE [fdo#107807]
* igt@kms_atomic_transition@6x-modeset-transitions:
- shard-iclb: NOTRUN -> SKIP [fdo#109278] +3
* igt@kms_atomic_transition@6x-modeset-transitions-nonblocking:
- shard-glk: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +7
* igt@kms_busy@basic-flip-d:
- shard-snb: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +9
* igt@kms_chamelium@hdmi-crc-planes-random:
- shard-iclb: NOTRUN -> SKIP [fdo#109284] +2
* igt@kms_color@pipe-a-gamma:
- shard-skl: PASS -> FAIL [fdo#104782]
* igt@kms_cursor_crc@cursor-128x42-sliding:
- shard-glk: NOTRUN -> FAIL [fdo#103232] +2
* igt@kms_cursor_crc@cursor-256x256-suspend:
- shard-kbl: PASS -> DMESG-WARN [fdo#108566] +2
* igt@kms_cursor_crc@cursor-512x170-onscreen:
- shard-iclb: NOTRUN -> SKIP [fdo#109279]
* igt@kms_cursor_crc@cursor-64x21-random:
- shard-skl: PASS -> FAIL [fdo#103232]
* igt@kms_flip@2x-flip-vs-panning:
- shard-iclb: NOTRUN -> SKIP [fdo#109274] +6
* igt@kms_force_connector_basic@force-connector-state:
- shard-iclb: NOTRUN -> SKIP [fdo#109285] +1
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-pwrite:
- shard-snb: NOTRUN -> SKIP [fdo#109271] +74
* igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-mmap-gtt:
- shard-iclb: NOTRUN -> SKIP [fdo#109280] +12
* igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
- shard-iclb: PASS -> FAIL [fdo#103167] +7
* igt@kms_frontbuffer_tracking@fbcpsr-1p-indfb-fliptrack:
- shard-glk: NOTRUN -> SKIP [fdo#109271] +44
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt:
- shard-iclb: PASS -> FAIL [fdo#109247] +18
* igt@kms_frontbuffer_tracking@psr-shrfb-scaledprimary:
- shard-skl: PASS -> FAIL [fdo#103167] +2
* igt@kms_lease@page_flip_implicit_plane:
- shard-snb: NOTRUN -> FAIL [fdo#110281]
* igt@kms_lease@setcrtc_implicit_plane:
- shard-skl: NOTRUN -> FAIL [fdo#110281]
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-f:
- shard-skl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +9
* igt@kms_plane@pixel-format-pipe-b-planes:
- shard-glk: PASS -> SKIP [fdo#109271]
* igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
- shard-skl: PASS -> FAIL [fdo#108145] +1
* igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
- shard-skl: PASS -> FAIL [fdo#108145] / [fdo#110403]
* igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
- shard-glk: NOTRUN -> FAIL [fdo#108145]
* igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
- shard-skl: NOTRUN -> FAIL [fdo#108145]
* igt@kms_plane_lowres@pipe-a-tiling-x:
- shard-iclb: PASS -> FAIL [fdo#103166] +1
* igt@kms_plane_scaling@pipe-a-scaler-with-pixel-format:
- shard-glk: PASS -> SKIP [fdo#109271] / [fdo#109278]
* igt@kms_psr@primary_render:
- shard-iclb: PASS -> FAIL [fdo#107383] / [fdo#110215]
* igt@kms_psr@psr2_primary_blt:
- shard-iclb: NOTRUN -> SKIP [fdo#109441] +1
* igt@kms_psr@psr2_sprite_blt:
- shard-iclb: PASS -> SKIP [fdo#109441]
* igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
- shard-kbl: PASS -> DMESG-FAIL [fdo#105763]
* igt@prime_nv_test@nv_i915_sharing:
- shard-iclb: NOTRUN -> SKIP [fdo#109291] +2
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s3:
- shard-skl: INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS
* igt@i915_suspend@debugfs-reader:
- shard-apl: DMESG-WARN [fdo#108566] -> PASS +9
* igt@kms_color@pipe-b-ctm-0-5:
- shard-skl: FAIL [fdo#108682] -> PASS
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-skl: FAIL [fdo#105363] -> PASS
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
- shard-iclb: FAIL [fdo#109247] -> PASS +15
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render:
- shard-iclb: FAIL [fdo#103167] -> PASS +8
* igt@kms_plane@pixel-format-pipe-c-planes:
- shard-glk: SKIP [fdo#109271] -> PASS
* igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format:
- shard-glk: SKIP [fdo#109271] / [fdo#109278] -> PASS
* igt@kms_psr@cursor_mmap_gtt:
- shard-iclb: FAIL [fdo#107383] / [fdo#110215] -> PASS +1
* igt@kms_psr@psr2_cursor_render:
- shard-iclb: SKIP [fdo#109441] -> PASS +3
#### Warnings ####
* igt@gem_tiled_swapping@non-threaded:
- shard-iclb: FAIL [fdo#108686] -> INCOMPLETE [fdo#108686]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
[fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
[fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108682]: https://bugs.freedesktop.org/show_bug.cgi?id=108682
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109290]: https://bugs.freedesktop.org/show_bug.cgi?id=109290
[fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
[fdo#110281]: https://bugs.freedesktop.org/show_bug.cgi?id=110281
[fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
Participating hosts (10 -> 9)
------------------------------
Missing (1): shard-hsw
Build changes
-------------
* Linux: CI_DRM_5937 -> Patchwork_12815
CI_DRM_5937: f65ca0674f94ab6a66bcec4d3a27b5e2c06f1813 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4951: cc9a60c029432b5843724e4f2c57f9f815f7adbb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12815: 73e020a244ecd62e3619f91b0d55aad5b28249fa @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12815/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 1/2] drm/i915: start moving state checker to intel_verify.c
2019-04-16 10:36 [RFC 1/2] drm/i915: start moving state checker to intel_verify.c Jani Nikula
` (4 preceding siblings ...)
2019-04-16 17:45 ` ✓ Fi.CI.IGT: " Patchwork
@ 2019-05-15 9:33 ` Daniel Vetter
2019-05-15 9:38 ` Daniel Vetter
5 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2019-05-15 9:33 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 2/2] drm/i915: move pipe config compare to intel_verify.c
2019-04-16 10:36 ` [RFC 2/2] drm/i915: move pipe config compare " Jani Nikula
@ 2019-05-15 9:36 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2019-05-15 9:36 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Tue, Apr 16, 2019 at 01:36:03PM +0300, Jani Nikula wrote:
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
pipe_config_compare is used both for the atomic_check logic, and the
verifier. I think stuffing it into the verifier code is misplacing it as
much as e.g. stuffing it into intel_atomic_check.c file.
What I'd do instead is create an intel_crtc_state.[hc] pair and collect
everything that operates on struct intel_crtc_state, plus the struct
itself. Of course we might want to split out specific subtopics from this,
like e.g. the fairly massive amount of code related to compute clock
state.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 471 +--------------------------
> drivers/gpu/drm/i915/intel_drv.h | 9 +-
> drivers/gpu/drm/i915/intel_verify.c | 465 ++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_verify.h | 8 +
> 4 files changed, 478 insertions(+), 475 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 31a931..14899f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -202,9 +202,9 @@ static void intel_update_czclk(struct drm_i915_private *dev_priv)
> DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
> }
>
> -static inline u32 /* units of 100MHz */
> -intel_fdi_link_freq(struct drm_i915_private *dev_priv,
> - const struct intel_crtc_state *pipe_config)
> +/* units of 100MHz */
> +u32 intel_fdi_link_freq(struct drm_i915_private *dev_priv,
> + const struct intel_crtc_state *pipe_config)
> {
> if (HAS_DDI(dev_priv))
> return pipe_config->port_clock; /* SPLL */
> @@ -11904,471 +11904,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> return 0;
> }
>
> -static bool intel_fuzzy_clock_check(int clock1, int clock2)
> -{
> - int diff;
> -
> - if (clock1 == clock2)
> - return true;
> -
> - if (!clock1 || !clock2)
> - return false;
> -
> - diff = abs(clock1 - clock2);
> -
> - if (((((diff + clock1 + clock2) * 100)) / (clock1 + clock2)) < 105)
> - return true;
> -
> - return false;
> -}
> -
> -static bool
> -intel_compare_m_n(unsigned int m, unsigned int n,
> - unsigned int m2, unsigned int n2,
> - bool exact)
> -{
> - if (m == m2 && n == n2)
> - return true;
> -
> - if (exact || !m || !n || !m2 || !n2)
> - return false;
> -
> - BUILD_BUG_ON(DATA_LINK_M_N_MASK > INT_MAX);
> -
> - if (n > n2) {
> - while (n > n2) {
> - m2 <<= 1;
> - n2 <<= 1;
> - }
> - } else if (n < n2) {
> - while (n < n2) {
> - m <<= 1;
> - n <<= 1;
> - }
> - }
> -
> - if (n != n2)
> - return false;
> -
> - return intel_fuzzy_clock_check(m, m2);
> -}
> -
> -static bool
> -intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> - struct intel_link_m_n *m2_n2,
> - bool adjust)
> -{
> - if (m_n->tu == m2_n2->tu &&
> - intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
> - m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
> - intel_compare_m_n(m_n->link_m, m_n->link_n,
> - m2_n2->link_m, m2_n2->link_n, !adjust)) {
> - if (adjust)
> - *m2_n2 = *m_n;
> -
> - return true;
> - }
> -
> - return false;
> -}
> -
> -static bool
> -intel_compare_infoframe(const union hdmi_infoframe *a,
> - const union hdmi_infoframe *b)
> -{
> - return memcmp(a, b, sizeof(*a)) == 0;
> -}
> -
> -static void
> -pipe_config_infoframe_err(struct drm_i915_private *dev_priv,
> - bool adjust, const char *name,
> - const union hdmi_infoframe *a,
> - const union hdmi_infoframe *b)
> -{
> - if (adjust) {
> - if ((drm_debug & DRM_UT_KMS) == 0)
> - return;
> -
> - drm_dbg(DRM_UT_KMS, "mismatch in %s infoframe", name);
> - drm_dbg(DRM_UT_KMS, "expected:");
> - hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, a);
> - drm_dbg(DRM_UT_KMS, "found");
> - hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, b);
> - } else {
> - drm_err("mismatch in %s infoframe", name);
> - drm_err("expected:");
> - hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, a);
> - drm_err("found");
> - hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, b);
> - }
> -}
> -
> -static void __printf(3, 4)
> -pipe_config_err(bool adjust, const char *name, const char *format, ...)
> -{
> - struct va_format vaf;
> - va_list args;
> -
> - va_start(args, format);
> - vaf.fmt = format;
> - vaf.va = &args;
> -
> - if (adjust)
> - drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> - else
> - drm_err("mismatch in %s %pV", name, &vaf);
> -
> - va_end(args);
> -}
> -
> -static bool fastboot_enabled(struct drm_i915_private *dev_priv)
> -{
> - if (i915_modparams.fastboot != -1)
> - return i915_modparams.fastboot;
> -
> - /* Enable fastboot by default on Skylake and newer */
> - if (INTEL_GEN(dev_priv) >= 9)
> - return true;
> -
> - /* Enable fastboot by default on VLV and CHV */
> - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> - return true;
> -
> - /* Disabled by default on all others */
> - return false;
> -}
> -
> -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)
> -{
> - bool ret = true;
> - bool fixup_inherited = adjust &&
> - (current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
> - !(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
> -
> - if (fixup_inherited && !fastboot_enabled(dev_priv)) {
> - DRM_DEBUG_KMS("initial modeset and fastboot not set\n");
> - ret = false;
> - }
> -
> -#define PIPE_CONF_CHECK_X(name) do { \
> - if (current_config->name != pipe_config->name) { \
> - pipe_config_err(adjust, __stringify(name), \
> - "(expected 0x%08x, found 0x%08x)\n", \
> - current_config->name, \
> - pipe_config->name); \
> - ret = false; \
> - } \
> -} while (0)
> -
> -#define PIPE_CONF_CHECK_I(name) do { \
> - if (current_config->name != pipe_config->name) { \
> - pipe_config_err(adjust, __stringify(name), \
> - "(expected %i, found %i)\n", \
> - current_config->name, \
> - pipe_config->name); \
> - ret = false; \
> - } \
> -} while (0)
> -
> -#define PIPE_CONF_CHECK_BOOL(name) do { \
> - if (current_config->name != pipe_config->name) { \
> - pipe_config_err(adjust, __stringify(name), \
> - "(expected %s, found %s)\n", \
> - yesno(current_config->name), \
> - yesno(pipe_config->name)); \
> - ret = false; \
> - } \
> -} while (0)
> -
> -/*
> - * Checks state where we only read out the enabling, but not the entire
> - * state itself (like full infoframes or ELD for audio). These states
> - * require a full modeset on bootup to fix up.
> - */
> -#define PIPE_CONF_CHECK_BOOL_INCOMPLETE(name) do { \
> - if (!fixup_inherited || (!current_config->name && !pipe_config->name)) { \
> - PIPE_CONF_CHECK_BOOL(name); \
> - } else { \
> - pipe_config_err(adjust, __stringify(name), \
> - "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
> - yesno(current_config->name), \
> - yesno(pipe_config->name)); \
> - ret = false; \
> - } \
> -} while (0)
> -
> -#define PIPE_CONF_CHECK_P(name) do { \
> - if (current_config->name != pipe_config->name) { \
> - pipe_config_err(adjust, __stringify(name), \
> - "(expected %p, found %p)\n", \
> - current_config->name, \
> - pipe_config->name); \
> - ret = false; \
> - } \
> -} while (0)
> -
> -#define PIPE_CONF_CHECK_M_N(name) do { \
> - if (!intel_compare_link_m_n(¤t_config->name, \
> - &pipe_config->name,\
> - adjust)) { \
> - pipe_config_err(adjust, __stringify(name), \
> - "(expected tu %i gmch %i/%i link %i/%i, " \
> - "found tu %i, gmch %i/%i link %i/%i)\n", \
> - current_config->name.tu, \
> - current_config->name.gmch_m, \
> - current_config->name.gmch_n, \
> - current_config->name.link_m, \
> - current_config->name.link_n, \
> - pipe_config->name.tu, \
> - pipe_config->name.gmch_m, \
> - pipe_config->name.gmch_n, \
> - pipe_config->name.link_m, \
> - pipe_config->name.link_n); \
> - ret = false; \
> - } \
> -} while (0)
> -
> -/* This is required for BDW+ where there is only one set of registers for
> - * switching between high and low RR.
> - * This macro can be used whenever a comparison has to be made between one
> - * hw state and multiple sw state variables.
> - */
> -#define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
> - if (!intel_compare_link_m_n(¤t_config->name, \
> - &pipe_config->name, adjust) && \
> - !intel_compare_link_m_n(¤t_config->alt_name, \
> - &pipe_config->name, adjust)) { \
> - pipe_config_err(adjust, __stringify(name), \
> - "(expected tu %i gmch %i/%i link %i/%i, " \
> - "or tu %i gmch %i/%i link %i/%i, " \
> - "found tu %i, gmch %i/%i link %i/%i)\n", \
> - current_config->name.tu, \
> - current_config->name.gmch_m, \
> - current_config->name.gmch_n, \
> - current_config->name.link_m, \
> - current_config->name.link_n, \
> - current_config->alt_name.tu, \
> - current_config->alt_name.gmch_m, \
> - current_config->alt_name.gmch_n, \
> - current_config->alt_name.link_m, \
> - current_config->alt_name.link_n, \
> - pipe_config->name.tu, \
> - pipe_config->name.gmch_m, \
> - pipe_config->name.gmch_n, \
> - pipe_config->name.link_m, \
> - pipe_config->name.link_n); \
> - ret = false; \
> - } \
> -} while (0)
> -
> -#define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
> - if ((current_config->name ^ pipe_config->name) & (mask)) { \
> - pipe_config_err(adjust, __stringify(name), \
> - "(%x) (expected %i, found %i)\n", \
> - (mask), \
> - current_config->name & (mask), \
> - pipe_config->name & (mask)); \
> - ret = false; \
> - } \
> -} while (0)
> -
> -#define PIPE_CONF_CHECK_CLOCK_FUZZY(name) do { \
> - if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
> - pipe_config_err(adjust, __stringify(name), \
> - "(expected %i, found %i)\n", \
> - current_config->name, \
> - pipe_config->name); \
> - ret = false; \
> - } \
> -} while (0)
> -
> -#define PIPE_CONF_CHECK_INFOFRAME(name) do { \
> - if (!intel_compare_infoframe(¤t_config->infoframes.name, \
> - &pipe_config->infoframes.name)) { \
> - pipe_config_infoframe_err(dev_priv, adjust, __stringify(name), \
> - ¤t_config->infoframes.name, \
> - &pipe_config->infoframes.name); \
> - ret = false; \
> - } \
> -} while (0)
> -
> -#define PIPE_CONF_QUIRK(quirk) \
> - ((current_config->quirks | pipe_config->quirks) & (quirk))
> -
> - PIPE_CONF_CHECK_I(cpu_transcoder);
> -
> - PIPE_CONF_CHECK_BOOL(has_pch_encoder);
> - PIPE_CONF_CHECK_I(fdi_lanes);
> - PIPE_CONF_CHECK_M_N(fdi_m_n);
> -
> - PIPE_CONF_CHECK_I(lane_count);
> - PIPE_CONF_CHECK_X(lane_lat_optim_mask);
> -
> - if (INTEL_GEN(dev_priv) < 8) {
> - PIPE_CONF_CHECK_M_N(dp_m_n);
> -
> - if (current_config->has_drrs)
> - PIPE_CONF_CHECK_M_N(dp_m2_n2);
> - } else
> - PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
> -
> - PIPE_CONF_CHECK_X(output_types);
> -
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal);
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_start);
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_end);
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hsync_start);
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hsync_end);
> -
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vdisplay);
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vtotal);
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vblank_start);
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vblank_end);
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_start);
> - PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_end);
> -
> - PIPE_CONF_CHECK_I(pixel_multiplier);
> - PIPE_CONF_CHECK_I(output_format);
> - PIPE_CONF_CHECK_BOOL(has_hdmi_sink);
> - if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
> - IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> - PIPE_CONF_CHECK_BOOL(limited_color_range);
> -
> - PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> - PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> - PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe);
> -
> - PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
> -
> - PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
> - DRM_MODE_FLAG_INTERLACE);
> -
> - if (!PIPE_CONF_QUIRK(PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS)) {
> - PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
> - DRM_MODE_FLAG_PHSYNC);
> - PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
> - DRM_MODE_FLAG_NHSYNC);
> - PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
> - DRM_MODE_FLAG_PVSYNC);
> - PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
> - DRM_MODE_FLAG_NVSYNC);
> - }
> -
> - PIPE_CONF_CHECK_X(gmch_pfit.control);
> - /* pfit ratios are autocomputed by the hw on gen4+ */
> - if (INTEL_GEN(dev_priv) < 4)
> - PIPE_CONF_CHECK_X(gmch_pfit.pgm_ratios);
> - PIPE_CONF_CHECK_X(gmch_pfit.lvds_border_bits);
> -
> - if (!adjust) {
> - PIPE_CONF_CHECK_I(pipe_src_w);
> - PIPE_CONF_CHECK_I(pipe_src_h);
> -
> - PIPE_CONF_CHECK_BOOL(pch_pfit.enabled);
> - if (current_config->pch_pfit.enabled) {
> - PIPE_CONF_CHECK_X(pch_pfit.pos);
> - PIPE_CONF_CHECK_X(pch_pfit.size);
> - }
> -
> - PIPE_CONF_CHECK_I(scaler_state.scaler_id);
> - PIPE_CONF_CHECK_CLOCK_FUZZY(pixel_rate);
> -
> - PIPE_CONF_CHECK_X(gamma_mode);
> - if (IS_CHERRYVIEW(dev_priv))
> - PIPE_CONF_CHECK_X(cgm_mode);
> - else
> - PIPE_CONF_CHECK_X(csc_mode);
> - PIPE_CONF_CHECK_BOOL(gamma_enable);
> - PIPE_CONF_CHECK_BOOL(csc_enable);
> - }
> -
> - PIPE_CONF_CHECK_BOOL(double_wide);
> -
> - PIPE_CONF_CHECK_P(shared_dpll);
> - PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
> - PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
> - PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
> - PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
> - PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
> - PIPE_CONF_CHECK_X(dpll_hw_state.spll);
> - PIPE_CONF_CHECK_X(dpll_hw_state.ctrl1);
> - PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr1);
> - PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr2);
> - PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr0);
> - PIPE_CONF_CHECK_X(dpll_hw_state.ebb0);
> - PIPE_CONF_CHECK_X(dpll_hw_state.ebb4);
> - PIPE_CONF_CHECK_X(dpll_hw_state.pll0);
> - PIPE_CONF_CHECK_X(dpll_hw_state.pll1);
> - PIPE_CONF_CHECK_X(dpll_hw_state.pll2);
> - PIPE_CONF_CHECK_X(dpll_hw_state.pll3);
> - PIPE_CONF_CHECK_X(dpll_hw_state.pll6);
> - PIPE_CONF_CHECK_X(dpll_hw_state.pll8);
> - PIPE_CONF_CHECK_X(dpll_hw_state.pll9);
> - PIPE_CONF_CHECK_X(dpll_hw_state.pll10);
> - PIPE_CONF_CHECK_X(dpll_hw_state.pcsdw12);
> - PIPE_CONF_CHECK_X(dpll_hw_state.mg_refclkin_ctl);
> - PIPE_CONF_CHECK_X(dpll_hw_state.mg_clktop2_coreclkctl1);
> - PIPE_CONF_CHECK_X(dpll_hw_state.mg_clktop2_hsclkctl);
> - PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_div0);
> - PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_div1);
> - PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_lf);
> - PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_frac_lock);
> - PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_ssc);
> - PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_bias);
> - PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_tdc_coldst_bias);
> -
> - PIPE_CONF_CHECK_X(dsi_pll.ctrl);
> - PIPE_CONF_CHECK_X(dsi_pll.div);
> -
> - if (IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5)
> - PIPE_CONF_CHECK_I(pipe_bpp);
> -
> - PIPE_CONF_CHECK_CLOCK_FUZZY(base.adjusted_mode.crtc_clock);
> - PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock);
> -
> - PIPE_CONF_CHECK_I(min_voltage_level);
> -
> - PIPE_CONF_CHECK_X(infoframes.enable);
> - PIPE_CONF_CHECK_X(infoframes.gcp);
> - PIPE_CONF_CHECK_INFOFRAME(avi);
> - PIPE_CONF_CHECK_INFOFRAME(spd);
> - PIPE_CONF_CHECK_INFOFRAME(hdmi);
> -
> -#undef PIPE_CONF_CHECK_X
> -#undef PIPE_CONF_CHECK_I
> -#undef PIPE_CONF_CHECK_BOOL
> -#undef PIPE_CONF_CHECK_BOOL_INCOMPLETE
> -#undef PIPE_CONF_CHECK_P
> -#undef PIPE_CONF_CHECK_FLAGS
> -#undef PIPE_CONF_CHECK_CLOCK_FUZZY
> -#undef PIPE_CONF_QUIRK
> -
> - return ret;
> -}
> -
> -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),
> - &pipe_config->fdi_m_n);
> - int dotclock = pipe_config->base.adjusted_mode.crtc_clock;
> -
> - /*
> - * FDI already provided one idea for the dotclock.
> - * Yell if the encoder disagrees.
> - */
> - WARN(!intel_fuzzy_clock_check(fdi_dotclock, dotclock),
> - "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
> - fdi_dotclock, dotclock);
> - }
> -}
> -
> static void update_scanline_offset(const struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 77767c..ed853b2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1668,17 +1668,12 @@ 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);
> +u32 intel_fdi_link_freq(struct drm_i915_private *dev_priv,
> + const struct intel_crtc_state *pipe_config);
> static inline bool
> intel_crtc_has_type(const struct intel_crtc_state *crtc_state,
> enum intel_output_type type)
> diff --git a/drivers/gpu/drm/i915/intel_verify.c b/drivers/gpu/drm/i915/intel_verify.c
> index 4c8990..9785ad 100644
> --- a/drivers/gpu/drm/i915/intel_verify.c
> +++ b/drivers/gpu/drm/i915/intel_verify.c
> @@ -10,6 +10,471 @@
> #include "intel_pm.h"
> #include "intel_verify.h"
>
> +static bool intel_fuzzy_clock_check(int clock1, int clock2)
> +{
> + int diff;
> +
> + if (clock1 == clock2)
> + return true;
> +
> + if (!clock1 || !clock2)
> + return false;
> +
> + diff = abs(clock1 - clock2);
> +
> + if (((((diff + clock1 + clock2) * 100)) / (clock1 + clock2)) < 105)
> + return true;
> +
> + return false;
> +}
> +
> +static bool
> +intel_compare_m_n(unsigned int m, unsigned int n,
> + unsigned int m2, unsigned int n2,
> + bool exact)
> +{
> + if (m == m2 && n == n2)
> + return true;
> +
> + if (exact || !m || !n || !m2 || !n2)
> + return false;
> +
> + BUILD_BUG_ON(DATA_LINK_M_N_MASK > INT_MAX);
> +
> + if (n > n2) {
> + while (n > n2) {
> + m2 <<= 1;
> + n2 <<= 1;
> + }
> + } else if (n < n2) {
> + while (n < n2) {
> + m <<= 1;
> + n <<= 1;
> + }
> + }
> +
> + if (n != n2)
> + return false;
> +
> + return intel_fuzzy_clock_check(m, m2);
> +}
> +
> +static bool
> +intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> + struct intel_link_m_n *m2_n2,
> + bool adjust)
> +{
> + if (m_n->tu == m2_n2->tu &&
> + intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
> + m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
> + intel_compare_m_n(m_n->link_m, m_n->link_n,
> + m2_n2->link_m, m2_n2->link_n, !adjust)) {
> + if (adjust)
> + *m2_n2 = *m_n;
> +
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static bool
> +intel_compare_infoframe(const union hdmi_infoframe *a,
> + const union hdmi_infoframe *b)
> +{
> + return memcmp(a, b, sizeof(*a)) == 0;
> +}
> +
> +static void
> +pipe_config_infoframe_err(struct drm_i915_private *dev_priv,
> + bool adjust, const char *name,
> + const union hdmi_infoframe *a,
> + const union hdmi_infoframe *b)
> +{
> + if (adjust) {
> + if ((drm_debug & DRM_UT_KMS) == 0)
> + return;
> +
> + drm_dbg(DRM_UT_KMS, "mismatch in %s infoframe", name);
> + drm_dbg(DRM_UT_KMS, "expected:");
> + hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, a);
> + drm_dbg(DRM_UT_KMS, "found");
> + hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, b);
> + } else {
> + drm_err("mismatch in %s infoframe", name);
> + drm_err("expected:");
> + hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, a);
> + drm_err("found");
> + hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, b);
> + }
> +}
> +
> +static void __printf(3, 4)
> +pipe_config_err(bool adjust, const char *name, const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + va_start(args, format);
> + vaf.fmt = format;
> + vaf.va = &args;
> +
> + if (adjust)
> + drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> + else
> + drm_err("mismatch in %s %pV", name, &vaf);
> +
> + va_end(args);
> +}
> +
> +static bool fastboot_enabled(struct drm_i915_private *dev_priv)
> +{
> + if (i915_modparams.fastboot != -1)
> + return i915_modparams.fastboot;
> +
> + /* Enable fastboot by default on Skylake and newer */
> + if (INTEL_GEN(dev_priv) >= 9)
> + return true;
> +
> + /* Enable fastboot by default on VLV and CHV */
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + return true;
> +
> + /* Disabled by default on all others */
> + return false;
> +}
> +
> +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)
> +{
> + bool ret = true;
> + bool fixup_inherited = adjust &&
> + (current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
> + !(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
> +
> + if (fixup_inherited && !fastboot_enabled(dev_priv)) {
> + DRM_DEBUG_KMS("initial modeset and fastboot not set\n");
> + ret = false;
> + }
> +
> +#define PIPE_CONF_CHECK_X(name) do { \
> + if (current_config->name != pipe_config->name) { \
> + pipe_config_err(adjust, __stringify(name), \
> + "(expected 0x%08x, found 0x%08x)\n", \
> + current_config->name, \
> + pipe_config->name); \
> + ret = false; \
> + } \
> +} while (0)
> +
> +#define PIPE_CONF_CHECK_I(name) do { \
> + if (current_config->name != pipe_config->name) { \
> + pipe_config_err(adjust, __stringify(name), \
> + "(expected %i, found %i)\n", \
> + current_config->name, \
> + pipe_config->name); \
> + ret = false; \
> + } \
> +} while (0)
> +
> +#define PIPE_CONF_CHECK_BOOL(name) do { \
> + if (current_config->name != pipe_config->name) { \
> + pipe_config_err(adjust, __stringify(name), \
> + "(expected %s, found %s)\n", \
> + yesno(current_config->name), \
> + yesno(pipe_config->name)); \
> + ret = false; \
> + } \
> +} while (0)
> +
> +/*
> + * Checks state where we only read out the enabling, but not the entire
> + * state itself (like full infoframes or ELD for audio). These states
> + * require a full modeset on bootup to fix up.
> + */
> +#define PIPE_CONF_CHECK_BOOL_INCOMPLETE(name) do { \
> + if (!fixup_inherited || (!current_config->name && !pipe_config->name)) { \
> + PIPE_CONF_CHECK_BOOL(name); \
> + } else { \
> + pipe_config_err(adjust, __stringify(name), \
> + "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
> + yesno(current_config->name), \
> + yesno(pipe_config->name)); \
> + ret = false; \
> + } \
> +} while (0)
> +
> +#define PIPE_CONF_CHECK_P(name) do { \
> + if (current_config->name != pipe_config->name) { \
> + pipe_config_err(adjust, __stringify(name), \
> + "(expected %p, found %p)\n", \
> + current_config->name, \
> + pipe_config->name); \
> + ret = false; \
> + } \
> +} while (0)
> +
> +#define PIPE_CONF_CHECK_M_N(name) do { \
> + if (!intel_compare_link_m_n(¤t_config->name, \
> + &pipe_config->name,\
> + adjust)) { \
> + pipe_config_err(adjust, __stringify(name), \
> + "(expected tu %i gmch %i/%i link %i/%i, " \
> + "found tu %i, gmch %i/%i link %i/%i)\n", \
> + current_config->name.tu, \
> + current_config->name.gmch_m, \
> + current_config->name.gmch_n, \
> + current_config->name.link_m, \
> + current_config->name.link_n, \
> + pipe_config->name.tu, \
> + pipe_config->name.gmch_m, \
> + pipe_config->name.gmch_n, \
> + pipe_config->name.link_m, \
> + pipe_config->name.link_n); \
> + ret = false; \
> + } \
> +} while (0)
> +
> +/* This is required for BDW+ where there is only one set of registers for
> + * switching between high and low RR.
> + * This macro can be used whenever a comparison has to be made between one
> + * hw state and multiple sw state variables.
> + */
> +#define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
> + if (!intel_compare_link_m_n(¤t_config->name, \
> + &pipe_config->name, adjust) && \
> + !intel_compare_link_m_n(¤t_config->alt_name, \
> + &pipe_config->name, adjust)) { \
> + pipe_config_err(adjust, __stringify(name), \
> + "(expected tu %i gmch %i/%i link %i/%i, " \
> + "or tu %i gmch %i/%i link %i/%i, " \
> + "found tu %i, gmch %i/%i link %i/%i)\n", \
> + current_config->name.tu, \
> + current_config->name.gmch_m, \
> + current_config->name.gmch_n, \
> + current_config->name.link_m, \
> + current_config->name.link_n, \
> + current_config->alt_name.tu, \
> + current_config->alt_name.gmch_m, \
> + current_config->alt_name.gmch_n, \
> + current_config->alt_name.link_m, \
> + current_config->alt_name.link_n, \
> + pipe_config->name.tu, \
> + pipe_config->name.gmch_m, \
> + pipe_config->name.gmch_n, \
> + pipe_config->name.link_m, \
> + pipe_config->name.link_n); \
> + ret = false; \
> + } \
> +} while (0)
> +
> +#define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
> + if ((current_config->name ^ pipe_config->name) & (mask)) { \
> + pipe_config_err(adjust, __stringify(name), \
> + "(%x) (expected %i, found %i)\n", \
> + (mask), \
> + current_config->name & (mask), \
> + pipe_config->name & (mask)); \
> + ret = false; \
> + } \
> +} while (0)
> +
> +#define PIPE_CONF_CHECK_CLOCK_FUZZY(name) do { \
> + if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
> + pipe_config_err(adjust, __stringify(name), \
> + "(expected %i, found %i)\n", \
> + current_config->name, \
> + pipe_config->name); \
> + ret = false; \
> + } \
> +} while (0)
> +
> +#define PIPE_CONF_CHECK_INFOFRAME(name) do { \
> + if (!intel_compare_infoframe(¤t_config->infoframes.name, \
> + &pipe_config->infoframes.name)) { \
> + pipe_config_infoframe_err(dev_priv, adjust, __stringify(name), \
> + ¤t_config->infoframes.name, \
> + &pipe_config->infoframes.name); \
> + ret = false; \
> + } \
> +} while (0)
> +
> +#define PIPE_CONF_QUIRK(quirk) \
> + ((current_config->quirks | pipe_config->quirks) & (quirk))
> +
> + PIPE_CONF_CHECK_I(cpu_transcoder);
> +
> + PIPE_CONF_CHECK_BOOL(has_pch_encoder);
> + PIPE_CONF_CHECK_I(fdi_lanes);
> + PIPE_CONF_CHECK_M_N(fdi_m_n);
> +
> + PIPE_CONF_CHECK_I(lane_count);
> + PIPE_CONF_CHECK_X(lane_lat_optim_mask);
> +
> + if (INTEL_GEN(dev_priv) < 8) {
> + PIPE_CONF_CHECK_M_N(dp_m_n);
> +
> + if (current_config->has_drrs)
> + PIPE_CONF_CHECK_M_N(dp_m2_n2);
> + } else
> + PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
> +
> + PIPE_CONF_CHECK_X(output_types);
> +
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal);
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_start);
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_end);
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hsync_start);
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hsync_end);
> +
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vdisplay);
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vtotal);
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vblank_start);
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vblank_end);
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_start);
> + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_end);
> +
> + PIPE_CONF_CHECK_I(pixel_multiplier);
> + PIPE_CONF_CHECK_I(output_format);
> + PIPE_CONF_CHECK_BOOL(has_hdmi_sink);
> + if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
> + IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + PIPE_CONF_CHECK_BOOL(limited_color_range);
> +
> + PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> + PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> + PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe);
> +
> + PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
> +
> + PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
> + DRM_MODE_FLAG_INTERLACE);
> +
> + if (!PIPE_CONF_QUIRK(PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS)) {
> + PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
> + DRM_MODE_FLAG_PHSYNC);
> + PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
> + DRM_MODE_FLAG_NHSYNC);
> + PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
> + DRM_MODE_FLAG_PVSYNC);
> + PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags,
> + DRM_MODE_FLAG_NVSYNC);
> + }
> +
> + PIPE_CONF_CHECK_X(gmch_pfit.control);
> + /* pfit ratios are autocomputed by the hw on gen4+ */
> + if (INTEL_GEN(dev_priv) < 4)
> + PIPE_CONF_CHECK_X(gmch_pfit.pgm_ratios);
> + PIPE_CONF_CHECK_X(gmch_pfit.lvds_border_bits);
> +
> + if (!adjust) {
> + PIPE_CONF_CHECK_I(pipe_src_w);
> + PIPE_CONF_CHECK_I(pipe_src_h);
> +
> + PIPE_CONF_CHECK_BOOL(pch_pfit.enabled);
> + if (current_config->pch_pfit.enabled) {
> + PIPE_CONF_CHECK_X(pch_pfit.pos);
> + PIPE_CONF_CHECK_X(pch_pfit.size);
> + }
> +
> + PIPE_CONF_CHECK_I(scaler_state.scaler_id);
> + PIPE_CONF_CHECK_CLOCK_FUZZY(pixel_rate);
> +
> + PIPE_CONF_CHECK_X(gamma_mode);
> + if (IS_CHERRYVIEW(dev_priv))
> + PIPE_CONF_CHECK_X(cgm_mode);
> + else
> + PIPE_CONF_CHECK_X(csc_mode);
> + PIPE_CONF_CHECK_BOOL(gamma_enable);
> + PIPE_CONF_CHECK_BOOL(csc_enable);
> + }
> +
> + PIPE_CONF_CHECK_BOOL(double_wide);
> +
> + PIPE_CONF_CHECK_P(shared_dpll);
> + PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
> + PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
> + PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
> + PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
> + PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
> + PIPE_CONF_CHECK_X(dpll_hw_state.spll);
> + PIPE_CONF_CHECK_X(dpll_hw_state.ctrl1);
> + PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr1);
> + PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr2);
> + PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr0);
> + PIPE_CONF_CHECK_X(dpll_hw_state.ebb0);
> + PIPE_CONF_CHECK_X(dpll_hw_state.ebb4);
> + PIPE_CONF_CHECK_X(dpll_hw_state.pll0);
> + PIPE_CONF_CHECK_X(dpll_hw_state.pll1);
> + PIPE_CONF_CHECK_X(dpll_hw_state.pll2);
> + PIPE_CONF_CHECK_X(dpll_hw_state.pll3);
> + PIPE_CONF_CHECK_X(dpll_hw_state.pll6);
> + PIPE_CONF_CHECK_X(dpll_hw_state.pll8);
> + PIPE_CONF_CHECK_X(dpll_hw_state.pll9);
> + PIPE_CONF_CHECK_X(dpll_hw_state.pll10);
> + PIPE_CONF_CHECK_X(dpll_hw_state.pcsdw12);
> + PIPE_CONF_CHECK_X(dpll_hw_state.mg_refclkin_ctl);
> + PIPE_CONF_CHECK_X(dpll_hw_state.mg_clktop2_coreclkctl1);
> + PIPE_CONF_CHECK_X(dpll_hw_state.mg_clktop2_hsclkctl);
> + PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_div0);
> + PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_div1);
> + PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_lf);
> + PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_frac_lock);
> + PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_ssc);
> + PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_bias);
> + PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_tdc_coldst_bias);
> +
> + PIPE_CONF_CHECK_X(dsi_pll.ctrl);
> + PIPE_CONF_CHECK_X(dsi_pll.div);
> +
> + if (IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5)
> + PIPE_CONF_CHECK_I(pipe_bpp);
> +
> + PIPE_CONF_CHECK_CLOCK_FUZZY(base.adjusted_mode.crtc_clock);
> + PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock);
> +
> + PIPE_CONF_CHECK_I(min_voltage_level);
> +
> + PIPE_CONF_CHECK_X(infoframes.enable);
> + PIPE_CONF_CHECK_X(infoframes.gcp);
> + PIPE_CONF_CHECK_INFOFRAME(avi);
> + PIPE_CONF_CHECK_INFOFRAME(spd);
> + PIPE_CONF_CHECK_INFOFRAME(hdmi);
> +
> +#undef PIPE_CONF_CHECK_X
> +#undef PIPE_CONF_CHECK_I
> +#undef PIPE_CONF_CHECK_BOOL
> +#undef PIPE_CONF_CHECK_BOOL_INCOMPLETE
> +#undef PIPE_CONF_CHECK_P
> +#undef PIPE_CONF_CHECK_FLAGS
> +#undef PIPE_CONF_CHECK_CLOCK_FUZZY
> +#undef PIPE_CONF_QUIRK
> +
> + return ret;
> +}
> +
> +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),
> + &pipe_config->fdi_m_n);
> + int dotclock = pipe_config->base.adjusted_mode.crtc_clock;
> +
> + /*
> + * FDI already provided one idea for the dotclock.
> + * Yell if the encoder disagrees.
> + */
> + WARN(!intel_fuzzy_clock_check(fdi_dotclock, dotclock),
> + "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
> + fdi_dotclock, dotclock);
> + }
> +}
> +
> static void verify_wm_state(struct drm_crtc *crtc,
> struct drm_crtc_state *new_state)
> {
> diff --git a/drivers/gpu/drm/i915/intel_verify.h b/drivers/gpu/drm/i915/intel_verify.h
> index 4b751ea..faaf8f 100644
> --- a/drivers/gpu/drm/i915/intel_verify.h
> +++ b/drivers/gpu/drm/i915/intel_verify.h
> @@ -10,7 +10,15 @@ struct drm_atomic_state;
> struct drm_crtc;
> struct drm_crtc_state;
> struct drm_device;
> +struct drm_i915_private;
> +struct intel_crtc_state;
>
> +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_verify_modeset_crtc(struct drm_crtc *crtc,
> struct drm_atomic_state *state,
> struct drm_crtc_state *old_state,
> --
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 1/2] drm/i915: start moving state checker to intel_verify.c
2019-05-15 9:33 ` [RFC 1/2] " Daniel Vetter
@ 2019-05-15 9:38 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2019-05-15 9:38 UTC (permalink / raw)
To: Jani Nikula; +Cc: 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-05-15 9:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-16 10:36 [RFC 1/2] drm/i915: start moving state checker to intel_verify.c Jani Nikula
2019-04-16 10:36 ` [RFC 2/2] drm/i915: move pipe config compare " Jani Nikula
2019-05-15 9:36 ` Daniel Vetter
2019-04-16 12:14 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [RFC,1/2] drm/i915: start moving state checker " Patchwork
2019-04-16 12:15 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-04-16 12:42 ` ✓ Fi.CI.BAT: success " Patchwork
2019-04-16 17:45 ` ✓ Fi.CI.IGT: " Patchwork
2019-05-15 9:33 ` [RFC 1/2] " Daniel Vetter
2019-05-15 9:38 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox