Messages in this thread | | | Date | Thu, 27 Jan 2022 01:12:47 -0800 | From | Lucas De Marchi <> | Subject | Re: [Intel-gfx] [PATCH 02/19] dma-buf-map: Add helper to initialize second map |
| |
On Thu, Jan 27, 2022 at 09:55:05AM +0100, Christian König wrote: >Am 27.01.22 um 09:18 schrieb Lucas De Marchi: >>On Thu, Jan 27, 2022 at 09:02:54AM +0100, Christian König wrote: >>>Am 27.01.22 um 08:57 schrieb Lucas De Marchi: >>>>On Thu, Jan 27, 2022 at 08:27:11AM +0100, Christian König wrote: >>>>>Am 26.01.22 um 21:36 schrieb Lucas De Marchi: >>>>>>[SNIP] >>>>humn... not sure if I was clear. There is no importer and >>>>exporter here. >>> >>>Yeah, and exactly that's what I'm pointing out as problem here. >>> >>>You are using the inter driver framework for something internal to >>>the driver. That is an absolutely clear NAK! >>> >>>We could discuss that, but you guys are just sending around >>>patches to do this without any consensus that this is a good idea. >> >>s/you guys/you/ if you have to blame anyone - I'm the only s-o-b in >>these patches. I'm sending these to _build consensus_ on what may be >>a good >>use for it showing a real problem it's helping to fix. > >Well a cover letter would have been helpful, my impression was that >you have a larger set and just want to upstream some minor DMA-buf >changes necessary for it.
I missed adding this sentence to the cover letter, as my impression was that dma-buf-map was already used outside inter-driver framework. But there is actually a cover letter:
https://lore.kernel.org/all/20220126203702.1784589-1-lucas.demarchi@intel.com/
And looking at it now, it seems I missed adding Thomas Zimmermann to Cc.
> >Now I know why people are bugging me all the time to add cover letters >to add more context to my sets. > >> >>From its documentation: >> >> * The type :c:type:`struct dma_buf_map <dma_buf_map>` and its >>helpers are >> * actually independent from the dma-buf infrastructure. When >>sharing buffers >> * among devices, drivers have to know the location of the memory to >>access >> * the buffers in a safe way. :c:type:`struct dma_buf_map <dma_buf_map>` >> * solves this problem for dma-buf and its users. If other drivers or >> * sub-systems require similar functionality, the type could be >>generalized >> * and moved to a more prominent header file. >> >>if there is no consensus and a better alternative, I'm perfectly fine in >>throwing it out and using the better approach. > >When Thomas Zimmermann upstreamed the dma_buf_map work we had a >discussion if that shouldn't be independent of the DMA-buf framework. > >The consensus was that as soon as we have more widely use for it this >should be made independent. So basically that is what's happening now. > >I suggest the following approach: >1. Find a funky name for this, something like iomem_, kiomap_ or similar.
iosys_map?
>2. Separate this from all you driver dependent work and move the >dma_buf_map structure out of DMA-buf into this new whatever_ prefix.
should this be a follow up to the driver work or a prerequisite?
thanks Lucas De Marchi
>3. Ping Thomas, LKML, me and probably a couple of other core people if >this is the right idea or not. >4. Work on dropping the map parameter from dma_buf_vunmap(). This is >basically why we can't modify the pointers returned from >dma_buf_vmap() and has already cause a few problems with >dma_buf_map_incr(). > >Regards, >Christian. > >> >>Lucas De Marchi >
| |