diff mbox

[v8,2/4] drm: Add API for capturing frame CRCs

Message ID 1473415010-1389-3-git-send-email-tomeu.vizoso@collabora.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomeu Vizoso Sept. 9, 2016, 9:56 a.m. UTC
Adds files and directories to debugfs for controlling and reading frame
CRCs, per CRTC:

dri/0/crtc-0/crc
dri/0/crtc-0/crc/control
dri/0/crtc-0/crc/data

Drivers can implement the set_crc_source callback() in drm_crtc_funcs to
start and stop generating frame CRCs and can add entries to the output
by calling drm_crtc_add_crc_entry.

v2:
    - Lots of good fixes suggested by Thierry.
    - Added documentation.
    - Changed the debugfs layout.
    - Moved to allocate the entries circular queue once when frame
      generation gets enabled for the first time.
v3:
    - Use the control file just to select the source, and start and stop
      capture when the data file is opened and closed, respectively.
    - Make variable the number of CRC values per entry, per source.
    - Allocate entries queue each time we start capturing as now there
      isn't a fixed number of CRC values per entry.
    - Store the frame counter in the data file as a 8-digit hex number.
    - For sources that cannot provide useful frame numbers, place
      XXXXXXXX in the frame field.

v4:
    - Build only if CONFIG_DEBUG_FS is enabled.
    - Use memdup_user_nul.
    - Consolidate calculation of the size of an entry in a helper.
    - Add 0x prefix to hex numbers in the data file.
    - Remove unnecessary snprintf and strlen usage in read callback.

v5:
    - Made the crcs array in drm_crtc_crc_entry fixed-size
    - Lots of other smaller improvements suggested by Emil Velikov

v7:
    - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h

v8:
    - Call debugfs_remove_recursive when we fail to create the minor
      device

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

 Documentation/gpu/drm-uapi.rst    |   6 +
 drivers/gpu/drm/Makefile          |   3 +-
 drivers/gpu/drm/drm_crtc.c        |  29 +++-
 drivers/gpu/drm/drm_debugfs.c     |  34 +++-
 drivers/gpu/drm/drm_debugfs_crc.c | 351 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_drv.c         |  19 +++
 drivers/gpu/drm/drm_internal.h    |  16 ++
 include/drm/drm_crtc.h            |  41 +++++
 include/drm/drm_debugfs_crc.h     |  73 ++++++++
 9 files changed, 569 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c
 create mode 100644 include/drm/drm_debugfs_crc.h

Comments

Jon Hunter Oct. 4, 2016, 10:10 a.m. UTC | #1
Hi Tomeu,

On 09/09/16 10:56, Tomeu Vizoso wrote:
> Adds files and directories to debugfs for controlling and reading frame
> CRCs, per CRTC:
> 
> dri/0/crtc-0/crc
> dri/0/crtc-0/crc/control
> dri/0/crtc-0/crc/data
> 
> Drivers can implement the set_crc_source callback() in drm_crtc_funcs to
> start and stop generating frame CRCs and can add entries to the output
> by calling drm_crtc_add_crc_entry.
> 
> v2:
>     - Lots of good fixes suggested by Thierry.
>     - Added documentation.
>     - Changed the debugfs layout.
>     - Moved to allocate the entries circular queue once when frame
>       generation gets enabled for the first time.
> v3:
>     - Use the control file just to select the source, and start and stop
>       capture when the data file is opened and closed, respectively.
>     - Make variable the number of CRC values per entry, per source.
>     - Allocate entries queue each time we start capturing as now there
>       isn't a fixed number of CRC values per entry.
>     - Store the frame counter in the data file as a 8-digit hex number.
>     - For sources that cannot provide useful frame numbers, place
>       XXXXXXXX in the frame field.
> 
> v4:
>     - Build only if CONFIG_DEBUG_FS is enabled.
>     - Use memdup_user_nul.
>     - Consolidate calculation of the size of an entry in a helper.
>     - Add 0x prefix to hex numbers in the data file.
>     - Remove unnecessary snprintf and strlen usage in read callback.
> 
> v5:
>     - Made the crcs array in drm_crtc_crc_entry fixed-size
>     - Lots of other smaller improvements suggested by Emil Velikov
> 
> v7:
>     - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h
> 
> v8:
>     - Call debugfs_remove_recursive when we fail to create the minor
>       device
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> 
>  Documentation/gpu/drm-uapi.rst    |   6 +
>  drivers/gpu/drm/Makefile          |   3 +-
>  drivers/gpu/drm/drm_crtc.c        |  29 +++-
>  drivers/gpu/drm/drm_debugfs.c     |  34 +++-
>  drivers/gpu/drm/drm_debugfs_crc.c | 351 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_drv.c         |  19 +++
>  drivers/gpu/drm/drm_internal.h    |  16 ++
>  include/drm/drm_crtc.h            |  41 +++++
>  include/drm/drm_debugfs_crc.h     |  73 ++++++++
>  9 files changed, 569 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c
>  create mode 100644 include/drm/drm_debugfs_crc.h

...

> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -40,7 +40,7 @@
>  #include <drm/drm_modeset_lock.h>
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_auth.h>
> -#include <drm/drm_framebuffer.h>
> +#include <drm/drm_debugfs_crc.h>
>  
>  #include "drm_crtc_internal.h"
>  #include "drm_internal.h"
> @@ -141,6 +141,25 @@ static void drm_crtc_unregister_all(struct drm_device *dev)
>  	}
>  }
>  
> +static int drm_crtc_crc_init(struct drm_crtc *crtc)
> +{
> +#ifdef CONFIG_DEBUG_FS
> +	spin_lock_init(&crtc->crc.lock);
> +	init_waitqueue_head(&crtc->crc.wq);
> +	crtc->crc.source = kstrdup("auto", GFP_KERNEL);
> +	if (!crtc->crc.source)
> +		return -ENOMEM;
> +#endif
> +	return 0;
> +}
> +
> +static void drm_crtc_crc_fini(struct drm_crtc *crtc)
> +{
> +#ifdef CONFIG_DEBUG_FS
> +	kfree(crtc->crc.source);
> +#endif
> +}
> +
>  /**
>   * drm_crtc_init_with_planes - Initialise a new CRTC object with
>   *    specified primary and cursor planes.
> @@ -206,6 +225,12 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
>  	if (cursor)
>  		cursor->possible_crtcs = 1 << drm_crtc_index(crtc);
>  
> +	ret = drm_crtc_crc_init(crtc);
> +	if (ret) {
> +		drm_mode_object_unregister(dev, &crtc->base);
> +		return ret;
> +	}
> +
>  	if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
>  		drm_object_attach_property(&crtc->base, config->prop_active, 0);
>  		drm_object_attach_property(&crtc->base, config->prop_mode_id, 0);
> @@ -232,6 +257,8 @@ void drm_crtc_cleanup(struct drm_crtc *crtc)
>  	 * the indices on the drm_crtc after us in the crtc_list.
>  	 */
>  
> +	drm_crtc_crc_fini(crtc);
> +
>  	kfree(crtc->gamma_store);
>  	crtc->gamma_store = NULL;
>  
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 1205790ed960..800055c39cdb 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -415,5 +415,37 @@ void drm_debugfs_connector_remove(struct drm_connector *connector)
>  	connector->debugfs_entry = NULL;
>  }
>  
> -#endif /* CONFIG_DEBUG_FS */
> +int drm_debugfs_crtc_add(struct drm_crtc *crtc)
> +{
> +	struct drm_minor *minor = crtc->dev->primary;

After this patch was applied Tegra boards started crashing here when
dereferencing crtc pointer above ...

[    6.789318] Unable to handle kernel paging request at virtual address fffffff8
[    6.796537] pgd = c0004000
[    6.799270] [fffffff8] *pgd=afffd861, *pte=00000000, *ppte=00000000
[    6.805572] Internal error: Oops: 37 [#1] PREEMPT SMP ARM
[    6.810969] Modules linked in:
[    6.814038] CPU: 2 PID: 72 Comm: kworker/2:1 Not tainted 4.8.0-next-20161004-126151-gc7d3b91 #1
[    6.822728] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[    6.829000] Workqueue: events deferred_probe_work_func
[    6.834150] task: ee00f880 task.stack: ee010000
[    6.838682] PC is at drm_debugfs_crtc_add+0x8/0x70
[    6.843481] LR is at drm_minor_register+0xa4/0x1b0
[    6.848267] pc : [<c0440b80>]    lr : [<c0428a60>]    psr: a0000113
[    6.848267] sp : ee011d20  ip : ee2f4914  fp : c0d02100
[    6.859732] r10: 00000001  r9 : 00000000  r8 : 00000000
[    6.864949] r7 : ee2f3800  r6 : ee2f3a4c  r5 : ee2f4900  r4 : fffffff8
[    6.871472] r3 : 00000001  r2 : 00000000  r1 : ee011c70  r0 : fffffff8
[    6.877992] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[    6.885123] Control: 10c5387d  Table: 8000406a  DAC: 00000051
[    6.890871] Process kworker/2:1 (pid: 72, stack limit = 0xee010210)
[    6.897129] Stack: (0xee011d20 to 0xee012000)
[    6.901491] 1d20: fffffff8 ee2f4900 ee2f3a4c c0428a60 ee00f880 ee2f3800 00000000 00000000
[    6.909670] 1d40: c0d34794 0000001e 00000000 c0428be8 ee2f3800 eebae800 c0d3467c c0442008
[    6.917839] 1d60: eebae818 00000000 c0d34794 0000001e eebae810 c0dabfec 00000000 c0407578
[    6.926014] 1d80: c040755c c045ac3c 00000000 ee011db8 c045af10 00000001 c0dabfc8 c045922c
[    6.934190] 1da0: eeaaa370 eebac0b8 eebae810 eebae844 c0d33fa0 c045a9c4 eebae810 00000001
[    6.942366] 1dc0: c0d02100 eebae810 eebae810 c0d33fa0 ee9b6e10 c045a0b4 eebae810 00000000
[    6.950544] 1de0: eebae818 c0458624 c0d6404c 60000113 c0d02100 c0817664 eebae800 ee2f3c10
[    6.958713] 1e00: ee034ec0 eebae9d8 eebae9b0 eefa5580 eebae810 c0407744 eefbf974 eebaeb94
[    6.966887] 1e20: c0d3400c c0d33f68 ee2f3c10 eebaea10 00000000 c0408058 ee2f3c10 ee9b6810
[    6.975064] 1e40: 00000000 ee9b6800 00000000 c044bb4c 00000000 ee9b4940 ee2f3c10 00000000
[    6.983241] 1e60: ffffffed ee9b6810 fffffdfb c0d348f8 0000001e c045c20c c045c1bc ee9b6810
[    6.991417] 1e80: c0dabfec 00000000 c0d348f8 c045ac3c 00000000 ee011ec0 c045af10 00000001
[    6.999595] 1ea0: 00000000 c045922c ee8a3d70 eebacfb8 ee9b6810 ee9b6844 c0d34de8 c045a9c4
[    7.007763] 1ec0: ee9b6810 00000001 c0d02100 ee9b6810 ee9b6810 c0d34de8 eefa8800 c045a0b4
[    7.015938] 1ee0: ee9b6810 c0d34d70 c0d34d90 c045a4e8 eeb78f00 c0d34d98 eefa5580 c0134890
[    7.024115] 1f00: ee171484 eefa5580 eeb78f00 eeb78f18 00000001 eefa5580 eeb78f18 eefa5580
[    7.032292] 1f20: 00000008 c0134ab4 eefa88f5 eeb78f00 eefa5598 c0134cc8 00000000 eeaf6fc0
[    7.040469] 1f40: 00000000 eeaf6fc0 00000000 eeb78f00 c0134ac4 00000000 00000000 00000000
[    7.048637] 1f60: 00000000 c0139d68 fffcffff 00000000 ffefffff eeb78f00 00000000 00000000
[    7.056812] 1f80: ee011f80 ee011f80 00000000 00000000 ee011f90 ee011f90 ee011fac eeaf6fc0
[    7.064988] 1fa0: c0139c90 00000000 00000000 c0107938 00000000 00000000 00000000 00000000
[    7.073165] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    7.081342] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 fffffff3 ffffefbf
[    7.089538] [<c0440b80>] (drm_debugfs_crtc_add) from [<c0428a60>] (drm_minor_register+0xa4/0x1b0)
[    7.098405] [<c0428a60>] (drm_minor_register) from [<c0428be8>] (drm_dev_register+0x7c/0xd0)
[    7.106856] [<c0428be8>] (drm_dev_register) from [<c0442008>] (host1x_drm_probe+0x38/0x90)
[    7.115135] [<c0442008>] (host1x_drm_probe) from [<c0407578>] (host1x_device_probe+0x1c/0x28)
[    7.123672] [<c0407578>] (host1x_device_probe) from [<c045ac3c>] (driver_probe_device+0x1f0/0x2a8)
[    7.132642] [<c045ac3c>] (driver_probe_device) from [<c045922c>] (bus_for_each_drv+0x44/0x8c)
[    7.141178] [<c045922c>] (bus_for_each_drv) from [<c045a9c4>] (__device_attach+0x9c/0x100)
[    7.149454] [<c045a9c4>] (__device_attach) from [<c045a0b4>] (bus_probe_device+0x84/0x8c)
[    7.157624] [<c045a0b4>] (bus_probe_device) from [<c0458624>] (device_add+0x380/0x528)
[    7.165551] [<c0458624>] (device_add) from [<c0407744>] (host1x_subdev_register+0xb0/0xd4)
[    7.173827] [<c0407744>] (host1x_subdev_register) from [<c0408058>] (host1x_client_register+0xf4/0x11c)
[    7.183231] [<c0408058>] (host1x_client_register) from [<c044bb4c>] (tegra_hdmi_probe+0x1c8/0x2c8)
[    7.192201] [<c044bb4c>] (tegra_hdmi_probe) from [<c045c20c>] (platform_drv_probe+0x50/0xb0)
[    7.200653] [<c045c20c>] (platform_drv_probe) from [<c045ac3c>] (driver_probe_device+0x1f0/0x2a8)
[    7.209536] [<c045ac3c>] (driver_probe_device) from [<c045922c>] (bus_for_each_drv+0x44/0x8c)
[    7.218053] [<c045922c>] (bus_for_each_drv) from [<c045a9c4>] (__device_attach+0x9c/0x100)
[    7.226326] [<c045a9c4>] (__device_attach) from [<c045a0b4>] (bus_probe_device+0x84/0x8c)
[    7.234515] [<c045a0b4>] (bus_probe_device) from [<c045a4e8>] (deferred_probe_work_func+0x60/0x8c)
[    7.243486] [<c045a4e8>] (deferred_probe_work_func) from [<c0134890>] (process_one_work+0x120/0x31c)
[    7.252628] [<c0134890>] (process_one_work) from [<c0134ab4>] (process_scheduled_works+0x28/0x38)
[    7.261510] [<c0134ab4>] (process_scheduled_works) from [<c0134cc8>] (worker_thread+0x204/0x4b4)
[    7.270305] [<c0134cc8>] (worker_thread) from [<c0139d68>] (kthread+0xd8/0xf4)
[    7.277524] [<c0139d68>] (kthread) from [<c0107938>] (ret_from_fork+0x14/0x3c)
[    7.284749] Code: e584334c e8bd8010 e92d4070 e1a04000 (e5943000) 
[    7.290952] ---[ end trace 988a54919c1729f9 ]---

Looks like crtc is a errno in the above case. I see this function is
called by looping through all the crtc and we never check to see if
they are valid. Should we?

Cheers
Jon
Daniel Vetter Oct. 4, 2016, 11:25 a.m. UTC | #2
On Tue, Oct 4, 2016 at 12:10 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> Looks like crtc is a errno in the above case. I see this function is
> called by looping through all the crtc and we never check to see if
> they are valid. Should we?

Tegra is still using the load/unload hooks. That didn't mesh well with
Tomeu's patches (and Tomeu's patches have been thrown out meanwhile
because of that). Still would be neat if tegra could be demidlayered
and loose it's load/unload hooks. See the kerneldoc in drm_drv.c
(especially the DOC: section).
-Daniel
Lukas Wunner Oct. 4, 2016, 12:23 p.m. UTC | #3
On Tue, Oct 04, 2016 at 01:25:23PM +0200, Daniel Vetter wrote:
> On Tue, Oct 4, 2016 at 12:10 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> > Looks like crtc is a errno in the above case. I see this function is
> > called by looping through all the crtc and we never check to see if
> > they are valid. Should we?

