<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, May 5, 2017 at 8:32 AM, Emil Velikov <span dir="ltr"><<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 4 May 2017 at 21:56, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> On Thu, May 4, 2017 at 12:21 PM, Kristian Høgsberg <<a href="mailto:hoegsberg@gmail.com">hoegsberg@gmail.com</a>><br>
> wrote:<br>
>><br>
>> On Thu, May 4, 2017 at 11:43 AM, Lionel Landwerlin<br>
>> <<a href="mailto:lionel.g.landwerlin@intel.com">lionel.g.landwerlin@intel.com</a><wbr>> wrote:<br>
>> > On 04/05/17 07:52, Emil Velikov wrote:<br>
>> >><br>
>> >> On 4 May 2017 at 14:46, Daniel Vetter <<a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a>> wrote:<br>
>> >>><br>
>> >>> On Thu, Apr 27, 2017 at 10:58:42AM -0700, Lionel Landwerlin wrote:<br>
>> >>>><br>
>> >>>> On 27/04/17 08:20, Eric Anholt wrote:<br>
>> >>>>><br>
>> >>>>> Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>> writes:<br>
>> >>>>><br>
>> >>>>>> On 25 April 2017 at 23:56, Lionel Landwerlin<br>
>> >>>>>> <<a href="mailto:lionel.g.landwerlin@intel.com">lionel.g.landwerlin@intel.com</a><wbr>> wrote:<br>
>> >>>>>>><br>
>> >>>>>>> Hi,<br>
>> >>>>>>><br>
>> >>>>>>> While working with changes that span from kernel to user space,<br>
>> >>>>>>> I've<br>
>> >>>>>>> been wondering whether we need to depend on libdrm at all for the<br>
>> >>>>>>> anv<br>
>> >>>>>>> & i965 drivers. Indeed with Ken's recent changes, we only depend<br>
>> >>>>>>> on<br>
>> >>>>>>> libdrm for its kernel header files which we could just embed<br>
>> >>>>>>> ourselves.<br>
>> >>>>>>><br>
>> >>>>>>> I've only included the minimal set of header files we need from<br>
>> >>>>>>> the<br>
>> >>>>>>> kernel for anv & i965. Maybe other drivers would be interested and<br>
>> >>>>>>> maybe we should put all the kernel drm uapi headers into include?<br>
>> >>>>>>><br>
>> >>>>>> AFAICT the goal behind the libdrm_intel fold was to allow rapid and<br>
>> >>>>>> seamless rework of the interface.<br>
>> >>>>>> With a potential goal to make it a shared one, as it gets<br>
>> >>>>>> stabilised.<br>
>> >>>>>><br>
>> >>>>>> Currently ANV uses more than just the UAPI headers. But if we<br>
>> >>>>>> ignore<br>
>> >>>>>> that, coping them is a bad idea.<br>
>> >>>><br>
>> >>>> What else is anv using from libdrm? (maybe I missed something)<br>
>> >>>><br>
>> >> Lionel, I stand corrected, we have instances in anv, wsi/x11, loader<br>
>> >> and dri/i965 (the dri/common ones are just comments).<br>
>> >> In total they seem to be over three dozen instances. Experiment with<br>
>> >> the following:<br>
>> >><br>
>> >> for i in `nm -CD --defined-only /lib/libdrm.so | cut -c 20-| grep -v<br>
>> >> "^_" `; do git grep -w --name-only $i -- src ; done<br>
>> ><br>
>> ><br>
>> > Thanks for the code snippet.<br>
>> ><br>
>> > For anv :<br>
>> > drmGetDevices2<br>
>><br>
>> anv was designed to not rely on libdrm or libdrm_intel. I see the<br>
>> commit that added the libdrm dependency is from you and was not<br>
>> Reviewed or acked by Jason or any other core anv developer. I suggest<br>
>> we revert that, I don't see anything in the drmGetDevices2 code that<br>
>> is better suited for anv than what was there before.<br>
><br>
><br>
> I did comment on e-mail that I was begrudgingly ok with it. In retrospect,<br>
> it looks pretty pointless. As far as I can tell, drmGetDevices2 gains us<br>
> exactly two things over the old method of just trial and error:<br>
><br>
> 1. It stats the file to make sure that it's an actual DRM device before<br>
> opening the file. Does this actually matter? If so, it's easy enough to<br>
> add the dozen or so lines of code to do it in ANV.<br>
> 2. It walks over all files in /dev/dri rather than just /dev/dri/renderD#.<br>
> That's also very easy to add.<br>
><br>
> I agree with Kristian that the right thing to do is to revert that patch and<br>
> just write the dozen lines of code needed to do it "properly" if it even<br>
> matters.<br>
><br>
</div></div>Reverting the patch will lead to<br>
- 3s starup delays since the open() will wake up the $other GPU<br>
Rare, but we've had reports already.<br>
- slower porting/adoption of Vulkan on !Linux Unixes<span class="HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>I had a chat with Dave Airlie about it on IRC and I'm back to begrudgingly accepting the dependency. The "porting to BSD" argument was what finally won me over. In any case, if all we rely on is drmGetDevice2, then the version of libdrm you have basically stops mattering which is nice. <br></div></div><br></div></div>