Re: [PATCH v3 RESEND 0/7] New DRM driver for Intel VPU

[Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>]

Hi,

On 11/1/2022 11:17 AM, Thomas Zimmermann wrote:
> Hi
> 
> Am 01.11.22 um 09:58 schrieb Thomas Zimmermann:
>> Hi
>>
>> Am 24.09.22 um 17:11 schrieb Jacek Lawrynowicz:
>>> 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.
>>>
>>> Driver is part of gpu/drm subsystem because VPU is similar in operation to
>>> an integrated GPU. Reusing drm driver init, ioctl handling, gem and prime
>>> helpers and drm_mm allows to minimize code duplication in the kernel.
>>>
>>> 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 stack consists of Level Zero API driver and OpenVINO plugin.
>>> Both should be open-sourced by the end of Q4.
>>> The firmware for the VPU will be distributed as a closed source binary.
>>>
>>> I'm resending v3 patchset with correct Dave's email.
>>>
>>> Regards,
>>> Jacek
>>>
>>> v3:
>>> - Fixed alignment warning in ivpu_ipc.c when building with W=1
>>>
>>> v2: https://lore.kernel.org/all/20220913121017.993825-1-jacek.lawrynowicz@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.lawrynowicz@linux.intel.com/
>>>
>>> Jacek Lawrynowicz (7):
>>>    drm/ivpu: Introduce a new DRM driver for Intel VPU
>>>    drm/ivpu: Add Intel VPU MMU support
>>>    drm/ivpu: Add GEM buffer object management
>>>    drm/ivpu: Add IPC driver and JSM messages
>>>    drm/ivpu: Implement firmware parsing and booting
>>>    drm/ivpu: Add command buffer submission logic
>>>    drm/ivpu: Add PM support
>>>
>>>   MAINTAINERS                             |    8 +
>>>   drivers/gpu/drm/Kconfig                 |    2 +
>>>   drivers/gpu/drm/Makefile                |    1 +
>>>   drivers/gpu/drm/ivpu/Kconfig            |   12 +
>>>   drivers/gpu/drm/ivpu/Makefile           |   16 +
>>>   drivers/gpu/drm/ivpu/TODO               |    7 +
>>>   drivers/gpu/drm/ivpu/ivpu_drv.c         |  643 ++++++++++++++
>>>   drivers/gpu/drm/ivpu/ivpu_drv.h         |  178 ++++
>>>   drivers/gpu/drm/ivpu/ivpu_fw.c          |  426 +++++++++
>>>   drivers/gpu/drm/ivpu/ivpu_fw.h          |   38 +
>>>   drivers/gpu/drm/ivpu/ivpu_gem.c         |  836 ++++++++++++++++++
>>>   drivers/gpu/drm/ivpu/ivpu_gem.h         |  128 +++
>>>   drivers/gpu/drm/ivpu/ivpu_hw.h          |  169 ++++
>>>   drivers/gpu/drm/ivpu/ivpu_hw_mtl.c      | 1060 +++++++++++++++++++++++
>>>   drivers/gpu/drm/ivpu/ivpu_hw_mtl_reg.h  |  468 ++++++++++
>>>   drivers/gpu/drm/ivpu/ivpu_hw_reg_io.h   |  115 +++
>>>   drivers/gpu/drm/ivpu/ivpu_ipc.c         |  508 +++++++++++
>>>   drivers/gpu/drm/ivpu/ivpu_ipc.h         |   90 ++
>>>   drivers/gpu/drm/ivpu/ivpu_job.c         |  629 ++++++++++++++
>>>   drivers/gpu/drm/ivpu/ivpu_job.h         |   73 ++
>>>   drivers/gpu/drm/ivpu/ivpu_jsm_msg.c     |  220 +++++
>>>   drivers/gpu/drm/ivpu/ivpu_jsm_msg.h     |   25 +
>>>   drivers/gpu/drm/ivpu/ivpu_mmu.c         |  888 +++++++++++++++++++
>>>   drivers/gpu/drm/ivpu/ivpu_mmu.h         |   53 ++
>>>   drivers/gpu/drm/ivpu/ivpu_mmu_context.c |  419 +++++++++
>>>   drivers/gpu/drm/ivpu/ivpu_mmu_context.h |   49 ++
>>>   drivers/gpu/drm/ivpu/ivpu_pm.c          |  352 ++++++++
>>>   drivers/gpu/drm/ivpu/ivpu_pm.h          |   38 +
>>>   drivers/gpu/drm/ivpu/vpu_boot_api.h     |  241 ++++++
>>>   drivers/gpu/drm/ivpu/vpu_jsm_api.h      |  616 +++++++++++++
>>>   include/uapi/drm/ivpu_drm.h             |  343 ++++++++
>>
>> You have to add yourself to MAINTAINERS.
>>
>> I assume that there's a FOSS userspace available?
> 
> Rereading the cover letter, it will be opensourced. I think DRM policy is that we first need an OSS userspace befor the kernel driver get's merged. Nevertheless it's now a good time to get the driver in shape.
> 
> I've finished looking trough the patchset and left comments on some of them.

Thanks for the in-depth review and sorry for late response but it took us some time to implement all review changes.
The user space should be opensourced in coming weeks.

Regards,
Jacek