Messages in this thread | | | Subject | Re: [PATCH] fpga: altera-cvp: Truncated bitstream error support | From | Tom Rix <> | Date | Wed, 18 May 2022 13:08:31 -0700 |
| |
On 5/17/22 11:48 PM, tien.sung.ang@intel.com wrote: > From: Ang Tien Sung <tien.sung.ang@intel.com> > > To support the error handling of a truncated bitstream sent. > The current AIB CvP firmware is not capable of handling a > data stream smaller than 4096bytes. The firmware's limitation > causes a hung-up as it's DMA engine waits forever for the > completion of the instructed 4096bytes. > To resolve this design limitation, both firmware and CvP > driver made several changes. At the CvP driver, we just > have to ensure that anything lesser than 4096bytes are > padded with extra bytes. The CvP will then, initiate the > tear-down by clearing the START_XFER and CVP_CONFIG bits. > We should also check for CVP_ERROR during the CvP completion. > A send_buf which is always 4096bytes is used to copy the > data during every transaction. > > Signed-off-by: Ang Tien Sung <tien.sung.ang@intel.com> > --- > drivers/fpga/altera-cvp.c | 24 +++++++++++++++++++----- > 1 file changed, 19 insertions(+), 5 deletions(-) > > diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c > index 4ffb9da537d8..80edcfb5e5fc 100644 > --- a/drivers/fpga/altera-cvp.c > +++ b/drivers/fpga/altera-cvp.c > @@ -81,6 +81,7 @@ struct altera_cvp_conf { > u8 numclks; > u32 sent_packets; > u32 vsec_offset; > + u8 *send_buf;
Why is it necessary to hold the send_buf in this structure ?
If it is used only in *_write, could it alloc/freed there ?
Because the write happens rarely, my preference is to alloc/free in *_write().
> const struct cvp_priv *priv; > }; > > @@ -453,7 +454,11 @@ static int altera_cvp_write(struct fpga_manager *mgr, const char *buf, > } > > len = min(conf->priv->block_size, remaining); > - altera_cvp_send_block(conf, data, len); > + /* Copy the requested host data into the transmit buffer */ > + > + memcpy(conf->send_buf, data, len); Is a memset needed for a short buffer ? > + altera_cvp_send_block(conf, (const u32 *)conf->send_buf, > + conf->priv->block_size); > data += len / sizeof(u32); > done += len; > remaining -= len; > @@ -492,10 +497,13 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr, > if (ret) > return ret; > > - /* STEP 16 - check CVP_CONFIG_ERROR_LATCHED bit */ > - 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"); > + /* > + * STEP 16 - If bitstream error (truncated/miss-matched), > + * we shall exit here. > + */ > + ret = altera_read_config_dword(conf, VSE_CVP_STATUS, &val); Should this be STEP 17 ? the old 16 checked something else.
Tom
> + if (ret || (val & VSE_CVP_STATUS_CFG_ERR)) { > + dev_err(&mgr->dev, "CVP_CONFIG_ERROR!\n"); > return -EPROTO; > } > > @@ -661,6 +669,12 @@ static int altera_cvp_probe(struct pci_dev *pdev, > > pci_set_drvdata(pdev, mgr); > > + /* Allocate the 4096 block size transmit buffer */ > + conf->send_buf = devm_kzalloc(&pdev->dev, conf->priv->block_size, GFP_KERNEL); > + if (!conf->send_buf) { > + ret = -ENOMEM; > + goto err_unmap; > + } > return 0; > > err_unmap:
| |