On Wed, 5 Feb 2025 at 23:57, Frank Oltmanns <frank@xxxxxxxxxxxx> wrote:
>
> On xiaomi-beryllium and oneplus-enchilada audio does not work reliably
> with the in-kernel pd-mapper. Deferring the probe solves these issues.
> Specifically, audio only works reliably with the in-kernel pd-mapper, if
> the probe succeeds when remoteproc3 triggers the first successful probe.
> I.e., probes from remoteproc0, 1, and 2 need to be deferred until
> remoteproc3 has been probed.
The cause for such issues also must be identified. I mean, in-kernel
pd-mapper uncovered a lot of issues that were not noticed because
usually the starting order was a bit different. But we need to
understand how to actually fix those issues instead of working them
around.
>
> Introduce a device specific quirk that lists the first auxdev for which
> the probe must be executed. Until then, defer probes from other auxdevs.
>
> Fixes: 1ebcde047c54 ("soc: qcom: add pd-mapper implementation")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Frank Oltmanns <frank@xxxxxxxxxxxx>
> ---
> The in-kernel pd-mapper has been causing audio issues on sdm845
> devices (specifically, xiaomi-beryllium and oneplus-enchilada). I
> observed that Stephan’s approach [1] - which defers module probing by
> blocklisting the module and triggering a later probe - works reliably.
>
> Inspired by this, I experimented with delaying the probe within the
> module itself by returning -EPROBE_DEFER in qcom_pdm_probe() until a
> certain time (13.9 seconds after boot, based on ktime_get()) had
> elapsed. This method also restored audio functionality.
>
> Further logging of auxdev->id in qcom_pdm_probe() led to an interesting
> discovery: audio only works reliably with the in-kernel pd-mapper when
> the first successful probe is triggered by remoteproc3. In other words,
> probes from remoteproc0, 1, and 2 must be deferred until remoteproc3 has
> been probed.
>
> To address this, I propose introducing a quirk table (which currently
> only contains sdm845) to defer probing until the correct auxiliary
> device (remoteproc3) initiates the probe.
>
> I look forward to your feedback.
>
> Thanks,
> Frank
>
> [1]: https://lore.kernel.org/linux-arm-msm/Zwj3jDhc9fRoCCn6@xxxxxxxxxx/
> ---
> drivers/soc/qcom/qcom_pd_mapper.c | 43 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/drivers/soc/qcom/qcom_pd_mapper.c b/drivers/soc/qcom/qcom_pd_mapper.c
> index 154ca5beb47160cc404a46a27840818fe3187420..34b26df665a888ac4872f56e948e73b561ae3b6b 100644
> --- a/drivers/soc/qcom/qcom_pd_mapper.c
> +++ b/drivers/soc/qcom/qcom_pd_mapper.c
> @@ -46,6 +46,11 @@ struct qcom_pdm_data {
> struct list_head services;
> };
>
> +struct qcom_pdm_probe_first_dev_quirk {
> + const char *name;
> + u32 id;
> +};
> +
> static DEFINE_MUTEX(qcom_pdm_mutex); /* protects __qcom_pdm_data */
> static struct qcom_pdm_data *__qcom_pdm_data;
>
> @@ -526,6 +531,11 @@ static const struct qcom_pdm_domain_data *x1e80100_domains[] = {
> NULL,
> };
>
> +static const struct qcom_pdm_probe_first_dev_quirk first_dev_remoteproc3 = {
> + .id = 3,
> + .name = "pd-mapper"
> +};
> +
> static const struct of_device_id qcom_pdm_domains[] __maybe_unused = {
> { .compatible = "qcom,apq8016", .data = NULL, },
> { .compatible = "qcom,apq8064", .data = NULL, },
> @@ -566,6 +576,10 @@ static const struct of_device_id qcom_pdm_domains[] __maybe_unused = {
> {},
> };
>
> +static const struct of_device_id qcom_pdm_defer[] __maybe_unused = {
> + { .compatible = "qcom,sdm845", .data = &first_dev_remoteproc3, },
> + {},
> +};
> static void qcom_pdm_stop(struct qcom_pdm_data *data)
> {
> qcom_pdm_free_domains(data);
> @@ -637,6 +651,25 @@ static struct qcom_pdm_data *qcom_pdm_start(void)
> return ERR_PTR(ret);
> }
>
> +static bool qcom_pdm_ready(struct auxiliary_device *auxdev)
> +{
> + const struct of_device_id *match;
> + struct device_node *root;
> + struct qcom_pdm_probe_first_dev_quirk *first_dev;
> +
> + root = of_find_node_by_path("/");
> + if (!root)
> + return true;
> +
> + match = of_match_node(qcom_pdm_defer, root);
> + of_node_put(root);
> + if (!match)
> + return true;
> +
> + first_dev = (struct qcom_pdm_probe_first_dev_quirk *) match->data;
> + return (auxdev->id == first_dev->id) && !strcmp(auxdev->name, first_dev->name);
> +}
> +
> static int qcom_pdm_probe(struct auxiliary_device *auxdev,
> const struct auxiliary_device_id *id)
>
> @@ -647,6 +680,15 @@ static int qcom_pdm_probe(struct auxiliary_device *auxdev,
> mutex_lock(&qcom_pdm_mutex);
>
> if (!__qcom_pdm_data) {
> + if (!qcom_pdm_ready(auxdev)) {
> + pr_debug("%s: Deferring probe for device %s (id: %u)\n",
> + __func__, auxdev->name, auxdev->id);
> + ret = -EPROBE_DEFER;
> + goto probe_stop;
> + }
> + pr_debug("%s: Probing for device %s (id: %u), starting pdm\n",
> + __func__, auxdev->name, auxdev->id);
> +
> data = qcom_pdm_start();
>
> if (IS_ERR(data))
> @@ -659,6 +701,7 @@ static int qcom_pdm_probe(struct auxiliary_device *auxdev,
>
> auxiliary_set_drvdata(auxdev, __qcom_pdm_data);
>
> +probe_stop:
> mutex_unlock(&qcom_pdm_mutex);
>
> return ret;
>
> ---
> base-commit: 7f048b202333b967782a98aa21bb3354dc379bbf
> change-id: 20250205-qcom_pdm_defer-3dc1271d74d9
>
> Best regards,
> --
> Frank Oltmanns <frank@xxxxxxxxxxxx>
>
--
With best wishes
Dmitry
