On 4 May 2017 at 19:43, 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 > For i965 : > drmCommandNone > drmCommandWriteRead > drmIoctl > drmPrimeFDToHandle > drmPrimeHandleToFD > > You're right, I'll update the patch to just drop libdrm_intel. > Don't forget to remove the link against libdrm.
>> >>>>>> Why - adds a, yet another, copy and making synchronisation more >>>>>> annoying. First example - you blindly copied the files rather than >>>>>> using `make headers_install' first ;-) >>>>>> Not to mention that it makes the chicken and egg problem* even more >>>>>> confusing and error prone. >>>> >>>> It doesn't feel like that to me. Actually instead of modifying 3 >>>> different >>>> repos, you end up modifying just 2. >>>> Sounds less error prone. >>>> >> Lionel, are you saying that we can ignore IGT? Or you're suggesting >> that IGT should depend on Mesa copy of the headers? > > > If you look closely at IGT, you'll notice quite a few tests actually define > their own version of the structures/ioctl of the various driver interfaces. > So it's more or less already happening. > > git grep DRM_IO ./tests/ | grep define > git grep local_drm > > IGT has local defines which get purged every so often. Still "modifying just 2" is off-by one. >> Seriously, your argument of "let's import because we can" is iffy. Why >> stop with the DRM UAPI - let's import headers from glibc ;-) > > > I think you have to look at what we're doing here. i965 & anv are graphics > drivers tightly coupled with the kernel driver. > libdrm_intel isn't, it's mostly generic enough code that is shared across > some of our drivers. > And since we drop that dependency, why bother with it at all? > > We don't really have the same relationship to other components (like glibc). > >> >> If pulling new libdrm is that much of a nuisance to your workflow - >> just copy the blob we have for the Travis instance. >> It automatically tracks the libdrm version, builds and installs it as >> needed. > > > It's not about pulling, it's about maintaining. > The libdrm headers will still need to be synced and maintained regardless of this series. I wish we could make it disappear but sadly we cannot. Speaking of maintenance can you comment on this patch [1]? We have a similar instance in Mesa which we might want to address. [1] https://patchwork.kernel.org/patch/9626861/ >> >>>>>> Emil >>>>>> * Which patches land first - kernel or userspace >>>>> >>>>> I don't see how it does that at all. It simplifies the process: Now >>>>> you >>>>> send out your proposed new userspace to one repo, instead of two. >>>>> >>>>> And, after the first commit, it'll be obvious when you screw up using >>>>> make headers_install because there will be surprising diff. >>>> >>>> Right now we have to update libdrm, then update the mesa to depend on >>>> the >>>> right libdrm to actually get the header files we want. >>>> If you depend on libdrm from more than just uapi headers, it might now >>>> be >>>> too much of a burden. But it seems we're clearly moving away from that >>>> in >>>> anv/i965. >>>> As a developer it feels a lot easier to have just the update mesa with >>>> both >>>> the new kernel API you depend on & the changes to the user space driver >>>> using that API. >>> >>> As long as the headers are never installed into the system I'm in >>> principle ok with pulling all the i915.ko specific stuff into mesa. Seems >>> like a reasonable thing to do. >>> >> Daniel, Lionel's earlier suggestion (see the "modifying just 2" part >> above) implies that either a) Mesa should install these or b) we can >> ignore other components such as IGT. >> Neither of which sounds cool IMHO. > > > Feel pretty cool to me. Which part - a or b? > I don't think I can put it better than Eric did : > > On 04/05/17 09:38, Eric Anholt wrote: >> >> And it works great, because kernel headers are backwards compatible. >> When you need a feature, you just merge the header update necessary and >> no other developers get disrupted. >> >> Have you done kernel API feature development? I feel like this is the >> kind of thing you need to do yourself several times, with several >> revisions over the course of months, before understanding the >> limitations of our current process. > Once we find a volunteer to fix the bugs that I go through, I would gladly dive into kernel API development. At the same time - the only downside AFAICT of the current process is that it's not clear and concise enough. Before I wrote a document for libdrm - close to nobody knew the process, let alone have a change to follow it. I wish that after our chat (which inspired the v4 of [2]) you would trust me a little bit more. Gents, do as you wish - I won't interfere with any of this. I do reserve myself the right of "I told you so" should any of my predictions come true ;-) Thanks Emil [2] https://cgit.freedesktop.org/mesa/drm/commit/?id=3c717f61f885240980bfc4273dbd1fc837edc391 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
