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"?
> 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?
>
> 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.
> +{
> + 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.
> + *
> + * 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.
> *
> - * 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;
>
