[libcamera-devel] [PATCH v4 2/5] libcamera: request: Add tracepoints
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Nov 3 01:19:50 CET 2020
Hi Paul,
Thank you for the patch.
On Fri, Oct 30, 2020 at 05:57:53PM +0900, Paul Elder wrote:
> Add and use tracepoints in Request. Requests are core to libcamera
> operation, thus detecting delays in their processing is important, and
> serves as a good usage example of tracepoints.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> ---
> Changes in v4:
> - move enum tp definitions to "header" tp files
> - add them to meson accordingly
> - remove cancelled field from request_complete_buffer tracepoint
>
> Changes in v3:
> - expand the changelog
> - add enum tracepoint values so that request status and buffer status
> are strings instead of ints in the trace
> - remove cancelled from all request tracepoints, except complete_buffer
> - expand the complete_buffer tracepoint to include information on the
> buffer
> - display the address of the request in all tracepoints
> - remove tracepoints for add_buffer and find_buffer
>
> Changes in v2:
> - remove tracepoints from uvcvideo
> - remove comment in changelog that this is only used for demonstration
> - use Request pointers instead of feeding the fields manually to the
> tracepoint
> ---
> .../internal/tracepoints/buffer_enums.tp | 9 +++
> .../internal/tracepoints/meson.build | 5 ++
> .../libcamera/internal/tracepoints/request.tp | 68 +++++++++++++++++++
> .../internal/tracepoints/request_enums.tp | 9 +++
> src/libcamera/request.cpp | 11 +++
> 5 files changed, 102 insertions(+)
> create mode 100644 include/libcamera/internal/tracepoints/buffer_enums.tp
> create mode 100644 include/libcamera/internal/tracepoints/request.tp
> create mode 100644 include/libcamera/internal/tracepoints/request_enums.tp
>
> diff --git a/include/libcamera/internal/tracepoints/buffer_enums.tp b/include/libcamera/internal/tracepoints/buffer_enums.tp
> new file mode 100644
> index 00000000..fdbea69a
> --- /dev/null
> +++ b/include/libcamera/internal/tracepoints/buffer_enums.tp
> @@ -0,0 +1,9 @@
> +TRACEPOINT_ENUM(
> + libcamera,
> + buffer_status,
> + TP_ENUM_VALUES(
> + ctf_enum_value("FrameSuccess", 0)
> + ctf_enum_value("FrameError", 1)
> + ctf_enum_value("FrameCancelled", 2)
> + )
> +)
> diff --git a/include/libcamera/internal/tracepoints/meson.build b/include/libcamera/internal/tracepoints/meson.build
> index 2dd6733e..fee0758f 100644
> --- a/include/libcamera/internal/tracepoints/meson.build
> +++ b/include/libcamera/internal/tracepoints/meson.build
> @@ -1,4 +1,9 @@
> # SPDX-License-Identifier: CC0-1.0
>
> +# enum files must go first
> tracepoint_files = files([
> + 'buffer_enums.tp',
> + 'request_enums.tp',
> +
> + 'request.tp',
> ])
How about
# enum files must go first
tracepoint_files = files([
'buffer_enums.tp',
'request_enums.tp',
])
tracepoint_files += files([
'request.tp',
])
?
> diff --git a/include/libcamera/internal/tracepoints/request.tp b/include/libcamera/internal/tracepoints/request.tp
> new file mode 100644
> index 00000000..dd28a714
> --- /dev/null
> +++ b/include/libcamera/internal/tracepoints/request.tp
> @@ -0,0 +1,68 @@
> +#include <libcamera/buffer.h>
> +#include <libcamera/request.h>
> +
> +TRACEPOINT_EVENT_CLASS(
> + libcamera,
> + request,
> + TP_ARGS(
> + libcamera::Request *, req
> + ),
> + TP_FIELDS(
> + ctf_integer_hex(uintptr_t, request, reinterpret_cast<uintptr_t>(req))
> + ctf_integer(uint64_t, cookie, req->cookie())
> + ctf_enum(libcamera, request_status, uint32_t, status, req->status())
> + )
> +)
> +
> +TRACEPOINT_EVENT_INSTANCE(
> + libcamera,
> + request,
> + request_construct,
> + TP_ARGS(
> + libcamera::Request *, req
> + )
> +)
> +
> +TRACEPOINT_EVENT_INSTANCE(
> + libcamera,
> + request,
> + request_destroy,
> + TP_ARGS(
> + libcamera::Request *, req
> + )
> +)
> +
> +TRACEPOINT_EVENT_INSTANCE(
> + libcamera,
> + request,
> + request_reuse,
> + TP_ARGS(
> + libcamera::Request *, req
> + )
> +)
> +
> +TRACEPOINT_EVENT_INSTANCE(
> + libcamera,
> + request,
> + request_complete,
> + TP_ARGS(
> + libcamera::Request *, req
> + )
> +)
> +
> +
> +TRACEPOINT_EVENT(
> + libcamera,
> + request_complete_buffer,
> + TP_ARGS(
> + libcamera::Request *, req,
> + libcamera::FrameBuffer *, buf
> + ),
> + TP_FIELDS(
> + ctf_integer_hex(uintptr_t, request, reinterpret_cast<uintptr_t>(req))
> + ctf_integer(uint64_t, cookie, req->cookie())
> + ctf_integer(int, status, req->status())
> + ctf_integer_hex(uintptr_t, buffer, reinterpret_cast<uintptr_t>(buf))
> + ctf_enum(libcamera, buffer_status, uint32_t, buf_status, buf->metadata().status)
> + )
> +)
> diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp
> new file mode 100644
> index 00000000..371f6544
> --- /dev/null
> +++ b/include/libcamera/internal/tracepoints/request_enums.tp
> @@ -0,0 +1,9 @@
> +TRACEPOINT_ENUM(
> + libcamera,
> + request_status,
> + TP_ENUM_VALUES(
> + ctf_enum_value("RequestPending", 0)
> + ctf_enum_value("RequestComplete", 1)
> + ctf_enum_value("RequestCancelled", 2)
> + )
> +)
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index ae8b1660..a68684ef 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -16,6 +16,7 @@
>
> #include "libcamera/internal/camera_controls.h"
> #include "libcamera/internal/log.h"
> +#include "libcamera/internal/tracepoints.h"
>
> /**
> * \file request.h
> @@ -85,10 +86,14 @@ Request::Request(Camera *camera, uint64_t cookie)
> * \todo: Add a validator for metadata controls.
> */
> metadata_ = new ControlList(controls::controls);
> +
> + LIBCAMERA_TRACEPOINT(request_construct, this);
> }
>
> Request::~Request()
> {
> + LIBCAMERA_TRACEPOINT(request_destroy, this);
> +
> delete metadata_;
> delete controls_;
> delete validator_;
> @@ -106,6 +111,8 @@ Request::~Request()
> */
> void Request::reuse(ReuseFlag flags)
> {
> + LIBCAMERA_TRACEPOINT(request_reuse, this);
> +
> pending_.clear();
> if (flags & ReuseBuffers) {
> for (auto pair : bufferMap_) {
> @@ -259,6 +266,8 @@ void Request::complete()
> LOG(Request, Debug)
> << "Request has completed - cookie: " << cookie_
> << (cancelled_ ? " [Cancelled]" : "");
> +
> + LIBCAMERA_TRACEPOINT(request_complete, this);
> }
>
> /**
> @@ -276,6 +285,8 @@ void Request::complete()
> */
> bool Request::completeBuffer(FrameBuffer *buffer)
> {
> + LIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer);
> +
> int ret = pending_.erase(buffer);
> ASSERT(ret == 1);
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list