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

[Alan Tull <atull@kernel.org>]

On Wed, Jul 26, 2017 at 9:20 AM, Alan Tull <atull@kernel.org> wrote:
> On Wed, Jul 26, 2017 at 4:50 AM, Wu Hao <hao.wu@intel.com> 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@intel.com> wrote:
>>>
>>> Hi Hao,
>>>
>>> > On Fri, Mar 31, 2017 at 12:01:13PM -0700, matthew.gerlach@linux.intel.com 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.

> 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@vger.kernel.org
>>> >> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> >> >