2023-11-20 10:16:48

by Kang Yang

[permalink] [raw]
Subject: [PATCH 0/7] wifi: ath12k: support one MSI

This patch set is to support one MSI vector for WCN7850.

Kang Yang (7):
wifi: ath12k: get msi_data again after request_irq is called
wifi: ath12k: add CE and ext IRQ flag to indicate irq_handler
wifi: ath12k: use ATH12K_PCI_IRQ_DP_OFFSET for DP IRQ
wifi: ath12k: refactor multiple MSI vector implementation
wifi: ath12k: add support one MSI vector
wifi: ath12k: do not restore ASPM in case of single MSI vector
wifi: ath12k: set IRQ affinity to CPU0 in case of one MSI vector

drivers/net/wireless/ath/ath12k/core.h | 2 +
drivers/net/wireless/ath/ath12k/mhi.c | 16 ++-
drivers/net/wireless/ath/ath12k/pci.c | 180 +++++++++++++++++++++----
drivers/net/wireless/ath/ath12k/pci.h | 2 +
4 files changed, 172 insertions(+), 28 deletions(-)


base-commit: 9a36440d929d134c56030a8492405708a143f580
--
2.34.1


2023-11-20 10:16:50

by Kang Yang

[permalink] [raw]
Subject: [PATCH 6/7] wifi: ath12k: do not restore ASPM in case of single MSI vector

Current code enables ASPM by default, it allows MHI to enter M2 state.
In case of one MSI vector, system hang is observed if ath12k does MHI
register reading in this state.

The workaround here is to prevent MHI from entering M2 state, this can
be done by disabling ASPM if only one MSI vector is used. When using 32
vectors ASPM is enabled as before.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Kang Yang <[email protected]>
---
drivers/net/wireless/ath/ath12k/pci.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index 4484bac18f4e..3850331438de 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -1094,7 +1094,10 @@ int ath12k_pci_start(struct ath12k_base *ab)

set_bit(ATH12K_PCI_FLAG_INIT_DONE, &ab_pci->flags);

