| Message ID | 20190315164440.13209-4-fbarrat@linux.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | opencapi hw procedure fixes for mixed mode | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (c470806a2e5e25894aae772fc04069bbe35a21e4) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On 16/3/19 3:44 am, Frederic Barrat wrote: > For opencapi, we currently do impedance calibration when initializing > the PHY for the device, which could run in parallel if we were rich > and had multiple opencapi devices. But if 2 devices are on the same > obus, the 2 calibration sequences could overlap, which likely yields > bad results and is useless anyway since it only needs to be done once > per obus. > > This patch splits the opencapi PHY reset in 2 parts: > - a 'init' part called serially at boot. That's when zcal is done. If > we have 2 devices on the same socket, the zcal won't be redone, > since we're called serially and we'll see it has already be done for > the obus > - a 'reset' part called during fundamental reset as a prereq for link > training. It does the PHY setup for a set of lanes and the dccal. > > The PHY team confirmed there's no dependency between zcal and the > other reset steps and it can be moved earlier. > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> Does this need to go to stable? Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> > --- > hw/npu2-hw-procedures.c | 7 +++++-- > hw/npu2-opencapi.c | 5 +++-- > include/npu2.h | 3 ++- > 3 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/hw/npu2-hw-procedures.c b/hw/npu2-hw-procedures.c > index 2f330957..b671b8ac 100644 > --- a/hw/npu2-hw-procedures.c > +++ b/hw/npu2-hw-procedures.c > @@ -1017,17 +1017,20 @@ void npu2_opencapi_bump_ui_lane(struct npu2_dev *dev) > } > } > > -void npu2_opencapi_phy_setup(struct npu2_dev *dev) > +void npu2_opencapi_phy_init(struct npu2_dev *dev) > { > + run_procedure(dev, 5); /* procedure_phy_tx_zcal */ > /* > * This is only required for OpenCAPI - Hostboot tries to set this > * on systems where it can tell a link is OpenCAPI, but for > * Witherspoon it needs to be done in skiboot after device detection. > */ > phy_write(dev, &NPU2_PHY_RX_RC_ENABLE_AUTO_RECAL, 0x1); > +} > > +void npu2_opencapi_phy_reset(struct npu2_dev *dev) > +{ > run_procedure(dev, 4); /* procedure_phy_reset */ > - run_procedure(dev, 5); /* procedure_phy_tx_zcal */ > run_procedure(dev, 6); /* procedure_phy_rx_dccal */ > } > > diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c > index b2740f38..090223e2 100644 > --- a/hw/npu2-opencapi.c > +++ b/hw/npu2-opencapi.c > @@ -1194,7 +1194,7 @@ static int64_t npu2_opencapi_freset(struct pci_slot *slot) > } > dev->train_need_fence = true; > slot->link_retries = OCAPI_LINK_TRAINING_RETRIES; > - npu2_opencapi_phy_setup(dev); > + npu2_opencapi_phy_reset(dev); > /* fall-through */ > case OCAPI_SLOT_FRESET_INIT: > assert_odl_reset(chip_id, dev->brick_index); > @@ -1603,7 +1603,7 @@ static int setup_irq(struct npu2 *p) > > static void setup_debug_training_state(struct npu2_dev *dev) > { > - npu2_opencapi_phy_setup(dev); > + npu2_opencapi_phy_reset(dev); > > switch (npu2_ocapi_training_state) { > case NPU2_TRAIN_PRBS31: > @@ -1675,6 +1675,7 @@ static void setup_device(struct npu2_dev *dev) > /* Procedure 13.1.3.9 - AFU Config BARs */ > setup_afu_config_bars(dev->npu->chip_id, dev->npu->xscom_base, dev); > setup_perf_counters(dev); > + npu2_opencapi_phy_init(dev); > > set_fence_control(dev->npu->chip_id, dev->npu->xscom_base, dev->brick_index, 0b00); > > diff --git a/include/npu2.h b/include/npu2.h > index b62d1748..a2bcc450 100644 > --- a/include/npu2.h > +++ b/include/npu2.h > @@ -224,7 +224,8 @@ void npu2_set_link_flag(struct npu2_dev *ndev, uint8_t flag); > void npu2_clear_link_flag(struct npu2_dev *ndev, uint8_t flag); > uint32_t reset_ntl(struct npu2_dev *ndev); > extern int nv_zcal_nominal; > -void npu2_opencapi_phy_setup(struct npu2_dev *dev); > +void npu2_opencapi_phy_init(struct npu2_dev *dev); > +void npu2_opencapi_phy_reset(struct npu2_dev *dev); > void npu2_opencapi_phy_prbs31(struct npu2_dev *dev); > void npu2_opencapi_bump_ui_lane(struct npu2_dev *dev); > int64_t npu2_freeze_status(struct phb *phb __unused, >
Le 18/03/2019 à 01:57, Andrew Donnellan a écrit : > On 16/3/19 3:44 am, Frederic Barrat wrote: >> For opencapi, we currently do impedance calibration when initializing >> the PHY for the device, which could run in parallel if we were rich >> and had multiple opencapi devices. But if 2 devices are on the same >> obus, the 2 calibration sequences could overlap, which likely yields >> bad results and is useless anyway since it only needs to be done once >> per obus. >> >> This patch splits the opencapi PHY reset in 2 parts: >> - a 'init' part called serially at boot. That's when zcal is done. If >> we have 2 devices on the same socket, the zcal won't be redone, >> since we're called serially and we'll see it has already be done for >> the obus >> - a 'reset' part called during fundamental reset as a prereq for link >> training. It does the PHY setup for a set of lanes and the dccal. >> >> The PHY team confirmed there's no dependency between zcal and the >> other reset steps and it can be moved earlier. >> >> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> > > Does this need to go to stable? Yes, and I keep forgetting :-( This patch doesn't apply on v6.1. I'll backport it later this week. Fred > > Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> > >> --- >> hw/npu2-hw-procedures.c | 7 +++++-- >> hw/npu2-opencapi.c | 5 +++-- >> include/npu2.h | 3 ++- >> 3 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/hw/npu2-hw-procedures.c b/hw/npu2-hw-procedures.c >> index 2f330957..b671b8ac 100644 >> --- a/hw/npu2-hw-procedures.c >> +++ b/hw/npu2-hw-procedures.c >> @@ -1017,17 +1017,20 @@ void npu2_opencapi_bump_ui_lane(struct >> npu2_dev *dev) >> } >> } >> -void npu2_opencapi_phy_setup(struct npu2_dev *dev) >> +void npu2_opencapi_phy_init(struct npu2_dev *dev) >> { >> + run_procedure(dev, 5); /* procedure_phy_tx_zcal */ >> /* >> * This is only required for OpenCAPI - Hostboot tries to set this >> * on systems where it can tell a link is OpenCAPI, but for >> * Witherspoon it needs to be done in skiboot after device >> detection. >> */ >> phy_write(dev, &NPU2_PHY_RX_RC_ENABLE_AUTO_RECAL, 0x1); >> +} >> +void npu2_opencapi_phy_reset(struct npu2_dev *dev) >> +{ >> run_procedure(dev, 4); /* procedure_phy_reset */ >> - run_procedure(dev, 5); /* procedure_phy_tx_zcal */ >> run_procedure(dev, 6); /* procedure_phy_rx_dccal */ >> } >> diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c >> index b2740f38..090223e2 100644 >> --- a/hw/npu2-opencapi.c >> +++ b/hw/npu2-opencapi.c >> @@ -1194,7 +1194,7 @@ static int64_t npu2_opencapi_freset(struct >> pci_slot *slot) >> } >> dev->train_need_fence = true; >> slot->link_retries = OCAPI_LINK_TRAINING_RETRIES; >> - npu2_opencapi_phy_setup(dev); >> + npu2_opencapi_phy_reset(dev); >> /* fall-through */ >> case OCAPI_SLOT_FRESET_INIT: >> assert_odl_reset(chip_id, dev->brick_index); >> @@ -1603,7 +1603,7 @@ static int setup_irq(struct npu2 *p) >> static void setup_debug_training_state(struct npu2_dev *dev) >> { >> - npu2_opencapi_phy_setup(dev); >> + npu2_opencapi_phy_reset(dev); >> switch (npu2_ocapi_training_state) { >> case NPU2_TRAIN_PRBS31: >> @@ -1675,6 +1675,7 @@ static void setup_device(struct npu2_dev *dev) >> /* Procedure 13.1.3.9 - AFU Config BARs */ >> setup_afu_config_bars(dev->npu->chip_id, dev->npu->xscom_base, >> dev); >> setup_perf_counters(dev); >> + npu2_opencapi_phy_init(dev); >> set_fence_control(dev->npu->chip_id, dev->npu->xscom_base, >> dev->brick_index, 0b00); >> diff --git a/include/npu2.h b/include/npu2.h >> index b62d1748..a2bcc450 100644 >> --- a/include/npu2.h >> +++ b/include/npu2.h >> @@ -224,7 +224,8 @@ void npu2_set_link_flag(struct npu2_dev *ndev, >> uint8_t flag); >> void npu2_clear_link_flag(struct npu2_dev *ndev, uint8_t flag); >> uint32_t reset_ntl(struct npu2_dev *ndev); >> extern int nv_zcal_nominal; >> -void npu2_opencapi_phy_setup(struct npu2_dev *dev); >> +void npu2_opencapi_phy_init(struct npu2_dev *dev); >> +void npu2_opencapi_phy_reset(struct npu2_dev *dev); >> void npu2_opencapi_phy_prbs31(struct npu2_dev *dev); >> void npu2_opencapi_bump_ui_lane(struct npu2_dev *dev); >> int64_t npu2_freeze_status(struct phb *phb __unused, >> >
Frederic Barrat <fbarrat@linux.ibm.com> writes: > Le 18/03/2019 à 01:57, Andrew Donnellan a écrit : >> On 16/3/19 3:44 am, Frederic Barrat wrote: >>> For opencapi, we currently do impedance calibration when initializing >>> the PHY for the device, which could run in parallel if we were rich >>> and had multiple opencapi devices. But if 2 devices are on the same >>> obus, the 2 calibration sequences could overlap, which likely yields >>> bad results and is useless anyway since it only needs to be done once >>> per obus. >>> >>> This patch splits the opencapi PHY reset in 2 parts: >>> - a 'init' part called serially at boot. That's when zcal is done. If >>> we have 2 devices on the same socket, the zcal won't be redone, >>> since we're called serially and we'll see it has already be done for >>> the obus >>> - a 'reset' part called during fundamental reset as a prereq for link >>> training. It does the PHY setup for a set of lanes and the dccal. >>> >>> The PHY team confirmed there's no dependency between zcal and the >>> other reset steps and it can be moved earlier. >>> >>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> >> >> Does this need to go to stable? > > Yes, and I keep forgetting :-( > > This patch doesn't apply on v6.1. I'll backport it later this week. We only really care about 6.0 and 6.2 at this point - and given about 2 weeks we won't care about 6.2 anymore.
diff --git a/hw/npu2-hw-procedures.c b/hw/npu2-hw-procedures.c index 2f330957..b671b8ac 100644 --- a/hw/npu2-hw-procedures.c +++ b/hw/npu2-hw-procedures.c @@ -1017,17 +1017,20 @@ void npu2_opencapi_bump_ui_lane(struct npu2_dev *dev) } } -void npu2_opencapi_phy_setup(struct npu2_dev *dev) +void npu2_opencapi_phy_init(struct npu2_dev *dev) { + run_procedure(dev, 5); /* procedure_phy_tx_zcal */ /* * This is only required for OpenCAPI - Hostboot tries to set this * on systems where it can tell a link is OpenCAPI, but for * Witherspoon it needs to be done in skiboot after device detection. */ phy_write(dev, &NPU2_PHY_RX_RC_ENABLE_AUTO_RECAL, 0x1); +} +void npu2_opencapi_phy_reset(struct npu2_dev *dev) +{ run_procedure(dev, 4); /* procedure_phy_reset */ - run_procedure(dev, 5); /* procedure_phy_tx_zcal */ run_procedure(dev, 6); /* procedure_phy_rx_dccal */ } diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c index b2740f38..090223e2 100644 --- a/hw/npu2-opencapi.c +++ b/hw/npu2-opencapi.c @@ -1194,7 +1194,7 @@ static int64_t npu2_opencapi_freset(struct pci_slot *slot) } dev->train_need_fence = true; slot->link_retries = OCAPI_LINK_TRAINING_RETRIES; - npu2_opencapi_phy_setup(dev); + npu2_opencapi_phy_reset(dev); /* fall-through */ case OCAPI_SLOT_FRESET_INIT: assert_odl_reset(chip_id, dev->brick_index); @@ -1603,7 +1603,7 @@ static int setup_irq(struct npu2 *p) static void setup_debug_training_state(struct npu2_dev *dev) { - npu2_opencapi_phy_setup(dev); + npu2_opencapi_phy_reset(dev); switch (npu2_ocapi_training_state) { case NPU2_TRAIN_PRBS31: @@ -1675,6 +1675,7 @@ static void setup_device(struct npu2_dev *dev) /* Procedure 13.1.3.9 - AFU Config BARs */ setup_afu_config_bars(dev->npu->chip_id, dev->npu->xscom_base, dev); setup_perf_counters(dev); + npu2_opencapi_phy_init(dev); set_fence_control(dev->npu->chip_id, dev->npu->xscom_base, dev->brick_index, 0b00); diff --git a/include/npu2.h b/include/npu2.h index b62d1748..a2bcc450 100644 --- a/include/npu2.h +++ b/include/npu2.h @@ -224,7 +224,8 @@ void npu2_set_link_flag(struct npu2_dev *ndev, uint8_t flag); void npu2_clear_link_flag(struct npu2_dev *ndev, uint8_t flag); uint32_t reset_ntl(struct npu2_dev *ndev); extern int nv_zcal_nominal; -void npu2_opencapi_phy_setup(struct npu2_dev *dev); +void npu2_opencapi_phy_init(struct npu2_dev *dev); +void npu2_opencapi_phy_reset(struct npu2_dev *dev); void npu2_opencapi_phy_prbs31(struct npu2_dev *dev); void npu2_opencapi_bump_ui_lane(struct npu2_dev *dev); int64_t npu2_freeze_status(struct phb *phb __unused,
For opencapi, we currently do impedance calibration when initializing the PHY for the device, which could run in parallel if we were rich and had multiple opencapi devices. But if 2 devices are on the same obus, the 2 calibration sequences could overlap, which likely yields bad results and is useless anyway since it only needs to be done once per obus. This patch splits the opencapi PHY reset in 2 parts: - a 'init' part called serially at boot. That's when zcal is done. If we have 2 devices on the same socket, the zcal won't be redone, since we're called serially and we'll see it has already be done for the obus - a 'reset' part called during fundamental reset as a prereq for link training. It does the PHY setup for a set of lanes and the dccal. The PHY team confirmed there's no dependency between zcal and the other reset steps and it can be moved earlier. Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> --- hw/npu2-hw-procedures.c | 7 +++++-- hw/npu2-opencapi.c | 5 +++-- include/npu2.h | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-)