[PATCH 3/3] habanalabs: change polling functions to macros

From: Oded Gabbay
Date: Thu May 09 2019 - 15:03:04 EST

Next message: Oded Gabbay: "[PATCH 2/3] habanalabs: remove redundant memory clear"
Previous message: Oded Gabbay: "[PATCH 1/3] habanalabs: remove redundant CB size adjustment"
In reply to: Oded Gabbay: "[PATCH 1/3] habanalabs: remove redundant CB size adjustment"
Next in thread: Oded Gabbay: "[PATCH 2/3] habanalabs: remove redundant memory clear"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

This patch changes two polling functions to macros, in order to make their
API the same as the standard readl_poll_timeout so we would be able to
define the "condition for exit" when calling these macros.

This will simplify the code as it will eliminate the need to check both
for timeout and for the (cond) in the calling function.

Signed-off-by: Oded Gabbay <oded.gabbay@xxxxxxxxx>
---
drivers/misc/habanalabs/command_submission.c | 10 +--
drivers/misc/habanalabs/device.c | 89 --------------------
drivers/misc/habanalabs/firmware_if.c | 29 +++----
drivers/misc/habanalabs/goya/goya.c | 25 +++---
drivers/misc/habanalabs/habanalabs.h | 57 ++++++++++++-
5 files changed, 81 insertions(+), 129 deletions(-)

diff --git a/drivers/misc/habanalabs/command_submission.c b/drivers/misc/habanalabs/command_submission.c
index 6fe785e26859..6ad83d5ef4b0 100644
--- a/drivers/misc/habanalabs/command_submission.c
+++ b/drivers/misc/habanalabs/command_submission.c
@@ -682,14 +682,12 @@ int hl_cs_ioctl(struct hl_fpriv *hpriv, void *data)
u32 tmp;

rc = hl_poll_timeout_memory(hdev,
- (u64) (uintptr_t) &ctx->thread_ctx_switch_wait_token,
- jiffies_to_usecs(hdev->timeout_jiffies),
- &tmp);
+ &ctx->thread_ctx_switch_wait_token, tmp, (tmp == 1),
+ 100, jiffies_to_usecs(hdev->timeout_jiffies));

- if (rc || !tmp) {
+ if (rc == -ETIMEDOUT) {
dev_err(hdev->dev,
- "context switch phase didn't finish in time\n");
- rc = -ETIMEDOUT;
+ "context switch phase timeout (%d)\n", tmp);
goto out;
}
}
diff --git a/drivers/misc/habanalabs/device.c b/drivers/misc/habanalabs/device.c
index 8289cb7ea759..feedf4810430 100644
--- a/drivers/misc/habanalabs/device.c
+++ b/drivers/misc/habanalabs/device.c
@@ -1171,95 +1171,6 @@ void hl_device_fini(struct hl_device *hdev)
pr_info("removed device successfully\n");
}

-/*
- * hl_poll_timeout_memory - Periodically poll a host memory address
- * until it is not zero or a timeout occurs
- * @hdev: pointer to habanalabs device structure
- * @addr: Address to poll
- * @timeout_us: timeout in us
- * @val: Variable to read the value into
- *
- * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
- * case, the last read value at @addr is stored in @val. Must not
- * be called from atomic context if sleep_us or timeout_us are used.
- *
- * The function sleeps for 100us with timeout value of
- * timeout_us
- */
-int hl_poll_timeout_memory(struct hl_device *hdev, u64 addr,
- u32 timeout_us, u32 *val)
-{
- /*
- * address in this function points always to a memory location in the
- * host's (server's) memory. That location is updated asynchronously
- * either by the direct access of the device or by another core
- */
- u32 *paddr = (u32 *) (uintptr_t) addr;
- ktime_t timeout;
-
- /* timeout should be longer when working with simulator */
- if (!hdev->pdev)
- timeout_us *= 10;
-
- timeout = ktime_add_us(ktime_get(), timeout_us);
-
- might_sleep();
-
- for (;;) {
- /*
- * Flush CPU read/write buffers to make sure we read updates
- * done by other cores or by the device
- */
- mb();
- *val = *paddr;
- if (*val)
- break;
- if (ktime_compare(ktime_get(), timeout) > 0) {
- *val = *paddr;
- break;
- }
- usleep_range((100 >> 2) + 1, 100);
- }
-
- return *val ? 0 : -ETIMEDOUT;
-}
-
-/*
- * hl_poll_timeout_devicememory - Periodically poll a device memory address
- * until it is not zero or a timeout occurs
- * @hdev: pointer to habanalabs device structure
- * @addr: Device address to poll
- * @timeout_us: timeout in us
- * @val: Variable to read the value into
- *
- * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
- * case, the last read value at @addr is stored in @val. Must not
- * be called from atomic context if sleep_us or timeout_us are used.
- *
- * The function sleeps for 100us with timeout value of
- * timeout_us
- */
-int hl_poll_timeout_device_memory(struct hl_device *hdev, void __iomem *addr,
- u32 timeout_us, u32 *val)
-{
- ktime_t timeout = ktime_add_us(ktime_get(), timeout_us);
-
- might_sleep();
-
- for (;;) {
- *val = readl(addr);
- if (*val)
- break;
- if (ktime_compare(ktime_get(), timeout) > 0) {
- *val = readl(addr);
- break;
- }
- usleep_range((100 >> 2) + 1, 100);
- }
-
- return *val ? 0 : -ETIMEDOUT;
-}
-
/*
* MMIO register access helper functions.
*/
diff --git a/drivers/misc/habanalabs/firmware_if.c b/drivers/misc/habanalabs/firmware_if.c
index de445a1d9f3d..0cbdfa0d7fba 100644
--- a/drivers/misc/habanalabs/firmware_if.c
+++ b/drivers/misc/habanalabs/firmware_if.c
@@ -117,33 +117,28 @@ int hl_fw_send_cpu_message(struct hl_device *hdev, u32 hw_queue_id, u32 *msg,
goto out;
}