- ath12k_pci_aspm_restore(ab_pci);
+ if (test_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
+ ath12k_pci_aspm_restore(ab_pci);
+ else
+ ath12k_info(ab, "leaving PCI ASPM disabled to avoid MHI M2 problems\n");

ath12k_pci_ce_irqs_enable(ab);
ath12k_ce_rx_post_buf(ab);
--
2.34.1

2023-11-20 10:16:51

by Kang Yang

[permalink] [raw]
Subject: [PATCH 5/7] wifi: ath12k: add support one MSI vector

On some platforms it's not possible to allocate 32 MSI vectors for
various reasons, maybe kernel configuration, VT-d disabled, buggy BIOS
etc. So ath12k was not able to use WCN7850 PCI devices on those
platforms. Add support for one MSI vector to solve that.

In case of one MSI vector, interrupt migration needs to be disabled.
This is because when interrupt migration happens, the msi_data may
change. However, msi_data is already programmed to rings during initial
phase and ath12k has no way to know that msi_data is changed during run
time and reprogram again.

In case of one MSI vector, MHI subsystem should not use IRQF_NO_SUSPEND
as WCN7850 doesn't set this flag too. Ath12k doesn't need to leave IRQ
enabled in suspend state.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Kang Yang <[email protected]>
---
drivers/net/wireless/ath/ath12k/mhi.c | 16 +++++++--
drivers/net/wireless/ath/ath12k/pci.c | 51 ++++++++++++++++++++-------
2 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mhi.c b/drivers/net/wireless/ath/ath12k/mhi.c
index 39e640293cdc..638b124eb895 100644
--- a/drivers/net/wireless/ath/ath12k/mhi.c
+++ b/drivers/net/wireless/ath/ath12k/mhi.c
@@ -251,6 +251,7 @@ static int ath12k_mhi_get_msi(struct ath12k_pci *ab_pci)
u32 user_base_data, base_vector;
int ret, num_vectors, i;
int *irq;
+ unsigned int msi_data;

ret = ath12k_pci_get_user_msi_assignment(ab,
"MHI", &num_vectors,
@@ -265,9 +266,15 @@ static int ath12k_mhi_get_msi(struct ath12k_pci *ab_pci)
if (!irq)
return -ENOMEM;

- for (i = 0; i < num_vectors; i++)
- irq[i] = ath12k_pci_get_msi_irq(ab->dev,
- base_vector + i);
+ msi_data = base_vector;
+ for (i = 0; i < num_vectors; i++) {
+ if (test_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
+ irq[i] = ath12k_pci_get_msi_irq(ab->dev,
+ msi_data++);
+ else
+ irq[i] = ath12k_pci_get_msi_irq(ab->dev,
+ msi_data);
+ }

ab_pci->mhi_ctrl->irq = irq;
ab_pci->mhi_ctrl->nr_irqs = num_vectors;
@@ -374,6 +381,9 @@ int ath12k_mhi_register(struct ath12k_pci *ab_pci)
goto free_controller;
}

+ if (!test_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
+ mhi_ctrl->irq_flags = IRQF_SHARED | IRQF_NOBALANCING;
+
mhi_ctrl->iova_start = 0;
mhi_ctrl->iova_stop = 0xffffffff;
mhi_ctrl->sbl_size = SZ_512K;
diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index 3bb2d622dc52..4484bac18f4e 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -61,6 +61,17 @@ static const struct ath12k_msi_config ath12k_msi_config[] = {
},
};

+static const struct ath12k_msi_config msi_config_one_msi = {
+ .total_vectors = 1,
+ .total_users = 4,
+ .users = (struct ath12k_msi_user[]) {
+ { .name = "MHI", .num_vectors = 3, .base_vector = 0 },
+ { .name = "CE", .num_vectors = 1, .base_vector = 0 },
+ { .name = "WAKE", .num_vectors = 1, .base_vector = 0 },
+ { .name = "DP", .num_vectors = 1, .base_vector = 0 },
+ },
+};
+
static const char *irq_name[ATH12K_IRQ_NUM_MAX] = {
"bhi",
"mhi-er0",
@@ -414,16 +425,18 @@ static void ath12k_pci_sync_ce_irqs(struct ath12k_base *ab)
static void ath12k_pci_ce_tasklet(struct tasklet_struct *t)
{
struct ath12k_ce_pipe *ce_pipe = from_tasklet(ce_pipe, t, intr_tq);
+ int irq_idx = ATH12K_PCI_IRQ_CE0_OFFSET + ce_pipe->pipe_num;

ath12k_ce_per_engine_service(ce_pipe->ab, ce_pipe->pipe_num);

- ath12k_pci_ce_irq_enable(ce_pipe->ab, ce_pipe->pipe_num);
+ enable_irq(ce_pipe->ab->irq_num[irq_idx]);
}

static irqreturn_t ath12k_pci_ce_interrupt_handler(int irq, void *arg)
{
struct ath12k_ce_pipe *ce_pipe = arg;
struct ath12k_base *ab = ce_pipe->ab;
+ int irq_idx = ATH12K_PCI_IRQ_CE0_OFFSET + ce_pipe->pipe_num;

if (!test_bit(ATH12K_FLAG_CE_IRQ_ENABLED, &ab->dev_flags))
return IRQ_HANDLED;
@@ -431,7 +444,8 @@ static irqreturn_t ath12k_pci_ce_interrupt_handler(int irq, void *arg)
/* last interrupt received for this CE */
ce_pipe->timestamp = jiffies;

- ath12k_pci_ce_irq_disable(ce_pipe->ab, ce_pipe->pipe_num);
+ disable_irq_nosync(ab->irq_num[irq_idx]);
+
tasklet_schedule(&ce_pipe->intr_tq);

return IRQ_HANDLED;
@@ -504,11 +518,13 @@ static int ath12k_pci_ext_grp_napi_poll(struct napi_struct *napi, int budget)
napi);
struct ath12k_base *ab = irq_grp->ab;
int work_done;
+ int i;

work_done = ath12k_dp_service_srng(ab, irq_grp, budget);
if (work_done < budget) {
napi_complete_done(napi, work_done);
- ath12k_pci_ext_grp_enable(irq_grp);
+ for (i = 0; i < irq_grp->num_irq; i++)
+ enable_irq(irq_grp->ab->irq_num[irq_grp->irqs[i]]);
}

if (work_done > budget)
@@ -521,6 +537,7 @@ static irqreturn_t ath12k_pci_ext_interrupt_handler(int irq, void *arg)
{
struct ath12k_ext_irq_grp *irq_grp = arg;
struct ath12k_base *ab = irq_grp->ab;
+ int i;

if (!test_bit(ATH12K_FLAG_EXT_IRQ_ENABLED, &ab->dev_flags))
return IRQ_HANDLED;
@@ -530,7 +547,8 @@ static irqreturn_t ath12k_pci_ext_interrupt_handler(int irq, void *arg)
/* last interrupt received for this group */
irq_grp->timestamp = jiffies;

- ath12k_pci_ext_grp_disable(irq_grp);
+ for (i = 0; i < irq_grp->num_irq; i++)
+ disable_irq_nosync(irq_grp->ab->irq_num[irq_grp->irqs[i]]);

napi_schedule(&irq_grp->napi);

@@ -713,18 +731,26 @@ static int ath12k_pci_msi_alloc(struct ath12k_pci *ab_pci)
msi_config->total_vectors,
msi_config->total_vectors,
PCI_IRQ_MSI);
- if (num_vectors != msi_config->total_vectors) {
- ath12k_err(ab, "failed to get %d MSI vectors, only %d available",
- msi_config->total_vectors, num_vectors);

- if (num_vectors >= 0)
- return -EINVAL;
- else
- return num_vectors;
- } else {
+ if (num_vectors == msi_config->total_vectors) {
set_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags);
ab_pci->irq_flags = IRQF_SHARED;
+ } else {
+ num_vectors = pci_alloc_irq_vectors(ab_pci->pdev,
+ 1,
+ 1,
+ PCI_IRQ_MSI);
+ if (num_vectors < 0) {
+ ret = -EINVAL;
+ goto reset_msi_config;
+ }
+ clear_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags);
+ ab_pci->msi_config = &msi_config_one_msi;
+ ab_pci->irq_flags = IRQF_SHARED | IRQF_NOBALANCING;
+ ath12k_dbg(ab, ATH12K_DBG_PCI, "request MSI one vector\n");
+
}
+ ath12k_info(ab, "MSI vectors: %d\n", num_vectors);

ath12k_pci_msi_disable(ab_pci);

@@ -746,6 +772,7 @@ static int ath12k_pci_msi_alloc(struct ath12k_pci *ab_pci)
free_msi_vector:
pci_free_irq_vectors(ab_pci->pdev);

+reset_msi_config:
return ret;
}

--
2.34.1

2023-11-20 10:17:02

by Kang Yang

[permalink] [raw]
Subject: [PATCH 2/7] wifi: ath12k: add CE and ext IRQ flag to indicate irq_handler

Add two flags to indicate whether IRQ handler for CE and DP can be called.
This is because in one MSI vector case, interrupt is not disabled in
hif_stop and hif_irq_disable. So if interrupt is disabled, MHI interrupt
is disabled too.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Kang Yang <[email protected]>
---
drivers/net/wireless/ath/ath12k/core.h | 2 ++
drivers/net/wireless/ath/ath12k/pci.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 68c42ca44fcb..63c4d84657c4 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -199,6 +199,8 @@ enum ath12k_dev_flags {
ATH12K_FLAG_REGISTERED,
ATH12K_FLAG_QMI_FAIL,
ATH12K_FLAG_HTC_SUSPEND_COMPLETE,
+ ATH12K_FLAG_CE_IRQ_ENABLED,
+ ATH12K_FLAG_EXT_IRQ_ENABLED,
};

enum ath12k_monitor_flags {
diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index c5bc9afaa9fc..7f984a80fb27 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -373,6 +373,8 @@ static void ath12k_pci_ce_irqs_disable(struct ath12k_base *ab)
{
int i;

+ clear_bit(ATH12K_FLAG_CE_IRQ_ENABLED, &ab->dev_flags);
+
for (i = 0; i < ab->hw_params->ce_count; i++) {
if (ath12k_ce_get_attr_flags(ab, i) & CE_ATTR_DIS_INTR)
continue;
@@ -406,6 +408,10 @@ static void ath12k_pci_ce_tasklet(struct tasklet_struct *t)
static irqreturn_t ath12k_pci_ce_interrupt_handler(int irq, void *arg)
{
struct ath12k_ce_pipe *ce_pipe = arg;
+ struct ath12k_base *ab = ce_pipe->ab;
+
+ if (!test_bit(ATH12K_FLAG_CE_IRQ_ENABLED, &ab->dev_flags))
+ return IRQ_HANDLED;

/* last interrupt received for this CE */
ce_pipe->timestamp = jiffies;
@@ -428,6 +434,8 @@ static void __ath12k_pci_ext_irq_disable(struct ath12k_base *ab)
{
int i;

+ clear_bit(ATH12K_FLAG_EXT_IRQ_ENABLED, &ab->dev_flags);
+
for (i = 0; i < ATH12K_EXT_IRQ_GRP_NUM_MAX; i++) {
struct ath12k_ext_irq_grp *irq_grp = &ab->ext_irq_grp[i];

@@ -483,6 +491,10 @@ static int ath12k_pci_ext_grp_napi_poll(struct napi_struct *napi, int budget)
static irqreturn_t ath12k_pci_ext_interrupt_handler(int irq, void *arg)
{
struct ath12k_ext_irq_grp *irq_grp = arg;
+ struct ath12k_base *ab = irq_grp->ab;
+
+ if (!test_bit(ATH12K_FLAG_EXT_IRQ_ENABLED, &ab->dev_flags))
+ return IRQ_HANDLED;

ath12k_dbg(irq_grp->ab, ATH12K_DBG_PCI, "ext irq:%d\n", irq);

@@ -626,6 +638,8 @@ static void ath12k_pci_ce_irqs_enable(struct ath12k_base *ab)
{
int i;

+ set_bit(ATH12K_FLAG_CE_IRQ_ENABLED, &ab->dev_flags);
+
for (i = 0; i < ab->hw_params->ce_count; i++) {
if (ath12k_ce_get_attr_flags(ab, i) & CE_ATTR_DIS_INTR)
continue;
@@ -975,6 +989,8 @@ void ath12k_pci_ext_irq_enable(struct ath12k_base *ab)
{
int i;

+ set_bit(ATH12K_FLAG_EXT_IRQ_ENABLED, &ab->dev_flags);
+
for (i = 0; i < ATH12K_EXT_IRQ_GRP_NUM_MAX; i++) {
struct ath12k_ext_irq_grp *irq_grp = &ab->ext_irq_grp[i];

--
2.34.1

2023-11-20 10:17:03

by Kang Yang

[permalink] [raw]
Subject: [PATCH 3/7] wifi: ath12k: use ATH12K_PCI_IRQ_DP_OFFSET for DP IRQ

Like ATH12K_PCI_IRQ_CE0_OFFSET, define ATH12K_PCI_IRQ_DP_OFFSET for
DP to save the IRQ instead of base_vector from MSI config.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Kang Yang <[email protected]>
---
drivers/net/wireless/ath/ath12k/pci.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index 7f984a80fb27..4b872eff087c 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -17,7 +17,8 @@
#define ATH12K_PCI_BAR_NUM 0
#define ATH12K_PCI_DMA_MASK 32

-#define ATH12K_PCI_IRQ_CE0_OFFSET 3
+#define ATH12K_PCI_IRQ_CE0_OFFSET 3
+#define ATH12K_PCI_IRQ_DP_OFFSET 14

#define WINDOW_ENABLE_BIT 0x40000000
#define WINDOW_REG_ADDRESS 0x310c
@@ -511,9 +512,8 @@ static irqreturn_t ath12k_pci_ext_interrupt_handler(int irq, void *arg)
static int ath12k_pci_ext_irq_config(struct ath12k_base *ab)
{
int i, j, ret, num_vectors = 0;
- u32 user_base_data = 0, base_vector = 0, base_idx;
+ u32 user_base_data = 0, base_vector = 0;

- base_idx = ATH12K_PCI_IRQ_CE0_OFFSET + CE_COUNT_MAX;
ret = ath12k_pci_get_user_msi_assignment(ab, "DP",
&num_vectors,
&user_base_data,
@@ -542,7 +542,7 @@ static int ath12k_pci_ext_irq_config(struct ath12k_base *ab)
}

irq_grp->num_irq = num_irq;
- irq_grp->irqs[0] = base_idx + i;
+ irq_grp->irqs[0] = ATH12K_PCI_IRQ_DP_OFFSET + i;

for (j = 0; j < irq_grp->num_irq; j++) {
int irq_idx = irq_grp->irqs[j];
--
2.34.1

2023-11-20 10:17:03

by Kang Yang

[permalink] [raw]
Subject: [PATCH 7/7] wifi: ath12k: set IRQ affinity to CPU0 in case of one MSI vector

With VT-d disabled on Intel platform, ath12k gets only one MSI
vector. In that case, ath12k does not free IRQ when doing suspend,
hence the kernel has to migrate it to CPU0 (if it was affine to
other CPUs) and allocates a new MSI vector. However, ath12k has
no chance to reconfig it to HW srngs during this phase, thus
ath12k fails to resume.

This issue can be fixed by setting IRQ affinity to CPU0 before
request_irq is called. With such affinity, migration will not
happen and thus the vector keeps unchanged during suspend/resume.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Kang Yang <[email protected]>
---
drivers/net/wireless/ath/ath12k/pci.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index 3850331438de..77074443ce00 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -617,6 +617,15 @@ static int ath12k_pci_ext_irq_config(struct ath12k_base *ab)
return 0;
}

+static int ath12k_pci_set_irq_affinity_hint(struct ath12k_pci *ab_pci,
+ const struct cpumask *m)
+{
+ if (test_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
+ return 0;
+
+ return irq_set_affinity_hint(ab_pci->pdev->irq, m);
+}
+
static int ath12k_pci_config_irq(struct ath12k_base *ab)
{
struct ath12k_pci *ab_pci = ath12k_pci_priv(ab);
@@ -1359,10 +1368,16 @@ static int ath12k_pci_probe(struct pci_dev *pdev,
if (ret)
goto err_pci_msi_free;

+ ret = ath12k_pci_set_irq_affinity_hint(ab_pci, cpumask_of(0));
+ if (ret) {
+ ath12k_err(ab, "failed to set irq affinity %d\n", ret);
+ goto err_pci_msi_free;
+ }
+
ret = ath12k_mhi_register(ab_pci);
if (ret) {
ath12k_err(ab, "failed to register mhi: %d\n", ret);
- goto err_pci_msi_free;
+ goto err_irq_affinity_cleanup;
}

ret = ath12k_hal_srng_init(ab);
@@ -1416,6 +1431,9 @@ static int ath12k_pci_probe(struct pci_dev *pdev,
err_pci_msi_free:
ath12k_pci_msi_free(ab_pci);

+err_irq_affinity_cleanup:
+ ath12k_pci_set_irq_affinity_hint(ab_pci, NULL);
+
err_pci_free_region:
ath12k_pci_free_region(ab_pci);

@@ -1430,6 +1448,8 @@ static void ath12k_pci_remove(struct pci_dev *pdev)
struct ath12k_base *ab = pci_get_drvdata(pdev);
struct ath12k_pci *ab_pci = ath12k_pci_priv(ab);

+ ath12k_pci_set_irq_affinity_hint(ab_pci, NULL);
+
if (test_bit(ATH12K_FLAG_QMI_FAIL, &ab->dev_flags)) {
ath12k_pci_power_down(ab);
ath12k_qmi_deinit_service(ab);
@@ -1456,7 +1476,9 @@ static void ath12k_pci_remove(struct pci_dev *pdev)
static void ath12k_pci_shutdown(struct pci_dev *pdev)
{
struct ath12k_base *ab = pci_get_drvdata(pdev);
+ struct ath12k_pci *ab_pci = ath12k_pci_priv(ab);

+ ath12k_pci_set_irq_affinity_hint(ab_pci, NULL);
ath12k_pci_power_down(ab);
}

--
2.34.1

2023-11-20 10:17:05

by Kang Yang

[permalink] [raw]
Subject: [PATCH 4/7] wifi: ath12k: refactor multiple MSI vector implementation

This is to prepare for one MSI vector support. IRQ enable and disable
of CE and DP are done only in case of multiple MSI vectors.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Kang Yang <[email protected]>
---
drivers/net/wireless/ath/ath12k/pci.c | 48 ++++++++++++++++++++++-----
drivers/net/wireless/ath/ath12k/pci.h | 2 ++
2 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index 4b872eff087c..3bb2d622dc52 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -356,16 +356,30 @@ static void ath12k_pci_free_irq(struct ath12k_base *ab)

static void ath12k_pci_ce_irq_enable(struct ath12k_base *ab, u16 ce_id)
{
+ struct ath12k_pci *ab_pci = ath12k_pci_priv(ab);
u32 irq_idx;

+ /* In case of one MSI vector, we handle irq enable/disable in a
+ * uniform way since we only have one irq
+ */
+ if (!test_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
+ return;
+
irq_idx = ATH12K_PCI_IRQ_CE0_OFFSET + ce_id;
enable_irq(ab->irq_num[irq_idx]);
}

static void ath12k_pci_ce_irq_disable(struct ath12k_base *ab, u16 ce_id)
{
+ struct ath12k_pci *ab_pci = ath12k_pci_priv(ab);
u32 irq_idx;

+ /* In case of one MSI vector, we handle irq enable/disable in a
+ * uniform way since we only have one irq
+ */
+ if (!test_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
+ return;
+
irq_idx = ATH12K_PCI_IRQ_CE0_OFFSET + ce_id;
disable_irq_nosync(ab->irq_num[irq_idx]);
}
@@ -425,8 +439,15 @@ static irqreturn_t ath12k_pci_ce_interrupt_handler(int irq, void *arg)

static void ath12k_pci_ext_grp_disable(struct ath12k_ext_irq_grp *irq_grp)
{
+ struct ath12k_pci *ab_pci = ath12k_pci_priv(irq_grp->ab);
int i;

+ /* In case of one MSI vector, we handle irq enable/disable
+ * in a uniform way since we only have one irq
+ */
+ if (!test_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
+ return;
+
for (i = 0; i < irq_grp->num_irq; i++)
disable_irq_nosync(irq_grp->ab->irq_num[irq_grp->irqs[i]]);
}
@@ -449,8 +470,15 @@ static void __ath12k_pci_ext_irq_disable(struct ath12k_base *ab)

static void ath12k_pci_ext_grp_enable(struct ath12k_ext_irq_grp *irq_grp)
{
+ struct ath12k_pci *ab_pci = ath12k_pci_priv(irq_grp->ab);
int i;

+ /* In case of one MSI vector, we handle irq enable/disable in a
+ * uniform way since we only have one irq
+ */
+ if (!test_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
+ return;
+
for (i = 0; i < irq_grp->num_irq; i++)
enable_irq(irq_grp->ab->irq_num[irq_grp->irqs[i]]);
}
@@ -511,6 +539,7 @@ static irqreturn_t ath12k_pci_ext_interrupt_handler(int irq, void *arg)

static int ath12k_pci_ext_irq_config(struct ath12k_base *ab)
{
+ struct ath12k_pci *ab_pci = ath12k_pci_priv(ab);
int i, j, ret, num_vectors = 0;
u32 user_base_data = 0, base_vector = 0;

@@ -556,16 +585,15 @@ static int ath12k_pci_ext_irq_config(struct ath12k_base *ab)

irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY);
ret = request_irq(irq, ath12k_pci_ext_interrupt_handler,
- IRQF_SHARED,
+ ab_pci->irq_flags,
"DP_EXT_IRQ", irq_grp);
if (ret) {
ath12k_err(ab, "failed request irq %d: %d\n",
vector, ret);
return ret;
}
-
- disable_irq_nosync(ab->irq_num[irq_idx]);
}
+ ath12k_pci_ext_grp_disable(irq_grp);
}

return 0;
@@ -573,6 +601,7 @@ static int ath12k_pci_ext_irq_config(struct ath12k_base *ab)

static int ath12k_pci_config_irq(struct ath12k_base *ab)
{
+ struct ath12k_pci *ab_pci = ath12k_pci_priv(ab);
struct ath12k_ce_pipe *ce_pipe;
u32 msi_data_start;
u32 msi_data_count, msi_data_idx;
@@ -601,7 +630,7 @@ static int ath12k_pci_config_irq(struct ath12k_base *ab)
tasklet_setup(&ce_pipe->intr_tq, ath12k_pci_ce_tasklet);

ret = request_irq(irq, ath12k_pci_ce_interrupt_handler,
- IRQF_SHARED, irq_name[irq_idx],
+ ab_pci->irq_flags, irq_name[irq_idx],
ce_pipe);
if (ret) {
ath12k_err(ab, "failed to request irq %d: %d\n",
@@ -692,6 +721,9 @@ static int ath12k_pci_msi_alloc(struct ath12k_pci *ab_pci)
return -EINVAL;
else
return num_vectors;
+ } else {
+ set_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags);
+ ab_pci->irq_flags = IRQF_SHARED;
}

ath12k_pci_msi_disable(ab_pci);
@@ -924,11 +956,11 @@ int ath12k_pci_get_user_msi_assignment(struct ath12k_base *ab, char *user_name,
for (idx = 0; idx < msi_config->total_users; idx++) {
if (strcmp(user_name, msi_config->users[idx].name) == 0) {
*num_vectors = msi_config->users[idx].num_vectors;
- *user_base_data = msi_config->users[idx].base_vector
- + ab_pci->msi_ep_base_data;
- *base_vector = msi_config->users[idx].base_vector;
+ *base_vector = msi_config->users[idx].base_vector;
+ *user_base_data = *base_vector + ab_pci->msi_ep_base_data;

- ath12k_dbg(ab, ATH12K_DBG_PCI, "Assign MSI to user: %s, num_vectors: %d, user_base_data: %u, base_vector: %u\n",
+ ath12k_dbg(ab, ATH12K_DBG_PCI,
+ "Assign MSI to user: %s, num_vectors: %d, user_base_data: %u, base_vector: %u\n",
user_name, *num_vectors, *user_base_data,
*base_vector);

diff --git a/drivers/net/wireless/ath/ath12k/pci.h b/drivers/net/wireless/ath/ath12k/pci.h
index 0f24fd9395cd..db32676fec50 100644
--- a/drivers/net/wireless/ath/ath12k/pci.h
+++ b/drivers/net/wireless/ath/ath12k/pci.h
@@ -84,6 +84,7 @@ enum ath12k_pci_flags {
ATH12K_PCI_FLAG_INIT_DONE,
ATH12K_PCI_FLAG_IS_MSI_64,
ATH12K_PCI_ASPM_RESTORE,
+ ATH12K_PCI_FLAG_MULTI_MSI_VECTORS,
};

struct ath12k_pci_ops {
@@ -108,6 +109,7 @@ struct ath12k_pci {
/* enum ath12k_pci_flags */
unsigned long flags;
u16 link_ctl;
+ unsigned long irq_flags;
const struct ath12k_pci_ops *pci_ops;
};

--
2.34.1

2023-11-20 15:52:51

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH 2/7] wifi: ath12k: add CE and ext IRQ flag to indicate irq_handler

On 11/20/2023 2:15 AM, Kang Yang wrote:
> Add two flags to indicate whether IRQ handler for CE and DP can be called.
> This is because in one MSI vector case, interrupt is not disabled in
> hif_stop and hif_irq_disable. So if interrupt is disabled, MHI interrupt
> is disabled too.
>
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Kang Yang <[email protected]>
Acked-by: Jeff Johnson <[email protected]>

2023-11-20 15:53:42

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH 3/7] wifi: ath12k: use ATH12K_PCI_IRQ_DP_OFFSET for DP IRQ

On 11/20/2023 2:15 AM, Kang Yang wrote:
> Like ATH12K_PCI_IRQ_CE0_OFFSET, define ATH12K_PCI_IRQ_DP_OFFSET for
> DP to save the IRQ instead of base_vector from MSI config.
>
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Kang Yang <[email protected]>
Acked-by: Jeff Johnson <[email protected]>

2023-11-20 15:53:43

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH 4/7] wifi: ath12k: refactor multiple MSI vector implementation

On 11/20/2023 2:15 AM, Kang Yang wrote:
> This is to prepare for one MSI vector support. IRQ enable and disable
> of CE and DP are done only in case of multiple MSI vectors.
>
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Kang Yang <[email protected]>
Acked-by: Jeff Johnson <[email protected]>

2023-11-20 15:54:08

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH 5/7] wifi: ath12k: add support one MSI vector

On 11/20/2023 2:15 AM, Kang Yang wrote:
> On some platforms it's not possible to allocate 32 MSI vectors for
> various reasons, maybe kernel configuration, VT-d disabled, buggy BIOS
> etc. So ath12k was not able to use WCN7850 PCI devices on those
> platforms. Add support for one MSI vector to solve that.
>
> In case of one MSI vector, interrupt migration needs to be disabled.
> This is because when interrupt migration happens, the msi_data may
> change. However, msi_data is already programmed to rings during initial
> phase and ath12k has no way to know that msi_data is changed during run
> time and reprogram again.
>
> In case of one MSI vector, MHI subsystem should not use IRQF_NO_SUSPEND
> as WCN7850 doesn't set this flag too. Ath12k doesn't need to leave IRQ
> enabled in suspend state.
>
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Kang Yang <[email protected]>
Acked-by: Jeff Johnson <[email protected]>

2023-11-20 15:54:50

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH 7/7] wifi: ath12k: set IRQ affinity to CPU0 in case of one MSI vector

On 11/20/2023 2:15 AM, Kang Yang wrote:
> With VT-d disabled on Intel platform, ath12k gets only one MSI
> vector. In that case, ath12k does not free IRQ when doing suspend,
> hence the kernel has to migrate it to CPU0 (if it was affine to
> other CPUs) and allocates a new MSI vector. However, ath12k has
> no chance to reconfig it to HW srngs during this phase, thus
> ath12k fails to resume.
>
> This issue can be fixed by setting IRQ affinity to CPU0 before
> request_irq is called. With such affinity, migration will not
> happen and thus the vector keeps unchanged during suspend/resume.
>
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Kang Yang <[email protected]>
Acked-by: Jeff Johnson <[email protected]>

2023-11-20 15:54:59

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH 6/7] wifi: ath12k: do not restore ASPM in case of single MSI vector

On 11/20/2023 2:15 AM, Kang Yang wrote:
> Current code enables ASPM by default, it allows MHI to enter M2 state.
> In case of one MSI vector, system hang is observed if ath12k does MHI
> register reading in this state.
>
> The workaround here is to prevent MHI from entering M2 state, this can
> be done by disabling ASPM if only one MSI vector is used. When using 32
> vectors ASPM is enabled as before.
>
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Kang Yang <[email protected]>
Acked-by: Jeff Johnson <[email protected]>

2023-11-21 00:39:31

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH 5/7] wifi: ath12k: add support one MSI vector

On 11/20/2023 2:15 AM, Kang Yang wrote:
> On some platforms it's not possible to allocate 32 MSI vectors for
> various reasons, maybe kernel configuration, VT-d disabled, buggy BIOS
> etc. So ath12k was not able to use WCN7850 PCI devices on those
> platforms. Add support for one MSI vector to solve that.
>
> In case of one MSI vector, interrupt migration needs to be disabled.
> This is because when interrupt migration happens, the msi_data may
> change. However, msi_data is already programmed to rings during initial
> phase and ath12k has no way to know that msi_data is changed during run
> time and reprogram again.
>
> In case of one MSI vector, MHI subsystem should not use IRQF_NO_SUSPEND
> as WCN7850 doesn't set this flag too. Ath12k doesn't need to leave IRQ
> enabled in suspend state.
>
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Kang Yang <[email protected]>
> ---
> drivers/net/wireless/ath/ath12k/mhi.c | 16 +++++++--
> drivers/net/wireless/ath/ath12k/pci.c | 51 ++++++++++++++++++++-------
> 2 files changed, 52 insertions(+), 15 deletions(-)

...

> diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
> index 3bb2d622dc52..4484bac18f4e 100644
> --- a/drivers/net/wireless/ath/ath12k/pci.c
> +++ b/drivers/net/wireless/ath/ath12k/pci.c

...

> @@ -713,18 +731,26 @@ static int ath12k_pci_msi_alloc(struct ath12k_pci *ab_pci)
> msi_config->total_vectors,
> msi_config->total_vectors,
> PCI_IRQ_MSI);
> - if (num_vectors != msi_config->total_vectors) {
> - ath12k_err(ab, "failed to get %d MSI vectors, only %d available",
> - msi_config->total_vectors, num_vectors);
>
> - if (num_vectors >= 0)
> - return -EINVAL;
> - else
> - return num_vectors;
> - } else {
> + if (num_vectors == msi_config->total_vectors) {
> set_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags);
> ab_pci->irq_flags = IRQF_SHARED;
> + } else {
> + num_vectors = pci_alloc_irq_vectors(ab_pci->pdev,
> + 1,
> + 1,
> + PCI_IRQ_MSI);
> + if (num_vectors < 0) {
> + ret = -EINVAL;
> + goto reset_msi_config;
> + }
> + clear_bit(ATH12K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags);
> + ab_pci->msi_config = &msi_config_one_msi;
> + ab_pci->irq_flags = IRQF_SHARED | IRQF_NOBALANCING;
> + ath12k_dbg(ab, ATH12K_DBG_PCI, "request MSI one vector\n");
> +

ath12k-check is flagging this:
drivers/net/wireless/ath/ath12k/pci.c:761: Blank lines aren't necessary
before a close brace '}'
<
> }
> + ath12k_info(ab, "MSI vectors: %d\n", num_vectors);
>
> ath12k_pci_msi_disable(ab_pci);
>