[PATCH v13 2/3] drm/i915: Use new CRC debugfs API
From: Tomeu Vizoso
Date: Wed Dec 14 2016 - 08:35:25 EST
The core provides now an ABI to userspace for generation of frame CRCs,
so implement the ->set_crc_source() callback and reuse as much code as
possible with the previous ABI implementation.
When handling the pageflip interrupt, we skip 1 or 2 frames depending on
the HW because they contain wrong values. For the legacy ABI for
generating frame CRCs, this was done in userspace but now that we have a
generic ABI it's better if it's not exposed by the kernel.
v2:
- Leave the legacy implementation in place as the ABI implementation
in the core is incompatible with it.
v3:
- Use the "cooked" vblank counter so we have a whole 32 bits.
- Make sure we don't mess with the state of the legacy CRC capture
ABI implementation.
v4:
- Keep use of get_vblank_counter as in the legacy code, will be
changed in a followup commit.
v5:
- Skip first frame or two as it's known that they contain wrong
data.
- A few fixes suggested by Emil Velikov.
v6:
- Rework programming of the HW registers to preserve previous
behavior.
v7:
- Address whitespace issue.
- Added a comment on why in the implementation of the new ABI we
skip the 1st or 2nd frames.
v9:
- Add stub for intel_crtc_set_crc_source.
v12:
- Rebased.
- Remove stub for intel_crtc_set_crc_source and instead set the
callback to NULL (Jani Nikula).
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx>
Reviewed-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 81 ++++++++++++++++++++----------
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 6 +++
drivers/gpu/drm/i915/intel_pipe_crc.c | 94 ++++++++++++++++++++++++++++++-----
5 files changed, 145 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 20bc04d5e617..d7c0b44a1447 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1874,6 +1874,7 @@ struct intel_pipe_crc {
enum intel_pipe_crc_source source;
int head, tail;
wait_queue_head_t wq;
+ int skipped;
};
struct i915_frontbuffer_tracking {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a0e70f5b3aad..24b7620c1005 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1553,41 +1553,70 @@ static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
{
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
struct intel_pipe_crc_entry *entry;
- int head, tail;
+ struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+ struct drm_driver *driver = dev_priv->drm.driver;
+ uint32_t crcs[5];
+ int head, tail, ret;
+ u32 frame;
spin_lock(&pipe_crc->lock);
+ if (pipe_crc->source) {
+ if (!pipe_crc->entries) {
+ spin_unlock(&pipe_crc->lock);
+ DRM_DEBUG_KMS("spurious interrupt\n");
+ return;
+ }
- if (!pipe_crc->entries) {
- spin_unlock(&pipe_crc->lock);
- DRM_DEBUG_KMS("spurious interrupt\n");
- return;
- }
-
- head = pipe_crc->head;
- tail = pipe_crc->tail;
+ head = pipe_crc->head;
+ tail = pipe_crc->tail;
- if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
- spin_unlock(&pipe_crc->lock);
- DRM_ERROR("CRC buffer overflowing\n");
- return;
- }
+ if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
+ spin_unlock(&pipe_crc->lock);
+ DRM_ERROR("CRC buffer overflowing\n");
+ return;
+ }
- entry = &pipe_crc->entries[head];
+ entry = &pipe_crc->entries[head];
- entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm,
- pipe);
- entry->crc[0] = crc0;
- entry->crc[1] = crc1;
- entry->crc[2] = crc2;
- entry->crc[3] = crc3;
- entry->crc[4] = crc4;
+ entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
+ entry->crc[0] = crc0;
+ entry->crc[1] = crc1;
+ entry->crc[2] = crc2;
+ entry->crc[3] = crc3;
+ entry->crc[4] = crc4;
- head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
- pipe_crc->head = head;
+ head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
+ pipe_crc->head = head;
- spin_unlock(&pipe_crc->lock);
+ spin_unlock(&pipe_crc->lock);
- wake_up_interruptible(&pipe_crc->wq);
+ wake_up_interruptible(&pipe_crc->wq);
+ } else {
+ /*
+ * For some not yet identified reason, the first CRC is
+ * bonkers. So let's just wait for the next vblank and read
+ * out the buggy result.
+ *
+ * On CHV sometimes the second CRC is bonkers as well, so
+ * don't trust that one either.
+ */
+ if (pipe_crc->skipped == 0 ||
+ (IS_CHERRYVIEW(dev_priv) && pipe_crc->skipped == 1)) {
+ pipe_crc->skipped++;
+ spin_unlock(&pipe_crc->lock);
+ return;
+ }
+ spin_unlock(&pipe_crc->lock);
+ crcs[0] = crc0;
+ crcs[1] = crc1;
+ crcs[2] = crc2;
+ crcs[3] = crc3;
+ crcs[4] = crc4;
+ frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
+ ret = drm_crtc_add_crc_entry(&crtc->base, true, frame, crcs);
+ if (!ret)
+ wake_up_interruptible(&crtc->base.crc.wq);
+ }
}
#else
static inline void
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bc1af87789bc..96802591e6b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14771,6 +14771,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
.page_flip = intel_crtc_page_flip,
.atomic_duplicate_state = intel_crtc_duplicate_state,
.atomic_destroy_state = intel_crtc_destroy_state,
+ .set_crc_source = intel_crtc_set_crc_source,
};
/**
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 362f698af2a0..99b48aa907fe 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1873,5 +1873,11 @@ void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
/* intel_pipe_crc.c */
int intel_pipe_crc_create(struct drm_minor *minor);
void intel_pipe_crc_cleanup(struct drm_minor *minor);
+#ifdef CONFIG_DEBUG_FS
+int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
+ size_t *values_cnt);
+#else
+#define intel_crtc_set_crc_source NULL
+#endif
extern const struct file_operations i915_display_crc_ctl_fops;
#endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
index ef0c0e195164..0f1da810cff0 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -613,6 +613,22 @@ static int ivb_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
return 0;
}
+static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
+ enum pipe pipe,
+ enum intel_pipe_crc_source *source, u32 *val)
+{
+ if (IS_GEN2(dev_priv))
+ return i8xx_pipe_crc_ctl_reg(source, val);
+ else if (INTEL_GEN(dev_priv) < 5)
+ return i9xx_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
+ else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ return vlv_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
+ else if (IS_GEN5(dev_priv) || IS_GEN6(dev_priv))
+ return ilk_pipe_crc_ctl_reg(source, val);
+ else
+ return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
+}
+
static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
enum pipe pipe,
enum intel_pipe_crc_source source)
@@ -636,17 +652,7 @@ static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
return -EIO;
}
- if (IS_GEN2(dev_priv))
- ret = i8xx_pipe_crc_ctl_reg(&source, &val);
- else if (INTEL_GEN(dev_priv) < 5)
- ret = i9xx_pipe_crc_ctl_reg(dev_priv, pipe, &source, &val);
- else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- ret = vlv_pipe_crc_ctl_reg(dev_priv, pipe, &source, &val);
- else if (IS_GEN5(dev_priv) || IS_GEN6(dev_priv))
- ret = ilk_pipe_crc_ctl_reg(&source, &val);
- else
- ret = ivb_pipe_crc_ctl_reg(dev_priv, pipe, &source, &val);
-
+ ret = get_new_crc_ctl_reg(dev_priv, pipe, &source, &val);
if (ret != 0)
goto out;
@@ -687,7 +693,7 @@ static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
POSTING_READ(PIPE_CRC_CTL(pipe));
/* real source -> none transition */
- if (source == INTEL_PIPE_CRC_SOURCE_NONE) {
+ if (!source) {
struct intel_pipe_crc_entry *entries;
struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
pipe);
@@ -809,6 +815,11 @@ display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
{
int i;
+ if (!buf) {
+ *s = INTEL_PIPE_CRC_SOURCE_NONE;
+ return 0;
+ }
+
for (i = 0; i < ARRAY_SIZE(pipe_crc_sources); i++)
if (!strcmp(buf, pipe_crc_sources[i])) {
*s = i;
@@ -937,3 +948,62 @@ void intel_pipe_crc_cleanup(struct drm_minor *minor)
drm_debugfs_remove_files(info_list, 1, minor);
}
}
+
+int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
+ size_t *values_cnt)
+{
+ struct drm_i915_private *dev_priv = crtc->dev->dev_private;
+ struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[crtc->index];
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ enum intel_display_power_domain power_domain;
+ enum intel_pipe_crc_source source;
+ u32 val = 0; /* shut up gcc */
+ int ret = 0;
+
+ if (display_crc_ctl_parse_source(source_name, &source) < 0) {
+ DRM_DEBUG_DRIVER("unknown source %s\n", source_name);
+ return -EINVAL;
+ }
+
+ power_domain = POWER_DOMAIN_PIPE(crtc->index);
+ if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) {
+ DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
+ return -EIO;
+ }
+
+ ret = get_new_crc_ctl_reg(dev_priv, crtc->index, &source, &val);
+ if (ret != 0)
+ goto out;
+
+ if (source) {
+ /*
+ * When IPS gets enabled, the pipe CRC changes. Since IPS gets
+ * enabled and disabled dynamically based on package C states,
+ * user space can't make reliable use of the CRCs, so let's just
+ * completely disable it.
+ */
+ hsw_disable_ips(intel_crtc);
+ }
+
+ I915_WRITE(PIPE_CRC_CTL(crtc->index), val);
+ POSTING_READ(PIPE_CRC_CTL(crtc->index));
+
+ if (!source) {
+ if (IS_G4X(dev_priv))
+ g4x_undo_pipe_scramble_reset(dev_priv, crtc->index);
+ else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ vlv_undo_pipe_scramble_reset(dev_priv, crtc->index);
+ else if (IS_HASWELL(dev_priv) && crtc->index == PIPE_A)
+ hsw_trans_edp_pipe_A_crc_wa(dev_priv, false);
+
+ hsw_enable_ips(intel_crtc);
+ }
+
+ pipe_crc->skipped = 0;
+ *values_cnt = 5;
+
+out:
+ intel_display_power_put(dev_priv, power_domain);
+
+ return ret;
+}
--
2.7.4