nouveau is exploding as well on driver load!


> Tegra is still using the load/unload hooks. That didn't mesh well with
> Tomeu's patches (and Tomeu's patches have been thrown out meanwhile
> because of that).

They're still on drm-intel-nightly, please revert if they're broken.

Thanks,

Lukas
Jon Hunter Oct. 4, 2016, 12:38 p.m. UTC | #4
On 04/10/16 12:25, Daniel Vetter wrote:
> On Tue, Oct 4, 2016 at 12:10 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
>> Looks like crtc is a errno in the above case. I see this function is
>> called by looping through all the crtc and we never check to see if
>> they are valid. Should we?
> 
> Tegra is still using the load/unload hooks. That didn't mesh well with
> Tomeu's patches (and Tomeu's patches have been thrown out meanwhile
> because of that). Still would be neat if tegra could be demidlayered
> and loose it's load/unload hooks. See the kerneldoc in drm_drv.c
> (especially the DOC: section).

Adding Thierry and Alex as this is more their domain and CC'ing linux-tegra.

Cheers
Jon
Lukas Wunner Oct. 4, 2016, 12:44 p.m. UTC | #5
On Tue, Oct 04, 2016 at 02:23:13PM +0200, Lukas Wunner wrote:
> On Tue, Oct 04, 2016 at 01:25:23PM +0200, Daniel Vetter wrote:
> > On Tue, Oct 4, 2016 at 12:10 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> > > Looks like crtc is a errno in the above case. I see this function is
> > > called by looping through all the crtc and we never check to see if
> > > they are valid. Should we?
> 
> nouveau is exploding as well on driver load!
> 
> > Tegra is still using the load/unload hooks. That didn't mesh well with
> > Tomeu's patches (and Tomeu's patches have been thrown out meanwhile
> > because of that).
> 
> They're still on drm-intel-nightly, please revert if they're broken.

