Re: [PATCH v5 0/7] New DRM accel driver for Intel VPU

2023-01-19 Thread Daniel Vetter
On Tue, Jan 10, 2023 at 09:34:00PM +0200, Oded Gabbay wrote:
> On Mon, Jan 9, 2023 at 2:23 PM Jacek Lawrynowicz
>  wrote:
> >
> > Hi,
> >
> > This patchset contains a new Linux* Kernel Driver for Intel® VPUs.
> >
> > VPU stands for Versatile Processing Unit and it is an AI inference 
> > accelerator
> > integrated with Intel non-server CPUs starting from 14th generation.
> > VPU enables efficient execution of Deep Learning applications
> > like object detection, classification etc.
> >
> > The whole driver is licensed under GPL-2.0-only except for two headers 
> > imported
> > from the firmware that are MIT licensed.
> >
> > User mode driver was open sourced in December 2022 and it is available at:
> > https://github.com/intel/linux-vpu-driver
> >
> > The firmware for the VPU will be distributed as a closed source binary.
> >
> > There are no major chcnages in this version of the patchset.
> > Detailed changes are listed below.
> >
> > Regards,
> > Jacek
> 
> Hi Jacek,
> For patches 1-4 and 6 (assuming you fix the small issue in patch 3) you can 
> add
> Reviewed-by: Oded Gabbay 
> 
> For patch 5 I had a comment/question, please reply to it. Once we sort
> that thing that patch is also r-b by me.
> 
> Patch 7 - I prefer someone else who knows about PM to look at it. In
> habana we don't support power management (in the driver) so I have
> never dealt with that and therefore I have no idea if you use the kapi
> correctly.

Usually the hard part is deadlocks between runtime_resume calls and the
rpm suspend callback. I'm not sure whether lockdep catches these I'm not
sure whether lockdep catches these.

>From a very quick look it looks reasonable.
-Daniel

