[PATCH v8 2/4] drm: Add API for capturing frame CRCs
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
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
Reviewed-by: Emil Velikov
---
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
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
#include
#include
-#include
+#include
#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,
Re: [PATCH v8 2/4] drm: Add API for capturing frame CRCs
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
> 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
> Reviewed-by: Emil Velikov
> ---
>
> 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
> #include
> #include
> -#include
> +#include
>
> #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
fff8
[6.796537] pgd = c0004000
[6.799270] [fff8] *pgd=afffd861, *pte=, *ppte=00
Re: [PATCH v8 2/4] drm: Add API for capturing frame CRCs
On Tue, Oct 4, 2016 at 12:10 PM, Jon Hunter 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/4] drm: Add API for capturing frame CRCs
On Tue, Oct 04, 2016 at 01:25:23PM +0200, Daniel Vetter wrote: > On Tue, Oct 4, 2016 at 12:10 PM, Jon Hunter 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 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/4] drm: Add API for capturing frame CRCs
On 04/10/16 12:25, Daniel Vetter wrote: > On Tue, Oct 4, 2016 at 12:10 PM, Jon Hunter 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 -- nvpublic -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/4] drm: Add API for capturing frame CRCs
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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
