Hi,
On 4/10/2020 9:45 AM, Stephen Boyd wrote:
Quoting Maulik Shah (2020-04-05 23:32:19)
Add changes to invoke rpmh flush() from CPU PM notification.
This is done when the last the cpu is entering power collapse and
'power collapse' is a qcom-ism. Maybe say something like "deep CPU idle
states"?
Okay, i updated in v17.
controller is not busy.
Controllers that do have 'HW solver' mode do not need to register
for CPU PM notification. They may be in autonomous mode executing
low power mode and do not require rpmh_flush() to happen from CPU
PM notification.
Can you provide an example of a HW solver mode controller? Presumably
the display RSC is one of these?
Sure, Added in v17 to mention display RSC.
Signed-off-by: Maulik Shah <mkshah@xxxxxxxxxxxxxx>
Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index b718221..fbe1f3e 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -521,8 +527,86 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg)
return tcs_ctrl_write(drv, msg);
}
+/**
+ * rpmh_rsc_ctrlr_is_busy() - Check if any of the AMCs are busy.
+ *
+ * @drv: The controller
+ *
+ * Checks if any of the AMCs are busy in handling ACTIVE sets.
+ * This is called from the last cpu powering down before flushing
+ * SLEEP and WAKE sets. If AMCs are busy, controller can not enter
+ * power collapse, so deny from the last cpu's pm notification.
+ *
+ * Return:
+ * * False - AMCs are idle
+ * * True - AMCs are busy
+ */
+static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv)
Can drv be const? Would be nice to make it const in some places in this
driver.
It can, but it will require changing multiple places to make it const.
some of those functions are
dropped/modified in doug's cleanup series. I can do in separate patch
once this series is pulled in.
+{
+ int m;
+ struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
+
+ /*
+ * If we made an active request on a RSC that does not have a
+ * dedicated TCS for active state use, then re-purposed wake TCSes
+ * should be checked for not busy.
not busy, because we use wake TCSes for active requests in this case.
Ok, added in v17.
+ *
+ * Since this is called from the last cpu, need not take drv or tcs
+ * lock before checking tcs_is_free().
+ */
+ if (!tcs->num_tcs)
+ tcs = get_tcs_of_type(drv, WAKE_TCS);
+
+ for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
+ if (!tcs_is_free(drv, m))
+ return true;
+ }
+
+ return false;
+}
[...]
+
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index a75f3df..88f8801 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -433,16 +430,17 @@ static int send_single(struct rpmh_ctrlr *ctrlr, enum rpmh_state state,
}
/**
- * rpmh_flush: Flushes the buffered active and sleep sets to TCS
+ * rpmh_flush() - Flushes the buffered sleep and wake sets to TCSes
*
- * @ctrlr: controller making request to flush cached data
+ * @ctrlr: Controller making request to flush cached data
*
- * Return: -EBUSY if the controller is busy, probably waiting on a response
- * to a RPMH request sent earlier.
+ * This function is called from sleep code on the last CPU
+ * (thus no spinlock needed).
Might be good to stick a lockdep_assert_irqs_disabled() in this function
then. That would document that this function should only be called when
irqs are disabled.
Okay. Added lockdep_assert_irqs_disabled(). But this may need to be
dropped when
we add support for display RSC.
*
- * This function is always called from the sleep code from the last CPU
- * that is powering down the entire system. Since no other RPMH API would be
- * executing at this time, it is safe to run lockless.
+ * Return:
+ * * 0 - Success
+ * * -EAGAIN - Retry again
+ * * Error code - Otherwise
*/
int rpmh_flush(struct rpmh_ctrlr *ctrlr)
This function name keeps throwing me off. Can we please call it
something like rpmh_configure_tcs_sleep_wake()? The word "flush" seems
to imply there's some sort of cache going on, but that's not really the
case. We're programming a couple TCS FIFOs so that they can be used
across deep CPU sleep states.
{
@@ -455,9 +453,7 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
}
/* Invalidate the TCSes first to avoid stale data */
- do {
- ret = rpmh_rsc_invalidate(ctrlr_to_drv(ctrlr));
- } while (ret == -EAGAIN);
+ ret = rpmh_rsc_invalidate(ctrlr_to_drv(ctrlr));
if (ret)
return ret;
Thanks,
Maulik
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