> 
> Thanks,
> Oded
> 
> 
> >
> > Changelog
> >
> > v5:
> > - Rename ivpu_drm.h to ivpu_accel.h
> > - Cleanup ivpu_mmu_config_check()
> > - Optimize locking in ivpu_mmu_cd_add()
> > - Invalidate user context if it has MMU faults
> > - Move ivpu_ipc_match_consumer() outside ivpu_ipc_dispatch()
> >
> > v4: 
> > https://lore.kernel.org/all/20221208110733.5498-1-jacek.lawrynow...@linux.intel.com/
> > - Switch to the accel framework (DRIVER_COMPUTE_ACCEL)
> > - Move the driver from drivers/gpu/drm to drivers/accel
> > - Rename kconfig DRM_IVPU option to DRM_ACCEL_IVPU and update dependencies
> > - Create context on open() instead of lazy allocating it
> > - Remove status_offset from submit ioctl, as status is now reported in 
> > bo_wait ioctl
> > - Use managed resources in ivpu_drv.c
> > - Optimize locking in ivpu_ipc.c - add new rx_msg_lock for consumer msg list
> > - Refactor vpu_hw_mtl_reg.h to use BIT_MASK() and GENMASK() macros
> > - Use module_pci_driver() for mudule init
> > - Remove mutex from "struct ivpu_pm_info"
> > - Add explicit "vdev" arg to ivpu_dbg()
> > - Use drm_WARN_ON() instead of WARN_ON() where possible
> > - Add comments for boot related functions
> > - Update firmware API headers
> >
> > v3: 
> > https://lore.kernel.org/all/20220924151149.323622-1-jacek.lawrynow...@linux.intel.com/
> > - Fixed alignment warning in ivpu_ipc.c when building with W=1
> >
> > v2: 
> > https://lore.kernel.org/all/20220913121017.993825-1-jacek.lawrynow...@linux.intel.com/
> > - Rename the driver from "drm/vpu" to "drm/ivpu"
> > - Add a TODO file
> > - Add support for WC buffers
> >
> > v1: 
> > https://lore.kernel.org/all/20220728131709.1087188-1-jacek.lawrynow...@linux.intel.com/
> >
> > Jacek Lawrynowicz (7):
> >   accel/ivpu: Introduce a new DRM driver for Intel VPU
> >   accel/ivpu: Add Intel VPU MMU support
> >   accel/ivpu: Add GEM buffer object management
> >   accel/ivpu: Add IPC driver and JSM messages
> >   accel/ivpu: Implement firmware parsing and booting
> >   accel/ivpu: Add command buffer submission logic
> >   accel/ivpu: Add PM support
> >
> >  MAINTAINERS   |9 +
> >  drivers/Makefile  |1 +
> >  drivers/accel/Kconfig |2 +
> >  drivers/accel/Makefile|3 +
> >  drivers/accel/ivpu/Kconfig|   15 +
> >  drivers/accel/ivpu/Makefile   |   16 +
> >  drivers/accel/ivpu/TODO   |9 +
> >  drivers/accel/ivpu/ivpu_drv.c |  661 +++
> >  drivers/accel/ivpu/ivpu_drv.h |  190 +
> >  drivers/accel/ivpu/ivpu_fw.c  |  423 ++
> >  drivers/accel/ivpu/ivpu_fw.h  |   38 +
> >  drivers/accel/ivpu/ivpu_gem.c |  846 +++
> >  drivers/accel/ivpu/ivpu_gem.h |  129 +++
> >  drivers/accel/ivpu/ivpu_hw.h  |  170 
> >  drivers/accel/ivpu/ivpu_hw_mtl.c  | 1084 +
> >  drivers/accel/ivpu/ivpu_hw_mtl_reg.h  |  280 +++
> >  drivers/accel/ivpu/ivpu_hw_reg_io.h   |  115 +++
> >  drivers/accel/ivpu/ivpu_ipc.c |  508 
> >  drivers/accel/ivpu/ivpu_ipc.h |   93 +++
> >  drivers/accel/ivpu/ivpu_job.c |  614 ++
> >  drivers/accel/ivpu/ivpu_job.h |   67 ++
> >  drivers/accel/ivpu/iv

Re: [PATCH v5 0/7] New DRM accel driver for Intel VPU

2023-01-13 Thread Jacek Lawrynowicz
Hi,

On 12.01.2023 18:10, Jeffrey Hugo wrote:
> On 1/9/2023 5:23 AM, Jacek Lawrynowicz wrote:
>> Hi,
>>
>> This patchset contains a new Linux* Kernel Driver for Intel® VPUs.
>>
>> VPU stands for Versatile Processing Unit and it is an AI inference 
>> accelerator
>> integrated with Intel non-server CPUs starting from 14th generation.
>> VPU enables efficient execution of Deep Learning applications
>> like object detection, classification etc.
>>
>> The whole driver is licensed under GPL-2.0-only except for two headers 
>> imported
>> from the firmware that are MIT licensed.
>>
>> User mode driver was open sourced in December 2022 and it is available at:
>> https://github.com/intel/linux-vpu-driver
> 
> I feel like I forgot to mention this earlier because I can't find a reference 
> to it in my mails.  I would like to see something in Documentation for this 
> driver/device.  Would be nice to get an overview of how it works (system 
> block diagram?), how it is intended to be used, etc.  Include relevant 
> references.  This would be a great place to document the UMD and the compiler 
> (I am positive you have mentioned the compiler before, but I am currently 
> failing to find a reference to it).
> 
> I feel that traditional DRM gets away from not needing this since their stuff 
> is pretty well established.  Everyone uses Mesa/igt and so how things are 
> structured/used is fairly well implied.  Accel is brand new and doesn't have 
> that yet so I suspect we'll be well situated if we take the extra effort to 
> spell out these things which might be just assumed elsewhere.  Hopefully, 
> over time, such documentation helps in identifying useful areas to build up 
> the common code of the subsystem.
> 
> I can't justify holding up this series for this though.  Please put it on 
> some todo list  :)

