Re: [PATCH v1] xf86drm: Add drmHandleMatch func

From: Robert Foss <robert.foss@collabora.com>
To: Emil Velikov <emil.l.velikov@gmail.com>,
	Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Tomasz Figa <tfiga@chromium.org>,
	Chih-Wei Huang <cwhuang@linux.org.tw>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Daniel Stone <daniel.stone@collabora.com>,
	Stefan Schake <stschake@gmail.com>
Subject: Re: [PATCH v1] xf86drm: Add drmHandleMatch func
Date: Mon, 30 Apr 2018 10:04:57 +0200	[thread overview]
Message-ID: <704a0ad4-5b60-0bf4-41e9-46e6c30d96d1@collabora.com> (raw)
In-Reply-To: <CACvgo52iQfuoO+u=m8S40+G3GaVYr-rq4BaAOPp=m1SUK7nELQ@mail.gmail.com>

Hey Emil,

On 27.04.2018 15:48, Emil Velikov wrote:
> On 27 April 2018 at 12:31, Robert Foss <robert.foss@collabora.com> wrote:
>> drmHandleMatch is intended to allow for userspace to filter out
>> devices that it does not want to open.
>>
>> Opening specific devices using paths alone is not a reliable due to
>> probing order. This function intends to provide a mechanism
>> for filtering out devices that don't fit what you need using an
>> extensible set of filters.
>>
>> drm_match_key_t is intended to be extended with whatever
>> filter that would come in handy down the line.
>>
>> As a catch-all filter, the DRM_MATCH_FUNCTION was included
>> which allows the caller to filter based on an arbitrary function.
>>
>> An function pointer filter could of course filter based on
>> anything. But for the sake of convenience a few other simple
>> filters have been included.
>>
>> If the function pointer filter ends up being called with a
>> boilerplate fp by mutliple libdrm users, perhaps that funtion
>> could be moved into libdrm at a future date.
>>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> ---
>>
>> This patch implements a simple function for matching DRM device FDs
>> against the desired properties of a device that you are looking for.
>>
>> The discussion that led to this series can be found here:
>> https://lists.freedesktop.org/archives/mesa-dev/2018-January/183878.html
>>
>> The RFC can be found here:
>> https://www.spinics.net/lists/dri-devel/msg172398.html
>>
>> Since the RFC I opted to rework the patch to be more extensible.
>> The previous implementation would have been problematic if the
>> drmVersion or drmDevice structs ever needed to be changed or
>> removed. Or indeed if more properties were to become interesting.
>>
>> As it is now, it is basially implemented as per Daniel Stones suggestion in
>> the RFC discussion.
>>
>> Changes since RFC:
>>   - Reworked proposal to be not be based on structs in order to be more
>>     flexible.
>>   - Now uses filters of different types.
>>   - Caller can supply any number of predefined and function pointer
>>     filter.
>>
>>   xf86drm.h     | 24 ++++++++++++++++++++
>>   xf86drmMode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 94 insertions(+)
>>
> Instead of spending too much time thinking of a future-proof API, can
> I suggest getting a handful of users first.
> And only after that lifting it to a helper - If needed.
> 
> Here are some examples - anv [1] and xf86-video-amdgpu [2]. Note
> latter is not merged yet.
> Practically any implementation is as trivial as:
> 
> ret = drmGetDevices2(...)
> for (i = 0; i < ret; i++) {
>      compare_device_specifics(...)
> }
> drmFreeDevices(...)
> 
> What do you guys think?

I was hoping to have all probing functionality eventually implemented in libdrm, 
since it is duplicated in multiple codebases. Where this function&patch would be 
the first step.

But there seems to be some friction to this idea, so maybe it dropping this 
patch is the way to go forward.

> Emil
> 
> [1] https://cgit.freedesktop.org/mesa/mesa/tree/src/intel/vulkan/anv_device.c#n615
> [2] https://lists.freedesktop.org/archives/amd-gfx/2018-April/020936.html
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel