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

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 5263 bytes --]

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.

> 
>>>    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.

Best regards
Thomas

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]