Re: [PATCH v10 20/38] x86, mpparse: Use memremap to map the mpf and mpc data
On 3 November 2017 at 16:31, Tom Lendacky wrote: > On 11/3/2017 10:12 AM, Tomeu Vizoso wrote: >> >> On 17 July 2017 at 23:10, Tom Lendacky wrote: >>> >>> The SMP MP-table is built by UEFI and placed in memory in a decrypted >>> state. These tables are accessed using a mix of early_memremap(), >>> early_memunmap(), phys_to_virt() and virt_to_phys(). Change all accesses >>> to use early_memremap()/early_memunmap(). This allows for proper setting >>> of the encryption mask so that the data can be successfully accessed when >>> SME is active. >>> >>> Reviewed-by: Borislav Petkov >>> Signed-off-by: Tom Lendacky >>> --- >>> arch/x86/kernel/mpparse.c | 98 >>> +-- >>> 1 file changed, 70 insertions(+), 28 deletions(-) >> >> >> Hi there, >> >> today I played a bit with crosvm [0] and noticed that 4.14-rc7 doesn't >> boot. git-bisect pointed to this patch, and reverting it indeed gets >> things working again. >> >> Anybody has an idea of why this could be? > > > If you send me your kernel config I'll see if I can reproduce the issue > and debug it. x86_64_defconfig should be enough. I have pasted my dev env instructions here in case they help: http://blog.tomeuvizoso.net/2017/11/experiments-with-crosvm_6.html Thanks, Tomeu -- 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 v10 20/38] x86, mpparse: Use memremap to map the mpf and mpc data
On 17 July 2017 at 23:10, Tom Lendacky wrote: > The SMP MP-table is built by UEFI and placed in memory in a decrypted > state. These tables are accessed using a mix of early_memremap(), > early_memunmap(), phys_to_virt() and virt_to_phys(). Change all accesses > to use early_memremap()/early_memunmap(). This allows for proper setting > of the encryption mask so that the data can be successfully accessed when > SME is active. > > Reviewed-by: Borislav Petkov > Signed-off-by: Tom Lendacky > --- > arch/x86/kernel/mpparse.c | 98 > +-- > 1 file changed, 70 insertions(+), 28 deletions(-) Hi there, today I played a bit with crosvm [0] and noticed that 4.14-rc7 doesn't boot. git-bisect pointed to this patch, and reverting it indeed gets things working again. Anybody has an idea of why this could be? Thanks, Tomeu [0] https://chromium.googlesource.com/chromiumos/platform/crosvm > > diff --git a/arch/x86/kernel/mpparse.c b/arch/x86/kernel/mpparse.c > index fd37f39..5cbb317 100644 > --- a/arch/x86/kernel/mpparse.c > +++ b/arch/x86/kernel/mpparse.c > @@ -429,7 +429,7 @@ static inline void __init > construct_default_ISA_mptable(int mpc_default_type) > } > } > > -static struct mpf_intel *mpf_found; > +static unsigned long mpf_base; > > static unsigned long __init get_mpc_size(unsigned long physptr) > { > @@ -451,6 +451,7 @@ static int __init check_physptr(struct mpf_intel *mpf, > unsigned int early) > > size = get_mpc_size(mpf->physptr); > mpc = early_memremap(mpf->physptr, size); > + > /* > * Read the physical hardware table. Anything here will > * override the defaults. > @@ -497,12 +498,12 @@ static int __init check_physptr(struct mpf_intel *mpf, > unsigned int early) > */ > void __init default_get_smp_config(unsigned int early) > { > - struct mpf_intel *mpf = mpf_found; > + struct mpf_intel *mpf; > > if (!smp_found_config) > return; > > - if (!mpf) > + if (!mpf_base) > return; > > if (acpi_lapic && early) > @@ -515,6 +516,12 @@ void __init default_get_smp_config(unsigned int early) > if (acpi_lapic && acpi_ioapic) > return; > > + mpf = early_memremap(mpf_base, sizeof(*mpf)); > + if (!mpf) { > + pr_err("MPTABLE: error mapping MP table\n"); > + return; > + } > + > pr_info("Intel MultiProcessor Specification v1.%d\n", > mpf->specification); > #if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_X86_32) > @@ -529,7 +536,7 @@ void __init default_get_smp_config(unsigned int early) > /* > * Now see if we need to read further. > */ > - if (mpf->feature1 != 0) { > + if (mpf->feature1) { > if (early) { > /* > * local APIC has default address > @@ -542,8 +549,10 @@ void __init default_get_smp_config(unsigned int early) > construct_default_ISA_mptable(mpf->feature1); > > } else if (mpf->physptr) { > - if (check_physptr(mpf, early)) > + if (check_physptr(mpf, early)) { > + early_memunmap(mpf, sizeof(*mpf)); > return; > + } > } else > BUG(); > > @@ -552,6 +561,8 @@ void __init default_get_smp_config(unsigned int early) > /* > * Only use the first configuration found. > */ > + > + early_memunmap(mpf, sizeof(*mpf)); > } > > static void __init smp_reserve_memory(struct mpf_intel *mpf) > @@ -561,15 +572,16 @@ static void __init smp_reserve_memory(struct mpf_intel > *mpf) > > static int __init smp_scan_config(unsigned long base, unsigned long length) > { > - unsigned int *bp = phys_to_virt(base); > + unsigned int *bp; > struct mpf_intel *mpf; > - unsigned long mem; > + int ret = 0; > > apic_printk(APIC_VERBOSE, "Scan for SMP in [mem %#010lx-%#010lx]\n", > base, base + length - 1); > BUILD_BUG_ON(sizeof(*mpf) != 16); > > while (length > 0) { > + bp = early_memremap(base, length); > mpf = (struct mpf_intel *)bp; > if ((*bp == SMP_MAGIC_IDENT) && > (mpf->length == 1) && > @@ -579,24 +591,26 @@ static int __init smp_scan_config(unsigned long base, > unsigned long length) > #ifdef CONFIG_X86_LOCAL_APIC > smp_found_config = 1; > #endif > - mpf_found = mpf; > + mpf_base = base; > > - pr_info("found SMP MP-table at [mem > %#010llx-%#010llx] mapped at [%p]\n", > - (unsigned long long) virt_to_phys(mpf), > - (unsigned long long) virt_to_phys(mpf) + > - sizeof(*mpf) - 1, mpf)
Re: [PATCH v11 2/4] drm: Add API for capturing frame CRCs
Adding Benjamin Gaignard to CC in case he wants to comment on the usage of the registration functions, as suggested by Daniel Vetter. Regards, Tomeu On 6 October 2016 at 17:21, 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 > > v9: > - Register the debugfs directory for a crtc from > drm_crtc_register_all() > > v10: > - Don't let debugfs failures interrupt CRTC registration (Emil > Velikov) > > v11: > - Remove extra brace that broke compilation. Sorry! > > Signed-off-by: Tomeu Vizoso > --- > > Documentation/gpu/drm-uapi.rst| 6 + > drivers/gpu/drm/Makefile | 3 +- > drivers/gpu/drm/drm_crtc.c| 34 +++- > drivers/gpu/drm/drm_debugfs.c | 34 +++- > drivers/gpu/drm/drm_debugfs_crc.c | 351 > ++ > drivers/gpu/drm/drm_internal.h| 16 ++ > include/drm/drm_crtc.h| 41 + > include/drm/drm_debugfs_crc.h | 73 > 8 files changed, 555 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 25c720454017..74579d2e796e 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 \ > @@ -23,6 +23,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 2d7bedf28647..60403bf7a4ff 100644 > --- a/drivers/gpu
[PATCH v11 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 v9: - Register the debugfs directory for a crtc from drm_crtc_register_all() v10: - Don't let debugfs failures interrupt CRTC registration (Emil Velikov) v11: - Remove extra brace that broke compilation. Sorry! Signed-off-by: Tomeu Vizoso --- Documentation/gpu/drm-uapi.rst| 6 + drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_crtc.c| 34 +++- drivers/gpu/drm/drm_debugfs.c | 34 +++- drivers/gpu/drm/drm_debugfs_crc.c | 351 ++ drivers/gpu/drm/drm_internal.h| 16 ++ include/drm/drm_crtc.h| 41 + include/drm/drm_debugfs_crc.h | 73 8 files changed, 555 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 25c720454017..74579d2e796e 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 \ @@ -23,6 +23,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 2d7bedf28647..60403bf7a4ff 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" @@ -122,6 +122,10 @@ static int drm_crtc_register_all(struct drm_device *dev) int ret = 0; drm_for_each_crtc(crtc, dev) { + if (drm_debugfs_crtc_add(crtc)) + DRM_ERROR("Failed to initialize debugfs entry for CRTC '%s'.\n", + crtc->name); + if (crtc->funcs->late_register) ret = crtc->funcs->late_register(crtc); if (ret) @@ -138,9 +142,29 @@ static void drm_crtc_unregiste
[PATCH v11 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. Sorry about that, but there was a dangling brace in v10 breaking the build so here is this v11. Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines Documentation/gpu/drm-uapi.rst|6 + drivers/gpu/drm/Makefile |3 +- drivers/gpu/drm/drm_crtc.c| 34 +- drivers/gpu/drm/drm_debugfs.c | 34 +- drivers/gpu/drm/drm_debugfs_crc.c | 351 drivers/gpu/drm/drm_internal.h| 16 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 886 +--- 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 | 13 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1013 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 73 +++ 15 files changed, 1645 insertions(+), 912 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4 -- 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
[PATCH v10 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 v9: - Register the debugfs directory for a crtc from drm_crtc_register_all() v10: - Don't let debugfs failures interrupt CRTC registration (Emil Velikov) Signed-off-by: Tomeu Vizoso --- Documentation/gpu/drm-uapi.rst| 6 + drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_crtc.c| 34 +++- drivers/gpu/drm/drm_debugfs.c | 34 +++- drivers/gpu/drm/drm_debugfs_crc.c | 351 ++ drivers/gpu/drm/drm_internal.h| 16 ++ include/drm/drm_crtc.h| 41 + include/drm/drm_debugfs_crc.h | 73 8 files changed, 555 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 25c720454017..74579d2e796e 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 \ @@ -23,6 +23,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 2d7bedf28647..b9891a724437 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" @@ -122,6 +122,10 @@ static int drm_crtc_register_all(struct drm_device *dev) int ret = 0; drm_for_each_crtc(crtc, dev) { + if (drm_debugfs_crtc_add(crtc)) { + DRM_ERROR("Failed to initialize debugfs entry for CRTC '%s'.\n", + crtc->name); + if (crtc->funcs->late_register) ret = crtc->funcs->late_register(crtc); if (ret) @@ -138,9 +142,29 @@ static void drm_crtc_unregister_all(struct drm_device *dev) drm_for_each_cr
[PATCH v10 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. In this v10 debugfs creation failures don't abort CRTC registration, as suggested by Emil Velikov. Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines Documentation/gpu/drm-uapi.rst|6 + drivers/gpu/drm/Makefile |3 +- drivers/gpu/drm/drm_crtc.c| 34 +- drivers/gpu/drm/drm_debugfs.c | 34 +- drivers/gpu/drm/drm_debugfs_crc.c | 351 drivers/gpu/drm/drm_internal.h| 16 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 886 +--- 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 | 13 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1013 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 73 +++ 15 files changed, 1645 insertions(+), 912 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4 -- 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 v9 2/4] drm: Add API for capturing frame CRCs
On 10/06/2016 01:13 PM, Emil Velikov wrote: > On 6 October 2016 at 09: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 >> >> v9: >> - Register the debugfs directory for a crtc from >> drm_crtc_register_all() >> >> Signed-off-by: Tomeu Vizoso >> --- >> >> Documentation/gpu/drm-uapi.rst| 6 + >> drivers/gpu/drm/Makefile | 3 +- >> drivers/gpu/drm/drm_crtc.c| 34 +++- >> drivers/gpu/drm/drm_debugfs.c | 34 +++- >> drivers/gpu/drm/drm_debugfs_crc.c | 351 >> ++ >> drivers/gpu/drm/drm_internal.h| 16 ++ >> include/drm/drm_crtc.h| 41 + >> include/drm/drm_debugfs_crc.h | 73 >> 8 files changed, 555 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 25c720454017..74579d2e796e 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 \ >> @@ -23,6 +23,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
[PATCH v9 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. In this v9, there are build fixes for !CONFIG_DEBUG_FS and a fix so we don't break probing of drivers that still use the .load callback (tested on Tegra124). Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines Documentation/gpu/drm-uapi.rst|6 + drivers/gpu/drm/Makefile |3 +- drivers/gpu/drm/drm_crtc.c| 34 +- drivers/gpu/drm/drm_debugfs.c | 34 +- drivers/gpu/drm/drm_debugfs_crc.c | 351 drivers/gpu/drm/drm_internal.h| 16 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 886 +--- 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 | 13 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1013 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 73 +++ 15 files changed, 1645 insertions(+), 912 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4 -- 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
[PATCH v9 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 v9: - Register the debugfs directory for a crtc from drm_crtc_register_all() Signed-off-by: Tomeu Vizoso --- Documentation/gpu/drm-uapi.rst| 6 + drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_crtc.c| 34 +++- drivers/gpu/drm/drm_debugfs.c | 34 +++- drivers/gpu/drm/drm_debugfs_crc.c | 351 ++ drivers/gpu/drm/drm_internal.h| 16 ++ include/drm/drm_crtc.h| 41 + include/drm/drm_debugfs_crc.h | 73 8 files changed, 555 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 25c720454017..74579d2e796e 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 \ @@ -23,6 +23,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 2d7bedf28647..151ff9805de1 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" @@ -126,6 +126,10 @@ static int drm_crtc_register_all(struct drm_device *dev) ret = crtc->funcs->late_register(crtc); if (ret) return ret; + + ret = drm_debugfs_crtc_add(crtc); + if (ret) + return ret; } return 0; @@ -136,11 +140,31 @@ static void drm_crtc_unregister_all(struct drm_device *dev) struct drm_crtc *crtc; drm_for_each_crtc(crtc, dev) { + drm_debugfs_crtc_remove(crtc); if (crtc->funcs->early_unregister) crtc->funcs->early_unregister(crtc); } } +static
[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_
[PATCH v8 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines 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 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 886 +--- drivers/gpu/drm/i915/i915_drv.c |2 +- drivers/gpu/drm/i915/i915_drv.h |3 +- drivers/gpu/drm/i915/i915_irq.c | 83 ++- drivers/gpu/drm/i915/intel_display.c |1 + drivers/gpu/drm/i915/intel_drv.h |7 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1014 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 73 +++ 17 files changed, 1656 insertions(+), 914 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4 -- 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
[PATCH v7 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 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 | 15 ++ drivers/gpu/drm/drm_internal.h| 16 ++ include/drm/drm_crtc.h| 41 + include/drm/drm_debugfs_crc.h | 73 9 files changed, 565 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 12b47c30fe2e..4d5f61b6c0f2 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -179,3 +179,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 4054c94a2301..9c99b051ce87 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 \ @@ -21,6 +21,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 7f2510524f09..17cc6891dfbb 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" @@ -309,6 +309,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. @@ -374,6 +393,12 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, if (cursor)
Re: [PATCH v6 2/4] drm: Add API for capturing frame CRCs
On 8 September 2016 at 15:24, Emil Velikov wrote: > Hi Tomeu, > > A couple of small nitpicks and a rather nasty looking bug, related to > your earlier question. > > On 7 September 2016 at 11:27, Tomeu Vizoso wrote: > >> +static ssize_t crc_control_write(struct file *file, const char __user *ubuf, >> +size_t len, loff_t *offp) >> +{ > >> + if (source[len] == '\n') >> + source[len] = '\0'; >> + > Considering the bug below, I'm considering if there's a case were we > don't want to explicitly set the terminating byte ? > > >> +/* >> + * 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. >> + */ > NULL-terminated what the things that was missing, explaining the > maths. Yet note the code sort of contradicts it. > > TL;DR: above we conditionally NULL terminate the data, yet (below) we > feed garbage (always) instead of \0 byte. > >> +#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]; > Here buf is filled with garbage... > > >> + if (entry->has_frame_counter) >> + sprintf(buf, "0x%08x", entry->frame); >> + else >> + sprintf(buf, "XX"); >> + >> + 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"); >> + > ... and now all the data is in, incl. the \n byte. > >> + if (copy_to_user(user_buf, buf, LINE_LEN(crc->values_cnt))) > And here we copy the whole thing incl. the 'should be \0 but is > actually garbage' byte. As discussed offline, sprintf does terminate the string for us, so I think this is fine. > This doesn't look good :-\ > >> --- a/drivers/gpu/drm/drm_drv.c >> +++ b/drivers/gpu/drm/drm_drv.c > >> @@ -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"); >> + } >> + } >> + > Minor: We're missing teardown in the error path. Isn't drm_debugfs_cleanup taking care of it? > >> --- /dev/null >> +++ b/include/drm/drm_debugfs_crc.h > >> +static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) > Minor: The function is internal only and used only when > defined(CONFIG_DEBUG_FS). Thus it should stay in > drivers/gpu/drm/foo.h. > > Rule of thumb: include/drm defines the API used by the drivers, while > drivers/gpu/drm/foo_internal.h the internal API between the core DRM > modules. Good one. Thanks! Tomeu -- 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
[PATCH v7 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines 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 | 15 + drivers/gpu/drm/drm_internal.h| 16 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 886 +--- drivers/gpu/drm/i915/i915_drv.c |2 +- drivers/gpu/drm/i915/i915_drv.h |3 +- drivers/gpu/drm/i915/i915_irq.c | 83 ++- drivers/gpu/drm/i915/intel_display.c |1 + drivers/gpu/drm/i915/intel_drv.h |7 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1014 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 73 +++ 17 files changed, 1652 insertions(+), 914 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4 -- 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
[PATCH v6 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 Signed-off-by: Tomeu Vizoso --- 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 | 15 ++ drivers/gpu/drm/drm_internal.h| 10 ++ include/drm/drm_crtc.h| 41 + include/drm/drm_debugfs_crc.h | 78 + 9 files changed, 564 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 12b47c30fe2e..4d5f61b6c0f2 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -179,3 +179,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 4054c94a2301..9c99b051ce87 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 \ @@ -21,6 +21,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 7f2510524f09..17cc6891dfbb 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" @@ -309,6 +309,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. @@ -374,6 +393,12 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, if (cursor) cursor->possible_crtcs = 1 << drm_crtc_in
[PATCH v6 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines 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 | 15 + drivers/gpu/drm/drm_internal.h| 10 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 886 +--- drivers/gpu/drm/i915/i915_drv.c |2 +- drivers/gpu/drm/i915/i915_drv.h |3 +- drivers/gpu/drm/i915/i915_irq.c | 83 ++- drivers/gpu/drm/i915/intel_display.c |1 + drivers/gpu/drm/i915/intel_drv.h |7 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1014 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 78 +++ 17 files changed, 1651 insertions(+), 914 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4 -- 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
[PATCH v5 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines 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 | 15 + drivers/gpu/drm/drm_internal.h| 10 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 886 + drivers/gpu/drm/i915/i915_drv.c |2 +- drivers/gpu/drm/i915/i915_drv.h |3 +- drivers/gpu/drm/i915/i915_irq.c | 83 ++- drivers/gpu/drm/i915/intel_display.c |1 + drivers/gpu/drm/i915/intel_drv.h |7 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1005 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 78 +++ 17 files changed, 1642 insertions(+), 914 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4 -- 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
[PATCH v5 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 Signed-off-by: Tomeu Vizoso --- 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 | 15 ++ drivers/gpu/drm/drm_internal.h| 10 ++ include/drm/drm_crtc.h| 41 + include/drm/drm_debugfs_crc.h | 78 + 9 files changed, 564 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 12b47c30fe2e..4d5f61b6c0f2 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -179,3 +179,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 4054c94a2301..9c99b051ce87 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 \ @@ -21,6 +21,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 7f2510524f09..17cc6891dfbb 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" @@ -309,6 +309,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. @@ -374,6 +393,12 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, if (cursor) cursor->possible_crtcs = 1 << drm_crtc_in
Re: [PATCH v4 2/4] drm: Add API for capturing frame CRCs
On 2 September 2016 at 16:50, Emil Velikov wrote: > Hi Tomeu, > > On 5 August 2016 at 11:45, Tomeu Vizoso wrote: > >> +#ifdef CONFIG_DEBUG_FS >> + spin_lock_init(&crtc->crc.lock); >> + init_waitqueue_head(&crtc->crc.wq); >> + crtc->crc.source = kstrdup("auto", GFP_KERNEL); > Pedantic: kstrdup() can never fail ? Dealt with. >> +#endif >> + >> 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); >> @@ -764,6 +771,11 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) >> * the indices on the drm_crtc after us in the crtc_list. >> */ >> >> +#ifdef CONFIG_DEBUG_FS >> + drm_debugfs_crtc_remove(crtc); >> + kfree(crtc->crc.source); >> +#endif >> + > Worth adding these two as static inlines in the header ? Have moved the ifdefs to static functions in the same file. > Things are a bit asymetrical here. drm_debugfs_crtc_add() is in > drm_minor_register(), so why isn't drm_debugfs_crtc_remove() in > drm_minor_free() ? Good call. Have moved drm_debugfs_crtc_remove to drm_minor_unregister. >> -#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); > Mildly related: some userspace projects define convenience helpers > which you use in order to allocate the correct amount of space on > stack. > Is there such a set for the kernel ? > > With those the above will become something like: > char name[5+DECIMAL_STR_MAX] = ksprintf("crtc-%d", crtc->index); Don't know of those but I think that in this specific case an allocation is not that much of a problem. >> --- /dev/null >> +++ b/drivers/gpu/drm/drm_debugfs_crc.c > >> + * >> + * Authors: >> + *Eric Anholt >> + *Keith Packard >> + * > Wondering about these authorship lines - is the code copied/derived > from somewhere ? If so please mention where from. Done, and also fixed the actual people that should be in there. >> +/* >> + * 1 frame field of 10 chars plus a number of CRC fields of 10 chars each, >> space >> + * separated and with a newline at the end. >> + */ >> +#define LINE_LEN(VALUES_CNT) (10 + 11 * VALUES_CNT + 1 + 1) > Hmm I seem to suck at math. The macro seems larger than expected (comment and > code wise) by 1, right ? The comment indeed should mention that it's NULL-terminated, but the code is correct, isn't it? >> +#define MAX_LINE_LEN (LINE_LEN(DRM_MAX_CRC_NR)) >> + > Worth using lowercase VALUES_CNT and less likely to clash macro names ? Done. > > >> + BUILD_BUG_ON_NOT_POWER_OF_2(DRM_CRC_ENTRIES_NR); >> + crc->tail = (crc->tail + 1) & (DRM_CRC_ENTRIES_NR - 1); > It's strange that the kernel does not have a macro for this. But for the sake > of me I cannot find one. Yeah. >> --- a/drivers/gpu/drm/drm_drv.c >> +++ b/drivers/gpu/drm/drm_drv.c >> @@ -192,6 +192,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; >> >> @@ -207,6 +208,14 @@ static int drm_minor_register(struct drm_device *dev, >> unsigned int type) >> return ret; >> } >> >> + if (type == DRM_MINOR_LEGACY) { > Sidenote: If we did not use DRM_MINOR_LEGACY for render-only GPUs we would > save a couple of cycles here :-) > >> + drm_for_each_crtc(crtc, dev) { >> + ret = drm_debugfs_crtc_add(crtc); >> + if (ret) >> + goto err_debugfs; > IMHO we don't want to bring down the whole setup if this fails, unlike > where we cannot create debug/dri all together. > > Regardless: we seems to be missing the cleanup in the error path ? Have changed it to only log an error and continue, so no cleanup is necessary. >> +#if defined(CONFIG_DEBUG_FS) >> +extern const struct file_operations drm_crc_control_fops; >> +extern const struct file_operations drm_crtc_crc_fops;
[PATCH v2] drm/doc: Add a few words on validation with IGT
Also provide some pointers for building IGT as some kernel hackers might not be that familiar with building stuff on Linux distros. Signed-off-by: Tomeu Vizoso Cc: Daniel Vetter --- Documentation/gpu/drm-uapi.rst | 37 + 1 file changed, 37 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 12b47c30fe2e..1ba301cebe16 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -156,6 +156,43 @@ other hand, a driver requires shared state between clients which is visible to user-space and accessible beyond open-file boundaries, they cannot support render nodes. +Validating changes with IGT +=== + +There's a collection of tests that aims to cover the whole functionality of +DRM drivers and that can be used to check that changes to DRM drivers or the +core don't regress existing functionality. This test suite is called IGT and +its code can be found in https://cgit.freedesktop.org/drm/igt-gpu-tools/. + +To build IGT, start by installing its build dependencies. In Debian-based +systems:: + + # apt-get build-dep intel-gpu-tools + +And in Fedora-based systems:: + + # dnf builddep intel-gpu-tools + +Then clone the repository:: + + $ git clone git://anongit.freedesktop.org/drm/igt-gpu-tools + +Configure the build system and start the build:: + + $ cd igt-gpu-tools && ./autogen.sh && make -j6 + +Download the piglit dependency:: + + $ ./scripts/run-tests.sh -d + +And run the tests:: + + $ ./scripts/run-tests.sh -t kms -t core -s + +run-tests.sh is a wrapper around piglit that will execute the tests matching +the -t options. A report in HTML format will be available in +./results/html/index.html. Results can be compared with piglit. + VBlank event handling = -- 2.7.4 -- 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
[PATCH] drm/doc: Add a few words on validation with IGT
Also provide some pointers for building IGT as some kernel hackers might not be that familiar with building stuff on Linux distros. Signed-off-by: Tomeu Vizoso Cc: Daniel Vetter --- Documentation/gpu/drm-uapi.rst | 37 + 1 file changed, 37 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 94876938aef3..5e425ab83dc1 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -112,3 +112,40 @@ 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. + +Validating changes with IGT +=== + +There's a collection of tests that aims to cover the whole functionality of +DRM drivers and that can be used to check that changes to DRM drivers or the +core don't regress existing functionality. This test suite is called IGT and +its code can be found in https://cgit.freedesktop.org/xorg/app/intel-gpu-tools. + +To build IGT, start by installing its build dependencies. In Debian-based +systems:: + + # apt-get build-dep intel-gpu-tools + +And in Fedora-based systems:: + + # dnf builddep intel-gpu-tools + +Then clone the repository:: + + $ git clone git://anongit.freedesktop.org/xorg/app/intel-gpu-tools + +Configure the build system and start the build:: + + $ cd intel-gpu-tools && ./autogen.sh && make -j6 + +Download the piglit dependency:: + + $ ./scripts/run-tests.sh -d + +And run the tests:: + + $ ./scripts/run-tests.sh -t kms -t core -s + +run-tests.sh is a wrapper around piglit that will execute the tests matching +the -t options. A report in HTML format will be available in +./results/html/index.html. Results can be compared with piglit. -- 2.7.4 -- 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 v3 2/3] drm: Add API for capturing frame CRCs
On 6 August 2016 at 19:04, Daniel Stone wrote: > Hi Tomeu, > > On 22 July 2016 at 15:10, Tomeu Vizoso wrote: >> +/** >> + * 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 > > s/can/must/ > > Is it worth having 'auto' as a default source perhaps? Yup, it's the case in v4, so you just cat the data file and start getting CRCs. >> + * 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 >> "none" >> + * and "auto" keywords. "none" will disable CRC generation and "auto" will >> let >> + * the driver select a default source of frame CRCs for this CRTC. > > Is it also worth having 'connector-%s' (named as per sysfs, e.g. > connector-HDMI-A-0) as a standardised entry, for cloneable CRTCs which > have CRC control on the connector rather than the CRTC? My impression right now is that only "auto" makes sense as a standardised entry, as any explicit sources are pretty much hw-dependent so the tests will need knowledge about the hw anyway. The IGT tests already try each connector in each CRTC when looking for a setup that supports CRC capture (with the auto source). Regards, Tomeu -- 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
[PATCH v4 0/4] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. Thanks, Tomeu Tomeu Vizoso (4): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API drm/i915: Put "cooked" vlank counters in frame CRC lines Documentation/gpu/drm-uapi.rst|6 + drivers/gpu/drm/Makefile |3 +- drivers/gpu/drm/drm_crtc.c| 12 + drivers/gpu/drm/drm_debugfs.c | 36 +- drivers/gpu/drm/drm_debugfs_crc.c | 368 drivers/gpu/drm/drm_drv.c |9 + drivers/gpu/drm/drm_internal.h| 10 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 892 + drivers/gpu/drm/i915/i915_irq.c | 69 ++- drivers/gpu/drm/i915/intel_display.c |1 + drivers/gpu/drm/i915/intel_drv.h |7 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1001 + include/drm/drmP.h|5 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 74 +++ 16 files changed, 1619 insertions(+), 917 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.7.4 -- 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
[PATCH v4 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. Signed-off-by: Tomeu Vizoso --- Documentation/gpu/drm-uapi.rst| 6 + drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_crtc.c| 12 ++ drivers/gpu/drm/drm_debugfs.c | 36 +++- drivers/gpu/drm/drm_debugfs_crc.c | 368 ++ drivers/gpu/drm/drm_drv.c | 9 + drivers/gpu/drm/drm_internal.h| 10 ++ include/drm/drmP.h| 5 + include/drm/drm_crtc.h| 41 + include/drm/drm_debugfs_crc.h | 74 10 files changed, 562 insertions(+), 2 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 536bf3eaadd4..33f778696ccd 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -109,3 +109,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 e3dba6f44a79..a920b4d60853 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 @@ -20,6 +20,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 f1d9f0569d7f..59f2b2746b91 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -40,6 +40,7 @@ #include #include #include +#include #include "drm_crtc_internal.h" #include "drm_internal.h" @@ -738,6 +739,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); +#ifdef CONFIG_DEBUG_FS + spin_lock_init(&crtc->crc.lock); + init_waitqueue_head(&crtc->crc.wq); + crtc->crc.source = kstrdup("auto", GFP_KERNEL); +#endif + 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); @@ -764,6 +771,11 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) * the indices on the drm_crtc after us in the crtc_list. */ +#ifdef CONFIG_DEBUG_FS + drm_debugfs_crtc_remove(crtc); + kfree(crtc->crc.source); +#endif + kfree(crtc->gamma_store); crtc->gamma_store = NULL; diff --git a/drivers/gpu/drm/drm_debugfs.c
Re: [PATCH v3 2/3] drm: Add API for capturing frame CRCs
On 3 August 2016 at 09:06, Ville Syrjälä wrote: > On Fri, Jul 22, 2016 at 04:10:44PM +0200, 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. >> >> Signed-off-by: Tomeu Vizoso >> --- ... >> +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 = kmalloc(len + 1, GFP_KERNEL); >> + if (!source) >> + return -ENOMEM; >> + >> + if (copy_from_user(source, ubuf, len)) { >> + kfree(source); >> + return -EFAULT; >> + } > > memdup_user_nul() ? Good call. >> + >> + if (source[len - 1] == '\n') >> + source[len - 1] = '\0'; >> + else >> + source[len] = '\0'; >> + >> + spin_lock_irq(&crc->lock); >> + >> + if (crc->opened) { >> + kfree(source); >> + return -EBUSY; >> + } > > Why not just start the thing here? For the sake of symmetry, as we are stopping when the data file is closed. >> +static struct drm_crtc_crc_entry *crtc_get_crc_entry(struct drm_crtc_crc >> *crc, >> + int index) >> +{ >> + void *p = crc->entries; >> + size_t entry_size = (sizeof(*crc->entries) + >> + sizeof(*crc->entries[0].crcs) * crc->values_cnt); > > This computation is duplicated also in crtc_crc_open(). could use a > common helper to do it. > > Shame the language doesn't have a way to deal with arrays of variable > sized arrays in a nice way. Ok. >> + >> + return p + entry_size * index; >> +} >> + >> +#define MAX_LINE_LEN (8 + 9 * DRM_MAX_CRC_NR + 1 + 1) >> + >> +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), >> + c
[PATCH v3 2/3] 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. Signed-off-by: Tomeu Vizoso --- Documentation/gpu/drm-uapi.rst| 6 + drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_crtc.c| 12 ++ drivers/gpu/drm/drm_debugfs.c | 36 +++- drivers/gpu/drm/drm_debugfs_crc.c | 370 ++ drivers/gpu/drm/drm_drv.c | 9 + drivers/gpu/drm/drm_internal.h| 10 ++ include/drm/drmP.h| 5 + include/drm/drm_crtc.h| 41 + include/drm/drm_debugfs_crc.h | 74 10 files changed, 564 insertions(+), 2 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 536bf3eaadd4..33f778696ccd 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -109,3 +109,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 e3dba6f44a79..b53b5aaaeb4d 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -12,7 +12,8 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \ drm_info.o drm_debugfs.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 + drm_modeset_lock.o drm_atomic.o drm_bridge.o \ + drm_debugfs_crc.o drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 10b73f68c023..087345af96e7 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -40,6 +40,7 @@ #include #include #include +#include #include "drm_crtc_internal.h" #include "drm_internal.h" @@ -738,6 +739,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); +#ifdef CONFIG_DEBUG_FS + spin_lock_init(&crtc->crc.lock); + init_waitqueue_head(&crtc->crc.wq); + crtc->crc.source = kstrdup("auto", GFP_KERNEL); +#endif + 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); @@ -764,6 +771,11 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) * the indices on the drm_crtc after us in the crtc_list. */ +#ifdef CONFIG_DEBUG_FS + drm_debugfs_crtc_remove(crtc); + kfree(crtc->crc.source); +#endif + 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 fa10cef2ba37..73530cbf1316 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -415,5 +415,39 @@ 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_
[PATCH v3 0/3] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. Thanks, Tomeu Tomeu Vizoso (3): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API Documentation/gpu/drm-uapi.rst|6 + drivers/gpu/drm/Makefile |3 +- drivers/gpu/drm/drm_crtc.c| 12 + drivers/gpu/drm/drm_debugfs.c | 36 +- drivers/gpu/drm/drm_debugfs_crc.c | 370 drivers/gpu/drm/drm_drv.c |9 + drivers/gpu/drm/drm_internal.h| 10 + drivers/gpu/drm/i915/Makefile |2 +- drivers/gpu/drm/i915/i915_debugfs.c | 892 + drivers/gpu/drm/i915/i915_irq.c | 69 ++- drivers/gpu/drm/i915/intel_display.c |1 + drivers/gpu/drm/i915/intel_drv.h |7 + drivers/gpu/drm/i915/intel_pipe_crc.c | 1001 + include/drm/drmP.h|5 + include/drm/drm_crtc.h| 41 ++ include/drm/drm_debugfs_crc.h | 74 +++ 16 files changed, 1621 insertions(+), 917 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.5.5 -- 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
[PATCH v2 0/3] New debugfs API for capturing CRC of frames
Hi, this series basically takes the facility for continuously capturing CRCs of frames from the i915 driver and into the DRM core. The idea is that test suites such as IGT use this information to check that frames that are exected to be identical, also have identical CRC values. Other drivers for hardware that can provide frame CRCs (including eDP panels that support self-refresh) can easily implement the new callback and provide userspace with the CRC values. Thanks, Tomeu Tomeu Vizoso (3): drm/i915/debugfs: Move out pipe CRC code drm: Add API for capturing frame CRCs drm/i915: Use new CRC debugfs API Documentation/gpu/drm-uapi.rst| 6 + drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_crtc.c| 11 + drivers/gpu/drm/drm_debugfs.c | 36 +- drivers/gpu/drm/drm_debugfs_crc.c | 394 ++ drivers/gpu/drm/drm_drv.c | 9 + drivers/gpu/drm/drm_internal.h| 10 + drivers/gpu/drm/i915/Makefile | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 892 +-- drivers/gpu/drm/i915/i915_irq.c | 57 +- drivers/gpu/drm/i915/intel_display.c | 1 + drivers/gpu/drm/i915/intel_drv.h | 6 + drivers/gpu/drm/i915/intel_pipe_crc.c | 970 ++ include/drm/drmP.h| 5 + include/drm/drm_crtc.h| 40 ++ include/drm/drm_debugfs_crc.h | 71 +++ 16 files changed, 1597 insertions(+), 916 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c create mode 100644 include/drm/drm_debugfs_crc.h -- 2.5.5 -- 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
[PATCH v2 2/3] 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. Signed-off-by: Tomeu Vizoso --- Documentation/gpu/drm-uapi.rst| 6 + drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_crtc.c| 11 ++ drivers/gpu/drm/drm_debugfs.c | 36 +++- drivers/gpu/drm/drm_debugfs_crc.c | 394 ++ drivers/gpu/drm/drm_drv.c | 9 + drivers/gpu/drm/drm_internal.h| 10 + include/drm/drmP.h| 5 + include/drm/drm_crtc.h| 40 include/drm/drm_debugfs_crc.h | 71 +++ 10 files changed, 583 insertions(+), 2 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 536bf3eaadd4..33f778696ccd 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -109,3 +109,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 e3dba6f44a79..b53b5aaaeb4d 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -12,7 +12,8 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \ drm_info.o drm_debugfs.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 + drm_modeset_lock.o drm_atomic.o drm_bridge.o \ + drm_debugfs_crc.o drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 10b73f68c023..48155e0439df 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -40,6 +40,7 @@ #include #include #include +#include #include "drm_crtc_internal.h" #include "drm_internal.h" @@ -738,6 +739,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, if (cursor) cursor->possible_crtcs = 1 << drm_crtc_index(crtc); +#ifdef CONFIG_DEBUG_FS + spin_lock_init(&crtc->crc.lock); + init_waitqueue_head(&crtc->crc.wq); +#endif + 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); @@ -764,6 +770,11 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) * the indices on the drm_crtc after us in the crtc_list. */ +#ifdef CONFIG_DEBUG_FS + drm_debugfs_crtc_remove(crtc); + kfree(crtc->crc.source); +#endif + 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 fa10cef2ba37..73530cbf1316 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -415,5 +415,39 @@ 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: + debugfs_remove_recursive(crtc->debugfs_entry); + crtc->debugfs_entry = NULL; + 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
Re: [PATCH v15 1/3] drm: rockchip: Add basic drm driver
On 15 March 2016 at 02:30, Mark yao wrote: > On 2016年03月14日 21:35, Tomeu Vizoso wrote: >> >> On 2 December 2014 at 10:15, Mark Yao wrote: >>> >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> new file mode 100644 >>> index 000..e7ca25b >>> --- /dev/null >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> @@ -0,0 +1,1455 @@ >> >> ... >>> >>> +static bool vop_crtc_mode_fixup(struct drm_crtc *crtc, >>> + const struct drm_display_mode *mode, >>> + struct drm_display_mode *adjusted_mode) >>> +{ >>> + if (adjusted_mode->htotal == 0 || adjusted_mode->vtotal == 0) >>> + return false; >> >> Hi Mark, >> >> what's the rationale for this? >> >> Disabling a CRTC as in [0] will cause mode_fixup() to be called with >> an empty mode, failing that test. >> >> Removing the check seems to get things working fine for a short while, >> but a later modeset invariably gets the VOP to hang (as reported by >> [1]). >> >> Do you know why that check was put in place and what exactly could be >> causing the hw to hang? >> >> [0] >> https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/lib/igt_kms.c#n1616 >> [1] >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rockchip/rockchip_drm_vop.c#n873 >> >> Thanks, >> >> Tomeu >> > Hi Tomeu > > Just thinking that "adjusted_mode->htotal == 0 || adjusted_mode->vtotal == > 0" is not a good mode for vop. Ah, ok. Guess it should be removed then so we don't break userspace? > And you said VOP hang, only WARN_ON error message? or system hang, die? Sorry, the symptom was only the warning, I just went a bit too far and assumed the VOP had stopped working at all. > I think maybe crtc disable too fast, vblank is off, then no one can feed the > wait_update_complete. > Can you test it again with following patch? Actually, in today's testing I don't see that happening any more, sorry about that :/ What I have been looking at today is a related issue when running the kms_flip_event_leak test from intel-gpu-tools. If I remove the check mentioned above so CRTCs can be disabled with the SetCRTC IOCTL, I see this page fault the second and subsequent times I run the test. [ 75.809031] rk_iommu ff930300.iommu: Page fault at 0x0100 of type read [ 75.809035] rk_iommu ff930300.iommu: iova = 0x0100: dte_index: 0x4 pte_index: 0x0 page_offset: 0x0 [ 75.809040] rk_iommu ff930300.iommu: mmu_dte_addr: 0x2c258000 dte@0x2c258010: 0x2c561001 valid: 1 pte@0x2c561000: 0x2a06 valid: 0 page@0x flags: 0x0 [ 76.951288] rk_iommu ff930300.iommu: Enable stall request timed out, status: 0x4b I have written a smaller standalone test that is attached in case you want to check it out, but I haven't been able to find out why it only happens when the test is rerun. Apparently the VOP is still trying to read a BO (0x0100) right when the kernel frees it, but from what I can see, it should be scanning another BO at that point. Do you have any ideas on what could be happening? Thanks, Tomeu > drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -503,6 +503,8 @@ static void vop_crtc_disable(struct drm_crtc *crtc) > if (!vop->is_enabled) > return; > + vop_crtc_wait_for_update(crtc); > + > drm_crtc_vblank_off(crtc); > > Thanks. > > -- > Mark Yao > > #include #include #include #include #include #include #include #include #include __attribute__((format(printf, 1, 2))) static void kmsg(const char *format, ...) #define KERN_EMER "<0>" #define KERN_ALERT "<1>" #define KERN_CRIT "<2>" #define KERN_ERR "<3>" #define KERN_WARNING "<4>" #define KERN_NOTICE "<5>" #define KERN_INFO "<6>" #define KERN_DEBUG "<7>" { va_list ap; FILE *file; file = fopen("/dev/kmsg", "w"); if (file == NULL) return; va_start(ap, format); fprintf(file, "userspace: "); vfprintf(file, format, ap); va_end(ap); fclose(file); } static uint32_t dumb_create(int fd, int width, int height, int bpp) { struct drm_mode_create_dumb create = {}; create.width = width; create.height = height; create.bpp = bpp; create.handle = 0; drmIoctl(fd, DRM_IOCTL_MODE_CREATE_DUMB, &create); return create.handle; } static uint32_t fb_create(int fd, int width, int height, int format, int bo, int pitch) {
Re: [PATCH v15 1/3] drm: rockchip: Add basic drm driver
On 16 March 2016 at 16:23, Tomeu Vizoso wrote: > On 15 March 2016 at 02:30, Mark yao wrote: >> On 2016年03月14日 21:35, Tomeu Vizoso wrote: >>> >>> On 2 December 2014 at 10:15, Mark Yao wrote: >>>> >>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>>> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>>> new file mode 100644 >>>> index 000..e7ca25b >>>> --- /dev/null >>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>>> @@ -0,0 +1,1455 @@ >>> >>> ... >>>> >>>> +static bool vop_crtc_mode_fixup(struct drm_crtc *crtc, >>>> + const struct drm_display_mode *mode, >>>> + struct drm_display_mode *adjusted_mode) >>>> +{ >>>> + if (adjusted_mode->htotal == 0 || adjusted_mode->vtotal == 0) >>>> + return false; >>> >>> Hi Mark, >>> >>> what's the rationale for this? >>> >>> Disabling a CRTC as in [0] will cause mode_fixup() to be called with >>> an empty mode, failing that test. >>> >>> Removing the check seems to get things working fine for a short while, >>> but a later modeset invariably gets the VOP to hang (as reported by >>> [1]). >>> >>> Do you know why that check was put in place and what exactly could be >>> causing the hw to hang? >>> >>> [0] >>> https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/lib/igt_kms.c#n1616 >>> [1] >>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rockchip/rockchip_drm_vop.c#n873 >>> >>> Thanks, >>> >>> Tomeu >>> >> Hi Tomeu >> >> Just thinking that "adjusted_mode->htotal == 0 || adjusted_mode->vtotal == >> 0" is not a good mode for vop. > > Ah, ok. Guess it should be removed then so we don't break userspace? > >> And you said VOP hang, only WARN_ON error message? or system hang, die? > > Sorry, the symptom was only the warning, I just went a bit too far and > assumed the VOP had stopped working at all. > >> I think maybe crtc disable too fast, vblank is off, then no one can feed the >> wait_update_complete. >> Can you test it again with following patch? > > Actually, in today's testing I don't see that happening any more, > sorry about that :/ > > What I have been looking at today is a related issue when running the > kms_flip_event_leak test from intel-gpu-tools. If I remove the check > mentioned above so CRTCs can be disabled with the SetCRTC IOCTL, I see > this page fault the second and subsequent times I run the test. > > [ 75.809031] rk_iommu ff930300.iommu: Page fault at 0x0100 of type read > [ 75.809035] rk_iommu ff930300.iommu: iova = 0x0100: dte_index: > 0x4 pte_index: 0x0 page_offset: 0x0 > [ 75.809040] rk_iommu ff930300.iommu: mmu_dte_addr: 0x2c258000 > dte@0x2c258010: 0x2c561001 valid: 1 pte@0x2c561000: 0x2a06 valid: > 0 page@0x flags: 0x0 > [ 76.951288] rk_iommu ff930300.iommu: Enable stall request timed > out, status: 0x4b > > I have written a smaller standalone test that is attached in case you > want to check it out, but I haven't been able to find out why it only > happens when the test is rerun. > > Apparently the VOP is still trying to read a BO (0x0100) right > when the kernel frees it, but from what I can see, it should be > scanning another BO at that point. > > Do you have any ideas on what could be happening? Apparently, when the VOP is re-enabled, it will start scanning from the framebuffer address that had been set last. Because DMA addresses are recycled and there's going to be a low number of framebuffers, this isn't going to be obvious unless we make sure that there isn't a FB allocated at that DMA address any more. The attached test case does just that. Regards, Tomeu > Thanks, > > Tomeu > >> drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> @@ -503,6 +503,8 @@ static void vop_crtc_disable(struct drm_crtc *crtc) >> if (!vop->is_enabled) >> return; >> + vop_crtc_wait_for_update(crtc); >> + >> drm_crtc_vblank_off(crtc); >> >> Thanks. >> >> -- >> Mark Yao >> >> #include #include #include #include #include #include #include #include #include __attribute__((format(printf, 1, 2))) static void kmsg(const char *format, ...) #define KERN_EMER "<0>" #define KERN_ALERT "
Re: [PATCH v15 1/3] drm: rockchip: Add basic drm driver
On 2 December 2014 at 10:15, Mark Yao wrote: > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > new file mode 100644 > index 000..e7ca25b > --- /dev/null > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -0,0 +1,1455 @@ ... > +static bool vop_crtc_mode_fixup(struct drm_crtc *crtc, > + const struct drm_display_mode *mode, > + struct drm_display_mode *adjusted_mode) > +{ > + if (adjusted_mode->htotal == 0 || adjusted_mode->vtotal == 0) > + return false; Hi Mark, what's the rationale for this? Disabling a CRTC as in [0] will cause mode_fixup() to be called with an empty mode, failing that test. Removing the check seems to get things working fine for a short while, but a later modeset invariably gets the VOP to hang (as reported by [1]). Do you know why that check was put in place and what exactly could be causing the hw to hang? [0] https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/lib/igt_kms.c#n1616 [1] https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rockchip/rockchip_drm_vop.c#n873 Thanks, Tomeu > + > + return true; > +} -- 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