Re: [PATCH v3 1/7] drm/ivpu: Introduce a new DRM driver for Intel VPU

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

Hi,

On 10/25/2022 1:57 PM, Thomas Zimmermann wrote:
> Hi
> 
> Am 25.10.22 um 13:42 schrieb Jacek Lawrynowicz:
>> Hi, thanks for detailed review. My responses inline.
>>
>> On 10/25/2022 1:00 AM, Jeffrey Hugo wrote:
>>> On 9/24/2022 9:11 AM, Jacek Lawrynowicz wrote:
>>>> VPU stands for Versatile Processing Unit and it's a CPU-integrated
>>>> inference accelerator for Computer Vision and Deep Learning
>>>> applications.
>>>>
>>>> The VPU device consist of following componensts:
>>>>     - Buttress - provides CPU to VPU integration, interrupt, frequency and
>>>>       power management.
>>>>     - Memory Management Unit (based on ARM MMU-600) - translates VPU to
>>>>       host DMA addresses, isolates user workloads.
>>>>     - RISC based microcontroller - executes firmware that provides job
>>>>       execution API for the kernel-mode driver
>>>>     - Neural Compute Subsystem (NCS) - does the actual work, provides
>>>>       Compute and Copy engines.
>>>>     - Network on Chip (NoC) - network fabric connecting all the components
>>>>
>>>> This driver supports VPU IP v2.7 integrated into Intel Meteor Lake
>>>> client CPUs (14th generation).
>>>>
>>>> Module sources are at drivers/gpu/drm/ivpu and module name is
>>>> "intel_vpu.ko".
>>>>
>>>> This patch includes only very besic functionality:
>>>>     - module, PCI device and IRQ initialization
>>>>     - register definitions and low level register manipulation functions
>>>>     - SET/GET_PARAM ioctls
>>>>     - power up without firmware
>>>>
>>>> Signed-off-by: Krystian Pradzynski <krystian.pradzynski@linux.intel.com>
>>>> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
>>>> ---
>>>>    MAINTAINERS                            |    8 +
>>>>    drivers/gpu/drm/Kconfig                |    2 +
>>>>    drivers/gpu/drm/Makefile               |    1 +
>>>>    drivers/gpu/drm/ivpu/Kconfig           |   12 +
>>>>    drivers/gpu/drm/ivpu/Makefile          |    8 +
>>>>    drivers/gpu/drm/ivpu/TODO              |    7 +
>>>>    drivers/gpu/drm/ivpu/ivpu_drv.c        |  392 +++++++++
>>>>    drivers/gpu/drm/ivpu/ivpu_drv.h        |  153 ++++
>>>>    drivers/gpu/drm/ivpu/ivpu_hw.h         |  169 ++++
>>>>    drivers/gpu/drm/ivpu/ivpu_hw_mtl.c     | 1021 ++++++++++++++++++++++++
>>>>    drivers/gpu/drm/ivpu/ivpu_hw_mtl_reg.h |  468 +++++++++++
>>>>    drivers/gpu/drm/ivpu/ivpu_hw_reg_io.h  |  115 +++
>>>>    include/uapi/drm/ivpu_drm.h            |   95 +++
>>>>    13 files changed, 2451 insertions(+)
>>>>    create mode 100644 drivers/gpu/drm/ivpu/Kconfig
>>>>    create mode 100644 drivers/gpu/drm/ivpu/Makefile
>>>>    create mode 100644 drivers/gpu/drm/ivpu/TODO
>>>>    create mode 100644 drivers/gpu/drm/ivpu/ivpu_drv.c
>>>>    create mode 100644 drivers/gpu/drm/ivpu/ivpu_drv.h
>>>>    create mode 100644 drivers/gpu/drm/ivpu/ivpu_hw.h
>>>>    create mode 100644 drivers/gpu/drm/ivpu/ivpu_hw_mtl.c
>>>>    create mode 100644 drivers/gpu/drm/ivpu/ivpu_hw_mtl_reg.h
>>>>    create mode 100644 drivers/gpu/drm/ivpu/ivpu_hw_reg_io.h
>>>>    create mode 100644 include/uapi/drm/ivpu_drm.h
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 9475aa701a3f..d72ceef107e6 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -7046,6 +7046,14 @@ F:    Documentation/devicetree/bindings/gpu/vivante,gc.yaml
>>>>    F:    drivers/gpu/drm/etnaviv/
>>>>    F:    include/uapi/drm/etnaviv_drm.h
>>>>    +DRM DRIVERS FOR VPU
>>>> +M:    Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
>>>> +M:    Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
>>>> +S:    Supported
>>>> +T:    git git://anongit.freedesktop.org/drm/drm-misc
>>>> +F:    drivers/gpu/drm/ivpu/
>>>> +F:    include/uapi/drm/ivpu_drm.h
>>>
>>> No mail list?
>>
>> OK, I will add a link to dri-devel.
>>
>>>> +
>>>>    DRM DRIVERS FOR XEN
>>>>    M:    Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>    L:    dri-devel@lists.freedesktop.org
>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>>> index 198ba846d34b..0aaac0e5366f 100644
>>>> --- a/drivers/gpu/drm/Kconfig
>>>> +++ b/drivers/gpu/drm/Kconfig
>>>> @@ -364,6 +364,8 @@ source "drivers/gpu/drm/v3d/Kconfig"
>>>>      source "drivers/gpu/drm/vc4/Kconfig"
>>>>    +source "drivers/gpu/drm/ivpu/Kconfig"
>>>> +
>>>
>>> Why here of all places?  Just randomly in the middle of the list of sourced Kconfigs?
>>
>> I'll move it to the end.
> 
> I known that the Kconfigs and Makefiles are chaotic. But if you can, try to sort it alphabetically.

Man, this is an impossible task :)
Would the line under i915 be OK?

>>
>>>>    source "drivers/gpu/drm/etnaviv/Kconfig"
>>>>      source "drivers/gpu/drm/hisilicon/Kconfig"
>>>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>>>> index 25d0ba310509..1bfd7415c2d0 100644
>>>> --- a/drivers/gpu/drm/Makefile
>>>> +++ b/drivers/gpu/drm/Makefile
>>>> @@ -94,6 +94,7 @@ obj-$(CONFIG_DRM_KMB_DISPLAY)  += kmb/
>>>>    obj-$(CONFIG_DRM_MGAG200) += mgag200/
>>>>    obj-$(CONFIG_DRM_V3D)  += v3d/
>>>>    obj-$(CONFIG_DRM_VC4)  += vc4/
>>>> +obj-$(CONFIG_DRM_IVPU)  += ivpu/
>>>
>>> Again, why here?
>>
>> I'll move it to the end.
> 
> Same.

Under i915?

Regards,
Jacek