[PATCH libdrm v2] xf86drm: Parse the separate files to retrieve the vendor, ... info

[Emil Velikov]

On 10 November 2016 at 12:40, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 09.11.2016 19:08, Emil Velikov wrote:
>>
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> 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 at amd.com>
>> Cc: Mauro Santos <registo.mailling at gmail.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>> 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] ?

I can do that, as long as he's OK with the idea/approach.

Thanks
Emil
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 !