On 10 November 2016 at 12:44, Nicolai Hähnle <[email protected]> wrote: > On 09.11.2016 19:09, Emil Velikov wrote: >> >> From: Emil Velikov <[email protected]> >> >> Currently the drmGetDevice[s] API parses the config sysfs file to >> retrieve the revision field. >> >> That is required since there's no separate file (nor a libudev call >> afaict) that can be used. At the same time doing so causes the device to >> be awaken, so if an application creates/destroys a GL context multiple >> times they can observe noticeable delays. 2-3s in the case of >> firefox/thunderbird/chromium + radeon module. >> >> There's a kernel patch on the PCI mailing list, but until then we can >> use drmDeviceUseRevisionFile() which will use separate sysfs files and >> default to 0 if the revision one is missing. >> >> Since we don't care about the revision id, we can use that. >> >> Cc: "13.0" <[email protected]> > > > Can a patch that raises the minimum build requirements go into a stable > release? I agree that fixing this is important, just wondering whether there > are downstream problems we need to be aware of. > I agree that bumping the requirement is quite nasty and my first suggestion [rework the libdrm implementation to silently return revision = 0] might seem nastier.
> I also have a concern with the API, which I've sent as a response to the > libdrm patch. > > Thanks, > Nicolai > > > >> Cc: Mauro Santos <[email protected]> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502 >> Signed-off-by: Emil Velikov <[email protected]> >> --- >> The libdrm version is tentative. An alternative solution is to use >> revision file [and default to 0] behind the scenes. >> >> Both options are rather nasty, so any preferences and suggestions are >> greatly appreciated. >> >> Mauro, feel free to drop the version hunk (or bump the libdrm version >> while building). >> --- >> configure.ac | 2 +- >> src/loader/loader.c | 1 + >> 2 files changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/configure.ac b/configure.ac >> index d215b63..f529f2cf 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -68,7 +68,7 @@ OPENCL_VERSION=1 >> AC_SUBST([OPENCL_VERSION]) >> >> dnl Versions for external dependencies >> -LIBDRM_REQUIRED=2.4.66 >> +LIBDRM_REQUIRED=2.4.72 >> LIBDRM_RADEON_REQUIRED=2.4.56 >> LIBDRM_AMDGPU_REQUIRED=2.4.63 >> LIBDRM_INTEL_REQUIRED=2.4.61 >> diff --git a/src/loader/loader.c b/src/loader/loader.c >> index fe90307..e2e4156 100644 >> --- a/src/loader/loader.c >> +++ b/src/loader/loader.c >> @@ -275,6 +275,7 @@ drm_get_pci_id_for_fd(int fd, int *vendor_id, int >> *chip_id) >> drmDevicePtr device; >> int ret; >> >> + drmDeviceUseRevisionFile(); > > > That API is nasty. > Couldn't agree more - I even mentioned it a few lines above :-\ But even with the GetDevice(s)2 API, one will still require a libdrm version bump. Either way - let's keep that into the libdrm thread. Thanks Emil _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