- rc = hl_poll_timeout_memory(hdev, (u64) (uintptr_t) &pkt->fence,
- timeout, &tmp);
+ rc = hl_poll_timeout_memory(hdev, &pkt->fence, tmp,
+ (tmp == ARMCP_PACKET_FENCE_VAL), 1000, timeout);

hl_hw_queue_inc_ci_kernel(hdev, hw_queue_id);

if (rc == -ETIMEDOUT) {
- dev_err(hdev->dev, "Timeout while waiting for device CPU\n");
+ dev_err(hdev->dev, "Device CPU packet timeout (0x%x)\n", tmp);
hdev->device_cpu_disabled = true;
goto out;
}

- if (tmp == ARMCP_PACKET_FENCE_VAL) {
- u32 ctl = le32_to_cpu(pkt->ctl);
+ tmp = le32_to_cpu(pkt->ctl);

- rc = (ctl & ARMCP_PKT_CTL_RC_MASK) >> ARMCP_PKT_CTL_RC_SHIFT;
- if (rc) {
- dev_err(hdev->dev,
- "F/W ERROR %d for CPU packet %d\n",
- rc, (ctl & ARMCP_PKT_CTL_OPCODE_MASK)
+ rc = (tmp & ARMCP_PKT_CTL_RC_MASK) >> ARMCP_PKT_CTL_RC_SHIFT;
+ if (rc) {
+ dev_err(hdev->dev, "F/W ERROR %d for CPU packet %d\n",
+ rc,
+ (tmp & ARMCP_PKT_CTL_OPCODE_MASK)
>> ARMCP_PKT_CTL_OPCODE_SHIFT);
- rc = -EINVAL;
- } else if (result) {
- *result = (long) le64_to_cpu(pkt->result);
- }
- } else {
- dev_err(hdev->dev, "CPU packet wrong fence value\n");
- rc = -EINVAL;
+ rc = -EIO;
+ } else if (result) {
+ *result = (long) le64_to_cpu(pkt->result);
}

out:
diff --git a/drivers/misc/habanalabs/goya/goya.c b/drivers/misc/habanalabs/goya/goya.c
index a2459cb106dd..ffc7997d4898 100644
--- a/drivers/misc/habanalabs/goya/goya.c
+++ b/drivers/misc/habanalabs/goya/goya.c
@@ -2844,14 +2844,14 @@ static int goya_send_job_on_qman0(struct hl_device *hdev, struct hl_cs_job *job)
goto free_fence_ptr;
}

- rc = hl_poll_timeout_memory(hdev, (u64) (uintptr_t) fence_ptr, timeout,
- &tmp);
+ rc = hl_poll_timeout_memory(hdev, fence_ptr, tmp,
+ (tmp == GOYA_QMAN0_FENCE_VAL), 1000, timeout);

hl_hw_queue_inc_ci_kernel(hdev, GOYA_QUEUE_ID_DMA_0);

- if ((rc) || (tmp != GOYA_QMAN0_FENCE_VAL)) {
- dev_err(hdev->dev, "QMAN0 Job hasn't finished in time\n");
- rc = -ETIMEDOUT;
+ if (rc == -ETIMEDOUT) {
+ dev_err(hdev->dev, "QMAN0 Job timeout (0x%x)\n", tmp);
+ goto free_fence_ptr;
}

free_fence_ptr:
@@ -2925,20 +2925,19 @@ int goya_test_queue(struct hl_device *hdev, u32 hw_queue_id)
goto free_pkt;
}

- rc = hl_poll_timeout_memory(hdev, (u64) (uintptr_t) fence_ptr,
- GOYA_TEST_QUEUE_WAIT_USEC, &tmp);
+ rc = hl_poll_timeout_memory(hdev, fence_ptr, tmp, (tmp == fence_val),
+ 1000, GOYA_TEST_QUEUE_WAIT_USEC);

hl_hw_queue_inc_ci_kernel(hdev, hw_queue_id);

- if ((!rc) && (tmp == fence_val)) {
- dev_info(hdev->dev,
- "queue test on H/W queue %d succeeded\n",
- hw_queue_id);
- } else {
+ if (rc == -ETIMEDOUT) {
dev_err(hdev->dev,
"H/W queue %d test failed (scratch(0x%08llX) == 0x%08X)\n",
hw_queue_id, (unsigned long long) fence_dma_addr, tmp);
- rc = -EINVAL;
+ rc = -EIO;
+ } else {
+ dev_info(hdev->dev, "queue test on H/W queue %d succeeded\n",
+ hw_queue_id);
}

free_pkt:
diff --git a/drivers/misc/habanalabs/habanalabs.h b/drivers/misc/habanalabs/habanalabs.h
index f09029339d5e..00b3339f4828 100644
--- a/drivers/misc/habanalabs/habanalabs.h
+++ b/drivers/misc/habanalabs/habanalabs.h
@@ -1065,6 +1065,59 @@ void hl_wreg(struct hl_device *hdev, u32 reg, u32 val);
(cond) ? 0 : -ETIMEDOUT; \
})