Never mind, I hadn't rebased on today's drm-intel-nightly yet,
I see it's gone now.

Thanks,

Lukas
diff mbox

Patch

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 1ba301cebe16..de3ac9f90f8f 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -216,3 +216,9 @@  interfaces. Especially since all hardware-acceleration interfaces to
 userspace are driver specific for efficiency and other reasons these
 interfaces can be rather substantial. Hence every driver has its own
 chapter.
+
+Testing and validation
+======================
+
+.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c
+   :doc: CRC ABI
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 439d89b25ae0..60db76bbb02a 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -9,7 +9,7 @@  drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
 		drm_scatter.o drm_pci.o \
 		drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
 		drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
-		drm_info.o drm_debugfs.o drm_encoder_slave.o \
+		drm_info.o drm_encoder_slave.o \
 		drm_trace_points.o drm_global.o drm_prime.o \
 		drm_rect.o drm_vma_manager.o drm_flip_work.o \
 		drm_modeset_lock.o drm_atomic.o drm_bridge.o \
@@ -22,6 +22,7 @@  drm-$(CONFIG_PCI) += ati_pcigart.o
 drm-$(CONFIG_DRM_PANEL) += drm_panel.o
 drm-$(CONFIG_OF) += drm_of.o
 drm-$(CONFIG_AGP) += drm_agpsupport.o
+drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
 
 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
 		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d84a0ead8100..9363dd597d3c 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -40,7 +40,7 @@ 
 #include <drm/drm_modeset_lock.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_auth.h>
-#include <drm/drm_framebuffer.h>
+#include <drm/drm_debugfs_crc.h>
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -141,6 +141,25 @@  static void drm_crtc_unregister_all(struct drm_device *dev)
 	}
 }
 
