[PATCH i-g-t v1] tools/intel_hdcp: Introduce intel_hdcp tool
Reddy Guddati, Santhosh
santhosh.reddy.guddati at intel.com
Tue Feb 11 13:27:57 UTC 2025
On 24-12-2024 14:01, Reddy Guddati, Santhosh wrote:
> Hi Kamil,
>
> ------------------------------------------------------------------------
> *From:* Kamil Konieczny <kamil.konieczny at linux.intel.com>
> *Sent:* Friday, December 6, 2024 10:48 PM
> *To:* Reddy Guddati, Santhosh <santhosh.reddy.guddati at intel.com>
> *Cc:* igt-dev at lists.freedesktop.org <igt-dev at lists.freedesktop.org>;
> Kandpal, Suraj <suraj.kandpal at intel.com>; Nautiyal, Ankit K
> <ankit.k.nautiyal at intel.com>; Thasleem, Mohammed
> <mohammed.thasleem at intel.com>
> *Subject:* Re: [PATCH i-g-t v1] tools/intel_hdcp: Introduce intel_hdcp tool
> Hi Santhosh,
> On 2024-12-04 at 15:26:54 +0530, Santhosh Reddy Guddati wrote:
> > Add a new HDCP tool for self-testing and easy deployment
> > to client machines. This tool helps diagnose issues,
> > determining whether the problem lies in the driver or user space.
> >
> > The current changes include tool skeleton and get hdcp info
> > on the connected outputs.
> >
> > Signed-off-by: Santhosh Reddy Guddati <santhosh.reddy.guddati at intel.com>
> > ---
> > tools/intel_hdcp.c | 203 +++++++++++++++++++++++++++++++++++++++++++++
> > tools/meson.build | 1 +
> > 2 files changed, 204 insertions(+)
> > create mode 100644 tools/intel_hdcp.c
> >
> > diff --git a/tools/intel_hdcp.c b/tools/intel_hdcp.c
> > new file mode 100644
> > index 000000000..8c0258092
> > --- /dev/null
> > +++ b/tools/intel_hdcp.c
> > @@ -0,0 +1,203 @@
> > +/*
> > + * Copyright © 2024 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> obtaining a
>
> Use SPDX, see other recent C files.
>
> > + * copy of this software and associated documentation files (the
> "Software"),
> > + * to deal in the Software without restriction, including without
> limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including
> the next
> > + * paragraph) shall be included in all copies or substantial
> portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > + * DEALINGS IN THE SOFTWARE.
> > + *
> > + * This tool designed to mimic an HDCP video player, capable of
> performing
> > + * self-tests and easily deployable to client machines. This tool
> helps identify
> > + * whether issues are rooted in the driver or user space.
> > + *
> > + */
> > +
> > +#include <stdio.h>
> > +#include "igt.h"
> > +#include <fcntl.h>
>
> Sort alphabetically, first system headers, then igt so:
>
> #include <fcntl.h>
> #include <stdio.h>
>
> #include "igt.h"
>
> Btw are you sure to include igt.h here?
>
> We need this to use the igt defintions here to stay consistent with the
> other tools in the igt_gpu_tools repository, We are using the igt_*
> functions in the existing tools like
> Igt_dp_compliance, igt_reg etc.
>
> Please let me know your thoughts or if my understanding is incorrect.
> > +
> > +#define MAX_HDCP_BUF_LEN 5000
> > +
> > +typedef struct data {
> > + int fd;
> > + igt_display_t display;
> > + struct igt_fb red, green;
> > + int height, width;
> > +} data_t;
> > +
> > +
> > +/* TODO: Implement the actual HDCP enabling logic here
> > + * Steps:
> > + * 1. Open the appropriate debugfs file for the connector.
> > + * 2. Enable HDCP on the output using connector properties
> > + * IGT_CONNECTOR_CONTENT_PROTECTION and
> IGT_CONNECTOR_HDCP_CONTENT_TYPE.
> > + * 3. Check the status to ensure HDCP is enabled.
> > + */
> > +static void enable_hdcp(void)
> > +{
> > + igt_debug("TODO: Enable HDCP\n");
>
> Tools is not a test, so maybe just fprintf(stderr, ...)
>
> > +}
> > +
> > +/* TODO: Implement the actual HDCP disabling logic here */
> > +static void disable_hdcp(void)
> > +{
> > + igt_debug("TODO: Disable HDCP\n");
>
> Same here.
>
> > +}
> > +
> > +static bool sink_hdcp2_capable(data_t *data, igt_output_t *output)
> > +{
> > + char buf[MAX_HDCP_BUF_LEN];
> > + int fd;
> > +
> > + fd = igt_debugfs_connector_dir(data->fd, output->name, O_RDONLY);
>
> You are using igt lib function here, make sure it will not assert.
>
> > + if (fd < 0)
> > + return false;
> > +
> > + if (is_intel_device(data->fd))
> > + igt_debugfs_simple_read(fd,
> "i915_hdcp_sink_capability", buf, sizeof(buf));
>
> Same here, using igt lib function intended for tests
> can bring an assert, it is no expected in a tool.
>
>
>
> > + else
> > + igt_debugfs_simple_read(fd, "hdcp_sink_capability",
> buf, sizeof(buf));
>
> Same here.
>
> > +
> > + close(fd);
> > +
> > + igt_debug("Sink capability: %s\n", buf);
> > + return strstr(buf, "HDCP2.2");
> > +}
> > +
> > +static bool sink_hdcp_capable(data_t *data, igt_output_t *output)
> > +{
> > + char buf[MAX_HDCP_BUF_LEN];
> > + int fd;
> > +
> > + fd = igt_debugfs_connector_dir(data->fd, output->name, O_RDONLY);
>
> Same here.
>
> > + if (fd < 0)
> > + return false;
> > +
> > + if (is_intel_device(data->fd))
> > + igt_debugfs_simple_read(fd,
> "i915_hdcp_sink_capability", buf, sizeof(buf));
>
> Same here.
>
> > + else
> > + igt_debugfs_simple_read(fd, "hdcp_sink_capability",
> buf, sizeof(buf));
>
> Same here.
>
> > +
> > + close(fd);
> > +
> > + igt_debug("Sink capability: %s\n", buf);
> > + return strstr(buf, "HDCP1.4");
> > +}
> > +
> > +static const char *get_hdcp_version(igt_output_t *output, data_t *data)
> > +{
> > + if (sink_hdcp2_capable(data, output))
> > + return "HDCP 2.2";
> > + else if (sink_hdcp_capable(data, output))
> > + return "HDCP 1.4";
> > + else
> > + return "No HDCP support";
> > +}
> > +
> > +static void get_hdcp_info(data_t *data)
> > +{
> > + igt_output_t *output;
> > + igt_display_t *display = &data->display;
> > +
> > + igt_info("\nHDCP Sink Information\n");
> > + for_each_connected_output(display, output) {
> > + igt_info("Output: %s\tHDCP Supported Version: %s\n",
> > + igt_output_name(output),
> get_hdcp_version(output, data));
> > + }
> > +}
> > +
> > +static void print_usage(void)
> > +{
> > + igt_info("Usage: intel_hdcp [OPTIONS]\n");
>
> Same here, imho better just printf() here.
> > + igt_info("Options:\n");
> > + igt_info("-i, --info Get HDCP Information\n");
> > + igt_info("-e, --enable Enable HDCP on the output\n");
> > + igt_info("-d, --disable Disable HDCP on the specific
> output\n");
> > + igt_info("-h, --help Display this help message\n");
> > +}
> > +
> > +static void create_frame_buffers(int fd, data_t *data)
> > +{
> > + igt_output_t *output;
> > + drmModeModeInfo *mode;
> > + uint16_t width, height;
> > + igt_display_t *display = &data->display;
> > +
> > + for_each_connected_output(display, output) {
> > + mode = igt_output_get_mode(output);
> > + igt_assert(mode);
> > +
> > + width = mode->hdisplay;
> > + height = mode->vdisplay;
> > + }
> > +
> > + data->width = width;
> > + data->height = height;
> > +
> > + igt_create_color_fb(data->fd, width, height,
> > + DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> > + 1.f, 0.f, 0.f, &data->red);
> > + igt_create_color_fb(data->fd, width, height,
> > + DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> > + 0.f, 1.f, 0.f, &data->green);
> > +}
> > +
> > +static void hdcp_init(data_t *data)
> > +{
> > + data->fd = drm_open_driver_master(DRIVER_ANY);
>
> Imho you should do an open yourself.
>
> > + if (data->fd < 0) {
> > + fprintf(stderr, "Failed to open DRM driver\n");
> > + exit(EXIT_FAILURE);
> > + }
> > + igt_display_require(&data->display, data->fd);
>
> You are not in test, you cannot 'require' a display in a tool,
> imho you should inform user that no displays were found and
> just exit, no igt SKIPs.
>
> We need this to use the igt defintions here to stay consistent with the
> other tools in the igt_gpu_tools repository, We are using the igt_*
> functions in the existing tools like
> Igt_dp_compliance, igt_reg etc.
>
>
> Regards,
> Kamil
>
> > + igt_display_require_output(&data->display);
> > + create_frame_buffers(data->fd, data);
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > + data_t data;
> > + int option;
> > + static const char optstr[] = "hied";
> > + struct option long_opts[] = {
> > + {"help", no_argument, NULL, 'h'},
> > + {"info", no_argument, NULL, 'i'},
> > + {"enable", no_argument, NULL, 'e'},
> > + {"disable", no_argument, NULL, 'd'},
> > + {NULL, 0, NULL, 0 }
> > + };
> > +
> > + hdcp_init(&data);
> > +
> > + while ((option = getopt_long(argc, argv, optstr, long_opts,
> NULL)) != -1) {
> > + switch (option) {
> > + case 'i':
> > + get_hdcp_info(&data);
> > + break;
> > + case 'e':
> > + enable_hdcp();
> > + break;
> > + case 'd':
> > + disable_hdcp();
> > + break;
> > + case 'h':
> > + default:
> > + print_usage();
> > + break;
> > + }
> > + }
> > +}
> > diff --git a/tools/meson.build b/tools/meson.build
> > index 48c9a4b50..8e24005eb 100644
> > --- a/tools/meson.build
> > +++ b/tools/meson.build
> > @@ -27,6 +27,7 @@ tools_progs = [
> > 'intel_gpu_time',
> > 'intel_gtt',
> > 'intel_guc_logger',
> > + 'intel_hdcp',
> > 'intel_infoframes',
> > 'intel_lid',
> > 'intel_opregion_decode',
> > --
> > 2.34.1
> >
More information about the igt-dev
mailing list