CRC generation can be impacted by commits coming from userspace, and
enabling CRC generation may itself trigger a commit. Add notes about
this to the kerneldoc.
Changes since v1:
- Clarified that anything that would disable CRCs counts as a full
modeset, and so userspace needs to reconfigure after full modesets
Changes since v2:
- Add these notes
- Rebase onto drm-misc-next (trivial conflict in comment)
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
Signed-off-by: Ayan Kumar Halder <ayan.halder@arm.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link:- https://patchwork.freedesktop.org/patch/321974/
drm_debugfs_crtc_crc_add() function checks that both .set_crc_source and
.verify_crc_source hooks are provided before enabling debugfs support for
reading per-frame CRC data. Make that explicit in the documentation.
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190703150330.21992-1-Liviu.Dudau@arm.com
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Because there is no need to check these functions, a number of local
functions can be made to return void to simplify things as nothing can
fail.
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20190613133439.GA6715@kroah.com
I. was. blind.
Caught with vkms, which has some really slow crc computation function.
Fixes: 1882018a70 ("drm/crc-debugfs: User irqsafe spinlock in drm_crtc_add_crc_entry")
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190606211544.5389-1-daniel.vetter@ffwll.ch
We can be called from any context, we need to be prepared.
Noticed this while hacking on vkms, which calls this function from a
normal worker. Which really upsets lockdep.
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190605194556.16744-1-daniel.vetter@ffwll.ch
The use of the drmP.h header file is deprecated.
Remove use from all files in drm/*
so people do not look there and follow a bad example.
Build tested allyesconfig,allmodconfig on x86, arm etc.
Including alpha that is as always more challenging than
the rest.
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Acked-by: Daniel Vetter <daniel@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Link: https://patchwork.freedesktop.org/patch/msgid/20190526173535.32701-8-sam@ravnborg.org
This reverts commit e8fa567118.
Don't wait for first CRC during crtc_crc_open. It avoids one frame wait
during open. If application want to wait after read call, it can use
poll/read blocking read() call.
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180821083858.26275-4-mahesh1.kumar@intel.com
This patch make changes to allocate crc-entries buffer before
enabling CRC generation.
It moves all the failure check early in the function before setting
the source or memory allocation.
Now set_crc_source takes only two variable inputs, values_cnt we
already gets as part of verify_crc_source.
Changes since V1:
- refactor code to use single spin lock
Changes since V2:
- rebase
Changes since V3:
- rebase on top of VKMS driver
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Acked-by: Leo Li <sunpeng.li@amd.com> (V2)
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> (V3)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180821083858.26275-3-mahesh1.kumar@intel.com
This patch introduce a callback function "get_crc_sources" which
will be called during read of control node. It is an optional
callback function and if driver implements this callback, driver
should return a constant pointer to an array of crc sources list
and update count according to the number of source in the list.
Changes Since V1: (Daniel)
- return const pointer to an array of crc sources list
- do validation of sources in CRC-core
Changes Since V2:
- update commit message
- update callback documentation
- print one source name per line
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180713135942.25061-3-mahesh1.kumar@intel.com
This patch adds a new callback function "verify_crc_source" which will
be used during setting the crc source in control node. This will help
in avoiding setting of wrong string for source.
Changes since V1:
- do not yet verify_crc_source during open.
Changes since V1:
- improve callback description
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180713135942.25061-2-mahesh1.kumar@intel.com
This will make it possible for userspace to know whether reading
will block, without blocking on the fd. This makes it possible to
drain all queued CRC's in blocking mode, without having to reopen
the fd.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180202142743.68527-1-maarten.lankhorst@linux.intel.com
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
[mlankhorst: Use POLLIN|POLLRDNORM, based on Ville's suggestion]
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>
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>
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]
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>
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
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
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
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
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
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