+static int drm_crtc_crc_init(struct drm_crtc *crtc)
+{
+#ifdef CONFIG_DEBUG_FS
+	spin_lock_init(&crtc->crc.lock);
+	init_waitqueue_head(&crtc->crc.wq);
+	crtc->crc.source = kstrdup("auto", GFP_KERNEL);
+	if (!crtc->crc.source)
+		return -ENOMEM;
+#endif
+	return 0;
+}
+
+static void drm_crtc_crc_fini(struct drm_crtc *crtc)
+{
+#ifdef CONFIG_DEBUG_FS
+	kfree(crtc->crc.source);
+#endif
+}
+
 /**
  * drm_crtc_init_with_planes - Initialise a new CRTC object with
  *    specified primary and cursor planes.
@@ -206,6 +225,12 @@  int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
 	if (cursor)
 		cursor->possible_crtcs = 1 << drm_crtc_index(crtc);
 
+	ret = drm_crtc_crc_init(crtc);
+	if (ret) {
+		drm_mode_object_unregister(dev, &crtc->base);
+		return ret;
+	}
+
 	if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
 		drm_object_attach_property(&crtc->base, config->prop_active, 0);
 		drm_object_attach_property(&crtc->base, config->prop_mode_id, 0);
@@ -232,6 +257,8 @@  void drm_crtc_cleanup(struct drm_crtc *crtc)
 	 * the indices on the drm_crtc after us in the crtc_list.
 	 */
 
+	drm_crtc_crc_fini(crtc);
+
 	kfree(crtc->gamma_store);
 	crtc->gamma_store = NULL;
 
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 1205790ed960..800055c39cdb 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -415,5 +415,37 @@  void drm_debugfs_connector_remove(struct drm_connector *connector)
 	connector->debugfs_entry = NULL;
 }
 
