[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