Re: [PATCH 11/16] fpga: intel: fme: add partial reconfiguration sub feature support

[Wu Hao <hao.wu@intel.com>]

On Mon, Apr 03, 2017 at 03:49:41PM -0700, matthew.gerlach@linux.intel.com wrote:
> 
> 
> On Mon, 3 Apr 2017, Alan Tull wrote:
> 
> >On Thu, Mar 30, 2017 at 7:08 AM, Wu Hao <hao.wu@intel.com> wrote:
> >>From: Kang Luwei <luwei.kang@intel.com>
> >>
> >>Partial Reconfiguration (PR) is the most important function for FME. It
> >>allows reconfiguration for given Port/Accelerated Function Unit (AFU).
> >>
> >>This patch adds support for PR sub feature. In this patch, it registers
> >>a fpga_mgr and implements fpga_manager_ops, and invoke fpga_mgr_buf_load
> >>for PR operation once PR request received via ioctl.
> >
> >The code that invokes fpga_mgr_buf_load should be a different layer.
> >
> >>Below user space
> >>interfaces are exposed by this sub feature.
> >>
> >>Sysfs interface:
> >>* /sys/class/fpga/<fpga.x>/<intel-fpga-fme.x>/interface_id
> >>  Read-only. Indicate the hardware interface information. Userspace
> >>  applications need to check this interface to select correct green
> >>  bitstream format before PR.
> >>
> >>Ioctl interface:
> >>* FPGA_FME_PORT_PR
> >>  Do partial reconfiguration per information from userspace, including
> >>  target port(AFU), buffer size and address info. It returns the PR status
> >>  (PR error code if failed) to userspace.
> >>
> >>Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
> >>Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
> >>Signed-off-by: Shiva Rao <shiva.rao@intel.com>
> >>Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
> >>Signed-off-by: Alan Tull <alan.tull@intel.com>
> >>Signed-off-by: Kang Luwei <luwei.kang@intel.com>
> >>Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> >>Signed-off-by: Wu Hao <hao.wu@intel.com>
> >>---
> >> drivers/fpga/intel/Makefile      |   2 +-
> >> drivers/fpga/intel/feature-dev.h |  58 ++++++
> >> drivers/fpga/intel/fme-main.c    |  44 ++++-
> >> drivers/fpga/intel/fme-pr.c      | 400 +++++++++++++++++++++++++++++++++++++++
> >> drivers/fpga/intel/fme.h         |  32 ++++
> >> include/uapi/linux/intel-fpga.h  |  44 +++++
> >> 6 files changed, 578 insertions(+), 2 deletions(-)
> >> create mode 100644 drivers/fpga/intel/fme-pr.c
> >> create mode 100644 drivers/fpga/intel/fme.h
> >>
> >>diff --git a/drivers/fpga/intel/Makefile b/drivers/fpga/intel/Makefile
> >>index 546861d..0452cb6 100644
> >>--- a/drivers/fpga/intel/Makefile
> >>+++ b/drivers/fpga/intel/Makefile
> >>@@ -2,4 +2,4 @@ obj-$(CONFIG_INTEL_FPGA_PCI) += intel-fpga-pci.o
> >> obj-$(CONFIG_INTEL_FPGA_FME) += intel-fpga-fme.o
> >>
> >> intel-fpga-pci-objs := pcie.o feature-dev.o
> >>-intel-fpga-fme-objs := fme-main.o
> >>+intel-fpga-fme-objs := fme-main.o fme-pr.o
> >>diff --git a/drivers/fpga/intel/feature-dev.h b/drivers/fpga/intel/feature-dev.h
> >>index dccc283..5a25c915 100644
> >>--- a/drivers/fpga/intel/feature-dev.h
> >>+++ b/drivers/fpga/intel/feature-dev.h
> >>@@ -150,8 +150,66 @@ struct feature_fme_err {
> >> };
> >>
> >> /* FME Partial Reconfiguration Sub Feature Register Set */
> >>+/* FME PR Control Register */
> >>+struct feature_fme_pr_ctl {
> >>+       union {
> >>+               u64 csr;
> >>+               struct {
> >>+                       u8  pr_reset:1;         /* Reset PR Engine */
> >>+                       u8  rsvdz1:3;
> >>+                       u8  pr_reset_ack:1;     /* Reset PR Engine Ack */
> >>+                       u8  rsvdz2:3;
> >>+                       u8  pr_regionid:2;      /* PR Region ID */
> >>+                       u8  rsvdz3:2;
> >>+                       u8  pr_start_req:1;     /* PR Start Request */
> >>+                       u8  pr_push_complete:1; /* PR Data push complete */
> >>+                       u8  pr_kind:1;          /* Load Customer or Intel GBS */
> >>+                       u32 rsvdz4:17;
> >>+                       u32 config_data;
> >>+               };
> >>+       };
> >>+};
> >>+
> >>+/* FME PR Status Register */
> >>+struct feature_fme_pr_status {
> >>+       union {
> >>+               u64 csr;
> >>+               struct {
> >>+                       u16 pr_credit:9;        /* Number of PR Credits */
> >>+                       u8  rsvdz1:7;
> >>+                       u8  pr_status:1;        /* PR Operation status */
> >>+                       u8  rsvdz2:3;
> >>+                       u8  pr_ctrlr_status:3;  /* Controller status */
> >>+                       u8  rsvdz3:1;
> >>+                       u8  pr_host_status:4;   /* PR Host status */
> >>+                       u64 rsvdz4:36;
> >>+               };
> >>+       };
> >>+};
> >>+
> >>+/* FME PR Data Register */
> >>+struct feature_fme_pr_data {
> >>+       union {
> >>+               u64 csr;
> >>+               struct {
> >>+                       /* PR data from the raw-binary file */
> >>+                       u32 pr_data_raw;
> >>+                       u32 rsvd;
> >>+               };
> >>+       };
> >>+};
> >>+
> >> struct feature_fme_pr {
> >>        struct feature_header header;
> >>+       struct feature_fme_pr_ctl control;
> >>+       struct feature_fme_pr_status status;
> >>+       struct feature_fme_pr_data data;
> >>+       u64 error;
> >>+
> >>+       u64 rsvd[16];
> >>+
> >>+       u64 intfc_id_l;         /* PR interface Id Low */
> >>+       u64 intfc_id_h;         /* PR interface Id High */
> >> };
> >>
> >> /* PORT Header Register Set */
> >>diff --git a/drivers/fpga/intel/fme-main.c b/drivers/fpga/intel/fme-main.c
> >>index 36d0c4c..0d9a7a6 100644
> >>--- a/drivers/fpga/intel/fme-main.c
> >>+++ b/drivers/fpga/intel/fme-main.c
> >>@@ -23,6 +23,7 @@
> >> #include <linux/intel-fpga.h>
> >>
> >> #include "feature-dev.h"
> >>+#include "fme.h"
> >>
> >> static ssize_t ports_num_show(struct device *dev,
> >>                              struct device_attribute *attr, char *buf)
> >>@@ -101,6 +102,10 @@ static struct feature_driver fme_feature_drvs[] = {
> >>                .ops = &fme_hdr_ops,
> >>        },
> >>        {
> >>+               .name = FME_FEATURE_PR_MGMT,
> >>+               .ops = &pr_mgmt_ops,
> >>+       },
> >>+       {
> >>                .ops = NULL,
> >>        },
> >> };
> >>@@ -182,14 +187,48 @@ static const struct file_operations fme_fops = {
> >>        .unlocked_ioctl = fme_ioctl,
> >> };
> >>
> >>+static int fme_dev_init(struct platform_device *pdev)
> >>+{
> >>+       struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
> >>+       struct fpga_fme *fme;
> >>+
> >>+       fme = devm_kzalloc(&pdev->dev, sizeof(*fme), GFP_KERNEL);
> >>+       if (!fme)
> >>+               return -ENOMEM;
> >>+
> >>+       fme->pdata = pdata;
> >>+
> >>+       mutex_lock(&pdata->lock);
> >>+       fpga_pdata_set_private(pdata, fme);
> >>+       mutex_unlock(&pdata->lock);
> >>+       return 0;
> >>+}
> >>+
> >>+static void fme_dev_destroy(struct platform_device *pdev)
> >>+{
> >>+       struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
> >>+       struct fpga_fme *fme;
> >>+
> >>+       mutex_lock(&pdata->lock);
> >>+       fme = fpga_pdata_get_private(pdata);
> >>+       fpga_pdata_set_private(pdata, NULL);
> >>+       mutex_unlock(&pdata->lock);
> >>+
> >>+       devm_kfree(&pdev->dev, fme);
> >>+}
> >>+
> >> static int fme_probe(struct platform_device *pdev)
> >> {
> >>        int ret;
> >>
> >>-       ret = fpga_dev_feature_init(pdev, fme_feature_drvs);
> >>+       ret = fme_dev_init(pdev);
> >>        if (ret)
> >>                goto exit;
> >>
> >>+       ret = fpga_dev_feature_init(pdev, fme_feature_drvs);
> >>+       if (ret)
> >>+               goto dev_destroy;
> >>+
> >>        ret = fpga_register_dev_ops(pdev, &fme_fops, THIS_MODULE);
> >>        if (ret)
> >>                goto feature_uinit;
> >>@@ -198,6 +237,8 @@ static int fme_probe(struct platform_device *pdev)
> >>
> >> feature_uinit:
> >>        fpga_dev_feature_uinit(pdev);
> >>+dev_destroy:
> >>+       fme_dev_destroy(pdev);
> >> exit:
> >>        return ret;
> >> }
> >>@@ -206,6 +247,7 @@ static int fme_remove(struct platform_device *pdev)
> >> {
> >>        fpga_dev_feature_uinit(pdev);
> >>        fpga_unregister_dev_ops(pdev);
> >>+       fme_dev_destroy(pdev);
> >>        return 0;
> >> }
> >>
> >>diff --git a/drivers/fpga/intel/fme-pr.c b/drivers/fpga/intel/fme-pr.c
> >>new file mode 100644
> >>index 0000000..3b44a3e
> >>--- /dev/null
> >>+++ b/drivers/fpga/intel/fme-pr.c
> >>@@ -0,0 +1,400 @@
> >>+/*
> >>+ * Driver for Intel FPGA Management Engine (FME) Partial Reconfiguration
> >>+ *
> >>+ * Copyright (C) 2017 Intel Corporation, Inc.
> >>+ *
> >>+ * Authors:
> >>+ *   Kang Luwei <luwei.kang@intel.com>
> >>+ *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
> >>+ *   Joseph Grecco <joe.grecco@intel.com>
> >>+ *   Enno Luebbers <enno.luebbers@intel.com>
> >>+ *   Tim Whisonant <tim.whisonant@intel.com>
> >>+ *   Ananda Ravuri <ananda.ravuri@intel.com>
> >>+ *   Christopher Rauer <christopher.rauer@intel.com>
> >>+ *   Henry Mitchel <henry.mitchel@intel.com>
> >>+ *
> >>+ * This work is licensed under a dual BSD/GPLv2 license. When using or
> >>+ * redistributing this file, you may do so under either license. See the
> >>+ * LICENSE.BSD file under this directory for the BSD license and see
> >>+ * the COPYING file in the top-level directory for the GPLv2 license.
> >>+ */
> >>+
> >>+#include <linux/types.h>
> >>+#include <linux/device.h>
> >>+#include <linux/vmalloc.h>
> >>+#include <linux/uaccess.h>
> >>+#include <linux/fpga/fpga-mgr.h>
> >>+#include <linux/intel-fpga.h>
> >>+
> >>+#include "feature-dev.h"
> >>+#include "fme.h"
> >>+
> >>+#define PR_WAIT_TIMEOUT   8000000
> >>+
> >>+#define PR_HOST_STATUS_IDLE    0
> >>+
> >>+DEFINE_FPGA_PR_ERR_MSG(pr_err_msg);
> >>+
> >>+static ssize_t interface_id_show(struct device *dev,
> >>+                                struct device_attribute *attr, char *buf)
> >>+{
> >>+       u64 intfc_id_l, intfc_id_h;
> >>+       struct feature_fme_pr *fme_pr
> >>+               = get_feature_ioaddr_by_index(dev, FME_FEATURE_ID_PR_MGMT);
> >>+
> >>+       intfc_id_l = readq(&fme_pr->intfc_id_l);
> >>+       intfc_id_h = readq(&fme_pr->intfc_id_h);
> >>+
> >>+       return scnprintf(buf, PAGE_SIZE, "%016llx%016llx\n",
> >>+                       (unsigned long long)intfc_id_h,
> >>+                       (unsigned long long)intfc_id_l);
> >>+}
> >>+static DEVICE_ATTR_RO(interface_id);
> >>+
> >>+static struct attribute *pr_mgmt_attrs[] = {
> >>+       &dev_attr_interface_id.attr,
> >>+       NULL,
> >>+};
> >>+
> >>+struct attribute_group pr_mgmt_attr_group = {
> >>+       .attrs  = pr_mgmt_attrs,
> >>+       .name   = "pr",
> >>+};
> >>+
> >>+static u64
> >>+pr_err_handle(struct platform_device *pdev, struct feature_fme_pr *fme_pr)
> >>+{
> >>+       struct feature_fme_pr_status fme_pr_status;
> >>+       unsigned long err_code;
> >>+       u64 fme_pr_error;
> >>+       int i = 0;
> >>+
> >>+       fme_pr_status.csr = readq(&fme_pr->status);
> >>+       if (!fme_pr_status.pr_status)
> >>+               return 0;
> >>+
> >>+       err_code = fme_pr_error = readq(&fme_pr->error);
> >>+       for_each_set_bit(i, &err_code, PR_MAX_ERR_NUM)
> >>+               dev_dbg(&pdev->dev, "%s\n", pr_err_msg[i]);
> >>+       writeq(fme_pr_error, &fme_pr->error);
> >>+       return fme_pr_error;
> >>+}
> >>+
> >>+static int fme_pr_write_init(struct fpga_manager *mgr,
> >>+               struct fpga_image_info *info, const char *buf, size_t count)
> >>+{
> >>+       struct fpga_fme *fme = mgr->priv;
> >>+       struct platform_device *pdev;
> >>+       struct feature_fme_pr *fme_pr;
> >>+       struct feature_fme_pr_ctl fme_pr_ctl;
> >>+       struct feature_fme_pr_status fme_pr_status;
> >>+
> >>+       pdev = fme->pdata->dev;
> >>+       fme_pr = get_feature_ioaddr_by_index(&pdev->dev,
> >>+                               FME_FEATURE_ID_PR_MGMT);
> >>+       if (!fme_pr)
> >>+               return -EINVAL;
> >>+
> >>+       if (WARN_ON(info->flags != FPGA_MGR_PARTIAL_RECONFIG))
> >>+               return -EINVAL;
> >
> >flags is bitmapped so please do:
> >
> >if (WARN_ON(info->flags & FPGA_MGR_PARTIAL_RECONFIG))
> >
> >>+
> >>+       dev_dbg(&pdev->dev, "resetting PR before initiated PR\n");
> >>+
> >>+       fme_pr_ctl.csr = readq(&fme_pr->control);
> >>+       fme_pr_ctl.pr_reset = 1;
> >>+       writeq(fme_pr_ctl.csr, &fme_pr->control);
> >>+
> >>+       fme_pr_ctl.pr_reset_ack = 1;
> >>+
> >>+       if (fpga_wait_register_field(pr_reset_ack, fme_pr_ctl,
> >>+                                    &fme_pr->control, PR_WAIT_TIMEOUT, 1)) {
> >>+               dev_err(&pdev->dev, "maximum PR timeout\n");
> >>+               return -ETIMEDOUT;
> >>+       }
> >>+
> >>+       fme_pr_ctl.csr = readq(&fme_pr->control);
> >>+       fme_pr_ctl.pr_reset = 0;
> >>+       writeq(fme_pr_ctl.csr, &fme_pr->control);
> >>+
> >>+       dev_dbg(&pdev->dev,
> >>+               "waiting for PR resource in HW to be initialized and ready\n");
> >>+
> >>+       fme_pr_status.pr_host_status = PR_HOST_STATUS_IDLE;
> >>+
> >>+       if (fpga_wait_register_field(pr_host_status, fme_pr_status,
> >>+                                    &fme_pr->status, PR_WAIT_TIMEOUT, 1)) {
> >>+               dev_err(&pdev->dev, "maximum PR timeout\n");
> >>+               return -ETIMEDOUT;
> >>+       }
> >>+
> >>+       dev_dbg(&pdev->dev, "check if have any previous PR error\n");
> >>+       pr_err_handle(pdev, fme_pr);
> >>+       return 0;
> >>+}
> >>+
> >>+static int fme_pr_write(struct fpga_manager *mgr,
> >>+                       const char *buf, size_t count)
> >>+{
> >>+       struct fpga_fme *fme = mgr->priv;
> >>+       struct platform_device *pdev;
> >>+       struct feature_fme_pr *fme_pr;
> >>+       struct feature_fme_pr_ctl fme_pr_ctl;
> >>+       struct feature_fme_pr_status fme_pr_status;
> >>+       struct feature_fme_pr_data fme_pr_data;
> >>+       int delay, pr_credit, i = 0;
> >>+
> >>+       pdev = fme->pdata->dev;
> >>+       fme_pr = get_feature_ioaddr_by_index(&pdev->dev,
> >>+                               FME_FEATURE_ID_PR_MGMT);
> >>+
> >>+       dev_dbg(&pdev->dev, "set PR port ID and start request\n");
> >>+
> >>+       fme_pr_ctl.csr = readq(&fme_pr->control);
> >>+       fme_pr_ctl.pr_regionid = fme->port_id;
> >>+       fme_pr_ctl.pr_start_req = 1;
> >>+       writeq(fme_pr_ctl.csr, &fme_pr->control);
> >>+
> >>+       dev_dbg(&pdev->dev, "pushing data from bitstream to HW\n");
> >>+
> >>+       fme_pr_status.csr = readq(&fme_pr->status);
> >>+       pr_credit = fme_pr_status.pr_credit;
> >>+
> >>+       while (count > 0) {
> >>+               delay = 0;
> >>+               while (pr_credit <= 1) {
> >>+                       if (delay++ > PR_WAIT_TIMEOUT) {
> >>+                               dev_err(&pdev->dev, "maximum try\n");
> >>+                               return -ETIMEDOUT;
> >>+                       }
> >>+                       udelay(1);
> >>+
> >>+                       fme_pr_status.csr = readq(&fme_pr->status);
> >>+                       pr_credit = fme_pr_status.pr_credit;
> >>+               };
> >>+
> >>+               if (count >= 4) {
> >>+                       fme_pr_data.rsvd = 0;
> >>+                       fme_pr_data.pr_data_raw = *(((u32 *)buf) + i);
> >>+                       writeq(fme_pr_data.csr, &fme_pr->data);
> >>+                       count -= 4;
> >>+                       pr_credit--;
> >>+                       i++;
> >>+               } else {
> >>+                       WARN_ON(1);
> >>+                       return -EINVAL;
> >>+               }
> >>+       }
> >>+
> >>+       return 0;
> >>+}
> >>+
> >>+static int fme_pr_write_complete(struct fpga_manager *mgr,
> >>+                       struct fpga_image_info *info)
> >>+{
> >>+       struct fpga_fme *fme = mgr->priv;
> >>+       struct platform_device *pdev;
> >>+       struct feature_fme_pr *fme_pr;
> >>+       struct feature_fme_pr_ctl fme_pr_ctl;
> >>+
> >>+       pdev = fme->pdata->dev;
> >>+       fme_pr = get_feature_ioaddr_by_index(&pdev->dev,
> >>+                               FME_FEATURE_ID_PR_MGMT);
> >>+
> >>+       fme_pr_ctl.csr = readq(&fme_pr->control);
> >>+       fme_pr_ctl.pr_push_complete = 1;
> >>+       writeq(fme_pr_ctl.csr, &fme_pr->control);
> >>+
> >>+       dev_dbg(&pdev->dev, "green bitstream push complete\n");
> >>+       dev_dbg(&pdev->dev, "waiting for HW to release PR resource\n");
> >>+
> >>+       fme_pr_ctl.pr_start_req = 0;
> >>+
> >>+       if (fpga_wait_register_field(pr_start_req, fme_pr_ctl,
> >>+                                    &fme_pr->control, PR_WAIT_TIMEOUT, 1)) {
> >>+               dev_err(&pdev->dev, "maximum try.\n");
> >>+               return -ETIMEDOUT;
> >>+       }
> >>+
> >>+       dev_dbg(&pdev->dev, "PR operation complete, checking status\n");
> >>+       fme->pr_err = pr_err_handle(pdev, fme_pr);
> >>+       if (fme->pr_err)
> >>+               return -EIO;
> >>+
> >>+       dev_dbg(&pdev->dev, "PR done successfully\n");
> >>+       return 0;
> >>+}
> >>+
> >>+static enum fpga_mgr_states fme_pr_state(struct fpga_manager *mgr)
> >>+{
> >>+       return FPGA_MGR_STATE_UNKNOWN;
> >>+}
> 
> The functions that implement the fme_pr_ops are really a low level fpga
> manager driver for the Altera PR IP component.  A standalone version of
> such a driver has been reviewed and Acked.  See the links below.
> Could this file use those functions and remove this code?
> 
> http://marc.info/?l=linux-kernel&m=149019678925564&w=2
> http://marc.info/?l=linux-kernel&m=149019654225457&w=2
> http://marc.info/?l=linux-kernel&m=149019598025274&w=2
> http://marc.info/?l=linux-kernel&m=149013051007149&w=2
> 

Thanks for the info, I just checked these code quickly, but it seems the
register definitions are totally different, so I feel that we may not be
able to reuse this driver.

Thanks
Hao