Skip to contentSkip to conversation

fpga: altera-cvp: Add Stratix10 Support

PATCH Superseded [email protected] 2 participants 10 messages Jul 11, 2019 – Jul 15, 2019 FPGA manager framework

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

cover fpga: altera-cvp: Add Stratix10 Support

From: Thor Thayer <[email protected]>

Newer versions (V2) of Altera/Intel FPGAs CvP have different PCI Vendor Specific Capability offsets than the older (V1) Altera/FPGAs.

Most of the CvP registers and their bitfields remain the same between both the older parts and the newer parts.

This patchset implements changes to discover the Vendor Specific Capability offset and then add Stratix10 CvP support.

Thor Thayer (3):
fpga: altera-cvp: Discover Vendor Specific offset
fpga: altera-cvp: Preparation for V2 parts.
fpga: altera-cvp: Add Stratix10 (V2) Support

 drivers/fpga/altera-cvp.c | 322 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 253 insertions(+), 69 deletions(-)
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/3 fpga: altera-cvp: Discover Vendor Specific offset

From: Thor Thayer <[email protected]>

Newer Intel FPGAs have different Vendor Specific offsets than legacy parts. Use PCI discovery to find the CvP registers. Since the register positions remain the same, change the hard coded address to a more flexible way of indexing registers from the offset.
Adding new PCI read and write abstraction functions to handle the offset (altera_read_config_dword() and altera_write_config_dword()).

---
 drivers/fpga/altera-cvp.c | 91 +++++++++++++++++++++++++++++------------------
 1 file changed, 56 insertions(+), 35 deletions(-)
drivers/fpga/altera-cvp.c +56 -35
index 770915fb97f9..04f2b2a072a7 100644
@@ -22,10 +22,10 @@
 #define TIMEOUT_US	2000	/* CVP STATUS timeout for USERMODE polling */
 
 /* Vendor Specific Extended Capability Registers */
-#define VSE_PCIE_EXT_CAP_ID		0x200
+#define VSE_PCIE_EXT_CAP_ID		0x0
 #define VSE_PCIE_EXT_CAP_ID_VAL		0x000b	/* 16bit */
 
-#define VSE_CVP_STATUS			0x21c	/* 32bit */
+#define VSE_CVP_STATUS			0x1c	/* 32bit */
 #define VSE_CVP_STATUS_CFG_RDY		BIT(18)	/* CVP_CONFIG_READY */
 #define VSE_CVP_STATUS_CFG_ERR		BIT(19)	/* CVP_CONFIG_ERROR */
 #define VSE_CVP_STATUS_CVP_EN		BIT(20)	/* ctrl block is enabling CVP */
@@ -33,18 +33,18 @@
 #define VSE_CVP_STATUS_CFG_DONE		BIT(23)	/* CVP_CONFIG_DONE */
 #define VSE_CVP_STATUS_PLD_CLK_IN_USE	BIT(24)	/* PLD_CLK_IN_USE */
 
-#define VSE_CVP_MODE_CTRL		0x220	/* 32bit */
+#define VSE_CVP_MODE_CTRL		0x20	/* 32bit */
 #define VSE_CVP_MODE_CTRL_CVP_MODE	BIT(0)	/* CVP (1) or normal mode (0) */
 #define VSE_CVP_MODE_CTRL_HIP_CLK_SEL	BIT(1) /* PMA (1) or fabric clock (0) */
 #define VSE_CVP_MODE_CTRL_NUMCLKS_OFF	8	/* NUMCLKS bits offset */
 #define VSE_CVP_MODE_CTRL_NUMCLKS_MASK	GENMASK(15, 8)
 
-#define VSE_CVP_DATA			0x228	/* 32bit */
-#define VSE_CVP_PROG_CTRL		0x22c	/* 32bit */
+#define VSE_CVP_DATA			0x28	/* 32bit */
+#define VSE_CVP_PROG_CTRL		0x2c	/* 32bit */
 #define VSE_CVP_PROG_CTRL_CONFIG	BIT(0)
 #define VSE_CVP_PROG_CTRL_START_XFER	BIT(1)
 
-#define VSE_UNCOR_ERR_STATUS		0x234	/* 32bit */
+#define VSE_UNCOR_ERR_STATUS		0x34	/* 32bit */
 #define VSE_UNCOR_ERR_CVP_CFG_ERR	BIT(5)	/* CVP_CONFIG_ERROR_LATCHED */
 
 #define DRV_NAME		"altera-cvp"
@@ -60,14 +60,27 @@ struct altera_cvp_conf {
 	void			(*write_data)(struct altera_cvp_conf *, u32);
 	char			mgr_name[64];
 	u8			numclks;
+	u32			vsec_offset;
 };
 
+static inline void altera_read_config_dword(struct altera_cvp_conf *conf,
+					    int where, u32 *val)
+{
+	pci_read_config_dword(conf->pci_dev, conf->vsec_offset + where, val);
+}
+
+static inline void altera_write_config_dword(struct altera_cvp_conf *conf,
+					     int where, u32 val)
+{
+	pci_write_config_dword(conf->pci_dev, conf->vsec_offset + where, val);
+}
+
 static enum fpga_mgr_states altera_cvp_state(struct fpga_manager *mgr)
 {
 	struct altera_cvp_conf *conf = mgr->priv;
 	u32 status;
 
-	pci_read_config_dword(conf->pci_dev, VSE_CVP_STATUS, &status);
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &status);
 
 	if (status & VSE_CVP_STATUS_CFG_DONE)
 		return FPGA_MGR_STATE_OPERATING;
@@ -85,7 +98,8 @@ static void altera_cvp_write_data_iomem(struct altera_cvp_conf *conf, u32 val)
 
 static void altera_cvp_write_data_config(struct altera_cvp_conf *conf, u32 val)
 {
-	pci_write_config_dword(conf->pci_dev, VSE_CVP_DATA, val);
+	pci_write_config_dword(conf->pci_dev, conf->vsec_offset + VSE_CVP_DATA,
+			       val);
 }
 
 /* switches between CvP clock and internal clock */
@@ -95,10 +109,10 @@ static void altera_cvp_dummy_write(struct altera_cvp_conf *conf)
 	u32 val;
 
 	/* set 1 CVP clock cycle for every CVP Data Register Write */
