On Mon, Feb 5, 2018 at 7:47 PM, Wu Hao <hao.wu@xxxxxxxxx> wrote: > On Mon, Feb 05, 2018 at 10:36:45AM -0800, Luebbers, Enno wrote: >> Hi Hao, >> >> On Sun, Feb 04, 2018 at 05:37:06PM +0800, Wu Hao wrote: >> > On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote: >> > > Hi Hao, Alan, >> > > >> > > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote: >> > > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote: >> > > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao <hao.wu@xxxxxxxxx> wrote: >> > > > > >> > > > > Hi Hao, >> > > > > >> > > > > A few comments below. Besides that, looks good. >> > > > > >> > > > > > This patch adds fpga manager driver for FPGA Management Engine (FME). It >> > > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration function. >> > > > > > >> > > > > > Signed-off-by: Tim Whisonant <tim.whisonant@xxxxxxxxx> >> > > > > > Signed-off-by: Enno Luebbers <enno.luebbers@xxxxxxxxx> >> > > > > > Signed-off-by: Shiva Rao <shiva.rao@xxxxxxxxx> >> > > > > > Signed-off-by: Christopher Rauer <christopher.rauer@xxxxxxxxx> >> > > > > > Signed-off-by: Kang Luwei <luwei.kang@xxxxxxxxx> >> > > > > > Signed-off-by: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx> >> > > > > > Signed-off-by: Wu Hao <hao.wu@xxxxxxxxx> >> > > > > > ---- >> > > > > > v3: rename driver to dfl-fpga-fme-mgr >> > > > > > implemented status callback for fpga manager >> > > > > > rebased due to fpga api changes >> > > > > > --- >> > > > > > .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr | 8 + >> > > > > > drivers/fpga/Kconfig | 6 + >> > > > > > drivers/fpga/Makefile | 1 + >> > > > > > drivers/fpga/fpga-dfl-fme-mgr.c | 318 +++++++++++++++++++++ >> > > > > > drivers/fpga/fpga-dfl.h | 39 ++- >> > > > > > 5 files changed, 371 insertions(+), 1 deletion(-) >> > > > > > create mode 100644 Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr >> > > > > > create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c >> > > > > > >> > > > > > diff --git a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr >> > > > > > new file mode 100644 >> > > > > > index 0000000..2d4f917 >> > > > > > --- /dev/null >> > > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr >> > > > > > @@ -0,0 +1,8 @@ >> > > > > > +What: /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id >> > > > > > +Date: November 2017 >> > > > > > +KernelVersion: 4.15 >> > > > > > +Contact: Wu Hao <hao.wu@xxxxxxxxx> >> > > > > > +Description: Read-only. It returns interface id of partial reconfiguration >> > > > > > + hardware. Userspace could use this information to check if >> > > > > > + current hardware is compatible with given image before FPGA >> > > > > > + programming. >> > > > > >> > > > > I'm a little confused by this. I can understand that the PR bitstream >> > > > > has a dependency on the FPGA's static image, but I don't understand >> > > > > the dependency of the bistream on the hardware that is used to program >> > > > > the bitstream to the FPGA. >> > > > >> > > > Sorry for the confusion, the interface_id is used to indicate the version of >> > > > the hardware for partial reconfiguration (it's part of the static image of >> > > > the FPGA device). Will improve the description on this. >> > > > >> > > >> > > The interface_id expresses the compatibility of the static region with PR >> > > bitstreams generated for it. It changes every time a new static region is >> > > generated. >> > > >> > > Would it make more sense to have the interface_id exposed as part of the FME >> > > device (which represents the static region)? I'm not sure - it kind of also >> > > makes sense here, where you would have all the information in one place (if the >> > > interface_id matches, I can use this component to program a bitstream). >> > >> > Hi Enno >> > >> > Yes, this interface is under fpga-dfl-fme-mgr.0, and fpga-dfl-fme-mgr.0 is >> > under fpga-dfl-fme.0. It's part of the FME device for sure. From another >> > point of view, it means if anyone wants to do PR on this Intel FPGA device, >> > he needs to find the FME device firstly, and then check if any fpga manager >> > created under this FME device, if yes, check the interface_id before PR via >> > the FME device node ioctl. >> >> That sounds good, thank you! >> >> > >> > > >> > > Sorry for my limited understanding of the infrastructure - would this same >> > > "fpga-dfl-fme-mgr.0" be used for PR if we had multiple PR regions? In that case >> > > it would need to expose multiple interface_ids (or we'd have to track both >> > > interface IDs and an identifier for the target PR region). >> > >> > Yes, the fpga manager could be shared with different PR regions. >> > >> > Sorry, I'm not sure where we need to expose multiple interface_ids and why. >> >> It's basically a question of how to determine bitstream compatibility - either, >> there's a separate interface_id per reconfigurable region, or there is a single >> interface_id for the entire device. Both make sense from a certain perspective. >> >> If there are multiple interface_ids per device (one per region), the driver >> would need to expose all of them. If there's only a single one, the driver only >> exposes that one ID - compatibility would be determined by looking at both that >> single interface_id _and_ the identifier/number of the targeted region. >> >> I would prefer a separate interface_id per region - it seems more generic and >> flexible. Hi Enno, I agree with this. > > It's possible to have per region interface_id (or even both per dev interface_id > and per region interface_id at the same time), but per FME PR sub feature > implementation, it supports multiple PR regions, but only provide one interface > id, so at least in this case, it's not per-region information per my > understanding. We can consider it later when hardware really supports it. : ) Hi Hao, I understand that in the case of this PR hardware, the region to program is selected when the region_id to program is written to a PR hardware control register. For another example, Arria10 has a hard PR hardware and the PR bitstream lands in the area of the FPGA for which it was compiled. In that case, for the PR bitstream to be compatible with a PR region, the layout of the edge connections also needs to be compatible, so compatibility is per-region in that case instead of per-PR hardware. And besides, as I said yesterday, the hard PR hardware would not know what the static region ID is when this framework is used with such a device. That's why I think making the id per-region may be more future proof, even if it may see unnecessary in the case of the original blue bits this was written for. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
