From 240fdb4ce67d406ae10ca8c95b39aab3ef53c04c Mon Sep 17 00:00:00 2001 From: mglocker Date: Thu, 9 May 2024 08:06:42 +0000 Subject: [PATCH] Don't schedule interrupt aggregation when commands are still in-progress. As of the documentation: "NOTE Write operations to IACTH and IATOVAL are only allowed when no commands are outstanding." Instead we only schedule interrupt aggregation at the start of the SCSI command call, when all commands have completed. --- sys/dev/ic/ufshci.c | 112 ++++------------------------------------- sys/dev/ic/ufshcireg.h | 3 +- sys/dev/ic/ufshcivar.h | 3 +- 3 files changed, 13 insertions(+), 105 deletions(-) diff --git a/sys/dev/ic/ufshci.c b/sys/dev/ic/ufshci.c index 5c38cd4c85d..0045463b7e6 100644 --- a/sys/dev/ic/ufshci.c +++ b/sys/dev/ic/ufshci.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ufshci.c,v 1.11 2024/05/09 08:04:48 mglocker Exp $ */ +/* $OpenBSD: ufshci.c,v 1.12 2024/05/09 08:06:42 mglocker Exp $ */ /* * Copyright (c) 2022 Marcus Glocker @@ -130,6 +130,7 @@ ufshci_intr(void *arg) /* Reset Interrupt Aggregation Counter and Timer. */ UFSHCI_WRITE_4(sc, UFSHCI_REG_UTRIACR, UFSHCI_REG_UTRIACR_IAEN | UFSHCI_REG_UTRIACR_CTR); + sc->sc_intraggr_enabled = 0; ufshci_xfer_complete(sc); @@ -195,7 +196,12 @@ ufshci_attach(struct ufshci_softc *sc) printf("%s: NUTRS can't be >32 (is %d)!\n", sc->sc_dev.dv_xname, sc->sc_nutrs); return 1; + } else if (sc->sc_nutrs == 1) { + sc->sc_iacth = sc->sc_nutrs; + } else if (sc->sc_nutrs > 1) { + sc->sc_iacth = sc->sc_nutrs - 1; } + DPRINTF("IACTH=%d\n", sc->sc_iacth); ufshci_init(sc); @@ -373,24 +379,6 @@ ufshci_init(struct ufshci_softc *sc) /* 7.1.1 Host Controller Initialization: 11) */ reg = UFSHCI_READ_4(sc, UFSHCI_REG_UTRIACR); DPRINTF("%s: UTRIACR=0x%08x\n", __func__, reg); - /* - * Only enable interrupt aggregation when interrupts are available. - * Otherwise, the interrupt aggregation counter already starts to - * count completed commands, and will keep interrupts disabled once - * reaching the threshold. We only issue the interrupt aggregation - * counter reset in the interrupt handler during runtime, so we would - * have a kind of chicken/egg problem. - */ - if (!cold) { - DPRINTF("%s: Enable interrupt aggregation\n", __func__); - UFSHCI_WRITE_4(sc, UFSHCI_REG_UTRIACR, - UFSHCI_REG_UTRIACR_IAEN | - UFSHCI_REG_UTRIACR_IAPWEN | - UFSHCI_REG_UTRIACR_CTR | - UFSHCI_REG_UTRIACR_IACTH(UFSHCI_INTR_AGGR_COUNT) | - UFSHCI_REG_UTRIACR_IATOVAL(UFSHCI_INTR_AGGR_TIMEOUT)); - sc->sc_intraggr_enabled = 1; - } /* * 7.1.1 Host Controller Initialization: 12) @@ -577,16 +565,6 @@ ufshci_utr_cmd_nop(struct ufshci_softc *sc, struct ufshci_ccb *ccb, return -1; } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 10) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 11) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 12) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 13) */ - UFSHCI_WRITE_4(sc, UFSHCI_REG_UTRIACR, - UFSHCI_REG_UTRIACR_IAEN | - UFSHCI_REG_UTRIACR_IAPWEN | - UFSHCI_REG_UTRIACR_IACTH(UFSHCI_INTR_AGGR_COUNT) | - UFSHCI_REG_UTRIACR_IATOVAL(UFSHCI_INTR_AGGR_TIMEOUT)); - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 14) */ ccb->ccb_status = CCB_STATUS_INPROGRESS; ufshci_doorbell_write(sc, slot); @@ -687,16 +665,6 @@ ufshci_utr_cmd_lun(struct ufshci_softc *sc, struct ufshci_ccb *ccb, return -1; } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 10) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 11) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 12) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 13) */ - UFSHCI_WRITE_4(sc, UFSHCI_REG_UTRIACR, - UFSHCI_REG_UTRIACR_IAEN | - UFSHCI_REG_UTRIACR_IAPWEN | - UFSHCI_REG_UTRIACR_IACTH(UFSHCI_INTR_AGGR_COUNT) | - UFSHCI_REG_UTRIACR_IATOVAL(UFSHCI_INTR_AGGR_TIMEOUT)); - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 14) */ ccb->ccb_status = CCB_STATUS_INPROGRESS; ufshci_doorbell_write(sc, slot); @@ -795,18 +763,6 @@ ufshci_utr_cmd_inquiry(struct ufshci_softc *sc, struct ufshci_ccb *ccb, return -1; } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 10) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 11) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 12) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 13) */ - if (!ISSET(xs->flags, SCSI_POLL)) { - UFSHCI_WRITE_4(sc, UFSHCI_REG_UTRIACR, - UFSHCI_REG_UTRIACR_IAEN | - UFSHCI_REG_UTRIACR_IAPWEN | - UFSHCI_REG_UTRIACR_IACTH(UFSHCI_INTR_AGGR_COUNT) | - UFSHCI_REG_UTRIACR_IATOVAL(UFSHCI_INTR_AGGR_TIMEOUT)); - } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 14) */ ccb->ccb_status = CCB_STATUS_INPROGRESS; ufshci_doorbell_write(sc, slot); @@ -909,18 +865,6 @@ ufshci_utr_cmd_capacity16(struct ufshci_softc *sc, struct ufshci_ccb *ccb, return -1; } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 10) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 11) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 12) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 13) */ - if (!ISSET(xs->flags, SCSI_POLL)) { - UFSHCI_WRITE_4(sc, UFSHCI_REG_UTRIACR, - UFSHCI_REG_UTRIACR_IAEN | - UFSHCI_REG_UTRIACR_IAPWEN | - UFSHCI_REG_UTRIACR_IACTH(UFSHCI_INTR_AGGR_COUNT) | - UFSHCI_REG_UTRIACR_IATOVAL(UFSHCI_INTR_AGGR_TIMEOUT)); - } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 14) */ ccb->ccb_status = CCB_STATUS_INPROGRESS; ufshci_doorbell_write(sc, slot); @@ -1022,18 +966,6 @@ ufshci_utr_cmd_capacity(struct ufshci_softc *sc, struct ufshci_ccb *ccb, return -1; } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 10) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 11) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 12) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 13) */ - if (!ISSET(xs->flags, SCSI_POLL)) { - UFSHCI_WRITE_4(sc, UFSHCI_REG_UTRIACR, - UFSHCI_REG_UTRIACR_IAEN | - UFSHCI_REG_UTRIACR_IAPWEN | - UFSHCI_REG_UTRIACR_IACTH(UFSHCI_INTR_AGGR_COUNT) | - UFSHCI_REG_UTRIACR_IATOVAL(UFSHCI_INTR_AGGR_TIMEOUT)); - } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 14) */ ccb->ccb_status = CCB_STATUS_INPROGRESS; ufshci_doorbell_write(sc, slot); @@ -1138,18 +1070,6 @@ ufshci_utr_cmd_io(struct ufshci_softc *sc, struct ufshci_ccb *ccb, return -1; } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 10) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 11) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 12) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 13) */ - if (!ISSET(xs->flags, SCSI_POLL)) { - UFSHCI_WRITE_4(sc, UFSHCI_REG_UTRIACR, - UFSHCI_REG_UTRIACR_IAEN | - UFSHCI_REG_UTRIACR_IAPWEN | - UFSHCI_REG_UTRIACR_IACTH(UFSHCI_INTR_AGGR_COUNT) | - UFSHCI_REG_UTRIACR_IATOVAL(UFSHCI_INTR_AGGR_TIMEOUT)); - } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 14) */ ccb->ccb_status = CCB_STATUS_INPROGRESS; ufshci_doorbell_write(sc, slot); @@ -1242,18 +1162,6 @@ ufshci_utr_cmd_sync(struct ufshci_softc *sc, struct ufshci_ccb *ccb, return -1; } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 10) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 11) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 12) */ - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 13) */ - if (!ISSET(xs->flags, SCSI_POLL)) { - UFSHCI_WRITE_4(sc, UFSHCI_REG_UTRIACR, - UFSHCI_REG_UTRIACR_IAEN | - UFSHCI_REG_UTRIACR_IAPWEN | - UFSHCI_REG_UTRIACR_IACTH(UFSHCI_INTR_AGGR_COUNT) | - UFSHCI_REG_UTRIACR_IATOVAL(UFSHCI_INTR_AGGR_TIMEOUT)); - } - /* 7.2.1 Basic Steps when Building a UTP Transfer Request: 14) */ ccb->ccb_status = CCB_STATUS_INPROGRESS; ufshci_doorbell_write(sc, slot); @@ -1408,13 +1316,13 @@ ufshci_scsi_cmd(struct scsi_xfer *xs) DPRINTF("%s: cmd=0x%x\n", __func__, xs->cmd.opcode); - if (!cold && !sc->sc_intraggr_enabled) { - DPRINTF("%s: Enable interrupt aggregation\n", __func__); + /* Schedule interrupt aggregation. */ + if (ISSET(xs->flags, SCSI_POLL) == 0 && sc->sc_intraggr_enabled == 0) { UFSHCI_WRITE_4(sc, UFSHCI_REG_UTRIACR, UFSHCI_REG_UTRIACR_IAEN | UFSHCI_REG_UTRIACR_IAPWEN | UFSHCI_REG_UTRIACR_CTR | - UFSHCI_REG_UTRIACR_IACTH(UFSHCI_INTR_AGGR_COUNT) | + UFSHCI_REG_UTRIACR_IACTH(sc->sc_iacth) | UFSHCI_REG_UTRIACR_IATOVAL(UFSHCI_INTR_AGGR_TIMEOUT)); sc->sc_intraggr_enabled = 1; } diff --git a/sys/dev/ic/ufshcireg.h b/sys/dev/ic/ufshcireg.h index d251c5f35f1..37041472fbb 100644 --- a/sys/dev/ic/ufshcireg.h +++ b/sys/dev/ic/ufshcireg.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ufshcireg.h,v 1.5 2024/04/19 20:43:33 mglocker Exp $ */ +/* $OpenBSD: ufshcireg.h,v 1.6 2024/05/09 08:06:42 mglocker Exp $ */ /* * Copyright (c) 2022 Marcus Glocker @@ -21,7 +21,6 @@ */ #define UFSHCI_UCD_PRDT_MAX_SEGS 64 #define UFSHCI_UCD_PRDT_MAX_XFER (UFSHCI_UCD_PRDT_MAX_SEGS * PAGE_SIZE) -#define UFSHCI_INTR_AGGR_COUNT 1 /* Max. allowed value = 31 */ #define UFSHCI_INTR_AGGR_TIMEOUT 0x64 /* 4ms */ #define UFSHCI_MAX_UNITS 32 diff --git a/sys/dev/ic/ufshcivar.h b/sys/dev/ic/ufshcivar.h index 689c256f898..65b9d6fe007 100644 --- a/sys/dev/ic/ufshcivar.h +++ b/sys/dev/ic/ufshcivar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ufshcivar.h,v 1.3 2024/05/09 08:04:48 mglocker Exp $ */ +/* $OpenBSD: ufshcivar.h,v 1.4 2024/05/09 08:06:42 mglocker Exp $ */ /* * Copyright (c) 2022 Marcus Glocker @@ -58,6 +58,7 @@ struct ufshci_softc { bus_dma_tag_t sc_dmat; uint8_t sc_intraggr_enabled; + uint8_t sc_iacth; struct mutex sc_cmd_mtx; uint32_t sc_ver; -- 2.20.1