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.

Signed-off-by: Tomeu Vizoso <tomeu.viz...@collabora.com>
Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
---

 drivers/gpu/drm/i915/i915_drv.h       |  1 +
 drivers/gpu/drm/i915/i915_irq.c       | 83 +++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_display.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h      |  8 +++
 drivers/gpu/drm/i915/intel_pipe_crc.c | 94 ++++++++++++++++++++++++++++++-----
 5 files changed, 149 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f8c66eea06bc..20522f0a4c57 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1713,6 +1713,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 bd6c8b0eeaef..1549cc4f88ec 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1491,41 +1491,72 @@ 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 drm_crtc *crtc = intel_get_crtc_for_pipe(&dev_priv->drm, 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);
+               spin_lock(&crtc->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, true, frame, crcs);
+               spin_unlock(&crtc->crc.lock);
+               if (!ret)
+                       wake_up_interruptible(&crtc->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 23a6c7213eca..7412a05fa5d9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14636,6 +14636,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 737261b09110..31894b7c6517 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1844,6 +1844,14 @@ void intel_color_load_luts(struct drm_crtc_state 
*crtc_state);
 /* 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
+static inline int intel_crtc_set_crc_source(struct drm_crtc *crtc,
+                                           const char *source_name,
+                                           size_t *values_cnt) { return 0; }
+#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 6c38d4fdcaef..1a51e174e9e5 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -615,6 +615,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)
@@ -640,17 +656,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;
 
@@ -691,7 +697,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 =
                        to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
@@ -813,6 +819,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;
@@ -941,3 +952,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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to