-#endif /* CONFIG_DEBUG_FS */
+int drm_debugfs_crtc_add(struct drm_crtc *crtc)
+{
+	struct drm_minor *minor = crtc->dev->primary;
+	struct dentry *root;
+	char *name;
+
+	name = kasprintf(GFP_KERNEL, "crtc-%d", crtc->index);
+	if (!name)
+		return -ENOMEM;
+
+	root = debugfs_create_dir(name, minor->debugfs_root);
+	kfree(name);
+	if (!root)
+		return -ENOMEM;
+
+	crtc->debugfs_entry = root;
+
+	if (drm_debugfs_crtc_crc_add(crtc))
+		goto error;
 
+	return 0;
+
+error:
+	drm_debugfs_crtc_remove(crtc);
+	return -ENOMEM;
+}
+
+void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
+{
+	debugfs_remove_recursive(crtc->debugfs_entry);
+	crtc->debugfs_entry = NULL;
+}
+
+#endif /* CONFIG_DEBUG_FS */
diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
new file mode 100644
index 000000000000..4129405d17c0
--- /dev/null
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -0,0 +1,351 @@ 
+/*
+ * Copyright © 2008 Intel Corporation
+ * Copyright © 2016 Collabora Ltd
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Based on code from the i915 driver.
+ * Original author: Damien Lespiau <damien.lespiau@intel.com>
+ *
+ */
+
+#include <linux/circ_buf.h>
+#include <linux/ctype.h>
+#include <linux/debugfs.h>
+#include <drm/drmP.h>
+
+/**
+ * DOC: CRC ABI
+ *
+ * DRM device drivers can provide to userspace CRC information of each frame as
+ * it reached a given hardware component (a "source").
+ *
+ * Userspace can control generation of CRCs in a given CRTC by writing to the
+ * file dri/0/crtc-N/crc/control in debugfs, with N being the index of the CRTC.
+ * Accepted values are source names (which are driver-specific) and the "auto"
+ * keyword, which will let the driver select a default source of frame CRCs
+ * for this CRTC.
+ *
+ * Once frame CRC generation is enabled, userspace can capture them by reading
+ * the dri/0/crtc-N/crc/data file. Each line in that file contains the frame
+ * number in the first field and then a number of unsigned integer fields
+ * containing the CRC data. Fields are separated by a single space and the number
+ * of CRC fields is source-specific.
+ *
+ * Note that though in some cases the CRC is computed in a specified way and on
+ * the frame contents as supplied by userspace (eDP 1.3), in general the CRC
+ * computation is performed in an unspecified way and on frame contents that have
+ * been already processed in also an unspecified way and thus userspace cannot
+ * rely on being able to generate matching CRC values for the frame contents that
+ * it submits. In this general case, the maximum userspace can do is to compare
+ * the reported CRCs of frames that should have the same contents.
+ */
+
+static int crc_control_show(struct seq_file *m, void *data)
+{
+	struct drm_crtc *crtc = m->private;
+
+	seq_printf(m, "%s\n", crtc->crc.source);
+
+	return 0;
+}
+
+static int crc_control_open(struct inode *inode, struct file *file)
+{
+	struct drm_crtc *crtc = inode->i_private;
+
+	return single_open(file, crc_control_show, crtc);
+}
+
+static ssize_t crc_control_write(struct file *file, const char __user *ubuf,
+				 size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	struct drm_crtc *crtc = m->private;
+	struct drm_crtc_crc *crc = &crtc->crc;
+	char *source;
+
+	if (len == 0)
+		return 0;
+
+	if (len > PAGE_SIZE - 1) {
+		DRM_DEBUG_KMS("Expected < %lu bytes into crtc crc control\n",
+			      PAGE_SIZE);
+		return -E2BIG;
+	}
+
+	source = memdup_user_nul(ubuf, len);
+	if (IS_ERR(source))
+		return PTR_ERR(source);
+
+	if (source[len] == '\n')
+		source[len] = '\0';
+
+	spin_lock_irq(&crc->lock);
+
+	if (crc->opened) {
+		spin_unlock_irq(&crc->lock);
+		kfree(source);
+		return -EBUSY;
+	}
+
+	kfree(crc->source);
+	crc->source = source;
+
+	spin_unlock_irq(&crc->lock);
+
+	*offp += len;
+	return len;
+}
+
+const struct file_operations drm_crtc_crc_control_fops = {
+	.owner = THIS_MODULE,
+	.open = crc_control_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = crc_control_write
+};
+
+static int crtc_crc_open(struct inode *inode, struct file *filep)
+{
+	struct drm_crtc *crtc = inode->i_private;
+	struct drm_crtc_crc *crc = &crtc->crc;
+	struct drm_crtc_crc_entry *entries = NULL;
+	size_t values_cnt;
+	int ret;
+
+	if (crc->opened)
+		return -EBUSY;
+
+	ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt);
+	if (ret)
+		return ret;
+
+	if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) {
+		ret = -EINVAL;
+		goto err_disable;
+	}
+
+	if (WARN_ON(values_cnt == 0)) {
+		ret = -EINVAL;
+		goto err_disable;
+	}
+
+	entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL);
+	if (!entries) {
+		ret = -ENOMEM;
+		goto err_disable;
+	}
+
+	spin_lock_irq(&crc->lock);
+	crc->entries = entries;
+	crc->values_cnt = values_cnt;
+	crc->opened = true;
+	spin_unlock_irq(&crc->lock);
+
+	return 0;
+
+err_disable:
+	crtc->funcs->set_crc_source(crtc, NULL, &values_cnt);
+	return ret;
+}
+
+static int crtc_crc_release(struct inode *inode, struct file *filep)
+{
+	struct drm_crtc *crtc = filep->f_inode->i_private;
+	struct drm_crtc_crc *crc = &crtc->crc;
+	size_t values_cnt;
+
+	spin_lock_irq(&crc->lock);
+	kfree(crc->entries);
+	crc->entries = NULL;
+	crc->head = 0;
+	crc->tail = 0;
+	crc->values_cnt = 0;
+	crc->opened = false;
+	spin_unlock_irq(&crc->lock);
+
+	crtc->funcs->set_crc_source(crtc, NULL, &values_cnt);
+
+	return 0;
+}
+
+static int crtc_crc_data_count(struct drm_crtc_crc *crc)
+{
+	assert_spin_locked(&crc->lock);
+	return CIRC_CNT(crc->head, crc->tail, DRM_CRC_ENTRIES_NR);
+}
+
+/*
+ * 1 frame field of 10 chars plus a number of CRC fields of 10 chars each, space
+ * separated, with a newline at the end and null-terminated.
+ */
+#define LINE_LEN(values_cnt)	(10 + 11 * values_cnt + 1 + 1)
+#define MAX_LINE_LEN		(LINE_LEN(DRM_MAX_CRC_NR))
+
+static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
+			     size_t count, loff_t *pos)
+{
+	struct drm_crtc *crtc = filep->f_inode->i_private;
+	struct drm_crtc_crc *crc = &crtc->crc;
+	struct drm_crtc_crc_entry *entry;
+	char buf[MAX_LINE_LEN];
+	int ret, i;
+
+	spin_lock_irq(&crc->lock);
+
+	if (!crc->source) {
+		spin_unlock_irq(&crc->lock);
+		return 0;
+	}
+
+	/* Nothing to read? */
+	while (crtc_crc_data_count(crc) == 0) {
+		if (filep->f_flags & O_NONBLOCK) {
+			spin_unlock_irq(&crc->lock);
+			return -EAGAIN;
+		}
+
+		ret = wait_event_interruptible_lock_irq(crc->wq,
+							crtc_crc_data_count(crc),
+							crc->lock);
+		if (ret) {
+			spin_unlock_irq(&crc->lock);
+			return ret;
+		}
+	}
+
+	/* We know we have an entry to be read */
+	entry = &crc->entries[crc->tail];
+
+	if (count < LINE_LEN(crc->values_cnt)) {
+		spin_unlock_irq(&crc->lock);
+		return -EINVAL;
+	}
+
+	BUILD_BUG_ON_NOT_POWER_OF_2(DRM_CRC_ENTRIES_NR);
+	crc->tail = (crc->tail + 1) & (DRM_CRC_ENTRIES_NR - 1);
+
+	spin_unlock_irq(&crc->lock);
+
+	if (entry->has_frame_counter)
+		sprintf(buf, "0x%08x", entry->frame);
+	else
+		sprintf(buf, "XXXXXXXXXX");
+
+	for (i = 0; i < crc->values_cnt; i++)
+		sprintf(buf + 10 + i * 11, " 0x%08x", entry->crcs[i]);
+	sprintf(buf + 10 + crc->values_cnt * 11, "\n");
+
+	if (copy_to_user(user_buf, buf, LINE_LEN(crc->values_cnt)))
+		return -EFAULT;
+
+	return LINE_LEN(crc->values_cnt);
+}
+
+const struct file_operations drm_crtc_crc_data_fops = {
+	.owner = THIS_MODULE,
+	.open = crtc_crc_open,
+	.read = crtc_crc_read,
+	.release = crtc_crc_release,
+};
+
+/**
+ * drm_debugfs_crtc_crc_add - Add files to debugfs for capture of frame CRCs
+ * @crtc: CRTC to whom the frames will belong
+ *
+ * Adds files to debugfs directory that allows userspace to control the
+ * generation of frame CRCs and to read them.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc)
+{
+	struct dentry *crc_ent, *ent;
+
+	if (!crtc->funcs->set_crc_source)
+		return 0;
+
+	crc_ent = debugfs_create_dir("crc", crtc->debugfs_entry);
+	if (!crc_ent)
+		return -ENOMEM;
+
+	ent = debugfs_create_file("control", S_IRUGO, crc_ent, crtc,
+				  &drm_crtc_crc_control_fops);
+	if (!ent)
+		goto error;
+
+	ent = debugfs_create_file("data", S_IRUGO, crc_ent, crtc,
+				  &drm_crtc_crc_data_fops);
+	if (!ent)
+		goto error;
+
+	return 0;
+
+error:
+	debugfs_remove_recursive(crc_ent);
+
+	return -ENOMEM;
+}
+
+/**
+ * drm_crtc_add_crc_entry - Add entry with CRC information for a frame
+ * @crtc: CRTC to which the frame belongs
+ * @has_frame: whether this entry has a frame number to go with
+ * @frame: number of the frame these CRCs are about
+ * @crcs: array of CRC values, with length matching #drm_crtc_crc.values_cnt
+ *
+ * For each frame, the driver polls the source of CRCs for new data and calls
+ * this function to add them to the buffer from where userspace reads.
+ */
+int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame,
+			   uint32_t frame, uint32_t *crcs)
+{
+	struct drm_crtc_crc *crc = &crtc->crc;
+	struct drm_crtc_crc_entry *entry;
+	int head, tail;
+
+	assert_spin_locked(&crc->lock);
+
+	/* Caller may not have noticed yet that userspace has stopped reading */
+	if (!crc->opened)
+		return -EINVAL;
+
+	head = crc->head;
+	tail = crc->tail;
+
+	if (CIRC_SPACE(head, tail, DRM_CRC_ENTRIES_NR) < 1) {
+		DRM_ERROR("Overflow of CRC buffer, userspace reads too slow.\n");
+		return -ENOBUFS;
+	}
+
+	entry = &crc->entries[head];
+	entry->frame = frame;
+	entry->has_frame_counter = has_frame;
+	memcpy(&entry->crcs, crcs, sizeof(*crcs) * crc->values_cnt);
+
+	head = (head + 1) & (DRM_CRC_ENTRIES_NR - 1);
+	crc->head = head;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_crtc_add_crc_entry);
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index acf6a5f38920..fa33741746c0 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -206,6 +206,7 @@  static void drm_minor_free(struct drm_device *dev, unsigned int type)
 static int drm_minor_register(struct drm_device *dev, unsigned int type)
 {
 	struct drm_minor *minor;
+	struct drm_crtc *crtc;
 	unsigned long flags;
 	int ret;
 
@@ -221,6 +222,14 @@  static int drm_minor_register(struct drm_device *dev, unsigned int type)
 		return ret;
 	}
 
