[PATCH libdrm 4/5] xf86drm: introduce drmGetDevice[s]2

[Emil Velikov]

On 1 December 2016 at 03:56, Michel Dänzer <michel at daenzer.net> wrote:
> On 01/12/16 12:09 PM, Michel Dänzer wrote:
>> On 01/12/16 05:35 AM, Emil Velikov wrote:
>>> From: Emil Velikov <emil.velikov at collabora.com>
>>>
>>> Relative to the original version, here one can provide a flags bitmask.
>>> Currently only DRM_DEVICE_IGNORE_PCI_REVISION is supported.
>>>
>>> Implementation detail:
>>> If it's set, we will only parse the separate sysfs files and we won't
>>> touch the config one. The latter awakes the device (causing delays)
>>> which is the core reason why this API was introduced.
>>>
>>> Cc: Michel Dänzer <michel at daenzer.net>
>>> Cc: Nicolai Hähnle <nhaehnle at gmail.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>
>>> ---
>>> Michel, Nicolai any naming suggestions or input in general will be
>>> appreciated.
>>
>> It all looks good to me in general, thanks for doing this! I just have
>> a couple of minor suggestions for this patch which might make the code
>> clearer, feel free to take them or leave them. Either way, patches 3-5
>> are
>>
>> Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
>
> On further thought, I wonder if maybe drmGetDevice[s]2 should default to
> not retrieving the PCI revision, unless a flag is set, say
> DRM_DEVICE_GET_PCI_REVISION? That would avoid unnecessarily using the
> config file if the caller forgets to set the flag even though it doesn't
> need the revision.
>
Not 100% sold on the reasoning (if someone forgets...) yet making the
revision opt-in (as opposed to opt-out) makes sense.

> Though in that case, maybe the revision_id field should also be set to
> 0xff without the flag, to avoid callers forgetting to set the flag and
> getting an incorrect but plausible(?) 0 for the revision.
>
All suggestions sound amazing, thank you !

Barring any objections, I'll re-spin the series and send one for Mesa
later on today.

Thanks again,
Emil