On 14 November 2016 at 09:56, Michel Dänzer <michel@xxxxxxxxxxx> wrote: > On 10/11/16 10:38 PM, Emil Velikov wrote: >> On 10 November 2016 at 12:40, Nicolai Hähnle <nhaehnle@xxxxxxxxx> wrote: >>> On 09.11.2016 19:08, Emil Velikov wrote: >>>> >>>> From: Emil Velikov <emil.velikov@xxxxxxxxxxxxx> >>>> >>>> Parsing config sysfs file wakes up the device. The latter of which may >>>> be slow and isn't required to begin with. >>>> >>>> Reading through config is/was required since the revision is not >>>> available by other means, although with a kernel patch in the way that >>>> is about to change. >>>> >>>> Since returning 0 when one might expect a valid value is a no-go add a >>>> workaround drmDeviceUseRevisionFile() which one can use to say "I don't >>>> care if the revision file returns 0." >>>> >>>> v2: Complete rework - add new API to control the method, instead of >>>> changing it underneat the users' feet. >>>> >>>> Cc: Michel Dänzer <michel.daenzer@xxxxxxx> >>>> Cc: Mauro Santos <registo.mailling@xxxxxxxxx> >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502 >>>> Signed-off-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx> >>>> --- >>>> I don't have a strong preference for/against this or v1. >>>> >>>> With this Mesa will require a 2 line patch. With v1 things will get >>>> fixed magically, when rebuilt against newer libdrm. >>>> >>>> Mauro I'll send the mesa patch in a second. Feel free to give it a spin. >>>> >>>> Thanks >>>> --- >>>> xf86drm.c | 70 >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- >>>> xf86drm.h | 11 ++++++++++ >>>> 2 files changed, 78 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/xf86drm.c b/xf86drm.c >>>> index 52add5e..676effc 100644 >>>> --- a/xf86drm.c >>>> +++ b/xf86drm.c >>>> @@ -113,6 +113,13 @@ void drmSetServerInfo(drmServerInfoPtr info) >>>> drm_server_info = info; >>>> } >>>> >>>> +static bool use_revision_file; >>>> + >>>> +void drmDeviceUseRevisionFile(void) >>>> +{ >>>> + use_revision_file = true; >>>> +} >>> >>> >>> I think this API of setting a magic hidden global variable is really nasty. >>> >>> Can we add new drmGetDevice[s] entry points with a flags parameter and a >>> flag (per Michel's suggestion) which says "I don't care about the revision >>> ID"? It kind of sucks because they'll have to get a silly name like >>> drmGetDevice[s]2, but I think that's still better than magic global state. >>> >> AFACS Michel suggested (in his latest reply) to have >> parse_separate_sysfs_files always and make the lack of revision file >> fatal - falling back to ./config. Not a separate API [+ flags] ? > > You're mixing up two separate issues. Sorry if I was unclear before, let > me try again: > > My first comment was about the "drmDeviceUseRevisionFile" name being > misleading and not reflecting the intent. For this issue, I think > Nicolai's suggestion is even better than just fixing the name. > > My other comment was that parse_separate_sysfs_files should be tried > first even if the revision information is needed, and should only bail > in that case if the separate revision sysfs files are missing, in which > case parse_config_sysfs_file can be used as a fallback. > > Makes perfect sense, tyvm ! >> P.S. /me is dreaming of the day when [wh]e'll get to drop 90% of >> libdrm and it's hacks - viva la libdrm3 ! > > Given the issues with bumping SONAME, I'm afraid you'll have to continue > dreaming for a long time. :) > Sticking with libdrm3.so.X + libdrm3.pc should sort that, right ? That plus a simple sed job to make the symbols different/unique. Regardless, anything libdrm3 is unrelated to the topic in question. Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