+	if (type == DRM_MINOR_PRIMARY) {
+		drm_for_each_crtc(crtc, dev) {
+			ret = drm_debugfs_crtc_add(crtc);
+			if (ret)
+				DRM_ERROR("DRM: Failed to initialize CRC debugfs.\n");
+		}
+	}
+
 	ret = device_add(minor->kdev);
 	if (ret)
 		goto err_debugfs;
@@ -234,6 +243,10 @@  static int drm_minor_register(struct drm_device *dev, unsigned int type)
 	return 0;
 
 err_debugfs:
+	if (type == DRM_MINOR_PRIMARY) {
+		drm_for_each_crtc(crtc, dev)
+			drm_debugfs_crtc_remove(crtc);
+	}
 	drm_debugfs_cleanup(minor);
 	return ret;
 }
@@ -241,12 +254,18 @@  err_debugfs:
 static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
 {
 	struct drm_minor *minor;
+	struct drm_crtc *crtc;
 	unsigned long flags;
 
 	minor = *drm_minor_get_slot(dev, type);
 	if (!minor || !device_is_registered(minor->kdev))
 		return;
 
+	if (type == DRM_MINOR_PRIMARY) {
+		drm_for_each_crtc(crtc, dev)
+			drm_debugfs_crtc_remove(crtc);
+	}
+
 	/* replace @minor with NULL so lookups will fail from now on */
 	spin_lock_irqsave(&drm_minor_lock, flags);
 	idr_replace(&drm_minors_idr, NULL, minor->index);
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index b86dc9b921a5..8a1cd8e314c4 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -97,6 +97,9 @@  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 int drm_debugfs_cleanup(struct drm_minor *minor);
 int drm_debugfs_connector_add(struct drm_connector *connector);
 void drm_debugfs_connector_remove(struct drm_connector *connector);
+int drm_debugfs_crtc_add(struct drm_crtc *crtc);
+void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
+int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
 #else
 static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 				   struct dentry *root)