-	pci_read_config_dword(conf->pci_dev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
 	val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
 	val |= 1 << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
-	pci_write_config_dword(conf->pci_dev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
 	for (i = 0; i < CVP_DUMMY_WR; i++)
 		conf->write_data(conf, 0); /* dummy data, could be any value */
@@ -115,7 +129,7 @@ static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask,
 		retries++;
 
 	do {
-		pci_read_config_dword(conf->pci_dev, VSE_CVP_STATUS, &val);
+		altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
 		if ((val & status_mask) == status_val)
 			return 0;
 
@@ -130,18 +144,17 @@ static int altera_cvp_teardown(struct fpga_manager *mgr,
 			       struct fpga_image_info *info)
 {
 	struct altera_cvp_conf *conf = mgr->priv;
-	struct pci_dev *pdev = conf->pci_dev;
 	int ret;
 	u32 val;
 
 	/* STEP 12 - reset START_XFER bit */
-	pci_read_config_dword(pdev, VSE_CVP_PROG_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
 	val &= ~VSE_CVP_PROG_CTRL_START_XFER;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
 	/* STEP 13 - reset CVP_CONFIG bit */
 	val &= ~VSE_CVP_PROG_CTRL_CONFIG;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
 	/*
 	 * STEP 14
@@ -163,7 +176,6 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 				 const char *buf, size_t count)
 {
 	struct altera_cvp_conf *conf = mgr->priv;
-	struct pci_dev *pdev = conf->pci_dev;
 	u32 iflags, val;
 	int ret;
 
@@ -183,7 +195,7 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 		conf->numclks = 1; /* for uncompressed and unencrypted images */
 
 	/* STEP 1 - read CVP status and check CVP_EN flag */
-	pci_read_config_dword(pdev, VSE_CVP_STATUS, &val);
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
 	if (!(val & VSE_CVP_STATUS_CVP_EN)) {
 		dev_err(&mgr->dev, "CVP mode off: 0x%04x\n", val);
 		return -ENODEV;
@@ -201,14 +213,14 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	 * - set HIP_CLK_SEL and CVP_MODE (must be set in the order mentioned)
 	 */
 	/* switch from fabric to PMA clock */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
 	val |= VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
 	/* set CVP mode */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
 	val |= VSE_CVP_MODE_CTRL_CVP_MODE;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
 	/*
 	 * STEP 3
@@ -217,10 +229,10 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	altera_cvp_dummy_write(conf);
 
 	/* STEP 4 - set CVP_CONFIG bit */
-	pci_read_config_dword(pdev, VSE_CVP_PROG_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
 	/* request control block to begin transfer using CVP */
 	val |= VSE_CVP_PROG_CTRL_CONFIG;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
 	/* STEP 5 - poll CVP_CONFIG READY for 1 with 10us timeout */
 	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY,
@@ -237,15 +249,15 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	altera_cvp_dummy_write(conf);
 
 	/* STEP 7 - set START_XFER */
-	pci_read_config_dword(pdev, VSE_CVP_PROG_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
 	val |= VSE_CVP_PROG_CTRL_START_XFER;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
 	/* STEP 8 - start transfer (set CVP_NUMCLKS for bitstream) */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
 	val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
 	val |= conf->numclks << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
 	return 0;
 }
@@ -256,7 +268,7 @@ static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
 	u32 val;
 
 	/* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
-	pci_read_config_dword(conf->pci_dev, VSE_CVP_STATUS, &val);
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
 	if (val & VSE_CVP_STATUS_CFG_ERR) {
 		dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
 			bytes);
@@ -315,7 +327,6 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
 				     struct fpga_image_info *info)
 {
 	struct altera_cvp_conf *conf = mgr->priv;
-	struct pci_dev *pdev = conf->pci_dev;
 	int ret;
 	u32 mask;
 	u32 val;
@@ -325,17 +336,17 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
 		return ret;
 
 	/* STEP 16 - check CVP_CONFIG_ERROR_LATCHED bit */
-	pci_read_config_dword(pdev, VSE_UNCOR_ERR_STATUS, &val);
+	altera_read_config_dword(conf, VSE_UNCOR_ERR_STATUS, &val);
 	if (val & VSE_UNCOR_ERR_CVP_CFG_ERR) {
 		dev_err(&mgr->dev, "detected CVP_CONFIG_ERROR_LATCHED!\n");
 		return -EPROTO;
 	}
 
 	/* STEP 17 - reset CVP_MODE and HIP_CLK_SEL bit */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
 	val &= ~VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
 	val &= ~VSE_CVP_MODE_CTRL_CVP_MODE;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
 	/* STEP 18 - poll PLD_CLK_IN_USE and USER_MODE bits */
 	mask = VSE_CVP_STATUS_PLD_CLK_IN_USE | VSE_CVP_STATUS_USERMODE;
@@ -396,20 +407,27 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 	struct fpga_manager *mgr;
 	u16 cmd, val;
 	u32 regval;
-	int ret;
+	int ret, offset;
+
+	/* Discover the Vendor Specific Offset for this device */
+	offset = pci_find_next_ext_capability(pdev, 0, PCI_EXT_CAP_ID_VNDR);
+	if (!offset) {
+		dev_err(&pdev->dev, "No Vendor Specific Offset.\n");
+		return -ENODEV;
+	}
 
 	/*
 	 * First check if this is the expected FPGA device. PCI config
 	 * space access works without enabling the PCI device, memory
 	 * space access is enabled further down.
 	 */
-	pci_read_config_word(pdev, VSE_PCIE_EXT_CAP_ID, &val);
+	pci_read_config_word(pdev, offset + VSE_PCIE_EXT_CAP_ID, &val);
 	if (val != VSE_PCIE_EXT_CAP_ID_VAL) {
 		dev_err(&pdev->dev, "Wrong EXT_CAP_ID value 0x%x\n", val);
 		return -ENODEV;
 	}
 
-	pci_read_config_dword(pdev, VSE_CVP_STATUS, &regval);
+	pci_read_config_dword(pdev, offset + VSE_CVP_STATUS, &regval);
 	if (!(regval & VSE_CVP_STATUS_CVP_EN)) {
 		dev_err(&pdev->dev,
 			"CVP is disabled for this device: CVP_STATUS Reg 0x%x\n",
@@ -421,6 +439,9 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 	if (!conf)
 		return -ENOMEM;
 
+	conf->vsec_offset = offset;
+	dev_dbg(&pdev->dev, "Vendor Specific Data Offset at 0x%x\n", offset);
+
 	/*
 	 * Enable memory BAR access. We cannot use pci_enable_device() here
 	 * because it will make the driver unusable with FPGA devices that
Signature · 1 line
2.7.4
maintainer
#2 lore raw

Re: [PATCH 1/3] fpga: altera-cvp: Discover Vendor Specific offset

Quoted 58 lines · On Thu, Jul 11, 2019 at 03:32:48PM -0500, [email protected] wrote:

From: Thor Thayer <[email protected]>

Newer Intel FPGAs have different Vendor Specific offsets than legacy parts. Use PCI discovery to find the CvP registers. Since the register positions remain the same, change the hard coded address to a more flexible way of indexing registers from the offset.
Adding new PCI read and write abstraction functions to handle the offset (altera_read_config_dword() and altera_write_config_dword()).

---
 drivers/fpga/altera-cvp.c | 91 +++++++++++++++++++++++++++++------------------
 1 file changed, 56 insertions(+), 35 deletions(-)
drivers/fpga/altera-cvp.c +6 -6
index 770915fb97f9..04f2b2a072a7 100644
@@ -22,10 +22,10 @@
 #define TIMEOUT_US	2000	/* CVP STATUS timeout for USERMODE polling */
 
 /* Vendor Specific Extended Capability Registers */
-#define VSE_PCIE_EXT_CAP_ID		0x200
+#define VSE_PCIE_EXT_CAP_ID		0x0
 #define VSE_PCIE_EXT_CAP_ID_VAL		0x000b	/* 16bit */
 
-#define VSE_CVP_STATUS			0x21c	/* 32bit */
+#define VSE_CVP_STATUS			0x1c	/* 32bit */
 #define VSE_CVP_STATUS_CFG_RDY		BIT(18)	/* CVP_CONFIG_READY */
 #define VSE_CVP_STATUS_CFG_ERR		BIT(19)	/* CVP_CONFIG_ERROR */
 #define VSE_CVP_STATUS_CVP_EN		BIT(20)	/* ctrl block is enabling CVP */
@@ -33,18 +33,18 @@
 #define VSE_CVP_STATUS_CFG_DONE		BIT(23)	/* CVP_CONFIG_DONE */
 #define VSE_CVP_STATUS_PLD_CLK_IN_USE	BIT(24)	/* PLD_CLK_IN_USE */
 
-#define VSE_CVP_MODE_CTRL		0x220	/* 32bit */
+#define VSE_CVP_MODE_CTRL		0x20	/* 32bit */
 #define VSE_CVP_MODE_CTRL_CVP_MODE	BIT(0)	/* CVP (1) or normal mode (0) */
 #define VSE_CVP_MODE_CTRL_HIP_CLK_SEL	BIT(1) /* PMA (1) or fabric clock (0) */
 #define VSE_CVP_MODE_CTRL_NUMCLKS_OFF	8	/* NUMCLKS bits offset */
 #define VSE_CVP_MODE_CTRL_NUMCLKS_MASK	GENMASK(15, 8)
 
-#define VSE_CVP_DATA			0x228	/* 32bit */
-#define VSE_CVP_PROG_CTRL		0x22c	/* 32bit */
+#define VSE_CVP_DATA			0x28	/* 32bit */
+#define VSE_CVP_PROG_CTRL		0x2c	/* 32bit */
 #define VSE_CVP_PROG_CTRL_CONFIG	BIT(0)
 #define VSE_CVP_PROG_CTRL_START_XFER	BIT(1)
 
-#define VSE_UNCOR_ERR_STATUS		0x234	/* 32bit */
+#define VSE_UNCOR_ERR_STATUS		0x34	/* 32bit */
 #define VSE_UNCOR_ERR_CVP_CFG_ERR	BIT(5)	/* CVP_CONFIG_ERROR_LATCHED */
 
 #define DRV_NAME		"altera-cvp"
@@ -60,14 +60,27 @@ struct altera_cvp_conf {
 	void			(*write_data)(struct altera_cvp_conf *, u32);

drivers/fpga/altera-cvp.c struct altera_cvp_conf { · line 70

Quoted 10 lines
drivers/fpga/altera-cvp.c +6 -0
 	char			mgr_name[64];
 	u8			numclks;
+	u32			vsec_offset;
 };
 
+static inline void altera_read_config_dword(struct altera_cvp_conf *conf,
+					    int where, u32 *val)
+{
+	pci_read_config_dword(conf->pci_dev, conf->vsec_offset + where, val);
+}

I think the compiler would inline those anyway.

drivers/fpga/altera-cvp.c struct altera_cvp_conf { · line 76

Quoted 6 lines
drivers/fpga/altera-cvp.c +6 -0
+
+static inline void altera_write_config_dword(struct altera_cvp_conf *conf,
+					     int where, u32 val)
+{
+	pci_write_config_dword(conf->pci_dev, conf->vsec_offset + where, val);
+}

I think the compiler would inline those anyway.

Quoted 173 lines
drivers/fpga/altera-cvp.c +25 -26
+
 static enum fpga_mgr_states altera_cvp_state(struct fpga_manager *mgr)
 {
 	struct altera_cvp_conf *conf = mgr->priv;
 	u32 status;
 
-	pci_read_config_dword(conf->pci_dev, VSE_CVP_STATUS, &status);
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &status);
 
 	if (status & VSE_CVP_STATUS_CFG_DONE)
 		return FPGA_MGR_STATE_OPERATING;
@@ -85,7 +98,8 @@ static void altera_cvp_write_data_iomem(struct altera_cvp_conf *conf, u32 val)
 
 static void altera_cvp_write_data_config(struct altera_cvp_conf *conf, u32 val)
 {
-	pci_write_config_dword(conf->pci_dev, VSE_CVP_DATA, val);
+	pci_write_config_dword(conf->pci_dev, conf->vsec_offset + VSE_CVP_DATA,
+			       val);
 }
 
 /* switches between CvP clock and internal clock */
@@ -95,10 +109,10 @@ static void altera_cvp_dummy_write(struct altera_cvp_conf *conf)
 	u32 val;
 
 	/* set 1 CVP clock cycle for every CVP Data Register Write */
-	pci_read_config_dword(conf->pci_dev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
 	val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
 	val |= 1 << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
-	pci_write_config_dword(conf->pci_dev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
 	for (i = 0; i < CVP_DUMMY_WR; i++)
 		conf->write_data(conf, 0); /* dummy data, could be any value */
@@ -115,7 +129,7 @@ static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask,
 		retries++;
 
 	do {
-		pci_read_config_dword(conf->pci_dev, VSE_CVP_STATUS, &val);
+		altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
 		if ((val & status_mask) == status_val)
 			return 0;
 
@@ -130,18 +144,17 @@ static int altera_cvp_teardown(struct fpga_manager *mgr,
 			       struct fpga_image_info *info)
 {
 	struct altera_cvp_conf *conf = mgr->priv;
-	struct pci_dev *pdev = conf->pci_dev;
 	int ret;
 	u32 val;
 
 	/* STEP 12 - reset START_XFER bit */
-	pci_read_config_dword(pdev, VSE_CVP_PROG_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
 	val &= ~VSE_CVP_PROG_CTRL_START_XFER;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
 	/* STEP 13 - reset CVP_CONFIG bit */
 	val &= ~VSE_CVP_PROG_CTRL_CONFIG;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
 	/*
 	 * STEP 14
@@ -163,7 +176,6 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 				 const char *buf, size_t count)
 {
 	struct altera_cvp_conf *conf = mgr->priv;
-	struct pci_dev *pdev = conf->pci_dev;
 	u32 iflags, val;
 	int ret;
 
@@ -183,7 +195,7 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 		conf->numclks = 1; /* for uncompressed and unencrypted images */
 
 	/* STEP 1 - read CVP status and check CVP_EN flag */
-	pci_read_config_dword(pdev, VSE_CVP_STATUS, &val);
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
 	if (!(val & VSE_CVP_STATUS_CVP_EN)) {
 		dev_err(&mgr->dev, "CVP mode off: 0x%04x\n", val);
 		return -ENODEV;
@@ -201,14 +213,14 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	 * - set HIP_CLK_SEL and CVP_MODE (must be set in the order mentioned)
 	 */
 	/* switch from fabric to PMA clock */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
 	val |= VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
 	/* set CVP mode */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
 	val |= VSE_CVP_MODE_CTRL_CVP_MODE;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
 	/*
 	 * STEP 3
@@ -217,10 +229,10 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	altera_cvp_dummy_write(conf);
 
 	/* STEP 4 - set CVP_CONFIG bit */
-	pci_read_config_dword(pdev, VSE_CVP_PROG_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
 	/* request control block to begin transfer using CVP */
 	val |= VSE_CVP_PROG_CTRL_CONFIG;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
 	/* STEP 5 - poll CVP_CONFIG READY for 1 with 10us timeout */
 	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY,
@@ -237,15 +249,15 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	altera_cvp_dummy_write(conf);
 
 	/* STEP 7 - set START_XFER */
-	pci_read_config_dword(pdev, VSE_CVP_PROG_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
 	val |= VSE_CVP_PROG_CTRL_START_XFER;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
 	/* STEP 8 - start transfer (set CVP_NUMCLKS for bitstream) */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
 	val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
 	val |= conf->numclks << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
 	return 0;
 }
@@ -256,7 +268,7 @@ static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
 	u32 val;
 
 	/* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
-	pci_read_config_dword(conf->pci_dev, VSE_CVP_STATUS, &val);
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
 	if (val & VSE_CVP_STATUS_CFG_ERR) {
 		dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
 			bytes);
@@ -315,7 +327,6 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
 				     struct fpga_image_info *info)
 {
 	struct altera_cvp_conf *conf = mgr->priv;
-	struct pci_dev *pdev = conf->pci_dev;
 	int ret;
 	u32 mask;
 	u32 val;
@@ -325,17 +336,17 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
 		return ret;
 
 	/* STEP 16 - check CVP_CONFIG_ERROR_LATCHED bit */
-	pci_read_config_dword(pdev, VSE_UNCOR_ERR_STATUS, &val);
+	altera_read_config_dword(conf, VSE_UNCOR_ERR_STATUS, &val);
 	if (val & VSE_UNCOR_ERR_CVP_CFG_ERR) {
 		dev_err(&mgr->dev, "detected CVP_CONFIG_ERROR_LATCHED!\n");
 		return -EPROTO;
 	}
 
 	/* STEP 17 - reset CVP_MODE and HIP_CLK_SEL bit */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
 	val &= ~VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
 	val &= ~VSE_CVP_MODE_CTRL_CVP_MODE;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
 	/* STEP 18 - poll PLD_CLK_IN_USE and USER_MODE bits */
 	mask = VSE_CVP_STATUS_PLD_CLK_IN_USE | VSE_CVP_STATUS_USERMODE;

drivers/fpga/altera-cvp.c static int altera_cvp_probe(struct pci_dev *pdev, · line 410

Quoted 5 lines
drivers/fpga/altera-cvp.c +1 -1
@@ -396,20 +407,27 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 	struct fpga_manager *mgr;
 	u16 cmd, val;
 	u32 regval;
-	int ret;
+	int ret, offset;

Reverse tree, please:
struct fpga_manager *mgr;
+ int ret, offset;
u16 cmd, val;
u32 regval;
- int ret;

Quoted 26 lines
drivers/fpga/altera-cvp.c +9 -2
+
+	/* Discover the Vendor Specific Offset for this device */
+	offset = pci_find_next_ext_capability(pdev, 0, PCI_EXT_CAP_ID_VNDR);
+	if (!offset) {
+		dev_err(&pdev->dev, "No Vendor Specific Offset.\n");
+		return -ENODEV;
+	}
 
 	/*
 	 * First check if this is the expected FPGA device. PCI config
 	 * space access works without enabling the PCI device, memory
 	 * space access is enabled further down.
 	 */
-	pci_read_config_word(pdev, VSE_PCIE_EXT_CAP_ID, &val);
+	pci_read_config_word(pdev, offset + VSE_PCIE_EXT_CAP_ID, &val);
 	if (val != VSE_PCIE_EXT_CAP_ID_VAL) {
 		dev_err(&pdev->dev, "Wrong EXT_CAP_ID value 0x%x\n", val);
 		return -ENODEV;
 	}
 
-	pci_read_config_dword(pdev, VSE_CVP_STATUS, &regval);
+	pci_read_config_dword(pdev, offset + VSE_CVP_STATUS, &regval);
 	if (!(regval & VSE_CVP_STATUS_CVP_EN)) {
 		dev_err(&pdev->dev,
 			"CVP is disabled for this device: CVP_STATUS Reg 0x%x\n",

drivers/fpga/altera-cvp.c static int altera_cvp_probe(struct pci_dev *pdev, · line 442

Quoted 4 lines
drivers/fpga/altera-cvp.c +1 -0
@@ -421,6 +439,9 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 	if (!conf)
 		return -ENOMEM;
 
+	conf->vsec_offset = offset;

If you'd swap the order you could use your new helper earlier, right? not sure if that saves you some code. Your call.

Quoted 8 lines
drivers/fpga/altera-cvp.c +2 -0
+	dev_dbg(&pdev->dev, "Vendor Specific Data Offset at 0x%x\n", offset);
+
 	/*
 	 * Enable memory BAR access. We cannot use pci_enable_device() here
 	 * because it will make the driver unusable with FPGA devices that
Signature · 1 line
2.7.4

Thanks,
Moritz

#3 lore raw

Re: [PATCH 1/3] fpga: altera-cvp: Discover Vendor Specific offset

Hi Moritz,

Quoted 64 lines · Moritz Fischer
Quoted 63 lines · On Thu, Jul 11, 2019 at 03:32:48PM -0500, [email protected] wrote:

From: Thor Thayer <[email protected]>

Newer Intel FPGAs have different Vendor Specific offsets than legacy parts. Use PCI discovery to find the CvP registers. Since the register positions remain the same, change the hard coded address to a more flexible way of indexing registers from the offset.
Adding new PCI read and write abstraction functions to handle the offset (altera_read_config_dword() and altera_write_config_dword()).

---
  drivers/fpga/altera-cvp.c | 91 +++++++++++++++++++++++++++++------------------
  1 file changed, 56 insertions(+), 35 deletions(-)
drivers/fpga/altera-cvp.c +7 -6
index 770915fb97f9..04f2b2a072a7 100644
@@ -22,10 +22,10 @@
  #define TIMEOUT_US	2000	/* CVP STATUS timeout for USERMODE polling */
 
  /* Vendor Specific Extended Capability Registers */
-#define VSE_PCIE_EXT_CAP_ID		0x200
+#define VSE_PCIE_EXT_CAP_ID		0x0
  #define VSE_PCIE_EXT_CAP_ID_VAL		0x000b	/* 16bit */
 
-#define VSE_CVP_STATUS			0x21c	/* 32bit */
+#define VSE_CVP_STATUS			0x1c	/* 32bit */
  #define VSE_CVP_STATUS_CFG_RDY		BIT(18)	/* CVP_CONFIG_READY */
  #define VSE_CVP_STATUS_CFG_ERR		BIT(19)	/* CVP_CONFIG_ERROR */
  #define VSE_CVP_STATUS_CVP_EN		BIT(20)	/* ctrl block is enabling CVP */
@@ -33,18 +33,18 @@
  #define VSE_CVP_STATUS_CFG_DONE		BIT(23)	/* CVP_CONFIG_DONE */
  #define VSE_CVP_STATUS_PLD_CLK_IN_USE	BIT(24)	/* PLD_CLK_IN_USE */
 
-#define VSE_CVP_MODE_CTRL		0x220	/* 32bit */
+#define VSE_CVP_MODE_CTRL		0x20	/* 32bit */
  #define VSE_CVP_MODE_CTRL_CVP_MODE	BIT(0)	/* CVP (1) or normal mode (0) */
  #define VSE_CVP_MODE_CTRL_HIP_CLK_SEL	BIT(1) /* PMA (1) or fabric clock (0) */
  #define VSE_CVP_MODE_CTRL_NUMCLKS_OFF	8	/* NUMCLKS bits offset */
  #define VSE_CVP_MODE_CTRL_NUMCLKS_MASK	GENMASK(15, 8)
 
-#define VSE_CVP_DATA			0x228	/* 32bit */
-#define VSE_CVP_PROG_CTRL		0x22c	/* 32bit */
+#define VSE_CVP_DATA			0x28	/* 32bit */
+#define VSE_CVP_PROG_CTRL		0x2c	/* 32bit */
  #define VSE_CVP_PROG_CTRL_CONFIG	BIT(0)
  #define VSE_CVP_PROG_CTRL_START_XFER	BIT(1)
 
-#define VSE_UNCOR_ERR_STATUS		0x234	/* 32bit */
+#define VSE_UNCOR_ERR_STATUS		0x34	/* 32bit */
  #define VSE_UNCOR_ERR_CVP_CFG_ERR	BIT(5)	/* CVP_CONFIG_ERROR_LATCHED */
 
  #define DRV_NAME		"altera-cvp"
@@ -60,14 +60,27 @@ struct altera_cvp_conf {
  	void			(*write_data)(struct altera_cvp_conf *, u32);
  	char			mgr_name[64];
  	u8			numclks;
+	u32			vsec_offset;
  };
 
Quoted 7 lines

drivers/fpga/altera-cvp.c struct altera_cvp_conf { · line 70

Quoted 5 lines
drivers/fpga/altera-cvp.c +5 -0
+static inline void altera_read_config_dword(struct altera_cvp_conf *conf,
+					    int where, u32 *val)
+{
+	pci_read_config_dword(conf->pci_dev, conf->vsec_offset + where, val);
+}

I think the compiler would inline those anyway.

OK. I will take both of these out.

Quoted 188 lines

drivers/fpga/altera-cvp.c struct altera_cvp_conf { · line 76

Quoted 6 lines
drivers/fpga/altera-cvp.c +6 -0
+
+static inline void altera_write_config_dword(struct altera_cvp_conf *conf,
+					     int where, u32 val)
+{
+	pci_write_config_dword(conf->pci_dev, conf->vsec_offset + where, val);
+}

I think the compiler would inline those anyway.

Quoted 173 lines
drivers/fpga/altera-cvp.c +25 -26
+
  static enum fpga_mgr_states altera_cvp_state(struct fpga_manager *mgr)
  {
  	struct altera_cvp_conf *conf = mgr->priv;
  	u32 status;
 
-	pci_read_config_dword(conf->pci_dev, VSE_CVP_STATUS, &status);
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &status);
 
  	if (status & VSE_CVP_STATUS_CFG_DONE)
  		return FPGA_MGR_STATE_OPERATING;
@@ -85,7 +98,8 @@ static void altera_cvp_write_data_iomem(struct altera_cvp_conf *conf, u32 val)
 
  static void altera_cvp_write_data_config(struct altera_cvp_conf *conf, u32 val)
  {
-	pci_write_config_dword(conf->pci_dev, VSE_CVP_DATA, val);
+	pci_write_config_dword(conf->pci_dev, conf->vsec_offset + VSE_CVP_DATA,
+			       val);
  }
 
  /* switches between CvP clock and internal clock */
@@ -95,10 +109,10 @@ static void altera_cvp_dummy_write(struct altera_cvp_conf *conf)
  	u32 val;
 
  	/* set 1 CVP clock cycle for every CVP Data Register Write */
-	pci_read_config_dword(conf->pci_dev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
  	val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
  	val |= 1 << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
-	pci_write_config_dword(conf->pci_dev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
  	for (i = 0; i < CVP_DUMMY_WR; i++)
  		conf->write_data(conf, 0); /* dummy data, could be any value */
@@ -115,7 +129,7 @@ static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask,
  		retries++;
 
  	do {
-		pci_read_config_dword(conf->pci_dev, VSE_CVP_STATUS, &val);
+		altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
  		if ((val & status_mask) == status_val)
  			return 0;
 
@@ -130,18 +144,17 @@ static int altera_cvp_teardown(struct fpga_manager *mgr,
  			       struct fpga_image_info *info)
  {
  	struct altera_cvp_conf *conf = mgr->priv;
-	struct pci_dev *pdev = conf->pci_dev;
  	int ret;
  	u32 val;
 
  	/* STEP 12 - reset START_XFER bit */
-	pci_read_config_dword(pdev, VSE_CVP_PROG_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
  	val &= ~VSE_CVP_PROG_CTRL_START_XFER;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
  	/* STEP 13 - reset CVP_CONFIG bit */
  	val &= ~VSE_CVP_PROG_CTRL_CONFIG;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
  	/*
  	 * STEP 14
@@ -163,7 +176,6 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
  				 const char *buf, size_t count)
  {
  	struct altera_cvp_conf *conf = mgr->priv;
-	struct pci_dev *pdev = conf->pci_dev;
  	u32 iflags, val;
  	int ret;
 
@@ -183,7 +195,7 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
  		conf->numclks = 1; /* for uncompressed and unencrypted images */
 
  	/* STEP 1 - read CVP status and check CVP_EN flag */
-	pci_read_config_dword(pdev, VSE_CVP_STATUS, &val);
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
  	if (!(val & VSE_CVP_STATUS_CVP_EN)) {
  		dev_err(&mgr->dev, "CVP mode off: 0x%04x\n", val);
  		return -ENODEV;
@@ -201,14 +213,14 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
  	 * - set HIP_CLK_SEL and CVP_MODE (must be set in the order mentioned)
  	 */
  	/* switch from fabric to PMA clock */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
  	val |= VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
  	/* set CVP mode */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
  	val |= VSE_CVP_MODE_CTRL_CVP_MODE;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
  	/*
  	 * STEP 3
@@ -217,10 +229,10 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
  	altera_cvp_dummy_write(conf);
 
  	/* STEP 4 - set CVP_CONFIG bit */
-	pci_read_config_dword(pdev, VSE_CVP_PROG_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
  	/* request control block to begin transfer using CVP */
  	val |= VSE_CVP_PROG_CTRL_CONFIG;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
  	/* STEP 5 - poll CVP_CONFIG READY for 1 with 10us timeout */
  	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY,
@@ -237,15 +249,15 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
  	altera_cvp_dummy_write(conf);
 
  	/* STEP 7 - set START_XFER */
-	pci_read_config_dword(pdev, VSE_CVP_PROG_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
  	val |= VSE_CVP_PROG_CTRL_START_XFER;
-	pci_write_config_dword(pdev, VSE_CVP_PROG_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
  	/* STEP 8 - start transfer (set CVP_NUMCLKS for bitstream) */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
  	val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
  	val |= conf->numclks << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
  	return 0;
  }
@@ -256,7 +268,7 @@ static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
  	u32 val;
 
  	/* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
-	pci_read_config_dword(conf->pci_dev, VSE_CVP_STATUS, &val);
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
  	if (val & VSE_CVP_STATUS_CFG_ERR) {
  		dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
  			bytes);
@@ -315,7 +327,6 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
  				     struct fpga_image_info *info)
  {
  	struct altera_cvp_conf *conf = mgr->priv;
-	struct pci_dev *pdev = conf->pci_dev;
  	int ret;
  	u32 mask;
  	u32 val;
@@ -325,17 +336,17 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
  		return ret;
 
  	/* STEP 16 - check CVP_CONFIG_ERROR_LATCHED bit */
-	pci_read_config_dword(pdev, VSE_UNCOR_ERR_STATUS, &val);
+	altera_read_config_dword(conf, VSE_UNCOR_ERR_STATUS, &val);
  	if (val & VSE_UNCOR_ERR_CVP_CFG_ERR) {
  		dev_err(&mgr->dev, "detected CVP_CONFIG_ERROR_LATCHED!\n");
  		return -EPROTO;
  	}
 
  	/* STEP 17 - reset CVP_MODE and HIP_CLK_SEL bit */
-	pci_read_config_dword(pdev, VSE_CVP_MODE_CTRL, &val);
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
  	val &= ~VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
  	val &= ~VSE_CVP_MODE_CTRL_CVP_MODE;
-	pci_write_config_dword(pdev, VSE_CVP_MODE_CTRL, val);
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
 
  	/* STEP 18 - poll PLD_CLK_IN_USE and USER_MODE bits */
  	mask = VSE_CVP_STATUS_PLD_CLK_IN_USE | VSE_CVP_STATUS_USERMODE;

drivers/fpga/altera-cvp.c static int altera_cvp_probe(struct pci_dev *pdev, · line 410

Quoted 5 lines
drivers/fpga/altera-cvp.c +1 -1
@@ -396,20 +407,27 @@ static int altera_cvp_probe(struct pci_dev *pdev,
  	struct fpga_manager *mgr;
  	u16 cmd, val;
  	u32 regval;
-	int ret;
+	int ret, offset;

Reverse tree, please:

Quoted 6 lines
   	struct fpga_manager *mgr;
  +	int ret, offset;
   	u16 cmd, val;
   	u32 regval;

- int ret;

OK. I'll make the change. I'll also move regval above the u16 line.

Quoted 29 lines
Quoted 29 lines
drivers/fpga/altera-cvp.c +9 -2
+
+	/* Discover the Vendor Specific Offset for this device */
+	offset = pci_find_next_ext_capability(pdev, 0, PCI_EXT_CAP_ID_VNDR);
+	if (!offset) {
+		dev_err(&pdev->dev, "No Vendor Specific Offset.\n");
+		return -ENODEV;
+	}
 
  	/*
  	 * First check if this is the expected FPGA device. PCI config
  	 * space access works without enabling the PCI device, memory
  	 * space access is enabled further down.
  	 */
-	pci_read_config_word(pdev, VSE_PCIE_EXT_CAP_ID, &val);
+	pci_read_config_word(pdev, offset + VSE_PCIE_EXT_CAP_ID, &val);
  	if (val != VSE_PCIE_EXT_CAP_ID_VAL) {
  		dev_err(&pdev->dev, "Wrong EXT_CAP_ID value 0x%x\n", val);
  		return -ENODEV;
  	}
 
-	pci_read_config_dword(pdev, VSE_CVP_STATUS, &regval);
+	pci_read_config_dword(pdev, offset + VSE_CVP_STATUS, &regval);
  	if (!(regval & VSE_CVP_STATUS_CVP_EN)) {
  		dev_err(&pdev->dev,
  			"CVP is disabled for this device: CVP_STATUS Reg 0x%x\n",
@@ -421,6 +439,9 @@ static int altera_cvp_probe(struct pci_dev *pdev,
  	if (!conf)
  		return -ENOMEM;
 
Quoted 5 lines

drivers/fpga/altera-cvp.c static int altera_cvp_probe(struct pci_dev *pdev, · line 442

Quoted 1 line
drivers/fpga/altera-cvp.c +1 -0
+	conf->vsec_offset = offset;

If you'd swap the order you could use your new helper earlier, right? not sure if that saves you some code. Your call.

Yes. However, I figured it was cleaner to perform the 2 extra checks before commiting to the allocation of the altera_cvp_conf struct.

If you would prefer the move, I can do that in the next version.

Thanks for reviewing!

Thor

Quoted 12 lines
Quoted 8 lines
drivers/fpga/altera-cvp.c +2 -0
+	dev_dbg(&pdev->dev, "Vendor Specific Data Offset at 0x%x\n", offset);
+
  	/*
  	 * Enable memory BAR access. We cannot use pci_enable_device() here
  	 * because it will make the driver unusable with FPGA devices that
Signature · 1 line
2.7.4

Thanks,
Moritz

#4 lore raw

PATCH 2/3 fpga: altera-cvp: Preparation for V2 parts.

In reply to [email protected]

From: Thor Thayer <[email protected]>

In preparation for adding newer V2 parts that use a FIFO, reorganize altera_cvp_chk_error() and change the write function to block based.
V2 parts have a block size matching the FIFO while older V1 parts write a 32 bit word at a time.

---
 drivers/fpga/altera-cvp.c | 72 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 26 deletions(-)
drivers/fpga/altera-cvp.c +46 -26
index 04f2b2a072a7..59835f6f9b2d 100644
@@ -140,6 +140,41 @@ static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask,
 	return -ETIMEDOUT;
 }
 
+static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
+{
+	struct altera_cvp_conf *conf = mgr->priv;
+	u32 val;
+
+	/* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
+	if (val & VSE_CVP_STATUS_CFG_ERR) {
+		dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
+			bytes);
+		return -EPROTO;
+	}
+	return 0;
+}
+
+static int altera_cvp_send_block(struct altera_cvp_conf *conf,
+				 const u32 *data, size_t len)
+{
+	int i, remainder;
+	u32 mask, words = len / sizeof(u32);
+
+	for (i = 0; i < words; i++)
+		conf->write_data(conf, *data++);
+
+	/* write up to 3 trailing bytes, if any */
+	remainder = len % sizeof(u32);
+	if (remainder) {
+		mask = BIT(remainder * 8) - 1;
+		if (mask)
+			conf->write_data(conf, *data & mask);
+	}
+
+	return 0;
+}
+
 static int altera_cvp_teardown(struct fpga_manager *mgr,
 			       struct fpga_image_info *info)
 {
@@ -262,39 +297,29 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	return 0;
 }
 
-static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
-{
-	struct altera_cvp_conf *conf = mgr->priv;
-	u32 val;
-
-	/* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
-	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
-	if (val & VSE_CVP_STATUS_CFG_ERR) {
-		dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
-			bytes);
-		return -EPROTO;
-	}
-	return 0;
-}
-
 static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
 			    size_t count)
 {
 	struct altera_cvp_conf *conf = mgr->priv;
 	const u32 *data;
-	size_t done, remaining;
+	size_t done, remaining, len;
 	int status = 0;
-	u32 mask;
 
 	/* STEP 9 - write 32-bit data from RBF file to CVP data register */
 	data = (u32 *)buf;
 	remaining = count;
 	done = 0;
 
-	while (remaining >= 4) {
-		conf->write_data(conf, *data++);
-		done += 4;
-		remaining -= 4;
+	while (remaining) {
+		if (remaining >= sizeof(u32))
+			len = sizeof(u32);
+		else
+			len = remaining;
+
+		altera_cvp_send_block(conf, data, len);
+		data++;
+		done += len;
+		remaining -= len;
 
 		/*
 		 * STEP 10 (optional) and STEP 11
@@ -312,11 +337,6 @@ static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
 		}
 	}
 
-	/* write up to 3 trailing bytes, if any */
-	mask = BIT(remaining * 8) - 1;
-	if (mask)
-		conf->write_data(conf, *data & mask);
-
 	if (altera_cvp_chkcfg)
 		status = altera_cvp_chk_error(mgr, count);
 
Signature · 1 line
2.7.4
maintainer
#5 lore raw

Re: [PATCH 2/3] fpga: altera-cvp: Preparation for V2 parts.

Hi Thor,

Quoted 18 lines · On Thu, Jul 11, 2019 at 03:32:49PM -0500, [email protected] wrote:

From: Thor Thayer <[email protected]>

In preparation for adding newer V2 parts that use a FIFO, reorganize altera_cvp_chk_error() and change the write function to block based.
V2 parts have a block size matching the FIFO while older V1 parts write a 32 bit word at a time.

---
 drivers/fpga/altera-cvp.c | 72 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 26 deletions(-)
drivers/fpga/altera-cvp.c +0 -0
index 04f2b2a072a7..59835f6f9b2d 100644

drivers/fpga/altera-cvp.c static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask, · line 143

Quoted 4 lines
drivers/fpga/altera-cvp.c +1 -0
@@ -140,6 +140,41 @@ static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask,
 	return -ETIMEDOUT;
 }
 
+static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)

Please drop the inline here.

Quoted 9 lines
drivers/fpga/altera-cvp.c +9 -0
+{
+	struct altera_cvp_conf *conf = mgr->priv;
+	u32 val;
+
+	/* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
+	if (val & VSE_CVP_STATUS_CFG_ERR) {
+		dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
+			bytes);

drivers/fpga/altera-cvp.c static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask, · line 162

Quoted 10 lines
drivers/fpga/altera-cvp.c +10 -0
+		return -EPROTO;
+	}
+	return 0;
+}
+
+static int altera_cvp_send_block(struct altera_cvp_conf *conf,
+				 const u32 *data, size_t len)
+{
+	int i, remainder;
+	u32 mask, words = len / sizeof(u32);

Reverse xmas-tree, please.

Quoted 34 lines
drivers/fpga/altera-cvp.c +15 -12
+
+	for (i = 0; i < words; i++)
+		conf->write_data(conf, *data++);
+
+	/* write up to 3 trailing bytes, if any */
+	remainder = len % sizeof(u32);
+	if (remainder) {
+		mask = BIT(remainder * 8) - 1;
+		if (mask)
+			conf->write_data(conf, *data & mask);
+	}
+
+	return 0;
+}
+
 static int altera_cvp_teardown(struct fpga_manager *mgr,
 			       struct fpga_image_info *info)
 {
@@ -262,39 +297,29 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	return 0;
 }
 
-static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
-{
-	struct altera_cvp_conf *conf = mgr->priv;
-	u32 val;
-
-	/* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
-	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
-	if (val & VSE_CVP_STATUS_CFG_ERR) {
-		dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
-			bytes);
-		return -EPROTO;
-	}

drivers/fpga/altera-cvp.c static int altera_cvp_write_init(struct fpga_manager *mgr, · line 305

Quoted 10 lines
drivers/fpga/altera-cvp.c +1 -4
-	return 0;
-}
-
 static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
 			    size_t count)
 {
 	struct altera_cvp_conf *conf = mgr->priv;
 	const u32 *data;
-	size_t done, remaining;
+	size_t done, remaining, len;

Reverse xmas-tree please.

drivers/fpga/altera-cvp.c static int altera_cvp_write_init(struct fpga_manager *mgr, · line 309

Quoted 5 lines
drivers/fpga/altera-cvp.c +0 -1
 	int status = 0;
-	u32 mask;
 
 	/* STEP 9 - write 32-bit data from RBF file to CVP data register */
 	data = (u32 *)buf;

Are there endianness concerns with this?

Quoted 35 lines
drivers/fpga/altera-cvp.c +10 -9
 	remaining = count;
 	done = 0;
 
-	while (remaining >= 4) {
-		conf->write_data(conf, *data++);
-		done += 4;
-		remaining -= 4;
+	while (remaining) {
+		if (remaining >= sizeof(u32))
+			len = sizeof(u32);
+		else
+			len = remaining;
+
+		altera_cvp_send_block(conf, data, len);
+		data++;
+		done += len;
+		remaining -= len;
 
 		/*
 		 * STEP 10 (optional) and STEP 11
@@ -312,11 +337,6 @@ static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
 		}
 	}
 
-	/* write up to 3 trailing bytes, if any */
-	mask = BIT(remaining * 8) - 1;
-	if (mask)
-		conf->write_data(conf, *data & mask);
-
 	if (altera_cvp_chkcfg)
 		status = altera_cvp_chk_error(mgr, count);
 
Signature · 1 line
2.7.4

Thanks,
Moritz

#6 lore raw

Re: [PATCH 2/3] fpga: altera-cvp: Preparation for V2 parts.

Quoted 24 lines · Moritz Fischer

Hi Thor,

Quoted 21 lines · On Thu, Jul 11, 2019 at 03:32:49PM -0500, [email protected] wrote:

From: Thor Thayer <[email protected]>

In preparation for adding newer V2 parts that use a FIFO, reorganize altera_cvp_chk_error() and change the write function to block based.
V2 parts have a block size matching the FIFO while older V1 parts write a 32 bit word at a time.

---
  drivers/fpga/altera-cvp.c | 72 ++++++++++++++++++++++++++++++-----------------
  1 file changed, 46 insertions(+), 26 deletions(-)
drivers/fpga/altera-cvp.c +0 -0
index 04f2b2a072a7..59835f6f9b2d 100644
@@ -140,6 +140,41 @@ static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask,
  	return -ETIMEDOUT;
  }
 
Quoted 3 lines

drivers/fpga/altera-cvp.c static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask, · line 143

Quoted 1 line
drivers/fpga/altera-cvp.c +1 -0
+static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)

Please drop the inline here.

OK. Thanks.

Quoted 12 lines
Quoted 12 lines
drivers/fpga/altera-cvp.c +12 -0
+{
+	struct altera_cvp_conf *conf = mgr->priv;
+	u32 val;
+
+	/* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
+	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
+	if (val & VSE_CVP_STATUS_CFG_ERR) {
+		dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
+			bytes);
+		return -EPROTO;
+	}
+	return 0;
Quoted 8 lines

drivers/fpga/altera-cvp.c static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask, · line 162

Quoted 7 lines
drivers/fpga/altera-cvp.c +7 -0
+}
+
+static int altera_cvp_send_block(struct altera_cvp_conf *conf,
+				 const u32 *data, size_t len)
+{
+	int i, remainder;
+	u32 mask, words = len / sizeof(u32);

Reverse xmas-tree, please.

OK. I'm new to the reverse xmas-tree but if I read [0] correctly, it isn't the size of the variable that is important but the length of the declaration.

Quoted 38 lines
Quoted 38 lines
drivers/fpga/altera-cvp.c +15 -15
+
+	for (i = 0; i < words; i++)
+		conf->write_data(conf, *data++);
+
+	/* write up to 3 trailing bytes, if any */
+	remainder = len % sizeof(u32);
+	if (remainder) {
+		mask = BIT(remainder * 8) - 1;
+		if (mask)
+			conf->write_data(conf, *data & mask);
+	}
+
+	return 0;
+}
+
  static int altera_cvp_teardown(struct fpga_manager *mgr,
  			       struct fpga_image_info *info)
  {
@@ -262,39 +297,29 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
  	return 0;
  }
 
-static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
-{
-	struct altera_cvp_conf *conf = mgr->priv;
-	u32 val;
-
-	/* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
-	altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
-	if (val & VSE_CVP_STATUS_CFG_ERR) {
-		dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
-			bytes);
-		return -EPROTO;
-	}
-	return 0;
-}
-
  static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
Quoted 8 lines

drivers/fpga/altera-cvp.c static int altera_cvp_write_init(struct fpga_manager *mgr, · line 305

Quoted 6 lines
drivers/fpga/altera-cvp.c +1 -1
  			    size_t count)
  {
  	struct altera_cvp_conf *conf = mgr->priv;
  	const u32 *data;
-	size_t done, remaining;
+	size_t done, remaining, len;

Reverse xmas-tree please.

OK.

Quoted 7 lines

drivers/fpga/altera-cvp.c static int altera_cvp_write_init(struct fpga_manager *mgr, · line 309

Quoted 5 lines
drivers/fpga/altera-cvp.c +0 -1
  	int status = 0;
-	u32 mask;
 
  	/* STEP 9 - write 32-bit data from RBF file to CVP data register */
  	data = (u32 *)buf;

Are there endianness concerns with this?

No, this maintains the previous big endian byte handling for Cyclone5/Arria10. The Stratix10 bitstream is generated in 4KB chunks.

Quoted 32 lines
Quoted 32 lines
drivers/fpga/altera-cvp.c +10 -9
  	remaining = count;
  	done = 0;
 
-	while (remaining >= 4) {
-		conf->write_data(conf, *data++);
-		done += 4;
-		remaining -= 4;
+	while (remaining) {
+		if (remaining >= sizeof(u32))
+			len = sizeof(u32);
+		else
+			len = remaining;
+
+		altera_cvp_send_block(conf, data, len);
+		data++;
+		done += len;
+		remaining -= len;
 
  		/*
  		 * STEP 10 (optional) and STEP 11
@@ -312,11 +337,6 @@ static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
  		}
  	}
 
-	/* write up to 3 trailing bytes, if any */
-	mask = BIT(remaining * 8) - 1;
-	if (mask)
-		conf->write_data(conf, *data & mask);
-
  	if (altera_cvp_chkcfg)
  		status = altera_cvp_chk_error(mgr, count);
 
Quoted 7 lines
Quoted 3 lines
Signature · 1 line
2.7.4

Thanks,
Moritz

I wasn't sure if you preferred this as a separate patch or squashed into patch #3. Having a separate patch makes the migration a little clearer.

Thanks for reviewing!

[0] https://www.spinics.net/lists/netdev/msg402833.html

#7 lore raw

PATCH 3/3 fpga: altera-cvp: Add Stratix10 (V2) Support

In reply to [email protected]

From: Thor Thayer <[email protected]>

Add Stratix10 specific functions that use a credit mechanism to throttle data to the CvP FIFOs. Add a private structure with function pointers for V1 vs V2 functions.

---
 drivers/fpga/altera-cvp.c | 173 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 158 insertions(+), 15 deletions(-)
drivers/fpga/altera-cvp.c +158 -15
index 59835f6f9b2d..21bb08e5f52a 100644
@@ -43,16 +43,32 @@
 #define VSE_CVP_PROG_CTRL		0x2c	/* 32bit */
 #define VSE_CVP_PROG_CTRL_CONFIG	BIT(0)
 #define VSE_CVP_PROG_CTRL_START_XFER	BIT(1)
+#define VSE_CVP_PROG_CTRL_MASK		GENMASK(1, 0)
 
 #define VSE_UNCOR_ERR_STATUS		0x34	/* 32bit */
 #define VSE_UNCOR_ERR_CVP_CFG_ERR	BIT(5)	/* CVP_CONFIG_ERROR_LATCHED */
 
+/* V2 Defines */
+#define VSE_CVP_TX_CREDITS		0x49	/* 8bit */
+
+#define CREDIT_TIMEOUT_US		20000
+#define V2_POLL_TIMEOUT_US		1000000
+#define V2_USER_TIMEOUT_US		500000
+
+#define V1_POLL_TIMEOUT_US		10
+
 #define DRV_NAME		"altera-cvp"
 #define ALTERA_CVP_MGR_NAME	"Altera CvP FPGA Manager"
 
+/* Write block sizes */
+#define ALTERA_CVP_V1_SIZE	4
+#define ALTERA_CVP_V2_SIZE	4096
+
 /* Optional CvP config error status check for debugging */
 static bool altera_cvp_chkcfg;
 
+struct cvp_priv;
+
 struct altera_cvp_conf {
 	struct fpga_manager	*mgr;
 	struct pci_dev		*pci_dev;
@@ -60,9 +76,26 @@ struct altera_cvp_conf {
 	void			(*write_data)(struct altera_cvp_conf *, u32);
 	char			mgr_name[64];
 	u8			numclks;
+	u8			current_credit_byte;
 	u32			vsec_offset;
+	const struct cvp_priv	*priv;
+};
+
+struct cvp_priv {
+	void	(*switch_clk)(struct altera_cvp_conf *conf);
+	int	(*clear_state)(struct altera_cvp_conf *conf);
+	int	(*wait_credit)(struct fpga_manager *mgr, u32 blocks);
+	int	block_size;
+	int	poll_time_us;
+	int	user_time_us;
 };
 
+static inline void altera_read_config_byte(struct altera_cvp_conf *conf,
+					   int where, u8 *val)
+{
+	pci_read_config_byte(conf->pci_dev, conf->vsec_offset + where, val);
+}
+
 static inline void altera_read_config_dword(struct altera_cvp_conf *conf,
 					    int where, u32 *val)
 {
@@ -155,6 +188,57 @@ static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
 	return 0;
 }
 
+/*
+ * CvP Version2 Functions
+ * Recent Intel FPGAs use a credit mechanism to throttle incoming
+ * bitstreams and a different method of clearing the state.
+ */
+
+static int altera_cvp_v2_clear_state(struct altera_cvp_conf *conf)
+{
+	u32 val;
+
+	/* Clear the START_XFER and CVP_CONFIG bits */
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
+	val &= ~VSE_CVP_PROG_CTRL_MASK;
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+
+	return altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0,
+				      conf->priv->poll_time_us);
+}
+
+static int altera_cvp_v2_wait_for_credit(struct fpga_manager *mgr,
+					 u32 blocks)
+{
+	struct altera_cvp_conf *conf = mgr->priv;
+	u32 count = 0;
+	int ret;
+	u8 val, delta_credit;
+
+	do {
+		altera_read_config_byte(conf, VSE_CVP_TX_CREDITS, &val);
+		delta_credit = (val - conf->current_credit_byte) & 0xff;
+
+		ret = altera_cvp_chk_error(mgr, blocks * ALTERA_CVP_V2_SIZE);
+		if (ret) {
+			dev_err(&conf->pci_dev->dev,
+				"CE Bit error credits host[0x%x]:dev[0x%x]\n",
+				conf->current_credit_byte, val);
+			return -EAGAIN;
+		}
+
+		if (count++ >= CREDIT_TIMEOUT_US) {
+			dev_err(&conf->pci_dev->dev,
+				"Timeout waiting for credit\n");
+			return -ETIMEDOUT;
+		}
+
+		udelay(1);
+	} while (!delta_credit);
+
+	return 0;
+}
+
 static int altera_cvp_send_block(struct altera_cvp_conf *conf,
 				 const u32 *data, size_t len)
 {
@@ -196,10 +280,12 @@ static int altera_cvp_teardown(struct fpga_manager *mgr,
 	 * - set CVP_NUMCLKS to 1 and then issue CVP_DUMMY_WR dummy
 	 *   writes to the HIP
 	 */
-	altera_cvp_dummy_write(conf); /* from CVP clock to internal clock */
+	if (conf->priv->switch_clk)
+		conf->priv->switch_clk(conf);
 
 	/* STEP 15 - poll CVP_CONFIG_READY bit for 0 with 10us timeout */
-	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0, 10);
+	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0,
+				     conf->priv->poll_time_us);
 	if (ret)
 		dev_err(&mgr->dev, "CFG_RDY == 0 timeout\n");
 
@@ -261,7 +347,16 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	 * STEP 3
 	 * - set CVP_NUMCLKS to 1 and issue CVP_DUMMY_WR dummy writes to the HIP
 	 */
-	altera_cvp_dummy_write(conf);
+	if (conf->priv->switch_clk)
+		conf->priv->switch_clk(conf);
+
+	if (conf->priv->clear_state) {
+		ret = conf->priv->clear_state(conf);
+		if (ret) {
+			dev_err(&mgr->dev, "Problem clearing out state\n");
+			return ret;
+		}
+	}
 
 	/* STEP 4 - set CVP_CONFIG bit */
 	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
@@ -269,9 +364,10 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	val |= VSE_CVP_PROG_CTRL_CONFIG;
 	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
-	/* STEP 5 - poll CVP_CONFIG READY for 1 with 10us timeout */
+	/* STEP 5 - poll CVP_CONFIG READY for 1 with timeout */
 	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY,
-				     VSE_CVP_STATUS_CFG_RDY, 10);
+				     VSE_CVP_STATUS_CFG_RDY,
+				     conf->priv->poll_time_us);
 	if (ret) {
 		dev_warn(&mgr->dev, "CFG_RDY == 1 timeout\n");
 		return ret;
@@ -281,7 +377,16 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	 * STEP 6
 	 * - set CVP_NUMCLKS to 1 and issue CVP_DUMMY_WR dummy writes to the HIP
 	 */
-	altera_cvp_dummy_write(conf);
+	if (conf->priv->switch_clk)
+		conf->priv->switch_clk(conf);
+
+	if (altera_cvp_chkcfg) {
+		ret = altera_cvp_chk_error(mgr, 0);
+		if (ret) {
+			dev_warn(&mgr->dev, "CFG_RDY == 1 timeout\n");
+			return ret;
+		}
+	}
 
 	/* STEP 7 - set START_XFER */
 	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
@@ -289,11 +394,12 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
 	/* STEP 8 - start transfer (set CVP_NUMCLKS for bitstream) */
-	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
-	val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
-	val |= conf->numclks << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
-	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
-
+	if (conf->priv->switch_clk) {
+		altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+		val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
+		val |= conf->numclks << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
+		altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+	}
 	return 0;
 }
 
@@ -311,15 +417,26 @@ static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
 	done = 0;
 
 	while (remaining) {
-		if (remaining >= sizeof(u32))
-			len = sizeof(u32);
+		/* Use credit throttling if available */
+		if (conf->priv->wait_credit) {
+			status = conf->priv->wait_credit(mgr, done);
+			if (status) {
+				dev_err(&conf->pci_dev->dev,
+					"Wait Credit ERR: 0x%x\n", status);
+				return status;
+			}
+		}
+
+		if (remaining >= conf->priv->block_size)
+			len = conf->priv->block_size;
 		else
 			len = remaining;
 
 		altera_cvp_send_block(conf, data, len);
-		data++;
+		data += len / sizeof(u32);
 		done += len;
 		remaining -= len;
+		conf->current_credit_byte++;
 
 		/*
 		 * STEP 10 (optional) and STEP 11
@@ -370,7 +487,8 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
 
 	/* STEP 18 - poll PLD_CLK_IN_USE and USER_MODE bits */
 	mask = VSE_CVP_STATUS_PLD_CLK_IN_USE | VSE_CVP_STATUS_USERMODE;
-	ret = altera_cvp_wait_status(conf, mask, mask, TIMEOUT_US);
+	ret = altera_cvp_wait_status(conf, mask, mask,
+				     conf->priv->user_time_us);
 	if (ret)
 		dev_err(&mgr->dev, "PLD_CLK_IN_USE|USERMODE timeout\n");
 
@@ -384,6 +502,24 @@ static const struct fpga_manager_ops altera_cvp_ops = {
 	.write_complete	= altera_cvp_write_complete,
 };
 
+static const struct cvp_priv cvp_priv_v1 = {
+	.switch_clk	= altera_cvp_dummy_write,
+	.clear_state	= NULL,
+	.wait_credit	= NULL,
+	.block_size	= ALTERA_CVP_V1_SIZE,
+	.poll_time_us	= V1_POLL_TIMEOUT_US,
+	.user_time_us	= TIMEOUT_US,
+};
+
+static const struct cvp_priv cvp_priv_v2 = {
+	.switch_clk	= NULL,
+	.clear_state	= altera_cvp_v2_clear_state,
+	.wait_credit	= altera_cvp_v2_wait_for_credit,
+	.block_size	= ALTERA_CVP_V2_SIZE,
+	.poll_time_us	= V2_POLL_TIMEOUT_US,
+	.user_time_us	= V2_USER_TIMEOUT_US,
+};
+
 static ssize_t chkcfg_show(struct device_driver *dev, char *buf)
 {
 	return snprintf(buf, 3, "%d\n", altera_cvp_chkcfg);
@@ -486,6 +622,13 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 	conf->pci_dev = pdev;
 	conf->write_data = altera_cvp_write_data_iomem;
 
+	if (conf->vsec_offset == 0x200)
+		conf->priv = &cvp_priv_v1;
+	else
+		conf->priv = &cvp_priv_v2;
+
+	conf->current_credit_byte = 0;
+
 	conf->map = pci_iomap(pdev, CVP_BAR, 0);
 	if (!conf->map) {
 		dev_warn(&pdev->dev, "Mapping CVP BAR failed\n");
Signature · 1 line
2.7.4
maintainer
#8 lore raw

Re: [PATCH 3/3] fpga: altera-cvp: Add Stratix10 (V2) Support

Hi Thor,

Quoted 95 lines · On Thu, Jul 11, 2019 at 03:32:50PM -0500, [email protected] wrote:

From: Thor Thayer <[email protected]>

Add Stratix10 specific functions that use a credit mechanism to throttle data to the CvP FIFOs. Add a private structure with function pointers for V1 vs V2 functions.

---
 drivers/fpga/altera-cvp.c | 173 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 158 insertions(+), 15 deletions(-)
drivers/fpga/altera-cvp.c +49 -0
index 59835f6f9b2d..21bb08e5f52a 100644
@@ -43,16 +43,32 @@
 #define VSE_CVP_PROG_CTRL		0x2c	/* 32bit */
 #define VSE_CVP_PROG_CTRL_CONFIG	BIT(0)
 #define VSE_CVP_PROG_CTRL_START_XFER	BIT(1)
+#define VSE_CVP_PROG_CTRL_MASK		GENMASK(1, 0)
 
 #define VSE_UNCOR_ERR_STATUS		0x34	/* 32bit */
 #define VSE_UNCOR_ERR_CVP_CFG_ERR	BIT(5)	/* CVP_CONFIG_ERROR_LATCHED */
 
+/* V2 Defines */
+#define VSE_CVP_TX_CREDITS		0x49	/* 8bit */
+
+#define CREDIT_TIMEOUT_US		20000
+#define V2_POLL_TIMEOUT_US		1000000
+#define V2_USER_TIMEOUT_US		500000
+
+#define V1_POLL_TIMEOUT_US		10
+
 #define DRV_NAME		"altera-cvp"
 #define ALTERA_CVP_MGR_NAME	"Altera CvP FPGA Manager"
 
+/* Write block sizes */
+#define ALTERA_CVP_V1_SIZE	4
+#define ALTERA_CVP_V2_SIZE	4096
+
 /* Optional CvP config error status check for debugging */
 static bool altera_cvp_chkcfg;
 
+struct cvp_priv;
+
 struct altera_cvp_conf {
 	struct fpga_manager	*mgr;
 	struct pci_dev		*pci_dev;
@@ -60,9 +76,26 @@ struct altera_cvp_conf {
 	void			(*write_data)(struct altera_cvp_conf *, u32);
 	char			mgr_name[64];
 	u8			numclks;
+	u8			current_credit_byte;
 	u32			vsec_offset;
+	const struct cvp_priv	*priv;
+};
+
+struct cvp_priv {
+	void	(*switch_clk)(struct altera_cvp_conf *conf);
+	int	(*clear_state)(struct altera_cvp_conf *conf);
+	int	(*wait_credit)(struct fpga_manager *mgr, u32 blocks);
+	int	block_size;
+	int	poll_time_us;
+	int	user_time_us;
 };
 
+static inline void altera_read_config_byte(struct altera_cvp_conf *conf,
+					   int where, u8 *val)
+{
+	pci_read_config_byte(conf->pci_dev, conf->vsec_offset + where, val);
+}
+
 static inline void altera_read_config_dword(struct altera_cvp_conf *conf,
 					    int where, u32 *val)
 {
@@ -155,6 +188,57 @@ static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
 	return 0;
 }
 
+/*
+ * CvP Version2 Functions
+ * Recent Intel FPGAs use a credit mechanism to throttle incoming
+ * bitstreams and a different method of clearing the state.
+ */
+
+static int altera_cvp_v2_clear_state(struct altera_cvp_conf *conf)
+{
+	u32 val;
+
+	/* Clear the START_XFER and CVP_CONFIG bits */
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
+	val &= ~VSE_CVP_PROG_CTRL_MASK;
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+
+	return altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0,

drivers/fpga/altera-cvp.c static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes) · line 216

Quoted 10 lines
drivers/fpga/altera-cvp.c +10 -0
+				      conf->priv->poll_time_us);
+}
+
+static int altera_cvp_v2_wait_for_credit(struct fpga_manager *mgr,
+					 u32 blocks)
+{
+	struct altera_cvp_conf *conf = mgr->priv;
+	u32 count = 0;
+	int ret;
+	u8 val, delta_credit;

Reverse xmas-tree.

drivers/fpga/altera-cvp.c static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes) · line 220

Quoted 4 lines
drivers/fpga/altera-cvp.c +4 -0
+
+	do {
+		altera_read_config_byte(conf, VSE_CVP_TX_CREDITS, &val);
+		delta_credit = (val - conf->current_credit_byte) & 0xff;

Can 0xff be a named constant?

Quoted 6 lines
drivers/fpga/altera-cvp.c +6 -0
+
+		ret = altera_cvp_chk_error(mgr, blocks * ALTERA_CVP_V2_SIZE);
+		if (ret) {
+			dev_err(&conf->pci_dev->dev,
+				"CE Bit error credits host[0x%x]:dev[0x%x]\n",
+				conf->current_credit_byte, val);

drivers/fpga/altera-cvp.c static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes) · line 236

Quoted 10 lines
drivers/fpga/altera-cvp.c +10 -0
+			return -EAGAIN;
+		}
+
+		if (count++ >= CREDIT_TIMEOUT_US) {
+			dev_err(&conf->pci_dev->dev,
+				"Timeout waiting for credit\n");
+			return -ETIMEDOUT;
+		}
+
+		udelay(1);

A comment why this (1 us) is required would be nice :)

Quoted 172 lines
drivers/fpga/altera-cvp.c +79 -15
+	} while (!delta_credit);
+
+	return 0;
+}
+
 static int altera_cvp_send_block(struct altera_cvp_conf *conf,
 				 const u32 *data, size_t len)
 {
@@ -196,10 +280,12 @@ static int altera_cvp_teardown(struct fpga_manager *mgr,
 	 * - set CVP_NUMCLKS to 1 and then issue CVP_DUMMY_WR dummy
 	 *   writes to the HIP
 	 */
-	altera_cvp_dummy_write(conf); /* from CVP clock to internal clock */
+	if (conf->priv->switch_clk)
+		conf->priv->switch_clk(conf);
 
 	/* STEP 15 - poll CVP_CONFIG_READY bit for 0 with 10us timeout */
-	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0, 10);
+	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0,
+				     conf->priv->poll_time_us);
 	if (ret)
 		dev_err(&mgr->dev, "CFG_RDY == 0 timeout\n");
 
@@ -261,7 +347,16 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	 * STEP 3
 	 * - set CVP_NUMCLKS to 1 and issue CVP_DUMMY_WR dummy writes to the HIP
 	 */
-	altera_cvp_dummy_write(conf);
+	if (conf->priv->switch_clk)
+		conf->priv->switch_clk(conf);
+
+	if (conf->priv->clear_state) {
+		ret = conf->priv->clear_state(conf);
+		if (ret) {
+			dev_err(&mgr->dev, "Problem clearing out state\n");
+			return ret;
+		}
+	}
 
 	/* STEP 4 - set CVP_CONFIG bit */
 	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
@@ -269,9 +364,10 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	val |= VSE_CVP_PROG_CTRL_CONFIG;
 	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
-	/* STEP 5 - poll CVP_CONFIG READY for 1 with 10us timeout */
+	/* STEP 5 - poll CVP_CONFIG READY for 1 with timeout */
 	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY,
-				     VSE_CVP_STATUS_CFG_RDY, 10);
+				     VSE_CVP_STATUS_CFG_RDY,
+				     conf->priv->poll_time_us);
 	if (ret) {
 		dev_warn(&mgr->dev, "CFG_RDY == 1 timeout\n");
 		return ret;
@@ -281,7 +377,16 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	 * STEP 6
 	 * - set CVP_NUMCLKS to 1 and issue CVP_DUMMY_WR dummy writes to the HIP
 	 */
-	altera_cvp_dummy_write(conf);
+	if (conf->priv->switch_clk)
+		conf->priv->switch_clk(conf);
+
+	if (altera_cvp_chkcfg) {
+		ret = altera_cvp_chk_error(mgr, 0);
+		if (ret) {
+			dev_warn(&mgr->dev, "CFG_RDY == 1 timeout\n");
+			return ret;
+		}
+	}
 
 	/* STEP 7 - set START_XFER */
 	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
@@ -289,11 +394,12 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
 	/* STEP 8 - start transfer (set CVP_NUMCLKS for bitstream) */
-	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
-	val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
-	val |= conf->numclks << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
-	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
-
+	if (conf->priv->switch_clk) {
+		altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+		val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
+		val |= conf->numclks << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
+		altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+	}
 	return 0;
 }
 
@@ -311,15 +417,26 @@ static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
 	done = 0;
 
 	while (remaining) {
-		if (remaining >= sizeof(u32))
-			len = sizeof(u32);
+		/* Use credit throttling if available */
+		if (conf->priv->wait_credit) {
+			status = conf->priv->wait_credit(mgr, done);
+			if (status) {
+				dev_err(&conf->pci_dev->dev,
+					"Wait Credit ERR: 0x%x\n", status);
+				return status;
+			}
+		}
+
+		if (remaining >= conf->priv->block_size)
+			len = conf->priv->block_size;
 		else
 			len = remaining;
 
 		altera_cvp_send_block(conf, data, len);
-		data++;
+		data += len / sizeof(u32);
 		done += len;
 		remaining -= len;
+		conf->current_credit_byte++;
 
 		/*
 		 * STEP 10 (optional) and STEP 11
@@ -370,7 +487,8 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
 
 	/* STEP 18 - poll PLD_CLK_IN_USE and USER_MODE bits */
 	mask = VSE_CVP_STATUS_PLD_CLK_IN_USE | VSE_CVP_STATUS_USERMODE;
-	ret = altera_cvp_wait_status(conf, mask, mask, TIMEOUT_US);
+	ret = altera_cvp_wait_status(conf, mask, mask,
+				     conf->priv->user_time_us);
 	if (ret)
 		dev_err(&mgr->dev, "PLD_CLK_IN_USE|USERMODE timeout\n");
 
@@ -384,6 +502,24 @@ static const struct fpga_manager_ops altera_cvp_ops = {
 	.write_complete	= altera_cvp_write_complete,
 };
 
+static const struct cvp_priv cvp_priv_v1 = {
+	.switch_clk	= altera_cvp_dummy_write,
+	.clear_state	= NULL,
+	.wait_credit	= NULL,
+	.block_size	= ALTERA_CVP_V1_SIZE,
+	.poll_time_us	= V1_POLL_TIMEOUT_US,
+	.user_time_us	= TIMEOUT_US,
+};
+
+static const struct cvp_priv cvp_priv_v2 = {
+	.switch_clk	= NULL,
+	.clear_state	= altera_cvp_v2_clear_state,
+	.wait_credit	= altera_cvp_v2_wait_for_credit,
+	.block_size	= ALTERA_CVP_V2_SIZE,
+	.poll_time_us	= V2_POLL_TIMEOUT_US,
+	.user_time_us	= V2_USER_TIMEOUT_US,
+};
+
 static ssize_t chkcfg_show(struct device_driver *dev, char *buf)
 {
 	return snprintf(buf, 3, "%d\n", altera_cvp_chkcfg);
@@ -486,6 +622,13 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 	conf->pci_dev = pdev;
 	conf->write_data = altera_cvp_write_data_iomem;
 
+	if (conf->vsec_offset == 0x200)
+		conf->priv = &cvp_priv_v1;
+	else
+		conf->priv = &cvp_priv_v2;
+
+	conf->current_credit_byte = 0;
+
 	conf->map = pci_iomap(pdev, CVP_BAR, 0);
 	if (!conf->map) {
 		dev_warn(&pdev->dev, "Mapping CVP BAR failed\n");
Signature · 1 line
2.7.4

Thanks,
Moritz

#9 lore raw

Re: [PATCH 3/3] fpga: altera-cvp: Add Stratix10 (V2) Support

Hi Moritz,

Quoted 102 lines · Moritz Fischer

Hi Thor,

Quoted 99 lines · On Thu, Jul 11, 2019 at 03:32:50PM -0500, [email protected] wrote:

From: Thor Thayer <[email protected]>

Add Stratix10 specific functions that use a credit mechanism to throttle data to the CvP FIFOs. Add a private structure with function pointers for V1 vs V2 functions.

---
  drivers/fpga/altera-cvp.c | 173 ++++++++++++++++++++++++++++++++++++++++++----
  1 file changed, 158 insertions(+), 15 deletions(-)
drivers/fpga/altera-cvp.c +53 -0
index 59835f6f9b2d..21bb08e5f52a 100644
@@ -43,16 +43,32 @@
  #define VSE_CVP_PROG_CTRL		0x2c	/* 32bit */
  #define VSE_CVP_PROG_CTRL_CONFIG	BIT(0)
  #define VSE_CVP_PROG_CTRL_START_XFER	BIT(1)
+#define VSE_CVP_PROG_CTRL_MASK		GENMASK(1, 0)
 
  #define VSE_UNCOR_ERR_STATUS		0x34	/* 32bit */
  #define VSE_UNCOR_ERR_CVP_CFG_ERR	BIT(5)	/* CVP_CONFIG_ERROR_LATCHED */
 
+/* V2 Defines */
+#define VSE_CVP_TX_CREDITS		0x49	/* 8bit */
+
+#define CREDIT_TIMEOUT_US		20000
+#define V2_POLL_TIMEOUT_US		1000000
+#define V2_USER_TIMEOUT_US		500000
+
+#define V1_POLL_TIMEOUT_US		10
+
  #define DRV_NAME		"altera-cvp"
  #define ALTERA_CVP_MGR_NAME	"Altera CvP FPGA Manager"
 
+/* Write block sizes */
+#define ALTERA_CVP_V1_SIZE	4
+#define ALTERA_CVP_V2_SIZE	4096
+
  /* Optional CvP config error status check for debugging */
  static bool altera_cvp_chkcfg;
 
+struct cvp_priv;
+
  struct altera_cvp_conf {
  	struct fpga_manager	*mgr;
  	struct pci_dev		*pci_dev;
@@ -60,9 +76,26 @@ struct altera_cvp_conf {
  	void			(*write_data)(struct altera_cvp_conf *, u32);
  	char			mgr_name[64];
  	u8			numclks;
+	u8			current_credit_byte;
  	u32			vsec_offset;
+	const struct cvp_priv	*priv;
+};
+
+struct cvp_priv {
+	void	(*switch_clk)(struct altera_cvp_conf *conf);
+	int	(*clear_state)(struct altera_cvp_conf *conf);
+	int	(*wait_credit)(struct fpga_manager *mgr, u32 blocks);
+	int	block_size;
+	int	poll_time_us;
+	int	user_time_us;
  };
 
+static inline void altera_read_config_byte(struct altera_cvp_conf *conf,
+					   int where, u8 *val)
+{
+	pci_read_config_byte(conf->pci_dev, conf->vsec_offset + where, val);
+}
+
  static inline void altera_read_config_dword(struct altera_cvp_conf *conf,
  					    int where, u32 *val)
  {
@@ -155,6 +188,57 @@ static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
  	return 0;
  }
 
+/*
+ * CvP Version2 Functions
+ * Recent Intel FPGAs use a credit mechanism to throttle incoming
+ * bitstreams and a different method of clearing the state.
+ */
+
+static int altera_cvp_v2_clear_state(struct altera_cvp_conf *conf)
+{
+	u32 val;
+
+	/* Clear the START_XFER and CVP_CONFIG bits */
+	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
+	val &= ~VSE_CVP_PROG_CTRL_MASK;
+	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+
+	return altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0,
+				      conf->priv->poll_time_us);
+}
+
+static int altera_cvp_v2_wait_for_credit(struct fpga_manager *mgr,
Quoted 8 lines

drivers/fpga/altera-cvp.c static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes) · line 216

Quoted 6 lines
drivers/fpga/altera-cvp.c +6 -0
+					 u32 blocks)
+{
+	struct altera_cvp_conf *conf = mgr->priv;
+	u32 count = 0;
+	int ret;
+	u8 val, delta_credit;

Reverse xmas-tree.

OK.

Quoted 6 lines

drivers/fpga/altera-cvp.c static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes) · line 220

Quoted 4 lines
drivers/fpga/altera-cvp.c +4 -0
+
+	do {
+		altera_read_config_byte(conf, VSE_CVP_TX_CREDITS, &val);
+		delta_credit = (val - conf->current_credit_byte) & 0xff;

Can 0xff be a named constant?

Hmm. I'll rework this. After looking at this closer, this shouldn't be needed since all the values are u8. I need to handle the rollover but the mask is meaningless.

Quoted 10 lines
Quoted 10 lines
drivers/fpga/altera-cvp.c +10 -0
+
+		ret = altera_cvp_chk_error(mgr, blocks * ALTERA_CVP_V2_SIZE);
+		if (ret) {
+			dev_err(&conf->pci_dev->dev,
+				"CE Bit error credits host[0x%x]:dev[0x%x]\n",
+				conf->current_credit_byte, val);
+			return -EAGAIN;
+		}
+
+		if (count++ >= CREDIT_TIMEOUT_US) {
Quoted 8 lines

drivers/fpga/altera-cvp.c static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes) · line 236

Quoted 6 lines
drivers/fpga/altera-cvp.c +6 -0
+			dev_err(&conf->pci_dev->dev,
+				"Timeout waiting for credit\n");
+			return -ETIMEDOUT;
+		}
+
+		udelay(1);

A comment why this (1 us) is required would be nice :)

Yes, I'll add a comment. It is to limit the traffic from queries as well as having a consistent timeout.

Quoted 159 lines
Quoted 159 lines
drivers/fpga/altera-cvp.c +72 -15
+	} while (!delta_credit);
+
+	return 0;
+}
+
  static int altera_cvp_send_block(struct altera_cvp_conf *conf,
  				 const u32 *data, size_t len)
  {
@@ -196,10 +280,12 @@ static int altera_cvp_teardown(struct fpga_manager *mgr,
  	 * - set CVP_NUMCLKS to 1 and then issue CVP_DUMMY_WR dummy
  	 *   writes to the HIP
  	 */
-	altera_cvp_dummy_write(conf); /* from CVP clock to internal clock */
+	if (conf->priv->switch_clk)
+		conf->priv->switch_clk(conf);
 
  	/* STEP 15 - poll CVP_CONFIG_READY bit for 0 with 10us timeout */
-	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0, 10);
+	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0,
+				     conf->priv->poll_time_us);
  	if (ret)
  		dev_err(&mgr->dev, "CFG_RDY == 0 timeout\n");
 
@@ -261,7 +347,16 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
  	 * STEP 3
  	 * - set CVP_NUMCLKS to 1 and issue CVP_DUMMY_WR dummy writes to the HIP
  	 */
-	altera_cvp_dummy_write(conf);
+	if (conf->priv->switch_clk)
+		conf->priv->switch_clk(conf);
+
+	if (conf->priv->clear_state) {
+		ret = conf->priv->clear_state(conf);
+		if (ret) {
+			dev_err(&mgr->dev, "Problem clearing out state\n");
+			return ret;
+		}
+	}
 
  	/* STEP 4 - set CVP_CONFIG bit */
  	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
@@ -269,9 +364,10 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
  	val |= VSE_CVP_PROG_CTRL_CONFIG;
  	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
-	/* STEP 5 - poll CVP_CONFIG READY for 1 with 10us timeout */
+	/* STEP 5 - poll CVP_CONFIG READY for 1 with timeout */
  	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY,
-				     VSE_CVP_STATUS_CFG_RDY, 10);
+				     VSE_CVP_STATUS_CFG_RDY,
+				     conf->priv->poll_time_us);
  	if (ret) {
  		dev_warn(&mgr->dev, "CFG_RDY == 1 timeout\n");
  		return ret;
@@ -281,7 +377,16 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
  	 * STEP 6
  	 * - set CVP_NUMCLKS to 1 and issue CVP_DUMMY_WR dummy writes to the HIP
  	 */
-	altera_cvp_dummy_write(conf);
+	if (conf->priv->switch_clk)
+		conf->priv->switch_clk(conf);
+
+	if (altera_cvp_chkcfg) {
+		ret = altera_cvp_chk_error(mgr, 0);
+		if (ret) {
+			dev_warn(&mgr->dev, "CFG_RDY == 1 timeout\n");
+			return ret;
+		}
+	}
 
  	/* STEP 7 - set START_XFER */
  	altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
@@ -289,11 +394,12 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
  	altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
 
  	/* STEP 8 - start transfer (set CVP_NUMCLKS for bitstream) */
-	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
-	val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
-	val |= conf->numclks << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
-	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
-
+	if (conf->priv->switch_clk) {
+		altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+		val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
+		val |= conf->numclks << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
+		altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+	}
  	return 0;
  }
 
@@ -311,15 +417,26 @@ static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
  	done = 0;
 
  	while (remaining) {
-		if (remaining >= sizeof(u32))
-			len = sizeof(u32);
+		/* Use credit throttling if available */
+		if (conf->priv->wait_credit) {
+			status = conf->priv->wait_credit(mgr, done);
+			if (status) {
+				dev_err(&conf->pci_dev->dev,
+					"Wait Credit ERR: 0x%x\n", status);
+				return status;
+			}
+		}
+
+		if (remaining >= conf->priv->block_size)
+			len = conf->priv->block_size;
  		else
  			len = remaining;
 
  		altera_cvp_send_block(conf, data, len);
-		data++;
+		data += len / sizeof(u32);
  		done += len;
  		remaining -= len;
+		conf->current_credit_byte++;
 
  		/*
  		 * STEP 10 (optional) and STEP 11
@@ -370,7 +487,8 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
 
  	/* STEP 18 - poll PLD_CLK_IN_USE and USER_MODE bits */
  	mask = VSE_CVP_STATUS_PLD_CLK_IN_USE | VSE_CVP_STATUS_USERMODE;
-	ret = altera_cvp_wait_status(conf, mask, mask, TIMEOUT_US);
+	ret = altera_cvp_wait_status(conf, mask, mask,
+				     conf->priv->user_time_us);
  	if (ret)
  		dev_err(&mgr->dev, "PLD_CLK_IN_USE|USERMODE timeout\n");
 
@@ -384,6 +502,24 @@ static const struct fpga_manager_ops altera_cvp_ops = {
  	.write_complete	= altera_cvp_write_complete,
  };
 
+static const struct cvp_priv cvp_priv_v1 = {
+	.switch_clk	= altera_cvp_dummy_write,
+	.clear_state	= NULL,
+	.wait_credit	= NULL,
+	.block_size	= ALTERA_CVP_V1_SIZE,
+	.poll_time_us	= V1_POLL_TIMEOUT_US,
+	.user_time_us	= TIMEOUT_US,
+};
+
+static const struct cvp_priv cvp_priv_v2 = {
+	.switch_clk	= NULL,
+	.clear_state	= altera_cvp_v2_clear_state,
+	.wait_credit	= altera_cvp_v2_wait_for_credit,
+	.block_size	= ALTERA_CVP_V2_SIZE,
+	.poll_time_us	= V2_POLL_TIMEOUT_US,
+	.user_time_us	= V2_USER_TIMEOUT_US,
+};
+
  static ssize_t chkcfg_show(struct device_driver *dev, char *buf)
  {
  	return snprintf(buf, 3, "%d\n", altera_cvp_chkcfg);
@@ -486,6 +622,13 @@ static int altera_cvp_probe(struct pci_dev *pdev,
  	conf->pci_dev = pdev;
  	conf->write_data = altera_cvp_write_data_iomem;
 
Quoted 17 lines
Quoted 13 lines
drivers/fpga/altera-cvp.c +7 -0
+	if (conf->vsec_offset == 0x200)
+		conf->priv = &cvp_priv_v1;
+	else
+		conf->priv = &cvp_priv_v2;
+
+	conf->current_credit_byte = 0;
+
  	conf->map = pci_iomap(pdev, CVP_BAR, 0);
  	if (!conf->map) {
  		dev_warn(&pdev->dev, "Mapping CVP BAR failed\n");
Signature · 1 line
2.7.4

Thanks,
Moritz

Thank you for reviewing!

Thor