Re: [PATCH 02/16] fpga: add FPGA device framework

[Wu Hao]

On Wed, Jul 26, 2017 at 05:29:11PM -0500, Alan Tull wrote:
> On Wed, Jul 26, 2017 at 9:20 AM, Alan Tull <atull@xxxxxxxxxx> wrote:
> > On Wed, Jul 26, 2017 at 4:50 AM, Wu Hao <hao.wu@xxxxxxxxx> wrote:
> >> On Tue, Jul 25, 2017 at 04:32:10PM -0500, Alan Tull wrote:
> >>> On Sat, Apr 1, 2017 at 7:18 AM, Wu Hao <hao.wu@xxxxxxxxx> wrote:
> >>>
> >>> Hi Hao,
> >>>
> >>> > On Fri, Mar 31, 2017 at 12:01:13PM -0700, matthew.gerlach@xxxxxxxxxxxxxxx wrote:
> >>> >> On Fri, 31 Mar 2017, Wu Hao wrote:
> >>> >> >On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote:
> >>> >> >>On Thu, Mar 30, 2017 at 08:08:02PM +0800, Wu Hao wrote:
> >>> >> >>>During FPGA device (e.g PCI-based) discovery, platform devices are
> >>> >> >>>registered for different FPGA function units. But the device node path
> >>> >> >>>isn't quite friendly to applications.
> >>> >> >>>
> >>> >> >>>Consider this case, applications want to access child device's sysfs file
> >>> >> >>>for some information.
> >>> >> >>>
> >>> >> >>>1) Access using bus-based path (e.g PCI)
> >>> >> >>>
> >>> >> >>>  /sys/bus/pci/devices/xxxxx/fpga_func_a.0/sysfs_file
> >>> >> >>>
> >>> >> >>>  From the path, it's clear which PCI device is the parent, but not perfect
> >>> >> >>>  solution for applications. PCI device BDF is not fixed, application may
> >>> >> >>>  need to search all PCI device to find the actual FPGA Device.
> >>> >> >>>
> >>> >> >>>2) Or access using platform device path
> >>> >> >>>
> >>> >> >>>  /sys/bus/platform/devices/fpga_func_a.0/sysfs_file
> >>> >> >>>
> >>> >> >>>  Applications find the actual function by name easily, but no information
> >>> >> >>>  about which fpga device it belongs to. It's quite confusing if multiple
> >>> >> >>>  FPGA devices are in one system.
> >>> >> >>>
> >>> >> >>>'FPGA Device' class is introduced to resolve this problem. Each node under
> >>> >> >>>this class represents a fpga device, which may have one or more child
> >>> >> >>>devices. Applications only need to search under this FPGA Device class
> >>> >> >>>folder to find the child device node it needs.
> >>> >> >>>
> >>> >> >>>For example, for the platform has 2 fpga devices, each fpga device has
> >>> >> >>>3 child devices, the hierarchy looks like this.
> >>> >> >>>
> >>> >> >>>Two nodes are under /sys/class/fpga/:
> >>> >> >>>/sys/class/fpga/fpga.0
> >>> >> >>>/sys/class/fpga/fpga.1
> >>> >> >>>
> >>> >> >>>Each node has 1 function A device and 2 function B devices:
> >>> >> >>>/sys/class/fpga/fpga.0/func_a.0
> >>> >> >>>/sys/class/fpga/fpga.0/func_b.0
> >>> >> >>>/sys/class/fpga/fpga.0/func_b.1
> >>> >> >>>
> >>> >> >>>/sys/class/fpga/fpga.1/func_a.1
> >>> >> >>>/sys/class/fpga/fpga.1/func_b.2
> >>> >> >>>/sys/class/fpga/fpga.1/func_b.3
> >>> >> >>>
> >>> >> >>>This following APIs are provided by FPGA device framework:
> >>> >> >>>* fpga_dev_create
> >>> >> >>>  Create fpga device under the given parent device.
> >>> >> >>>* fpga_dev_destroy
> >>> >> >>>  Destroy fpga device
> >>> >> >>>
> >>> >> >>>The following sysfs files are created:
> >>> >> >>>* /sys/class/fpga/<fpga.x>/name
> >>> >> >>>  Name of the fpga device.
> >>> >> >>
> >>> >> >>How does this interact with the existing "fpga class" that is in the
> >>> >> >>kernel already?
> >>> >> >
> >>> >> >The fpga-dev introduced by this patch, is only a container device, and
> >>> >>
> >>> >> I completely understand the need for a container device.  The fpga-region is
> >>> >> also primarily a container, and in some cases the fpga-region may represent
> >>> >> the entire fpga.  Over time this code may become redundant.
> >>> >
> >>> > Thanks a lot for your review and comments.
> >>> >
> >>> > I feel that the fpga-region implies that it supports reconfiguration,
> >>>
> >>> On Arria10, we create base fpga region which does not support full
> >>> reconfiguration.  It corresponds to the whole FPGA area, which was
> >>> loaded with a static FPGA image in the bootloader.  The partial
> >>> reconfiguration regions are children of the base FPGA region.  Any
> >>> devices in the FPGA are child devices of either the base region or a
> >>> region which is a child of it.
> >>>
> >>> > but
> >>> > in our cases, the Intel FPGA device, doesn't have base fpga-region for
> >>> > full reconfiguration, but many accelerators with partial reconfiguration
> >>> > support. A fpga-region brings together everything needed for the
> >>> > reconfiguration, and a fpga-dev is trying to brings everything on a FPGA
> >>> > device together, including fpga-region/bridge/manager, access different
> >>> > accelerators and other function units.
> >>> >
> >>> > I think it's not mandatory to use fpga-dev, as fpga-dev is just trying to
> >>> > provide one more option here for some complex hardware.
> >>>
> >>> Now that you've put out v2 which uses fpga-regions, do you still need
> >>> fpga-dev class?
> >>
> >> Hi Alan
> >>
> >> Thanks for the comments.
> >>
> >> In v2, I have updated the driver organization section in intel-fpga.txt[1].
> >
> >
> > I've read your v2 of this document.  It's changed as you've said, but
> > not that much.
> 
> I should clarify here that, yes I see that in v2 you're now using
> regions and bridges and I appreciate that.  I'm just trying to see
> what a good relationship between the existing fpga classes and the new
> fpga-dev class would be.

