| Message ID | 20220613195137.8117-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: atomisp: Various hmm and other cleanups | expand |
On Mon, Jun 13, 2022 at 9:51 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi All, > > I want to start writing a libcamera pipeline handler for the atomisp2, > but before I can do that I first need to fix mmap support in atomisp2. > > My plan for this is to port the atomisp2 handler to videobuf2. To do that > I first need to understand the existing memory/buffer management so I've > started with cleaning up the hmm code (with a bit of a detour here and > there). > > This series is the result of that. There are likely more cleanups to > follow, but I've to focus on some other things for a bit. I hope to be > able to return to the cleanups + an eventual videobuf2 conversion soon. Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> For patches 1-8: The code wise it's good clean up, functionality it might be that intention was to implement it as some point, but looking into (internal) history of the driver development I believe it would require some firmware changes which is impossible for upstreamed version of the driver and as you noticed never productized that time. Hence, good that we got less LoCs after all. For patches 9-13: I believe that patch 10 and 9 should be swapped in the series. Logically you drop caller first followed by (unused) callee. For the rest: To be continued... > Hans de Goede (40): > media: atomisp: remove the unused RAW_BUF_STRIDE macro > media: atomisp: remove unused ia_css_frame_allocate_contiguous*() > functions > media: atomisp: drop contiguous argument from > ia_css_frame_allocate_with_buffer_size() > media: atomisp: drop contiguous argument from > frame_allocate_with_data() > media: atomisp: drop contiguous argument from frame_create() > media: atomisp: drop IA_CSS_FRAME_FORMAT_MIPI support from > ia_css_frame_init_planes() > media: atomisp: drop contiguous flag from struct ia_css_frame > media: atomisp: drop ATOMISP_MAP_FLAG_CONTIGUOUS > media: atomisp: remove dynamic and reserved pool code > media: atomisp: remove hmm_pool_[un]register() > media: atomisp: remove hmm pool code > media: atomisp: remove hmm_mem_stats > media: atomisp: remove pool related kernel cmdline options > media: atomisp: remove unused attribute argument from > ia_css_frame_map() > media: atomisp: drop hmm_page_type > media: atomisp: removed unused hmm_bo_get_page_info() function > media: atomisp: remove bogus comment above hmm_bo_allocated() > prototype > media: atomisp: remove private acceleration ioctls > media: atomisp: remove atomisp_acc.c > media: atomisp: remove unused atomisp_*css_* functions > media: atomisp: asc.acc.pipeline is always NULL > media: atomisp: remove no longer used atomisp_css_acc_done() function > media: atomisp: remove atomisp_is_acc_enabled() > media: atomisp: drop unused ATOMISP_ACC_FW_LOAD_* defines > media: atomisp: drop ATOMISP_MAP_FLAG_CLEARED > media: atomisp: drop unused ATOMISP_MAP_FLAG_* flags > media: atomisp: remove unused hmm address translation functions > media: atomisp: add hmm_create_from_userdata() helper > media: atomisp: Simplify hmm_alloc() calls > media: atomisp: drop highmem var/arg from the hmm code > media: atomisp: drop HMM_BO_SHARE type > media: atomisp: remove hmm_page_object > media: atomisp: fix __get_frame_info() error handling > media: atomisp: add error checking to atomisp_create_pipes_stream() > media: atomisp: add error logging to > atomisp_destroy_pipes_stream_force() > media: atomisp: use atomisp_create_pipes_stream() in more places > media: atomisp: use atomisp_css_update_stream() in more places > media: atomisp: use atomisp_destroy_pipes_stream_force() in more > places > media: atomisp: remove force argument from > __destroy_[stream[s]|pipe[s]]() > media: atomisp: Add a notes.txt file > > drivers/staging/media/atomisp/Makefile | 3 - > .../staging/media/atomisp/include/hmm/hmm.h | 32 +- > .../media/atomisp/include/hmm/hmm_bo.h | 37 +- > .../media/atomisp/include/hmm/hmm_common.h | 26 - > .../media/atomisp/include/hmm/hmm_pool.h | 116 ---- > .../media/atomisp/include/linux/atomisp.h | 146 ---- > drivers/staging/media/atomisp/notes.txt | 30 + > .../staging/media/atomisp/pci/atomisp_acc.c | 625 ------------------ > .../staging/media/atomisp/pci/atomisp_acc.h | 120 ---- > .../staging/media/atomisp/pci/atomisp_cmd.c | 33 +- > .../media/atomisp/pci/atomisp_compat.h | 29 +- > .../media/atomisp/pci/atomisp_compat_css20.c | 365 ++-------- > .../atomisp/pci/atomisp_compat_ioctl32.h | 58 -- > .../staging/media/atomisp/pci/atomisp_drvfs.c | 7 +- > .../staging/media/atomisp/pci/atomisp_fops.c | 13 - > .../staging/media/atomisp/pci/atomisp_ioctl.c | 73 +- > .../staging/media/atomisp/pci/atomisp_ioctl.h | 1 - > .../media/atomisp/pci/atomisp_subdev.c | 3 - > .../media/atomisp/pci/atomisp_subdev.h | 10 - > .../staging/media/atomisp/pci/atomisp_v4l2.c | 32 - > drivers/staging/media/atomisp/pci/hmm/hmm.c | 186 +----- > .../staging/media/atomisp/pci/hmm/hmm_bo.c | 261 ++------ > .../media/atomisp/pci/hmm/hmm_dynamic_pool.c | 234 ------- > .../media/atomisp/pci/hmm/hmm_reserved_pool.c | 253 ------- > .../media/atomisp/pci/ia_css_frame_public.h | 40 -- > .../kernels/sdis/sdis_1.0/ia_css_sdis.host.c | 2 +- > .../kernels/sdis/sdis_2/ia_css_sdis2.host.c | 2 +- > .../pci/isp/modes/interface/isp_const.h | 6 - > .../pci/runtime/debug/src/ia_css_debug.c | 2 - > .../runtime/frame/interface/ia_css_frame.h | 7 +- > .../atomisp/pci/runtime/frame/src/frame.c | 105 +-- > .../pci/runtime/isp_param/src/isp_param.c | 2 +- > .../atomisp/pci/runtime/rmgr/src/rmgr_vbuf.c | 3 +- > .../atomisp/pci/runtime/spctrl/src/spctrl.c | 2 +- > drivers/staging/media/atomisp/pci/sh_css.c | 5 - > .../media/atomisp/pci/sh_css_firmware.c | 2 +- > .../staging/media/atomisp/pci/sh_css_mipi.c | 3 +- > .../staging/media/atomisp/pci/sh_css_params.c | 47 +- > 38 files changed, 205 insertions(+), 2716 deletions(-) > delete mode 100644 drivers/staging/media/atomisp/include/hmm/hmm_pool.h > create mode 100644 drivers/staging/media/atomisp/notes.txt > delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_acc.c > delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_acc.h > delete mode 100644 drivers/staging/media/atomisp/pci/hmm/hmm_dynamic_pool.c > delete mode 100644 drivers/staging/media/atomisp/pci/hmm/hmm_reserved_pool.c > > -- > 2.36.0 >
Hi, On 6/14/22 11:25, Andy Shevchenko wrote: > On Mon, Jun 13, 2022 at 9:51 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> Hi All, >> >> I want to start writing a libcamera pipeline handler for the atomisp2, >> but before I can do that I first need to fix mmap support in atomisp2. >> >> My plan for this is to port the atomisp2 handler to videobuf2. To do that >> I first need to understand the existing memory/buffer management so I've >> started with cleaning up the hmm code (with a bit of a detour here and >> there). >> >> This series is the result of that. There are likely more cleanups to >> follow, but I've to focus on some other things for a bit. I hope to be >> able to return to the cleanups + an eventual videobuf2 conversion soon. > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > For patches 1-8: The code wise it's good clean up, functionality it > might be that intention was to implement it as some point, but looking > into (internal) history of the driver development I believe it would > require some firmware changes which is impossible for upstreamed > version of the driver and as you noticed never productized that time. > Hence, good that we got less LoCs after all. > > For patches 9-13: I believe that patch 10 and 9 should be swapped in > the series. Logically you drop caller first followed by (unused) > callee. Note the code removed in patch 9 was never called even before patch 10, the removed calls in patch 10 were already "#if 0"-ed out. So there is no bisect breakage here. With that said I get your point. Regards, Hans > > For the rest: To be continued... > >> Hans de Goede (40): >> media: atomisp: remove the unused RAW_BUF_STRIDE macro >> media: atomisp: remove unused ia_css_frame_allocate_contiguous*() >> functions >> media: atomisp: drop contiguous argument from >> ia_css_frame_allocate_with_buffer_size() >> media: atomisp: drop contiguous argument from >> frame_allocate_with_data() >> media: atomisp: drop contiguous argument from frame_create() >> media: atomisp: drop IA_CSS_FRAME_FORMAT_MIPI support from >> ia_css_frame_init_planes() >> media: atomisp: drop contiguous flag from struct ia_css_frame >> media: atomisp: drop ATOMISP_MAP_FLAG_CONTIGUOUS >> media: atomisp: remove dynamic and reserved pool code >> media: atomisp: remove hmm_pool_[un]register() >> media: atomisp: remove hmm pool code >> media: atomisp: remove hmm_mem_stats >> media: atomisp: remove pool related kernel cmdline options >> media: atomisp: remove unused attribute argument from >> ia_css_frame_map() >> media: atomisp: drop hmm_page_type >> media: atomisp: removed unused hmm_bo_get_page_info() function >> media: atomisp: remove bogus comment above hmm_bo_allocated() >> prototype >> media: atomisp: remove private acceleration ioctls >> media: atomisp: remove atomisp_acc.c >> media: atomisp: remove unused atomisp_*css_* functions >> media: atomisp: asc.acc.pipeline is always NULL >> media: atomisp: remove no longer used atomisp_css_acc_done() function >> media: atomisp: remove atomisp_is_acc_enabled() >> media: atomisp: drop unused ATOMISP_ACC_FW_LOAD_* defines >> media: atomisp: drop ATOMISP_MAP_FLAG_CLEARED >> media: atomisp: drop unused ATOMISP_MAP_FLAG_* flags >> media: atomisp: remove unused hmm address translation functions >> media: atomisp: add hmm_create_from_userdata() helper >> media: atomisp: Simplify hmm_alloc() calls >> media: atomisp: drop highmem var/arg from the hmm code >> media: atomisp: drop HMM_BO_SHARE type >> media: atomisp: remove hmm_page_object >> media: atomisp: fix __get_frame_info() error handling >> media: atomisp: add error checking to atomisp_create_pipes_stream() >> media: atomisp: add error logging to >> atomisp_destroy_pipes_stream_force() >> media: atomisp: use atomisp_create_pipes_stream() in more places >> media: atomisp: use atomisp_css_update_stream() in more places >> media: atomisp: use atomisp_destroy_pipes_stream_force() in more >> places >> media: atomisp: remove force argument from >> __destroy_[stream[s]|pipe[s]]() >> media: atomisp: Add a notes.txt file >> >> drivers/staging/media/atomisp/Makefile | 3 - >> .../staging/media/atomisp/include/hmm/hmm.h | 32 +- >> .../media/atomisp/include/hmm/hmm_bo.h | 37 +- >> .../media/atomisp/include/hmm/hmm_common.h | 26 - >> .../media/atomisp/include/hmm/hmm_pool.h | 116 ---- >> .../media/atomisp/include/linux/atomisp.h | 146 ---- >> drivers/staging/media/atomisp/notes.txt | 30 + >> .../staging/media/atomisp/pci/atomisp_acc.c | 625 ------------------ >> .../staging/media/atomisp/pci/atomisp_acc.h | 120 ---- >> .../staging/media/atomisp/pci/atomisp_cmd.c | 33 +- >> .../media/atomisp/pci/atomisp_compat.h | 29 +- >> .../media/atomisp/pci/atomisp_compat_css20.c | 365 ++-------- >> .../atomisp/pci/atomisp_compat_ioctl32.h | 58 -- >> .../staging/media/atomisp/pci/atomisp_drvfs.c | 7 +- >> .../staging/media/atomisp/pci/atomisp_fops.c | 13 - >> .../staging/media/atomisp/pci/atomisp_ioctl.c | 73 +- >> .../staging/media/atomisp/pci/atomisp_ioctl.h | 1 - >> .../media/atomisp/pci/atomisp_subdev.c | 3 - >> .../media/atomisp/pci/atomisp_subdev.h | 10 - >> .../staging/media/atomisp/pci/atomisp_v4l2.c | 32 - >> drivers/staging/media/atomisp/pci/hmm/hmm.c | 186 +----- >> .../staging/media/atomisp/pci/hmm/hmm_bo.c | 261 ++------ >> .../media/atomisp/pci/hmm/hmm_dynamic_pool.c | 234 ------- >> .../media/atomisp/pci/hmm/hmm_reserved_pool.c | 253 ------- >> .../media/atomisp/pci/ia_css_frame_public.h | 40 -- >> .../kernels/sdis/sdis_1.0/ia_css_sdis.host.c | 2 +- >> .../kernels/sdis/sdis_2/ia_css_sdis2.host.c | 2 +- >> .../pci/isp/modes/interface/isp_const.h | 6 - >> .../pci/runtime/debug/src/ia_css_debug.c | 2 - >> .../runtime/frame/interface/ia_css_frame.h | 7 +- >> .../atomisp/pci/runtime/frame/src/frame.c | 105 +-- >> .../pci/runtime/isp_param/src/isp_param.c | 2 +- >> .../atomisp/pci/runtime/rmgr/src/rmgr_vbuf.c | 3 +- >> .../atomisp/pci/runtime/spctrl/src/spctrl.c | 2 +- >> drivers/staging/media/atomisp/pci/sh_css.c | 5 - >> .../media/atomisp/pci/sh_css_firmware.c | 2 +- >> .../staging/media/atomisp/pci/sh_css_mipi.c | 3 +- >> .../staging/media/atomisp/pci/sh_css_params.c | 47 +- >> 38 files changed, 205 insertions(+), 2716 deletions(-) >> delete mode 100644 drivers/staging/media/atomisp/include/hmm/hmm_pool.h >> create mode 100644 drivers/staging/media/atomisp/notes.txt >> delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_acc.c >> delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_acc.h >> delete mode 100644 drivers/staging/media/atomisp/pci/hmm/hmm_dynamic_pool.c >> delete mode 100644 drivers/staging/media/atomisp/pci/hmm/hmm_reserved_pool.c >> >> -- >> 2.36.0 >> > >
On Tue, Jun 14, 2022 at 11:50:32AM +0200, Hans de Goede wrote: > On 6/14/22 11:25, Andy Shevchenko wrote: > > On Mon, Jun 13, 2022 at 9:51 PM Hans de Goede <hdegoede@redhat.com> wrote: > > For patches 9-13: I believe that patch 10 and 9 should be swapped in > > the series. Logically you drop caller first followed by (unused) > > callee. > > Note the code removed in patch 9 was never called even before patch 10, > the removed calls in patch 10 were already "#if 0"-ed out. So there > is no bisect breakage here. With that said I get your point. Yes, it's not about bisecting, but rather logic and (quite unlike) possibility of restoring that partially and when someone switches 0 to 1 in those #if:s, compilation will be broken.
On Tue, Jun 14, 2022 at 11:25:07AM +0200, Andy Shevchenko wrote: > On Mon, Jun 13, 2022 at 9:51 PM Hans de Goede <hdegoede@redhat.com> wrote: > > > > Hi All, > > > > I want to start writing a libcamera pipeline handler for the atomisp2, > > but before I can do that I first need to fix mmap support in atomisp2. > > > > My plan for this is to port the atomisp2 handler to videobuf2. To do that > > I first need to understand the existing memory/buffer management so I've > > started with cleaning up the hmm code (with a bit of a detour here and > > there). > > > > This series is the result of that. There are likely more cleanups to > > follow, but I've to focus on some other things for a bit. I hope to be > > able to return to the cleanups + an eventual videobuf2 conversion soon. > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > For patches 1-8: The code wise it's good clean up, functionality it > might be that intention was to implement it as some point, but looking > into (internal) history of the driver development I believe it would > require some firmware changes which is impossible for upstreamed > version of the driver and as you noticed never productized that time. > Hence, good that we got less LoCs after all. > > For patches 9-13: I believe that patch 10 and 9 should be swapped in > the series. Logically you drop caller first followed by (unused) > callee. > > For the rest: To be continued... For the rest: Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> I believe a couple of comments are easy to address, may keep my tag there.
Hi All, I want to start writing a libcamera pipeline handler for the atomisp2, but before I can do that I first need to fix mmap support in atomisp2. My plan for this is to port the atomisp2 handler to videobuf2. To do that I first need to understand the existing memory/buffer management so I've started with cleaning up the hmm code (with a bit of a detour here and there). This series is the result of that. There are likely more cleanups to follow, but I've to focus on some other things for a bit. I hope to be able to return to the cleanups + an eventual videobuf2 conversion soon. Regards, Hans Hans de Goede (40): media: atomisp: remove the unused RAW_BUF_STRIDE macro media: atomisp: remove unused ia_css_frame_allocate_contiguous*() functions media: atomisp: drop contiguous argument from ia_css_frame_allocate_with_buffer_size() media: atomisp: drop contiguous argument from frame_allocate_with_data() media: atomisp: drop contiguous argument from frame_create() media: atomisp: drop IA_CSS_FRAME_FORMAT_MIPI support from ia_css_frame_init_planes() media: atomisp: drop contiguous flag from struct ia_css_frame media: atomisp: drop ATOMISP_MAP_FLAG_CONTIGUOUS media: atomisp: remove dynamic and reserved pool code media: atomisp: remove hmm_pool_[un]register() media: atomisp: remove hmm pool code media: atomisp: remove hmm_mem_stats media: atomisp: remove pool related kernel cmdline options media: atomisp: remove unused attribute argument from ia_css_frame_map() media: atomisp: drop hmm_page_type media: atomisp: removed unused hmm_bo_get_page_info() function media: atomisp: remove bogus comment above hmm_bo_allocated() prototype media: atomisp: remove private acceleration ioctls media: atomisp: remove atomisp_acc.c media: atomisp: remove unused atomisp_*css_* functions media: atomisp: asc.acc.pipeline is always NULL media: atomisp: remove no longer used atomisp_css_acc_done() function media: atomisp: remove atomisp_is_acc_enabled() media: atomisp: drop unused ATOMISP_ACC_FW_LOAD_* defines media: atomisp: drop ATOMISP_MAP_FLAG_CLEARED media: atomisp: drop unused ATOMISP_MAP_FLAG_* flags media: atomisp: remove unused hmm address translation functions media: atomisp: add hmm_create_from_userdata() helper media: atomisp: Simplify hmm_alloc() calls media: atomisp: drop highmem var/arg from the hmm code media: atomisp: drop HMM_BO_SHARE type media: atomisp: remove hmm_page_object media: atomisp: fix __get_frame_info() error handling media: atomisp: add error checking to atomisp_create_pipes_stream() media: atomisp: add error logging to atomisp_destroy_pipes_stream_force() media: atomisp: use atomisp_create_pipes_stream() in more places media: atomisp: use atomisp_css_update_stream() in more places media: atomisp: use atomisp_destroy_pipes_stream_force() in more places media: atomisp: remove force argument from __destroy_[stream[s]|pipe[s]]() media: atomisp: Add a notes.txt file drivers/staging/media/atomisp/Makefile | 3 - .../staging/media/atomisp/include/hmm/hmm.h | 32 +- .../media/atomisp/include/hmm/hmm_bo.h | 37 +- .../media/atomisp/include/hmm/hmm_common.h | 26 - .../media/atomisp/include/hmm/hmm_pool.h | 116 ---- .../media/atomisp/include/linux/atomisp.h | 146 ---- drivers/staging/media/atomisp/notes.txt | 30 + .../staging/media/atomisp/pci/atomisp_acc.c | 625 ------------------ .../staging/media/atomisp/pci/atomisp_acc.h | 120 ---- .../staging/media/atomisp/pci/atomisp_cmd.c | 33 +- .../media/atomisp/pci/atomisp_compat.h | 29 +- .../media/atomisp/pci/atomisp_compat_css20.c | 365 ++-------- .../atomisp/pci/atomisp_compat_ioctl32.h | 58 -- .../staging/media/atomisp/pci/atomisp_drvfs.c | 7 +- .../staging/media/atomisp/pci/atomisp_fops.c | 13 - .../staging/media/atomisp/pci/atomisp_ioctl.c | 73 +- .../staging/media/atomisp/pci/atomisp_ioctl.h | 1 - .../media/atomisp/pci/atomisp_subdev.c | 3 - .../media/atomisp/pci/atomisp_subdev.h | 10 - .../staging/media/atomisp/pci/atomisp_v4l2.c | 32 - drivers/staging/media/atomisp/pci/hmm/hmm.c | 186 +----- .../staging/media/atomisp/pci/hmm/hmm_bo.c | 261 ++------ .../media/atomisp/pci/hmm/hmm_dynamic_pool.c | 234 ------- .../media/atomisp/pci/hmm/hmm_reserved_pool.c | 253 ------- .../media/atomisp/pci/ia_css_frame_public.h | 40 -- .../kernels/sdis/sdis_1.0/ia_css_sdis.host.c | 2 +- .../kernels/sdis/sdis_2/ia_css_sdis2.host.c | 2 +- .../pci/isp/modes/interface/isp_const.h | 6 - .../pci/runtime/debug/src/ia_css_debug.c | 2 - .../runtime/frame/interface/ia_css_frame.h | 7 +- .../atomisp/pci/runtime/frame/src/frame.c | 105 +-- .../pci/runtime/isp_param/src/isp_param.c | 2 +- .../atomisp/pci/runtime/rmgr/src/rmgr_vbuf.c | 3 +- .../atomisp/pci/runtime/spctrl/src/spctrl.c | 2 +- drivers/staging/media/atomisp/pci/sh_css.c | 5 - .../media/atomisp/pci/sh_css_firmware.c | 2 +- .../staging/media/atomisp/pci/sh_css_mipi.c | 3 +- .../staging/media/atomisp/pci/sh_css_params.c | 47 +- 38 files changed, 205 insertions(+), 2716 deletions(-) delete mode 100644 drivers/staging/media/atomisp/include/hmm/hmm_pool.h create mode 100644 drivers/staging/media/atomisp/notes.txt delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_acc.c delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_acc.h delete mode 100644 drivers/staging/media/atomisp/pci/hmm/hmm_dynamic_pool.c delete mode 100644 drivers/staging/media/atomisp/pci/hmm/hmm_reserved_pool.c