[PATCH i-g-t v1] tools/intel_hdcp: Introduce intel_hdcp tool

Kamil Konieczny kamil.konieczny at linux.intel.com
Fri Feb 14 14:42:43 UTC 2025


Hi Reddy,
On 2025-02-11 at 18:57:57 +0530, Reddy Guddati, Santhosh wrote:
> 
> 
> On 24-12-2024 14:01, Reddy Guddati, Santhosh wrote:
> > Hi Kamil,
> > 
> >  > 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
[...]

> >  > +
> >  > +static void hdcp_init(data_t *data)
> >  > +{
> >  > +     data->fd = drm_open_driver_master(DRIVER_ANY);
> > 
> > Imho you should do an open yourself.

One more thing, do not load any display driver in a tool.
We do that in tests when using drm_open_driver*() functions
but tool should expect driver is already loaded.

> > 
> >  > +     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.
[...]

imho you should use here __igt_has_display() function, and this
function should strive to _not_ use any of igt_assert nor igt_require nor
igt_info nor igt_warn, should be safe to be called outside of igt tests,
for example safe for tool useage here.

In a tool you should _not_ call SKIP, rather tell user that you
didn't find any display, just use printf(....)

I am adding Katarzyna and Zbigniew and few igt maintainers to Cc
as we are thinking about lib/* useage in tools/ and runner/

Please also add a man page for a new tool, for example see 
man/intel_reg.rst

Regards,
Kamil



More information about the igt-dev mailing list