OK, I've added the documentation to TODO.
For now I will add info about the compiler to v6 cover letter.
It is available at https://github.com/openvinotoolkit/vpux-plugin.

Regards,
Jacek


Re: [PATCH v5 0/7] New DRM accel driver for Intel VPU

2023-01-12 Thread Jeffrey Hugo

On 1/9/2023 5:23 AM, Jacek Lawrynowicz wrote:

Hi,

This patchset contains a new Linux* Kernel Driver for Intel® VPUs.

VPU stands for Versatile Processing Unit and it is an AI inference accelerator
integrated with Intel non-server CPUs starting from 14th generation.
VPU enables efficient execution of Deep Learning applications
like object detection, classification etc.

The whole driver is licensed under GPL-2.0-only except for two headers imported
from the firmware that are MIT licensed.

User mode driver was open sourced in December 2022 and it is available at:
https://github.com/intel/linux-vpu-driver


I feel like I forgot to mention this earlier because I can't find a 
reference to it in my mails.  I would like to see something in 
Documentation for this driver/device.  Would be nice to get an overview 
of how it works (system block diagram?), how it is intended to be used, 
etc.  Include relevant references.  This would be a great place to 
document the UMD and the compiler (I am positive you have mentioned the 
compiler before, but I am currently failing to find a reference to it).


I feel that traditional DRM gets away from not needing this since their 
stuff is pretty well established.  Everyone uses Mesa/igt and so how 
things are structured/used is fairly well implied.  Accel is brand new 
and doesn't have that yet so I suspect we'll be well situated if we take 
the extra effort to spell out these things which might be just assumed 
elsewhere.  Hopefully, over time, such documentation helps in 
identifying useful areas to build up the common code of the subsystem.


I can't justify holding up this series for this though.  Please put it 
on some todo list  :)


-Jeff


Re: [PATCH v5 0/7] New DRM accel driver for Intel VPU

2023-01-10 Thread Oded Gabbay
On Mon, Jan 9, 2023 at 2:23 PM Jacek Lawrynowicz
 wrote:
>
> Hi,
>
> This patchset contains a new Linux* Kernel Driver for Intel® VPUs.
>
> VPU stands for Versatile Processing Unit and it is an AI inference accelerator
> integrated with Intel non-server CPUs starting from 14th generation.
> VPU enables efficient execution of Deep Learning applications
> like object detection, classification etc.
>
> The whole driver is licensed under GPL-2.0-only except for two headers 
> imported
> from the firmware that are MIT licensed.
>
> User mode driver was open sourced in December 2022 and it is available at:
> https://github.com/intel/linux-vpu-driver
>
> The firmware for the VPU will be distributed as a closed source binary.
>
> There are no major chcnages in this version of the patchset.
> Detailed changes are listed below.
>
> Regards,
> Jacek

Hi Jacek,
For patches 1-4 and 6 (assuming you fix the small issue in patch 3) you can add
Reviewed-by: Oded Gabbay 

For patch 5 I had a comment/question, please reply to it. Once we sort
that thing that patch is also r-b by me.

Patch 7 - I prefer someone else who knows about PM to look at it. In
habana we don't support power management (in the driver) so I have
never dealt with that and therefore I have no idea if you use the kapi
correctly.

Thanks,
Oded


