mbox series

[00/40] media: atomisp: Various hmm and other cleanups

Message ID 20220613195137.8117-1-hdegoede@redhat.com
Headers show
Series media: atomisp: Various hmm and other cleanups | expand

Message

Hans de Goede June 13, 2022, 7:50 p.m. UTC
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

Comments

Andy Shevchenko June 14, 2022, 9:25 a.m. UTC | #1
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
>
Andy Shevchenko June 14, 2022, 10:04 a.m. UTC | #2
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.
Andy Shevchenko June 14, 2022, 1:13 p.m. UTC | #3
On Mon, Jun 13, 2022 at 09:51:29PM +0200, Hans de Goede wrote:
> hmm_page_object only stores a struct page pointer, so we can just use
> the hmm_bo.pages page pointer array everywhere.

Right!