Commit Graph

11 Commits

Author SHA1 Message Date
Maarten Lankhorst 6f8bcc744a drm/atomic: Prepare drm_modeset_lock infrastructure for interruptible waiting, v2.
When we want to make drm_atomic_commit interruptible, there are a lot of
places that call the lock function, which we don't have control over.

Rather than trying to convert every single one, it's easier to toggle
interruptible waiting per acquire_ctx. If drm_modeset_acquire_init is
called with DRM_MODESET_ACQUIRE_INTERRUPTIBLE, then we will perform
interruptible waits in drm_modeset_lock and drm_modeset_backoff.

Changes since v1:
- Fix locking example in drm_modeset_lock.c to be compatible
  with interruptible waiting (xexaxo) and make it default.
  Uninterruptible waiting shouldn't happen except in corner cases,
  but the example will still apply if the flag is removed.
- Add drm_modeset_lock_single_interruptible() to documentation.
- Fix dead link to removed drm_modeset_lock_interruptible() in
  drm_modeset_lock().

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> #v1
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170912133749.6532-2-maarten.lankhorst@linux.intel.com
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
2017-09-13 09:50:52 +02:00
Maarten Lankhorst 8038e09be5 drm/crc: Only open CRC on atomic drivers when the CRTC is active.
Commit e8fa567118 ("drm: crc: Wait for a frame before returning
from open()") adds a wait for CRC frame, but with the CRTC off
this will never be generated. For atomic drivers we know if a CRTC
is active through crtc_state->active, so when inactive reject the
open with -EIO.

Just like with the previous patch changing debugfs opening semantics,
this patch has been tested against igt.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: e8fa567118 ("drm: crc: Wait for a frame before returning from open()")
Testcase: debugfs_test.read_all_entries
Link: http://patchwork.freedesktop.org/patch/msgid/15f9d300-65d3-63aa-00e3-e83f5e4d5a7a@linux.intel.com
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2017-07-17 16:34:51 +02:00
Maarten Lankhorst eb42ea6d0b drm/crc: Handle opening and closing crc better
When I was doing a grep . -r /sys/kernel/debug/dri/0 I noticed a WARN
appearing when I aborted the grep with ^C.

After investigating I've also noticed that the error handling was
lacking and there are race conditions involving multiple calls to
open/close simultaneously.

Fix this by setting the opened flag first and using crc->entries to
decide when crc can be collected.

Also call unset crc source before cleaning up, this way there is
no race with a future open().

This patch has been tested with all the tests in igt with CRC in their
name.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170621110007.11674-1-maarten.lankhorst@linux.intel.com
Reviewed-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
[mlankhorst: Add description that this patch has been tested with IGT,
based on tomeu's feedback]
2017-07-17 16:32:43 +02:00
Sean Paul c98cdff94a Revert "drm: Don't allow interruptions when opening debugfs/crc"
This reverts

commit b8dfa821c2
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Apr 7 12:17:12 2017 +0100
    drm: Don't allow interruptions when opening debugfs/crc

It reportedly breaks things, so let's revert now and try again later.

Fixes: b8dfa821c2 ("drm: Don't allow interruptions when opening debugfs/crc")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
2017-04-07 16:18:28 -04:00
Chris Wilson b8dfa821c2 drm: Don't allow interruptions when opening debugfs/crc
The code does not like to be interrupted when waiting for the first
vblank after opening a debugfs/crc channel, so don't.

[66285.716870] WARNING: CPU: 1 PID: 16615 at drivers/gpu/drm/drm_debugfs_crc.c:185 crtc_crc_open+0x1d0/0x1f0 [drm]
[66285.716877] Modules linked in: i915 intel_powerclamp crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel cryptd intel_gtt i2c_algo_bit lpc_ich mfd_core drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops prime_numbers drm video button autofs4 sd_mod ahci libahci libata i2c_i801 scsi_mod i2c_designware_platform i2c_designware_core i2c_core
[66285.716929] CPU: 1 PID: 16615 Comm: kms_frontbuffer Not tainted 4.11.0-rc5+ #7
[66285.716935] Hardware name: GIGABYTE GB-BXBT-1900/MZBAYAB-00, BIOS F8 03/02/2016
[66285.716941] Call Trace:
[66285.716955]  dump_stack+0x4d/0x6f
[66285.716966]  __warn+0xc1/0xe0
[66285.716975]  warn_slowpath_null+0x18/0x20
[66285.717004]  crtc_crc_open+0x1d0/0x1f0 [drm]
[66285.717014]  ? wake_atomic_t_function+0x50/0x50
[66285.717024]  full_proxy_open+0xf0/0x1b0
[66285.717032]  ? full_proxy_release+0x80/0x80
[66285.717042]  do_dentry_open.isra.17+0x14b/0x2d0
[66285.717051]  vfs_open+0x42/0x60
[66285.717064]  path_openat+0x5e7/0x13d0
[66285.717074]  ? refcount_dec_and_test+0x11/0x20
[66285.717081]  ? down_read+0xd/0x30
[66285.717087]  do_filp_open+0x85/0xf0
[66285.717093]  ? __vfs_write+0x23/0x120
[66285.717100]  ? __alloc_fd+0x3a/0x170
[66285.717107]  do_sys_open+0x11e/0x1f0
[66285.717113]  ? do_sys_open+0x11e/0x1f0
[66285.717119]  SyS_openat+0xf/0x20
[66285.717125]  entry_SYSCALL_64_fastpath+0x17/0x98
[66285.717131] RIP: 0033:0x7f5f2235146a
[66285.717135] RSP: 002b:00007ffd892e6bc0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
[66285.717142] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f5f2235146a
[66285.717147] RDX: 0000000000000000 RSI: 00007ffd892e6c40 RDI: 0000000000000006
[66285.717151] RBP: 00007ffd892e6b20 R08: 0000000000000000 R09: 000000000000000f
[66285.717156] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[66285.717161] R13: 00007ffd892e6b10 R14: 0000000000000004 R15: 00000000007e61f4

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100610
Fixes: e8fa567118 ("drm: crc: Wait for a frame before returning from open()")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20170407111712.13962-1-chris@chris-wilson.co.uk
2017-04-07 18:42:23 +02:00
Daniel Vetter 760f71e72e drm: document driver interface for CRC capturing
This was missed in Tomeu's patch. Also remove the kerneldoc for the
internal function, we don't document that in general.

While at it word-smith the docs slightly for more clarity.

Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Reviewed-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170322083617.13361-4-daniel.vetter@ffwll.ch
2017-03-24 09:36:06 +01:00
Benjamin Gaignard 9f547425fd drm: crc: Call wake_up_interruptible() each time there is a new CRC entry
Each time new data has being added in CRC list inform reader by calling
wake_up_interruptible().
This should avoid to do it in all drivers.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1483694104-25627-1-git-send-email-benjamin.gaignard@linaro.org
2017-01-06 15:23:19 +01:00
Tomeu Vizoso e8fa567118 drm: crc: Wait for a frame before returning from open()
Don't return from the open() call on the crc/data file until the HW has
produced a first frame, as there's great variability in when the HW is
able to do that and userspace shouldn't have to guess when this specific
HW is ready to start giving frame CRCs.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20170102125912.22305-3-tomeu.vizoso@collabora.com
2017-01-05 09:53:12 +01:00
Tomeu Vizoso 1aa81be3af drm: Move locking into drm_debugfs_crtc_crc_add
There's no reason any more for callers of this function to take the lock
themselves, so just move the lock to the function to avoid confusion and
bugs when more callers are contributed.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Robert Foss <robert.foss@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20170102125912.22305-2-tomeu.vizoso@collabora.com
2017-01-05 09:53:06 +01:00
Jani Nikula e3f56b2953 drm: fix sparse warnings on undeclared symbols in crc debugfs
Fixes sparse warnings:

drivers/gpu/drm/drm_debugfs_crc.c:118:30: warning: symbol
'drm_crtc_crc_control_fops' was not declared. Should it be static?

drivers/gpu/drm/drm_debugfs_crc.c:264:30: warning: symbol
'drm_crtc_crc_data_fops' was not declared. Should it be static?

drivers/gpu/drm/drm_debugfs_crc.c:281:5: warning: symbol
'drm_debugfs_crtc_crc_add' was not declared. Should it be static?

Fixes: 9edbf1fa60 ("drm: Add API for capturing frame CRCs")
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1476790115-28665-1-git-send-email-jani.nikula@intel.com
2016-10-19 14:10:29 +03:00
Tomeu Vizoso 9edbf1fa60 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
      XXXXXXXX in the frame field.

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

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

v7:
    - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h

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

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 <tomeu.vizoso@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Acked-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1475767268-14379-3-git-send-email-tomeu.vizoso@collabora.com
2016-10-17 16:44:34 +02:00