>
> Changelog
>
> v5:
> - Rename ivpu_drm.h to ivpu_accel.h
> - Cleanup ivpu_mmu_config_check()
> - Optimize locking in ivpu_mmu_cd_add()
> - Invalidate user context if it has MMU faults
> - Move ivpu_ipc_match_consumer() outside ivpu_ipc_dispatch()
>
> v4: 
> https://lore.kernel.org/all/20221208110733.5498-1-jacek.lawrynow...@linux.intel.com/
> - Switch to the accel framework (DRIVER_COMPUTE_ACCEL)
> - Move the driver from drivers/gpu/drm to drivers/accel
> - Rename kconfig DRM_IVPU option to DRM_ACCEL_IVPU and update dependencies
> - Create context on open() instead of lazy allocating it
> - Remove status_offset from submit ioctl, as status is now reported in 
> bo_wait ioctl
> - Use managed resources in ivpu_drv.c
> - Optimize locking in ivpu_ipc.c - add new rx_msg_lock for consumer msg list
> - Refactor vpu_hw_mtl_reg.h to use BIT_MASK() and GENMASK() macros
> - Use module_pci_driver() for mudule init
> - Remove mutex from "struct ivpu_pm_info"
> - Add explicit "vdev" arg to ivpu_dbg()
> - Use drm_WARN_ON() instead of WARN_ON() where possible
> - Add comments for boot related functions
> - Update firmware API headers
>
> v3: 
> https://lore.kernel.org/all/20220924151149.323622-1-jacek.lawrynow...@linux.intel.com/
> - Fixed alignment warning in ivpu_ipc.c when building with W=1
>
> v2: 
> https://lore.kernel.org/all/20220913121017.993825-1-jacek.lawrynow...@linux.intel.com/
> - Rename the driver from "drm/vpu" to "drm/ivpu"
> - Add a TODO file
> - Add support for WC buffers
>
> v1: 
> https://lore.kernel.org/all/20220728131709.1087188-1-jacek.lawrynow...@linux.intel.com/
>
> Jacek Lawrynowicz (7):
>   accel/ivpu: Introduce a new DRM driver for Intel VPU
>   accel/ivpu: Add Intel VPU MMU support
>   accel/ivpu: Add GEM buffer object management
>   accel/ivpu: Add IPC driver and JSM messages
>   accel/ivpu: Implement firmware parsing and booting
>   accel/ivpu: Add command buffer submission logic
>   accel/ivpu: Add PM support
>
>  MAINTAINERS   |9 +
>  drivers/Makefile  |1 +
>  drivers/accel/Kconfig |2 +
>  drivers/accel/Makefile|3 +
>  drivers/accel/ivpu/Kconfig|   15 +
>  drivers/accel/ivpu/Makefile   |   16 +
>  drivers/accel/ivpu/TODO   |9 +
>  drivers/accel/ivpu/ivpu_drv.c |  661 +++
>  drivers/accel/ivpu/ivpu_drv.h |  190 +
>  drivers/accel/ivpu/ivpu_fw.c  |  423 ++
>  drivers/accel/ivpu/ivpu_fw.h  |   38 +
>  drivers/accel/ivpu/ivpu_gem.c |  846 +++
>  drivers/accel/ivpu/ivpu_gem.h |  129 +++
>  drivers/accel/ivpu/ivpu_hw.h  |  170 
>  drivers/accel/ivpu/ivpu_hw_mtl.c  | 1084 +
>  drivers/accel/ivpu/ivpu_hw_mtl_reg.h  |  280 +++
>  drivers/accel/ivpu/ivpu_hw_reg_io.h   |  115 +++
>  drivers/accel/ivpu/ivpu_ipc.c |  508 
>  drivers/accel/ivpu/ivpu_ipc.h |   93 +++
>  drivers/accel/ivpu/ivpu_job.c |  614 ++
>  drivers/accel/ivpu/ivpu_job.h |   67 ++
>  drivers/accel/ivpu/ivpu_jsm_msg.c |  170 
>  drivers/accel/ivpu/ivpu_jsm_msg.h |   23 +
>  drivers/accel/ivpu/ivpu_mmu.c |  883 
>  drivers/accel/ivpu/ivpu_mmu.h |   50 ++
>  drivers/accel/ivpu/ivpu_mmu_context.c |  398 +
>  drivers/accel/ivpu/ivpu_mmu_context.h |   50 ++
>  drivers/accel/ivpu/ivpu_pm.c  |  329 
>  drivers/accel/ivpu/ivpu_pm.h  |   38 +
>  drivers/accel/ivpu/vpu_boot_api.h |  349 
>  drivers/accel/ivpu/vpu_jsm_api.h  |  999