[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info

[Emil Velikov]

On 8 November 2016 at 08:44, Michel Dänzer <michel at daenzer.net> wrote:
> On 07/11/16 08:30 PM, Emil Velikov wrote:
>> On 7 November 2016 at 09:14, Michel Dänzer <michel at daenzer.net> wrote:
>>> On 05/11/16 03:14 AM, Emil Velikov wrote:
>>>> On 2 November 2016 at 03:07, Michel Dänzer <michel at daenzer.net> wrote:
>>>>>
>>>>> The first attached patch will result in drmParsePciDeviceInfo always
>>>>> reporting revision 0 on kernels without the second attached patch. Will
>>>>> that be an issue for the amdgpu-pro stack?
>>>>>
>>>>> Please follow up directly to the patch e-mails with any comments on the
>>>>> patches.
>>>>>
>>>> Fleshing out the question from the actual patches:
>>>>
>>>> Do the AMDGPU-PRO or the AMD stack [as a whole] depend on the revision
>>>> field as returned by the drmDevice API ?
>>>
>>> One answer is that https://patchwork.freedesktop.org/patch/120132/ uses
>>> the revision ID. In this case a wrong revision ID would only cause a
>>> cosmetic issue, but I can imagine that other code in the amdgpu-pro
>>> stack really needs the correct revision ID to accurately identify the GPU.
>>>
>> Don't mean to sound rude, but I was hoping for a definite answer.
>
> So was I. :}
>
> Digging further, the above patch actually doesn't use the revision_id
> field but amdgpu kernel driver functionality to determine the revision.
> Given that such functionality exists, I don't think we have do any more
> special consideration of either amdgpu stack.
>
>
>> Regardless, do you/fellow AMD devs, any preference on how to go with
>> this bug [1] ?
>>
>> Add an override to force use of the revision file - be that envvar,
>> new API {drmDeviceUseRevisionFile, drmDevice...v2}, or revert the 12 +
>> commits (pulling only the offending one won't cut it). Obviously I'm
>> not a huge fan of the last one :-\
>>
>> [1] https://bugs.freedesktop.org/show_bug.cgi?id=98502
>
> I'm afraid I don't have any particularly strong opinion to offer here.
> But it seems weird to me to have an API which pretends to provide the
> revision ID, but it can actually be incorrect. (The same would apply to
> any other information, not just the revision ID in particular)
>
Indeed it is quite nasty. A quick and short solution, which doesn't
break existing users, is to have
drmDeviceForceUseRevisionFile(). I'll give it a stab, adding a juicy
comment/doc about it.

As we get to libdrm3 we can remove this/other hacks, but until then
this will do just fine ?

Thanks
Emil