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