+/*
+ * address in this macro points always to a memory location in the
+ * host's (server's) memory. That location is updated asynchronously
+ * either by the direct access of the device or by another core
+ */
+#define hl_poll_timeout_memory(hdev, addr, val, cond, sleep_us, timeout_us) \
+({ \
+ ktime_t __timeout; \
+ /* timeout should be longer when working with simulator */ \
+ if (hdev->pdev) \
+ __timeout = ktime_add_us(ktime_get(), timeout_us); \
+ else \
+ __timeout = ktime_add_us(ktime_get(), (timeout_us * 10)); \
+ might_sleep_if(sleep_us); \
+ for (;;) { \
+ /* Verify we read updates done by other cores or by device */ \
+ mb(); \
+ (val) = *((u32 *) (uintptr_t) (addr)); \
+ if (cond) \
+ break; \
+ if (timeout_us && ktime_compare(ktime_get(), __timeout) > 0) { \
+ (val) = *((u32 *) (uintptr_t) (addr)); \
+ break; \
+ } \
+ if (sleep_us) \
+ usleep_range((sleep_us >> 2) + 1, sleep_us); \
+ } \
+ (cond) ? 0 : -ETIMEDOUT; \
+})
+
+#define hl_poll_timeout_device_memory(hdev, addr, val, cond, sleep_us, \
+ timeout_us) \
+({ \
+ ktime_t __timeout; \
+ /* timeout should be longer when working with simulator */ \
+ if (hdev->pdev) \
+ __timeout = ktime_add_us(ktime_get(), timeout_us); \
+ else \
+ __timeout = ktime_add_us(ktime_get(), (timeout_us * 10)); \
+ might_sleep_if(sleep_us); \
+ for (;;) { \
+ (val) = readl(addr); \
+ if (cond) \
+ break; \
+ if (timeout_us && ktime_compare(ktime_get(), __timeout) > 0) { \
+ (val) = readl(addr); \
+ break; \
+ } \
+ if (sleep_us) \
+ usleep_range((sleep_us >> 2) + 1, sleep_us); \
+ } \
+ (cond) ? 0 : -ETIMEDOUT; \
+})

#define HL_ENG_BUSY(buf, size, fmt, ...) ({ \
if (buf) \
@@ -1334,10 +1387,6 @@ int hl_device_set_debug_mode(struct hl_device *hdev, bool enable);
int create_hdev(struct hl_device **dev, struct pci_dev *pdev,
enum hl_asic_type asic_type, int minor);
void destroy_hdev(struct hl_device *hdev);
-int hl_poll_timeout_memory(struct hl_device *hdev, u64 addr, u32 timeout_us,
- u32 *val);
-int hl_poll_timeout_device_memory(struct hl_device *hdev, void __iomem *addr,
- u32 timeout_us, u32 *val);
int hl_hw_queues_create(struct hl_device *hdev);
void hl_hw_queues_destroy(struct hl_device *hdev);
int hl_hw_queue_send_cb_no_cmpl(struct hl_device *hdev, u32 hw_queue_id,
--
2.17.1

Next message: Oded Gabbay: "[PATCH 2/3] habanalabs: remove redundant memory clear"
Previous message: Oded Gabbay: "[PATCH 1/3] habanalabs: remove redundant CB size adjustment"
In reply to: Oded Gabbay: "[PATCH 1/3] habanalabs: remove redundant CB size adjustment"
Next in thread: Oded Gabbay: "[PATCH 2/3] habanalabs: remove redundant memory clear"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]