On Tue, Feb 20, 2018 at 4:26 AM, Tomasz Figa <tf...@chromium.org> wrote: > On Tue, Feb 20, 2018 at 6:51 PM, Robert Foss <robert.f...@collabora.com> > wrote: >> Hey Tomasz, >> >> On 02/20/2018 09:55 AM, Tomasz Figa wrote: >>> >>> Hi Rob, >>> >>> On Fri, Feb 16, 2018 at 11:48 PM, Tomasz Figa <tf...@chromium.org> wrote: >>>> >>>> On Fri, Feb 16, 2018 at 11:33 PM, Robert Foss <robert.f...@collabora.com> >>>> wrote: >>>>> >>>>> Hey Tomasz, >>>>> >>>>> >>>>> On 02/16/2018 05:10 AM, Tomaszzz Figa wrote: >>>>>> >>>>>> >>>>>> On Fri, Feb 9, 2018 at 11:06 PM, Rob Herring <r...@kernel.org> wrote: >>>>>>> >>>>>>> >>>>>>> On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa <tfiga@chromi >>>>>>>>>> >>>>>>>>>> On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa <tf...@chromium.org> >>>>>>>>>> >>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Hi Rob, >>>>>>>>>>> >>>>>>>>>>> On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss >>>>>>>>>>> <robert.f...@collabora.com> wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> uint32_t (*get_fd)(buffer_handle_t handle, uint32_t >>>>>>>>>>>>>>>>> plane); >>>>>>>>>>>>>>>>> uint64_t (*get_modifier)(buffer_handle_t handle, >>>>>>>>>>>>>>>>> uint32_t >>>>>>>>>>>>>>>>> plane); >>>>>>>>>>>>>>>>> uint32_t (*get_offsets)(buffer_handle_t handle, >>>>>>>>>>>>>>>>> uint32_t >>>>>>>>>>>>>>>>> plane); >>>>>>>>>>>>>>>>> uint32_t (*get_stride)(buffer_handle_t handle, >>>>>>>>>>>>>>>>> uint32_t >>>>>>>>>>>>>>>>> plane); >>>>>>>>>>>>>>>>> ... >>>>>>>>>>>>>>>>> } gralloc_funcs_t; >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> These ones? > >>>>>>>>>>>>> Yeah, if we could retrieve such function pointer struct using >>>>>>>>>>>>> perform >>>>>>>>>>>>> or any equivalent (like the implementation-specific methods in >>>>>>>>>>>>> gralloc1, but not sure if that's going to be used in practice >>>>>>>>>>>>> anywhere), it could work for us. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> So this is where you and Rob Herring lose me, I don't think I >>>>>>>>>>>> understand >>>>>>>>>>>> quite how the gralloc1 call would be used, and how it would tie >>>>>>>>>>>> into >>>>>>>>>>>> this >>>>>>>>>>>> handle struct. I think I could do with some guidance on this. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> This would be very similar to gralloc0 perform call. gralloc1 >>>>>>>>>>> implementations need to provide getFunction() callback [1], which >>>>>>>>>>> returns a pointer to given function. The list of standard >>>>>>>>>>> functions >>>>>>>>>>> is >>>>>>>>>>> defined in the gralloc1.h header [2], but we could take some >>>>>>>>>>> random >>>>>>>>>>> big number and use it for our function that fills in provided >>>>>>>>>>> gralloc_funcs_t struct with necessary pointers. >>>>>>>>>>> >>>>>>>>>>> [1] >>>>>>>>>>> >>>>>>>>>>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300 >>>>>>>>>>> [2] >>>>>>>>>>> >>>>>>>>>>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134 >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> This is a deadend because it won't work with a HIDL based >>>>>>>>>> implementation (aka gralloc 2.0). You can't set function pointers >>>>>>>>>> (or >>>>>>>>>> any pointers) because gralloc runs in a different process. Yes, >>>>>>>>>> currently gralloc is a pass-thru HAL, but AIUI that will go away. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Part of it. I can't see IMapper being implemented by a separate >>>>>>>>> process. You can't map a buffer into one process from another >>>>>>>>> process. >>>>>>>>> >>>>>>>>> But anyway, it's a good point, thanks, I almost forgot about its >>>>>>>>> existence. I'll do further investigation. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Okay, so IMapper indeed breaks the approach I suggested. I'm not sure >>>>>>>> at the moment what we could do about it. (The idea of a dynamic >>>>>>>> library of a pre-defined name, exporting functions we specify, might >>>>>>>> still work, though.) >>>>>>>> >>>>>>>> Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be >>>>>>>> impossible to implement with IAllocator/IMapper. (Although I still >>>>>>>> think Mesa and Gralloc are free to have separate logic for choosing >>>>>>>> the DRM device to use.) >>>>>>> >>>>>>> >>>>>>> >>>>>>> I think the need for GET_FD goes away when the render node is used. We >>>>>>> may still need the card node for s/w rendering (if I can ever get that >>>>>>> working) though. Of course, if we use the vgem approach like CrOS then >>>>>>> we wouldn't. >>>>>> >>>>>> >>>>>> >>>>>> Hmm, if so, then we probably wouldn't have any strict need for these >>>>>> function pointers anymore. We already have a makeshift format resolve >>>>>> in place and the only missing bits that we still need to patch up >>>>>> downstream are removing GET_FD, dropping drm_gralloc.h and adding a >>>>>> fallback to kms_swrast if hw driver loading fails. >>>>> >>>>> >>>>> >>>>> So this discussion is slightly unrelated, but it is where me looking at >>>>> this >>>>> started. >>>>> >>>>> So I've got a kms_swrast fallback series[1], that I've been wanting to >>>>> test >>>>> before trying to push upstream, but haven't been able to run it in the >>>>> Android environment and the arc++ + chromiumos has also been problematic >>>>> for >>>>> various reasons (which are being looked into). >>>>> >>>>> Tomasz: If you're interested in testing the series, it would be helpful. >>>>> Hopefully testing is everything that needed for upstreaming. >>>>> >>>>> [1] https://gitlab.collabora.com/robertfoss/mesa/commits/kms_swrast >>>> >>>> >>> >>> I checked the branch, but it isn't possible to test it directly with >>> Chrome OS, since we do not provide DRM_GRALLOC_GET_FD functionality >>> and adding support for probing devices changes the code introduced by >>> your patches significantly, which kind of defeats the purpose. >>> >>> I think it might make sense to actually remove the requirement for >>> DRM_GRALLOC_GET_FD from upstream first (as in my original attempt from >>> long time ago) and then add kms_swrast fallback on top of that. >>> >> >> I would like to look into this, > > That would be much appreciated! > >> is your previous DRM_GRALLOC_GET_FD removal >> attempt available somewhere? > > Yep. > > https://patchwork.freedesktop.org/patch/102547/ > >> >> What kind of snags did you end up hitting? Just upstream approval or >> technical ones? > > Well, something in the middle. The probing of render nodes didn't seem > to be very well received.
Perhaps worth revisiting. Given we've failed to progress at all since then may change opinions some. We already have to handle multiple opens share the same pipe_screen, so I don't think reusing the fd buys us anything. Maybe we're close to the point of removing the flink name support too. The android-x86 folks have been working to get dma-bufs working. Chih-Wei, any comments on this? > Having the device path read from a property > was suggested, but it wouldn't work for us, since we use the same > Android image on different boards, where the order of probing of DRM > drivers might be different (and so the paths). Reading DRM driver name > from a property could work, though... How would that work if you support different GPUs in one image? Rob _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
