|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
On Wed, Sep 30, 2020 at 2:34 PM Christian König
<christian.koenig@xxxxxxx> wrote:
>
> Am 30.09.20 um 11:47 schrieb Daniel Vetter:
> > On Wed, Sep 30, 2020 at 10:34:31AM +0200, Christian König wrote:
> >> Am 30.09.20 um 10:19 schrieb Thomas Zimmermann:
> >>> Hi
> >>>
> >>> Am 30.09.20 um 10:05 schrieb Christian König:
> >>>> Am 29.09.20 um 19:49 schrieb Thomas Zimmermann:
> >>>>> Hi Christian
> >>>>>
> >>>>> Am 29.09.20 um 17:35 schrieb Christian König:
> >>>>>> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann:
> >>>>>>> The new helper ttm_kmap_obj_to_dma_buf() extracts address and location
> >>>>>>> from and instance of TTM's kmap_obj and initializes struct dma_buf_map
> >>>>>>> with these values. Helpful for TTM-based drivers.
> >>>>>> We could completely drop that if we use the same structure inside TTM
> >>>>>> as
> >>>>>> well.
> >>>>>>
> >>>>>> Additional to that which driver is going to use this?
> >>>>> As Daniel mentioned, it's in patch 3. The TTM-based drivers will
> >>>>> retrieve the pointer via this function.
> >>>>>
> >>>>> I do want to see all that being more tightly integrated into TTM, but
> >>>>> not in this series. This one is about fixing the bochs-on-sparc64
> >>>>> problem for good. Patch 7 adds an update to TTM to the DRM TODO list.
> >>>> I should have asked which driver you try to fix here :)
> >>>>
> >>>> In this case just keep the function inside bochs and only fix it there.
> >>>>
> >>>> All other drivers can be fixed when we generally pump this through TTM.
> >>> Did you take a look at patch 3? This function will be used by VRAM
> >>> helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, we
> >>> have to duplicate the functionality in each if these drivers. Bochs
> >>> itself uses VRAM helpers and doesn't touch the function directly.
> >> Ah, ok can we have that then only in the VRAM helpers?
> >>
> >> Alternative you could go ahead and use dma_buf_map in ttm_bo_kmap_obj
> >> directly and drop the hack with the TTM_BO_MAP_IOMEM_MASK.
> >>
> >> What I want to avoid is to have another conversion function in TTM because
> >> what happens here is that we already convert from ttm_bus_placement to
> >> ttm_bo_kmap_obj and then to dma_buf_map.
> > Hm I'm not really seeing how that helps with a gradual conversion of
> > everything over to dma_buf_map and assorted helpers for access? There's
> > too many places in ttm drivers where is_iomem and related stuff is used to
> > be able to convert it all in one go. An intermediate state with a bunch of
> > conversions seems fairly unavoidable to me.
>
> Fair enough. I would just have started bottom up and not top down.
>
> Anyway feel free to go ahead with this approach as long as we can remove
> the new function again when we clean that stuff up for good.
Yeah I guess bottom up would make more sense as a refactoring. But the
main motivation to land this here is to fix the __mmio vs normal
memory confusion in the fbdev emulation helpers for sparc (and
anything else that needs this). Hence the top down approach for
rolling this out.
-Daniel
>
> Christian.
>
> > -Daniel
> >
> >> Thanks,
> >> Christian.
> >>
> >>> Best regards
> >>> Thomas
> >>>
> >>>> Regards,
> >>>> Christian.
> >>>>
> >>>>> Best regards
> >>>>> Thomas
> >>>>>
> >>>>>> Regards,
> >>>>>> Christian.
> >>>>>>
> >>>>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> >>>>>>> ---
> >>>>>>> include/drm/ttm/ttm_bo_api.h | 24 ++++++++++++++++++++++++
> >>>>>>> include/linux/dma-buf-map.h | 20 ++++++++++++++++++++
> >>>>>>> 2 files changed, 44 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/include/drm/ttm/ttm_bo_api.h
> >>>>>>> b/include/drm/ttm/ttm_bo_api.h
> >>>>>>> index c96a25d571c8..62d89f05a801 100644
> >>>>>>> --- a/include/drm/ttm/ttm_bo_api.h
> >>>>>>> +++ b/include/drm/ttm/ttm_bo_api.h
> >>>>>>> @@ -34,6 +34,7 @@
> >>>>>>> #include <drm/drm_gem.h>
> >>>>>>> #include <drm/drm_hashtab.h>
> >>>>>>> #include <drm/drm_vma_manager.h>
> >>>>>>> +#include <linux/dma-buf-map.h>
> >>>>>>> #include <linux/kref.h>
> >>>>>>> #include <linux/list.h>
> >>>>>>> #include <linux/wait.h>
> >>>>>>> @@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct
> >>>>>>> ttm_bo_kmap_obj *map,
> >>>>>>> return map->virtual;
> >>>>>>> }
> >>>>>>> +/**
> >>>>>>> + * ttm_kmap_obj_to_dma_buf_map
> >>>>>>> + *
> >>>>>>> + * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap.
> >>>>>>> + * @map: Returns the mapping as struct dma_buf_map
> >>>>>>> + *
> >>>>>>> + * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the
> >>>>>>> memory
> >>>>>>> + * is not mapped, the returned mapping is initialized to NULL.
> >>>>>>> + */
> >>>>>>> +static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj
> >>>>>>> *kmap,
> >>>>>>> + struct dma_buf_map *map)
> >>>>>>> +{
> >>>>>>> + bool is_iomem;
> >>>>>>> + void *vaddr = ttm_kmap_obj_virtual(kmap, &is_iomem);
> >>>>>>> +
> >>>>>>> + if (!vaddr)
> >>>>>>> + dma_buf_map_clear(map);
> >>>>>>> + else if (is_iomem)
> >>>>>>> + dma_buf_map_set_vaddr_iomem(map, (void __force __iomem
> >>>>>>> *)vaddr);
> >>>>>>> + else
> >>>>>>> + dma_buf_map_set_vaddr(map, vaddr);
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> /**
> >>>>>>> * ttm_bo_kmap
> >>>>>>> *
> >>>>>>> diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
> >>>>>>> index fd1aba545fdf..2e8bbecb5091 100644
> >>>>>>> --- a/include/linux/dma-buf-map.h
> >>>>>>> +++ b/include/linux/dma-buf-map.h
> >>>>>>> @@ -45,6 +45,12 @@
> >>>>>>> *
> >>>>>>> * dma_buf_map_set_vaddr(&map. 0xdeadbeaf);
> >>>>>>> *
> >>>>>>> + * To set an address in I/O memory, use
> >>>>>>> dma_buf_map_set_vaddr_iomem().
> >>>>>>> + *
> >>>>>>> + * .. code-block:: c
> >>>>>>> + *
> >>>>>>> + * dma_buf_map_set_vaddr_iomem(&map. 0xdeadbeaf);
> >>>>>>> + *
> >>>>>>> * Test if a mapping is valid with either dma_buf_map_is_set() or
> >>>>>>> * dma_buf_map_is_null().
> >>>>>>> *
> >>>>>>> @@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct
> >>>>>>> dma_buf_map *map, void *vaddr)
> >>>>>>> map->is_iomem = false;
> >>>>>>> }
> >>>>>>> +/**
> >>>>>>> + * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to
> >>>>>>> an address in I/O memory
> >>>>>>> + * @map: The dma-buf mapping structure
> >>>>>>> + * @vaddr_iomem: An I/O-memory address
> >>>>>>> + *
> >>>>>>> + * Sets the address and the I/O-memory flag.
> >>>>>>> + */
> >>>>>>> +static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map
> >>>>>>> *map,
> >>>>>>> + void __iomem *vaddr_iomem)
> >>>>>>> +{
> >>>>>>> + map->vaddr_iomem = vaddr_iomem;
> >>>>>>> + map->is_iomem = true;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> /**
> >>>>>>> * dma_buf_map_is_equal - Compares two dma-buf mapping structures
> >>>>>>> for equality
> >>>>>>> * @lhs: The dma-buf mapping structure
> >>>>>> _______________________________________________
> >>>>>> dri-devel mailing list
> >>>>>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> >>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7C472c3d655a61411deb6708d86525d1b8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637370560438965013&sdata=HdHOA%2F1VcIX%2F7YtfYTiAqYEvw7Ag%2FS%2BxS5VwJKOv5y0%3D&reserved=0
> >>>>> _______________________________________________
> >>>>> amd-gfx mailing list
> >>>>> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> >>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cchristian.koenig%40amd.com%7C472c3d655a61411deb6708d86525d1b8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637370560438965013&sdata=H%2B5HKCsTrksRV2EyEiFGSTyS79jsWCmJimSMoJYusx8%3D&reserved=0
> >>>> _______________________________________________
> >>>> dri-devel mailing list
> >>>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> >>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7C472c3d655a61411deb6708d86525d1b8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637370560438965013&sdata=HdHOA%2F1VcIX%2F7YtfYTiAqYEvw7Ag%2FS%2BxS5VwJKOv5y0%3D&reserved=0
> >>>>
> >>> _______________________________________________
> >>> amd-gfx mailing list
> >>> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cchristian.koenig%40amd.com%7C472c3d655a61411deb6708d86525d1b8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637370560438965013&sdata=H%2B5HKCsTrksRV2EyEiFGSTyS79jsWCmJimSMoJYusx8%3D&reserved=0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |