On 10/21/24 22:12, Chautru, Nicolas wrote:
Hi Maxime, Do you want me to send a v2 for this or you can change the comment before applying? "Also, take the opportunity to fix some unused VRB2 registers definitions".
I can do it while applying. Thanks, Maxime
-----Original Message----- From: Maxime Coquelin <maxime.coque...@redhat.com> Sent: Monday, October 21, 2024 8:06 AM To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org Cc: hemant.agra...@nxp.com; Vargas, Hernan <hernan.var...@intel.com> Subject: Re: [PATCH v1 1/2] baseband/acc: FFT support in VRB2 PRQ device Hi, On 10/18/24 20:42, Chautru, Nicolas wrote:Hi Maxime,-----Original Message----- From: Maxime Coquelin <maxime.coque...@redhat.com> Sent: Friday, October 18, 2024 12:54 AM To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org Cc: hemant.agra...@nxp.com; Vargas, Hernan<hernan.var...@intel.com>Subject: Re: [PATCH v1 1/2] baseband/acc: FFT support in VRB2 PRQ device Hi Nicolas, On 10/15/24 00:30, Nicolas Chautru wrote:Supporting recent change in the device to extend FFT capability processing in latest stepping. Also including cosmetic change to VRB2 register definition. Signed-off-by: Nicolas Chautru <nicolas.chau...@intel.com> --- drivers/baseband/acc/acc_common.h | 2 +- drivers/baseband/acc/rte_vrb_pmd.c | 30 +++++++++++++++++++++++++----drivers/baseband/acc/vrb2_vf_enum.h | 4 ++-- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h index 0c249d5b93..4c60b7896b 100644 --- a/drivers/baseband/acc/acc_common.h +++ b/drivers/baseband/acc/acc_common.h @@ -106,7 +106,7 @@ #define ACC_MAX_FCW_SIZE 128 #define ACC_IQ_SIZE 4 -#define ACC_FCW_FFT_BLEN_3 28 +#define ACC_FCW_FFT_BLEN_VRB2 128 /* Constants from K0 computation from 3GPP 38.212 Table 5.4.2.1-2 */ #define ACC_N_ZC_1 66 /* N = 66 Zc for BG 1 */ diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c index 0455320c2a..5eb3e8dd48 100644 --- a/drivers/baseband/acc/rte_vrb_pmd.c +++ b/drivers/baseband/acc/rte_vrb_pmd.c @@ -1006,7 +1006,7 @@ vrb_queue_setup(struct rte_bbdev *dev,uint16_t queue_id,case RTE_BBDEV_OP_FFT: fcw_len = ACC_FCW_FFT_BLEN; if (q->d->device_variant == VRB2_VARIANT) - fcw_len = ACC_FCW_FFT_BLEN_3; + fcw_len = ACC_FCW_FFT_BLEN_VRB2; break; case RTE_BBDEV_OP_MLDTS: fcw_len = ACC_FCW_MLDTS_BLEN; @@ -1402,7 +1402,11 @@ vrb_dev_info_get(struct rte_bbdev *dev, structrte_bbdev_driver_info *dev_info)RTE_BBDEV_FFT_FP16_INPUT|RTE_BBDEV_FFT_FP16_OUTPUT |RTE_BBDEV_FFT_POWER_MEAS |-RTE_BBDEV_FFT_WINDOWING_BYPASS,+RTE_BBDEV_FFT_WINDOWING_BYPASS |+RTE_BBDEV_FFT_TIMING_OFFSET_PER_CS |+RTE_BBDEV_FFT_TIMING_ERROR |+RTE_BBDEV_FFT_DEWINDOWING |+RTE_BBDEV_FFT_FREQ_RESAMPLING,.num_buffers_src = 1, .num_buffers_dst = 1, .fft_windows_num = ACC_MAX_FFT_WIN,@@ -3725,6 +3729,8 @@vrb1_fcw_fft_fill(struct rte_bbdev_fft_op *op, struct acc_fcw_fft *fcw) static inline void vrb2_fcw_fft_fill(struct rte_bbdev_fft_op *op, struct acc_fcw_fft_3*fcw){ + uint8_t cs; + fcw->in_frame_size = op->fft.input_sequence_size; fcw->leading_pad_size = op->fft.input_leading_padding; fcw->out_frame_size = op->fft.output_sequence_size; @@ -3760,6 +3766,16 @@ vrb2_fcw_fft_fill(struct rte_bbdev_fft_op *op, structacc_fcw_fft_3 *fcw)fcw->bypass = 3; else fcw->bypass = 0; + + fcw->enable_dewin = check_bit(op->fft.op_flags,RTE_BBDEV_FFT_DEWINDOWING);+ fcw->freq_resample_mode = op->fft.freq_resample_mode; + fcw->depad_output_size = fcw->freq_resample_mode == 0 ? + op->fft.output_sequence_size : op- fft.output_depadded_size; + for (cs = 0; cs < RTE_BBDEV_MAX_CS; cs++) { + fcw->cs_theta_0[cs] = op->fft.cs_theta_0[cs]; + fcw->cs_theta_d[cs] = op->fft.cs_theta_d[cs]; + fcw->cs_time_offset[cs] = op->fft.time_offset[cs]; + } } static inline int @@ -3782,8 +3798,14 @@ vrb_dma_desc_fft_fill(structrte_bbdev_fft_op*op,/* FCW already done */ acc_header_init(desc); - RTE_SET_USED(win_input); - RTE_SET_USED(win_offset); + if (win_en && win_input) { + desc->data_ptrs[bd_idx].address =rte_pktmbuf_iova_offset(win_input, *win_offset);+ desc->data_ptrs[bd_idx].blen = op->fft.output_depadded_size* 2;+ desc->data_ptrs[bd_idx].blkid = ACC_DMA_BLKID_DEWIN_IN; + desc->data_ptrs[bd_idx].last = 0; + desc->data_ptrs[bd_idx].dma_ext = 0; + bd_idx++; + } desc->data_ptrs[bd_idx].address = rte_pktmbuf_iova_offset(input,*in_offset);desc->data_ptrs[bd_idx].blen = op->fft.input_sequence_size * ACC_IQ_SIZE; diff --git a/drivers/baseband/acc/vrb2_vf_enum.h b/drivers/baseband/acc/vrb2_vf_enum.h index 9c6e451010..1cc6986c67 100644 --- a/drivers/baseband/acc/vrb2_vf_enum.h +++ b/drivers/baseband/acc/vrb2_vf_enum.h @@ -18,8 +18,8 @@ enum { VRB2_VfHiInfoRingIntWrEnVf = 0x00000020, VRB2_VfHiInfoRingPf2VfWrEnVf = 0x00000024, VRB2_VfHiMsixVectorMapperVf = 0x00000060, - VRB2_VfHiDeviceStatus = 0x00000068, - VRB2_VfHiInterruptSrc = 0x00000070, + VRB2_VfHiDeviceStatus = 0x00000064, + VRB2_VfHiInterruptSrc = 0x00000068,The offset of the registers change, is that what you describe as cosmetic change? Does it have an impact on older DPDK versions? i.e. should it bebackported?Hi Maxime, These registers are currently not used in the code so it reallyhas no impact.It is more for documentation purpose and in case we need to use them in the future, basically not to carry such discrepancy with specs forward. No actual impact and does not need to be back ported for the same reason.Ok, then I would suggest to just name things as they are in the commit message. "Also, take the opportunity to fix some unsused VRB2 registers definitions". Thanks, MaximeThanks NicThanks, MaximeVRB2_VfDmaFec5GulDescBaseLoRegVf = 0x00000120, VRB2_VfDmaFec5GulDescBaseHiRegVf = 0x00000124, VRB2_VfDmaFec5GulRespPtrLoRegVf = 0x00000128,