@@ -116,4 +119,17 @@  static inline int drm_debugfs_connector_add(struct drm_connector *connector)
 static inline void drm_debugfs_connector_remove(struct drm_connector *connector)
 {
 }
+
+static inline int drm_debugfs_crtc_add(struct drm_crtc *crtc)
+{
+	return 0;
+}
+static inline void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
+{
+}
+
+static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc)
+{
+	return 0;
+}
 #endif
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8ca71d66282b..151a90a5ca37 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -42,6 +42,7 @@ 
 #include <drm/drm_connector.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_property.h>
+#include <drm/drm_debugfs_crc.h>
 
 struct drm_device;
 struct drm_mode_set;
@@ -536,6 +537,30 @@  struct drm_crtc_funcs {
 	 * before data structures are torndown.
 	 */
 	void (*early_unregister)(struct drm_crtc *crtc);
+
+	/**
+	 * @set_crc_source:
+	 *
+	 * Changes the source of CRC checksums of frames at the request of
+	 * userspace, typically for testing purposes. The sources available are
+	 * specific of each driver and a %NULL value indicates that CRC
+	 * generation is to be switched off.
+	 *
+	 * When CRC generation is enabled, the driver should call
+	 * drm_crtc_add_crc_entry() at each frame, providing any information
+	 * that characterizes the frame contents in the crcN arguments, as
+	 * provided from the configured source. Drivers must accept a "auto"
+	 * source name that will select a default source for this CRTC.
+	 *
+	 * This callback is optional if the driver does not support any CRC
+	 * generation functionality.
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or a negative error code on failure.
+	 */
+	int (*set_crc_source)(struct drm_crtc *crtc, const char *source,
+			      size_t *values_cnt);
 };
 
 /**
@@ -652,6 +677,22 @@  struct drm_crtc {
 	 * context.
 	 */
 	struct drm_modeset_acquire_ctx *acquire_ctx;
+
+#ifdef CONFIG_DEBUG_FS
+	/**
+	 * @debugfs_entry:
+	 *
+	 * Debugfs directory for this CRTC.
+	 */
+	struct dentry *debugfs_entry;
+
+	/**
+	 * @crc:
+	 *
+	 * Configuration settings of CRC capture.
+	 */
+	struct drm_crtc_crc crc;
+#endif
 };
 
 /**
diff --git a/include/drm/drm_debugfs_crc.h b/include/drm/drm_debugfs_crc.h
new file mode 100644
index 000000000000..7d63b1d4adb9
--- /dev/null
+++ b/include/drm/drm_debugfs_crc.h
@@ -0,0 +1,73 @@ 
+/*
+ * Copyright © 2016 Collabora Ltd.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+#ifndef __DRM_DEBUGFS_CRC_H__
+#define __DRM_DEBUGFS_CRC_H__
+
+#define DRM_MAX_CRC_NR		10
+
+/**
+ * struct drm_crtc_crc_entry - entry describing a frame's content
+ * @has_frame_counter: whether the source was able to provide a frame number
+ * @frame: number of the frame this CRC is about, if @has_frame_counter is true
+ * @crc: array of values that characterize the frame
+ */
+struct drm_crtc_crc_entry {
+	bool has_frame_counter;
+	uint32_t frame;
+	uint32_t crcs[DRM_MAX_CRC_NR];
+};
+
+#define DRM_CRC_ENTRIES_NR	128
+
+/**
+ * struct drm_crtc_crc - data supporting CRC capture on a given CRTC
+ * @lock: protects the fields in this struct
+ * @source: name of the currently configured source of CRCs
+ * @opened: whether userspace has opened the data file for reading
+ * @entries: array of entries, with size of %DRM_CRC_ENTRIES_NR
+ * @head: head of circular queue
+ * @tail: tail of circular queue
+ * @values_cnt: number of CRC values per entry, up to %DRM_MAX_CRC_NR
+ * @wq: workqueue used to synchronize reading and writing
+ */
+struct drm_crtc_crc {
+	spinlock_t lock;
+	const char *source;
+	bool opened;
+	struct drm_crtc_crc_entry *entries;
+	int head, tail;
+	size_t values_cnt;
+	wait_queue_head_t wq;
+};
+
+#if defined(CONFIG_DEBUG_FS)
+int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame,
+			   uint32_t frame, uint32_t *crcs);
+#else
+static inline int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame,
+					 uint32_t frame, uint32_t *crcs)
+{
+	return -EINVAL;
+}
+#endif /* defined(CONFIG_DEBUG_FS) */
+
+#endif /* __DRM_DEBUGFS_CRC_H__ */