Sure, I think these existing fpga classes could be used as child nodes
under fpga-dev, or directly used without it. If we use fpga-dev as container
then user may be able to find the target region easily from sysfs hierarchy
(e.g user wants to do PR on 3rd fpga device's 7th PR region via its fpga
region user interface.). : )

Thanks
Hao

> 
> > I'm just continuing the previous conversation.  I'll
> > add further comments on the v2 version.
> >
> > Alan
> >
> >> The fpga-regions/bridges/manager are created as children of FME module, as
> >> the partial reconfiguration function is only a sub feature of FME module.
> >>
> >> If switch to fpga-region as container device, it may not be easy for user
> >> space applications to know which one represents a FPGA device and which one
> >> represents a reconfigurable region as all have the similar name 'regionx'
> >> in the same sysfs folder. Please consider this case, if we have 5 fpga
> >> devices on one system and each fpga device has multiple PR regions (e.g 20+).
> >> Then user space applications need to search all regions to locate the ones
> >> represent the FPGA device, even we add some attributes to it.
> >>
> >> [1]http://marc.info/?l=linux-fpga&m=149844234509825&w=2
> >>
> >> Thanks
> >> Hao
> >>
> >>>
> >>> Alan
> >>>
> >>> >
> >>> > Thanks
> >>> > Hao
> >>> >
> >>> >> >drivers could register different functions under it. Per my understanding,
> >>> >> >the existing "fpga class", including fpga-region, fpga-bridge and
> >>> >> >fpga-manager, is used to provide reconfiguration function for FPGA. So
> >>> >> >driver can create child node using this existing "fpga class" to provide
> >>> >> >FPGA reconfiguration function, and more nodes under this container for
> >>> >> >different functions for given FPGA device.
> >>> >> >
> >>> >> >For Intel FPGA device, partial reconfiguration is only one function of
> >>> >> >Intel FPGA Management Engine (FME). FME driver creates fpga_manager under
> >>> >> >below path for partial reconfiguration, and other interfaces for more
> >>> >> >functions, e.g power management, virtualization support and etc.
> >>> >> >
> >>> >> >/sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/fpga_manager
> >>> >> >
> >>> >> >Thanks
> >>> >> >Hao
> >>> >> >
> >>> >> >>
> >>> >> >>thanks,
> >>> >> >>
> >>> >> >>greg k-h
> >>> >> >--
> >>> >> >To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
> >>> >> >the body of a message to majordomo@xxxxxxxxxxxxxxx
> >>> >> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>> >> >