Re: [PATCH v4 1/2] lib/drmtest: add __drm_open_driver_path()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, May 27, 2026 at 03:22:47PM +0200, Kamil Konieczny wrote:
> Hi Jani,
> On 2026-05-26 at 15:07:57 +0300, Jani Nikula wrote:
> > On Tue, 26 May 2026, "Sebastian Brzezinka" <sebastian.brzezinka@intel.com> wrote:
> > > Hi Jani,
> > >
> > > On Tue May 26, 2026 at 1:51 PM CEST, Jani Nikula wrote:
> > >> On Tue, 26 May 2026, Sebastian Brzezinka <sebastian.brzezinka@intel.com> wrote:
> > >>> Add __drm_open_driver_path() as a path-based DRM open helper. It opens
> > >>> the given path with O_RDWR, verifies the resulting fd is a DRM device by
> > >>> issuing DRM_IOCTL_VERSION via __get_drm_device_name() before proceeding
> > >>> (returning -1 and closing on failure), logs the opened device path, and
> > >>> populates the xe_device cache when the device is Xe.
> > >>>
> > >>> The __ prefix signals that the function does not throw assertions or
> > >>> igt_require() calls, consistent with the existing __drm_open_driver*
> > >>> family.
> > >>
> > >> FWIW, I think the __ prefix should indicate "implementation detail,
> > >> don't call directly" or something along those lines.
> > >>
> > >> I think it's problematic to encourage tests to call __ prefixed
> > >> functions.
> 
> 
> > > In general I agree with you, but in v3 Kamil comment on this:
> > > ```
> > >>>Could you rename it into __drm_open_driver_path?
> > >>>The idea is that all __functions should not throw asserts nor
> > >>>require.
> > > ```
> > >
> > > So I added an explanation in the comment.
> > 
> > I obviously disagree with all of that, but *shrug*.
> 
> We could implement separete lib functions for tools but
> that will require duplicate effort.
> 
> The idea with __function is that it should be implemented
> without any igt_assert nor igt_require so tools could use them.
> We could go with function() but then developers could
> later on change behaviour and break a tool.
> 
> Or we could add some suffix to a name? function_safe()?
> 
> Btw why tool/igt_power couldn't just read sysfs? Why does it
> needs to open /dev/dri/card* to get any info? +cc Ville

It opens the device just because lib/igt_power and its
dependencies want the fd.

-- 
Ville Syrjälä
Intel