* [PATCH v3/RESEND 0/3] Even moar rpmh cleanups
@ 2020-05-21 6:04 Stephen Boyd
2020-05-21 6:04 ` [PATCHv3/RESEND 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Stephen Boyd @ 2020-05-21 6:04 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson
Cc: linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson
(Resent with more Ccs and To lines)
We remove the tcs_is_free() API and then do super micro optimizations on
the irq handler. I haven't tested anything here so most likely there's a
bug (again again)!
Changes from v2:
* Went back in time and used the v1 patch for the first patch with
the fixes to make it not so complicated
Changes from v1:
* First patch became even moar complicated because it combines
find_free_tcs() with the check for a request in flight
* Fixed subject in patch 2
* Put back unsigned long for bitmap operation to silence compiler
warning
* Picked up review tags
Stephen Boyd (3):
soc: qcom: rpmh-rsc: Remove tcs_is_free() API
soc: qcom: rpmh-rsc: Loop over fewer bits in irq handler
soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition
drivers/soc/qcom/rpmh-rsc.c | 65 +++++++++++++------------------------
1 file changed, 22 insertions(+), 43 deletions(-)
Cc: Maulik Shah <mkshah@codeaurora.org>
Cc: Douglas Anderson <dianders@chromium.org>
base-commit: 1f7a3eb785e4a4e196729cd3d5ec97bd5f9f2940
--
Sent by a computer, using git, on the internet
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCHv3/RESEND 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API
2020-05-21 6:04 [PATCH v3/RESEND 0/3] Even moar rpmh cleanups Stephen Boyd
@ 2020-05-21 6:04 ` Stephen Boyd
2020-05-26 7:07 ` Maulik Shah
2020-05-28 18:04 ` Doug Anderson
2020-05-21 6:04 ` [PATCHv3/RESEND 2/3] soc: qcom: rpmh-rsc: Loop over fewer bits in irq handler Stephen Boyd
` (2 subsequent siblings)
3 siblings, 2 replies; 7+ messages in thread
From: Stephen Boyd @ 2020-05-21 6:04 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson
Cc: linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson
This API does very little. Let's replace all the callsites with the
normal operations that would be done on top of the bitmap that
tcs_in_use is. This simplifies and reduces the code size.
Cc: Maulik Shah <mkshah@codeaurora.org>
Cc: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/soc/qcom/rpmh-rsc.c | 59 +++++++++++++------------------------
1 file changed, 20 insertions(+), 39 deletions(-)
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 076fd27f3081..60fc56987659 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -184,22 +184,6 @@ static void write_tcs_reg_sync(const struct rsc_drv *drv, int reg, int tcs_id,
data, tcs_id, reg);
}
-/**
- * tcs_is_free() - Return if a TCS is totally free.
- * @drv: The RSC controller.
- * @tcs_id: The global ID of this TCS.
- *
- * Returns true if nobody has claimed this TCS (by setting tcs_in_use).
- *
- * Context: Must be called with the drv->lock held.
- *
- * Return: true if the given TCS is free.
- */
-static bool tcs_is_free(struct rsc_drv *drv, int tcs_id)
-{
- return !test_bit(tcs_id, drv->tcs_in_use);
-}
-
/**
* tcs_invalidate() - Invalidate all TCSes of the given type (sleep or wake).
* @drv: The RSC controller.
@@ -512,7 +496,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
*
* Return: 0 if nothing in flight or -EBUSY if we should try again later.
* The caller must re-enable interrupts between tries since that's
- * the only way tcs_is_free() will ever return true and the only way
+ * the only way tcs_in_use will ever be updated and the only way
* RSC_DRV_CMD_ENABLE will ever be cleared.
*/
static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
@@ -520,17 +504,14 @@ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
{
unsigned long curr_enabled;
u32 addr;
- int i, j, k;
- int tcs_id = tcs->offset;
+ int j, k;
+ int i = tcs->offset;
- for (i = 0; i < tcs->num_tcs; i++, tcs_id++) {
- if (tcs_is_free(drv, tcs_id))
- continue;
-
- curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id);
+ for_each_set_bit_from(i, drv->tcs_in_use, tcs->offset + tcs->num_tcs) {
+ curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, i);
for_each_set_bit(j, &curr_enabled, MAX_CMDS_PER_TCS) {
- addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, tcs_id, j);
+ addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, i, j);
for (k = 0; k < msg->num_cmds; k++) {
if (addr == msg->cmds[k].addr)
return -EBUSY;
@@ -548,18 +529,19 @@ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
*
* Must be called with the drv->lock held since that protects tcs_in_use.
*
- * Return: The first tcs that's free.
+ * Return: The first tcs that's free or -EBUSY if all in use.
*/
static int find_free_tcs(struct tcs_group *tcs)
{
- int i;
+ const struct rsc_drv *drv = tcs->drv;
+ unsigned long i;
+ unsigned long max = tcs->offset + tcs->num_tcs;
- for (i = 0; i < tcs->num_tcs; i++) {
- if (tcs_is_free(tcs->drv, tcs->offset + i))
- return tcs->offset + i;
- }
+ i = find_next_zero_bit(drv->tcs_in_use, max, tcs->offset);
+ if (i >= max)
+ return -EBUSY;
- return -EBUSY;
+ return i;
}
/**
@@ -757,8 +739,9 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg)
*/
static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv)
{
- int m;
- struct tcs_group *tcs = &drv->tcs[ACTIVE_TCS];
+ unsigned long set;
+ const struct tcs_group *tcs = &drv->tcs[ACTIVE_TCS];
+ unsigned long max;
/*
* If we made an active request on a RSC that does not have a
@@ -769,12 +752,10 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv)
if (!tcs->num_tcs)
tcs = &drv->tcs[WAKE_TCS];
- for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
- if (!tcs_is_free(drv, m))
- return true;
- }
+ max = tcs->offset + tcs->num_tcs;
+ set = find_next_bit(drv->tcs_in_use, max, tcs->offset);
- return false;
+ return set < max;
}
/**
--
Sent by a computer, using git, on the internet
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCHv3/RESEND 2/3] soc: qcom: rpmh-rsc: Loop over fewer bits in irq handler
2020-05-21 6:04 [PATCH v3/RESEND 0/3] Even moar rpmh cleanups Stephen Boyd
2020-05-21 6:04 ` [PATCHv3/RESEND 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd
@ 2020-05-21 6:04 ` Stephen Boyd
2020-05-21 6:04 ` [PATCHv3/RESEND 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition Stephen Boyd
2020-05-21 6:11 ` [PATCH v3/RESEND 0/3] Even moar rpmh cleanups Randy Dunlap
3 siblings, 0 replies; 7+ messages in thread
From: Stephen Boyd @ 2020-05-21 6:04 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson
Cc: linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson
readl() returns a u32, and BITS_PER_LONG is different on 32-bit vs.
64-bit architectures. Let's loop over the possible bits set in that type
instead of looping over more bits than we ever may need to.
Cc: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/soc/qcom/rpmh-rsc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 60fc56987659..ce725d4ff097 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -383,7 +383,7 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
irq_status = readl_relaxed(drv->tcs_base + RSC_DRV_IRQ_STATUS);
- for_each_set_bit(i, &irq_status, BITS_PER_LONG) {
+ for_each_set_bit(i, &irq_status, BITS_PER_TYPE(u32)) {
req = get_req_from_tcs(drv, i);
if (!req) {
WARN_ON(1);
--
Sent by a computer, using git, on the internet
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCHv3/RESEND 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition
2020-05-21 6:04 [PATCH v3/RESEND 0/3] Even moar rpmh cleanups Stephen Boyd
2020-05-21 6:04 ` [PATCHv3/RESEND 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd
2020-05-21 6:04 ` [PATCHv3/RESEND 2/3] soc: qcom: rpmh-rsc: Loop over fewer bits in irq handler Stephen Boyd
@ 2020-05-21 6:04 ` Stephen Boyd
2020-05-21 6:11 ` [PATCH v3/RESEND 0/3] Even moar rpmh cleanups Randy Dunlap
3 siblings, 0 replies; 7+ messages in thread
From: Stephen Boyd @ 2020-05-21 6:04 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson
Cc: linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson
Move the WARN_ON() into the if condition so the compiler can see that
the branch is unlikely() and possibly optimize it better.
Cc: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/soc/qcom/rpmh-rsc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index ce725d4ff097..8381bd012de4 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -385,10 +385,8 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
for_each_set_bit(i, &irq_status, BITS_PER_TYPE(u32)) {
req = get_req_from_tcs(drv, i);
- if (!req) {
- WARN_ON(1);
+ if (WARN_ON(!req))
goto skip;
- }
err = 0;
for (j = 0; j < req->num_cmds; j++) {
--
Sent by a computer, using git, on the internet
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3/RESEND 0/3] Even moar rpmh cleanups
2020-05-21 6:04 [PATCH v3/RESEND 0/3] Even moar rpmh cleanups Stephen Boyd
` (2 preceding siblings ...)
2020-05-21 6:04 ` [PATCHv3/RESEND 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition Stephen Boyd
@ 2020-05-21 6:11 ` Randy Dunlap
3 siblings, 0 replies; 7+ messages in thread
From: Randy Dunlap @ 2020-05-21 6:11 UTC (permalink / raw)
To: Stephen Boyd, Andy Gross, Bjorn Andersson
Cc: linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson
On 5/20/20 11:04 PM, Stephen Boyd wrote:
> (Resent with more Ccs and To lines)
>
> We remove the tcs_is_free() API and then do super micro optimizations on
> the irq handler. I haven't tested anything here so most likely there's a
> bug (again again)!
>
Subject: s/moar/more/
--
~Randy
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3/RESEND 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API
2020-05-21 6:04 ` [PATCHv3/RESEND 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd
@ 2020-05-26 7:07 ` Maulik Shah
2020-05-28 18:04 ` Doug Anderson
1 sibling, 0 replies; 7+ messages in thread
From: Maulik Shah @ 2020-05-26 7:07 UTC (permalink / raw)
To: Stephen Boyd, Andy Gross, Bjorn Andersson
Cc: linux-kernel, linux-arm-msm, Douglas Anderson
Hi,
Reviewed-by: Maulik Shah <mkshah@codeaurora.org>
Thanks,
Maulik
On 5/21/2020 11:34 AM, Stephen Boyd wrote:
> This API does very little. Let's replace all the callsites with the
> normal operations that would be done on top of the bitmap that
> tcs_in_use is. This simplifies and reduces the code size.
>
> Cc: Maulik Shah <mkshah@codeaurora.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
> drivers/soc/qcom/rpmh-rsc.c | 59 +++++++++++++------------------------
> 1 file changed, 20 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index 076fd27f3081..60fc56987659 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -184,22 +184,6 @@ static void write_tcs_reg_sync(const struct rsc_drv *drv, int reg, int tcs_id,
> data, tcs_id, reg);
> }
>
> -/**
> - * tcs_is_free() - Return if a TCS is totally free.
> - * @drv: The RSC controller.
> - * @tcs_id: The global ID of this TCS.
> - *
> - * Returns true if nobody has claimed this TCS (by setting tcs_in_use).
> - *
> - * Context: Must be called with the drv->lock held.
> - *
> - * Return: true if the given TCS is free.
> - */
> -static bool tcs_is_free(struct rsc_drv *drv, int tcs_id)
> -{
> - return !test_bit(tcs_id, drv->tcs_in_use);
> -}
> -
> /**
> * tcs_invalidate() - Invalidate all TCSes of the given type (sleep or wake).
> * @drv: The RSC controller.
> @@ -512,7 +496,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
> *
> * Return: 0 if nothing in flight or -EBUSY if we should try again later.
> * The caller must re-enable interrupts between tries since that's
> - * the only way tcs_is_free() will ever return true and the only way
> + * the only way tcs_in_use will ever be updated and the only way
> * RSC_DRV_CMD_ENABLE will ever be cleared.
> */
> static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
> @@ -520,17 +504,14 @@ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
> {
> unsigned long curr_enabled;
> u32 addr;
> - int i, j, k;
> - int tcs_id = tcs->offset;
> + int j, k;
> + int i = tcs->offset;
>
> - for (i = 0; i < tcs->num_tcs; i++, tcs_id++) {
> - if (tcs_is_free(drv, tcs_id))
> - continue;
> -
> - curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id);
> + for_each_set_bit_from(i, drv->tcs_in_use, tcs->offset + tcs->num_tcs) {
> + curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, i);
>
> for_each_set_bit(j, &curr_enabled, MAX_CMDS_PER_TCS) {
> - addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, tcs_id, j);
> + addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, i, j);
> for (k = 0; k < msg->num_cmds; k++) {
> if (addr == msg->cmds[k].addr)
> return -EBUSY;
> @@ -548,18 +529,19 @@ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
> *
> * Must be called with the drv->lock held since that protects tcs_in_use.
> *
> - * Return: The first tcs that's free.
> + * Return: The first tcs that's free or -EBUSY if all in use.
> */
> static int find_free_tcs(struct tcs_group *tcs)
> {
> - int i;
> + const struct rsc_drv *drv = tcs->drv;
> + unsigned long i;
> + unsigned long max = tcs->offset + tcs->num_tcs;
>
> - for (i = 0; i < tcs->num_tcs; i++) {
> - if (tcs_is_free(tcs->drv, tcs->offset + i))
> - return tcs->offset + i;
> - }
> + i = find_next_zero_bit(drv->tcs_in_use, max, tcs->offset);
> + if (i >= max)
> + return -EBUSY;
>
> - return -EBUSY;
> + return i;
> }
>
> /**
> @@ -757,8 +739,9 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg)
> */
> static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv)
> {
> - int m;
> - struct tcs_group *tcs = &drv->tcs[ACTIVE_TCS];
> + unsigned long set;
> + const struct tcs_group *tcs = &drv->tcs[ACTIVE_TCS];
> + unsigned long max;
>
> /*
> * If we made an active request on a RSC that does not have a
> @@ -769,12 +752,10 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv)
> if (!tcs->num_tcs)
> tcs = &drv->tcs[WAKE_TCS];
>
> - for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
> - if (!tcs_is_free(drv, m))
> - return true;
> - }
> + max = tcs->offset + tcs->num_tcs;
> + set = find_next_bit(drv->tcs_in_use, max, tcs->offset);
>
> - return false;
> + return set < max;
> }
>
> /**
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3/RESEND 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API
2020-05-21 6:04 ` [PATCHv3/RESEND 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd
2020-05-26 7:07 ` Maulik Shah
@ 2020-05-28 18:04 ` Doug Anderson
1 sibling, 0 replies; 7+ messages in thread
From: Doug Anderson @ 2020-05-28 18:04 UTC (permalink / raw)
To: Stephen Boyd
Cc: Andy Gross, Bjorn Andersson, LKML, linux-arm-msm, Maulik Shah
Hi,
On Wed, May 20, 2020 at 11:04 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> This API does very little. Let's replace all the callsites with the
> normal operations that would be done on top of the bitmap that
> tcs_in_use is. This simplifies and reduces the code size.
>
> Cc: Maulik Shah <mkshah@codeaurora.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
> drivers/soc/qcom/rpmh-rsc.c | 59 +++++++++++++------------------------
> 1 file changed, 20 insertions(+), 39 deletions(-)
Sorry for the delay in getting to this review. Looks great.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-28 18:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 6:04 [PATCH v3/RESEND 0/3] Even moar rpmh cleanups Stephen Boyd
2020-05-21 6:04 ` [PATCHv3/RESEND 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd
2020-05-26 7:07 ` Maulik Shah
2020-05-28 18:04 ` Doug Anderson
2020-05-21 6:04 ` [PATCHv3/RESEND 2/3] soc: qcom: rpmh-rsc: Loop over fewer bits in irq handler Stephen Boyd
2020-05-21 6:04 ` [PATCHv3/RESEND 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition Stephen Boyd
2020-05-21 6:11 ` [PATCH v3/RESEND 0/3] Even moar rpmh cleanups Randy Dunlap
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.