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

From: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
To: Dave Airlie <airlied@gmail.com>
Cc: Dave Airlie <airlied@linux.ie>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>,
	andrzej.kacprowski@linux.intel.com, "Koenig,
	Christian" <Christian.Koenig@amd.com>
Subject: Re: [PATCH v1 0/7] New DRM driver for Intel VPU
Date: Mon, 8 Aug 2022 16:49:18 +0200	[thread overview]
Message-ID: <20220808144918.GA2666787@linux.intel.com> (raw)
In-Reply-To: <CAPM=9tyqsK3X_+s53Xy5jUs+cq3+aSkYne-E4NLLJ+NBnRqjNg@mail.gmail.com>

On Mon, Aug 08, 2022 at 12:34:59PM +1000, Dave Airlie wrote:
> On Thu, 28 Jul 2022 at 23:17, Jacek Lawrynowicz
> <jacek.lawrynowicz@linux.intel.com> 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.
> >
> > 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 Q3.
> > The firmware for the VPU will be distributed as a closed source binary.
> 
> 
> Thanks for the submission, this looks pretty good and well layed out,
> 
> just a few higher level things, I think I'd like this name intel-vpu
> or ivpu or something, VPU is a pretty generic namespace usage.

Thanks for the comments, we will consider renaming. 

> I think adding some sort of TODO file with what is missing and what
> future things need to happen would be useful to know when merging this
> might be a good idea.
> 
> I'm kinda thinking with a rename we could merge this sooner into a
> staging-lite model.

I'm not sure what we can add to TODO file, from driver perspective
I think it's pretty much ready for merging (except renaming), just
other components: F/W and user-space are not yet released.

> I think I'd like Christian/Maarten to maybe review the fencing/uapi,
> to make sure nothing too much is wrong there. The submit/waitbo model
> is getting a bit old, and using syncobjs might be useful to make it
> more modern. Is this device meant to be used by multiple users at
> once? Maybe we'd want scheduler integration for it as well (which I
> think I saw mentioned somewhere in passing).

The current approach with submit/wait_bo is simplistic but sufficient
for basic use case. In the future we are planning to add support for
HW based scheduling (we are also looking at SW scheduler) and we will
likely revisit submit/sync APIs at that time.

Regards
Stanislaw