Re: [PATCH 00/14] Rename dma-buf-map
From: Christian König
Date: Fri Jan 28 2022 - 04:48:54 EST
Am 28.01.22 um 10:40 schrieb Lucas De Marchi:
On Fri, Jan 28, 2022 at 10:22:00AM +0100, Christian König wrote:
Am 28.01.22 um 10:12 schrieb Lucas De Marchi:
On Fri, Jan 28, 2022 at 09:41:14AM +0100, Christian König wrote:
Rule #1 is to never ever break the build.
Because of this all those patches needs to be squashed into a
single one as far as I can see.
what config are you building on?
Well I'm not building at all, I'm just looking at the patches as an
engineer with 25 years of experience with Linux patches.
Just take a look at patch number 2:
-static int fastrpc_vmap(struct dma_buf *dmabuf, struct dma_buf_map
*map)
+static int fastrpc_vmap(struct dma_buf *dmabuf, struct iosys_map *map)
You are changing the functions signature without changing any of the
callers.
At bare minimum that causes a warning and on runtime this only works
by coincident now because the structure pointers just happen to have
the same layout. This is not something we usually do.
you missed the magic/hack on patch 1:
1) dma-buf-map.h includes iosys-map.h _at the end_
2) iosys-map.h includes dma-buf-map.h at the beginning
and initially does a "define iosys_map dma_buf_map".
So, it doesn't work by coincidence, It's because it was done to allow
converting it piecemeal.
Oh, my. Please never do stuff like that again.
But as I said, I don't really have a preference. When crossing
subsystems one thing that is hard is that different people have different
preferences on these things. At least squashing now is much easier than
if I had to split it
Try to imagine how much complain I received on going the other way in
25985edcedea6396277003854657b5f3cb31a628 with
2463 files changed, 4252 insertions(+), 4252 deletions(-)
Well exactly that is perfectly fine.
What you do here is applying your personal hack which is absolutely not
welcomed.
Regards,
Christian.
:)
Lucas De Marchi
Regards,
Christian.
I built this series, full config with
CONFIG_COMPILE_TEST and doing:
git rebase -i <base> -x "make -j$(nproc)"
I split these patches in a way that wouldn't break the build on
purpose.
There were a couple that I couldn't build without cross compiling:
tegra
and rockchip. The others were ok.
I'm not really against squashing everything in one to merge, though.
It will be hard on the conflicts later, but should get the job done
much
quicker.
Lucas De Marchi
Regards,
Christian.
Am 28.01.22 um 09:36 schrieb Lucas De Marchi:
Motivation for this started in
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20220126203702.1784589-1-lucas.demarchi%40intel.com%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C635084a520994d35a16e08d9e2423319%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637789596221829397%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ruHpD3DbyyqQuZIFEQU%2B2RH31OwsdFnn1v7N4z75U0Y%3D&reserved=0
when trying to extend the dma-buf-map API to cover new use cases:
help a
single driver with allocations and sharing code paths for IO and
system
memory. I'm leaving the API additions aside and first renaming the
interface as requested.
There are already some users in tree outside the context of dma-buf
importer/exporter. So before extending the API, let's dissociate
it from
dma-buf.
The iosys-map.h is introduced in the first patch in a way that allows
the conversion of each driver to happen separately. After all the
conversions are done we can remove the old one, which is the last
patch.
Another possible way is to squash everything and merge together,
but I believe this would make much harder for review.
The conversion was done with the following semantic patch:
@r1@
@@
- struct dma_buf_map
+ struct iosys_map
@r2@
@@
(
- DMA_BUF_MAP_INIT_VADDR
+ IOSYS_MAP_INIT_VADDR
|
- dma_buf_map_set_vaddr
+ iosys_map_set_vaddr
|
- dma_buf_map_set_vaddr_iomem
+ iosys_map_set_vaddr_iomem
|
- dma_buf_map_is_equal
+ iosys_map_is_equal
|
- dma_buf_map_is_null
+ iosys_map_is_null
|
- dma_buf_map_is_set
+ iosys_map_is_set
|
- dma_buf_map_clear
+ iosys_map_clear
|
- dma_buf_map_memcpy_to
+ iosys_map_memcpy_to
|
- dma_buf_map_incr
+ iosys_map_incr
)
@@
@@
- #include <linux/dma-buf-map.h>
+ #include <linux/iosys-map.h>
and then some files had their includes adjusted so we can build
everything on each commit in this series. Also some comments were
update
to remove mentions to dma-buf-map. Simply doing a sed to rename
didn't
work as dma-buf has some APIs using the dma_buf_map prefix.
Once finalized, I think most of this, if not all, could go through
the
drm-misc-next branch. I split i915, msm, nouveau, and radeon in their
own patches in case it's preferred to take those through their own
trees.
Lucas De Marchi
Lucas De Marchi (14):
iosys-map: Introduce renamed dma-buf-map
misc: fastrpc: Replace dma-buf-map with iosys-map
dma-buf: Replace dma-buf-map with iosys-map
media: Replace dma-buf-map with iosys-map
drm/ttm: Replace dma-buf-map with iosys-map
drm: Replace dma-buf-map with iosys-map in drivers
drm/i915: Replace dma-buf-map with iosys-map
drm/msm: Replace dma-buf-map with iosys-map
drm/nouveau: Replace dma-buf-map with iosys-map
drm/tegra: Replace dma-buf-map with iosys-map
drm/radeon: Replace dma-buf-map with iosys-map
drm: Replace dma-buf-map with iosys-map in common code
Documentation: Refer to iosys-map instead of dma-buf-map
dma-buf-map: Remove API in favor of iosys-map
Documentation/driver-api/dma-buf.rst | 4 +-
Documentation/gpu/todo.rst | 20 +-
MAINTAINERS | 2 +-
drivers/dma-buf/dma-buf.c | 22 +-
drivers/dma-buf/heaps/cma_heap.c | 10 +-
drivers/dma-buf/heaps/system_heap.c | 10 +-
drivers/gpu/drm/ast/ast_drv.h | 2 +-
drivers/gpu/drm/ast/ast_mode.c | 8 +-
drivers/gpu/drm/drm_cache.c | 18 +-
drivers/gpu/drm/drm_client.c | 9 +-
drivers/gpu/drm/drm_fb_helper.c | 12 +-
drivers/gpu/drm/drm_gem.c | 12 +-
drivers/gpu/drm/drm_gem_cma_helper.c | 9 +-
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 +-
drivers/gpu/drm/drm_gem_shmem_helper.c | 15 +-
drivers/gpu/drm/drm_gem_ttm_helper.c | 4 +-
drivers/gpu/drm/drm_gem_vram_helper.c | 25 +-
drivers/gpu/drm/drm_internal.h | 6 +-
drivers/gpu/drm/drm_mipi_dbi.c | 8 +-
drivers/gpu/drm/drm_prime.c | 4 +-
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 8 +-
drivers/gpu/drm/gud/gud_pipe.c | 4 +-
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 5 +-
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 8 +-
.../drm/i915/gem/selftests/i915_gem_dmabuf.c | 6 +-
.../gpu/drm/i915/gem/selftests/mock_dmabuf.c | 6 +-
drivers/gpu/drm/lima/lima_gem.c | 3 +-
drivers/gpu/drm/lima/lima_sched.c | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_gem.c | 7 +-
drivers/gpu/drm/mediatek/mtk_drm_gem.h | 5 +-
drivers/gpu/drm/mgag200/mgag200_mode.c | 4 +-
drivers/gpu/drm/msm/msm_drv.h | 4 +-
drivers/gpu/drm/msm/msm_gem_prime.c | 6 +-
drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +
drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 13 +-
drivers/gpu/drm/qxl/qxl_display.c | 8 +-
drivers/gpu/drm/qxl/qxl_draw.c | 6 +-
drivers/gpu/drm/qxl/qxl_drv.h | 10 +-
drivers/gpu/drm/qxl/qxl_object.c | 8 +-
drivers/gpu/drm/qxl/qxl_object.h | 4 +-
drivers/gpu/drm/qxl/qxl_prime.c | 4 +-
drivers/gpu/drm/radeon/radeon_gem.c | 1 +
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 9 +-
drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 5 +-
drivers/gpu/drm/tegra/gem.c | 10 +-
drivers/gpu/drm/tiny/cirrus.c | 8 +-
drivers/gpu/drm/tiny/gm12u320.c | 7 +-
drivers/gpu/drm/ttm/ttm_bo_util.c | 16 +-
drivers/gpu/drm/ttm/ttm_resource.c | 26 +-
drivers/gpu/drm/ttm/ttm_tt.c | 6 +-
drivers/gpu/drm/udl/udl_modeset.c | 3 +-
drivers/gpu/drm/vboxvideo/vbox_mode.c | 4 +-
drivers/gpu/drm/virtio/virtgpu_prime.c | 1 +
drivers/gpu/drm/vkms/vkms_composer.c | 4 +-
drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
drivers/gpu/drm/vkms/vkms_plane.c | 2 +-
drivers/gpu/drm/vkms/vkms_writeback.c | 2 +-
drivers/gpu/drm/xen/xen_drm_front_gem.c | 7 +-
drivers/gpu/drm/xen/xen_drm_front_gem.h | 6 +-
.../common/videobuf2/videobuf2-dma-contig.c | 8 +-
.../media/common/videobuf2/videobuf2-dma-sg.c | 9 +-
.../common/videobuf2/videobuf2-vmalloc.c | 11 +-
drivers/misc/fastrpc.c | 4 +-
include/drm/drm_cache.h | 6 +-
include/drm/drm_client.h | 7 +-
include/drm/drm_gem.h | 6 +-
include/drm/drm_gem_atomic_helper.h | 6 +-
include/drm/drm_gem_cma_helper.h | 6 +-
include/drm/drm_gem_framebuffer_helper.h | 8 +-
include/drm/drm_gem_shmem_helper.h | 12 +-
include/drm/drm_gem_ttm_helper.h | 6 +-
include/drm/drm_gem_vram_helper.h | 9 +-
include/drm/drm_prime.h | 6 +-
include/drm/ttm/ttm_bo_api.h | 10 +-
include/drm/ttm/ttm_kmap_iter.h | 10 +-
include/drm/ttm/ttm_resource.h | 6 +-
include/linux/dma-buf-map.h | 266
------------------
include/linux/dma-buf.h | 12 +-
include/linux/iosys-map.h | 257
+++++++++++++++++
80 files changed, 579 insertions(+), 552 deletions(-)
delete mode 100644 include/linux/dma-buf-map.h
create mode 100644 include/linux/iosys-map.h