| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0b6ad379-049a-5152-12e5-77c62ffc4541@wanadoo.fr>
Date: Sun, 10 Sep 2023 22:16:05 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: j@...nau.net, Thomas Zimmermann <tzimmermann@...e.de>,
Javier Martinez Canillas <javierm@...hat.com>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
asahi@...ts.linux.dev
Subject: Re: [PATCH] drm/simpledrm: Add support for multiple "power-domains"
Le 10/09/2023 à 18:39, Janne Grunau via B4 Relay a écrit :
> From: Janne Grunau <j@...nau.net>
>
> Multiple power domains need to be handled explicitly in each driver. The
> driver core can not handle it automatically since it is not aware of
> power sequencing requirements the hardware might have. This is not a
> problem for simpledrm since everything is expected to be powered on by
> the bootloader. simpledrm has just ensure it remains powered on during
> its lifetime.
> This is required on Apple silicon M2 and M2 Pro/Max/Ultra desktop
> systems. The HDMI output initialized by the bootloader requires keeping
> the display controller and a DP phy power domain on.
>
> Signed-off-by: Janne Grunau <j@...nau.net>
> ---
> drivers/gpu/drm/tiny/simpledrm.c | 106 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 106 insertions(+)
>
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index ff86ba1ae1b8..efedede57d42 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -6,6 +6,7 @@
> #include <linux/of_address.h>
> #include <linux/platform_data/simplefb.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> #include <linux/regulator/consumer.h>
>
> #include <drm/drm_aperture.h>
> @@ -227,6 +228,12 @@ struct simpledrm_device {
> unsigned int regulator_count;
> struct regulator **regulators;
> #endif
> + /* power-domains */
> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> + int pwr_dom_count;
> + struct device **pwr_dom_devs;
> + struct device_link **pwr_dom_links;
> +#endif
>
> /* simplefb settings */
> struct drm_display_mode mode;
> @@ -468,6 +475,102 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev)
> }
> #endif
>
> +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> +/*
> + * Generic power domain handling code.
> + *
> + * Here we handle the power-domains properties of our "simple-framebuffer"
> + * dt node. This is only necessary if there is more than one power-domain.
> + * A single power-domains is handled automatically by the driver core. Multiple
> + * power-domains have to be handled by drivers since the driver core can't know
> + * the correct power sequencing. Power sequencing is not an issue for simpledrm
> + * since the bootloader has put the power domains already in the correct state.
> + * simpledrm has only to ensure they remain active for its lifetime.
> + *
> + * When the driver unloads, we detach from the power-domains.
> + *
> + * We only complain about errors here, no action is taken as the most likely
> + * error can only happen due to a mismatch between the bootloader which set
> + * up the "simple-framebuffer" dt node, and the PM domain providers in the
> + * device tree. Chances are that there are no adverse effects, and if there are,
> + * a clean teardown of the fb probe will not help us much either. So just
> + * complain and carry on, and hope that the user actually gets a working fb at
> + * the end of things.
> + */
> +static void simpledrm_device_detach_genpd(void *res)
> +{
> + int i;
> + struct simpledrm_device *sdev = /*(struct simpledrm_device *)*/res;
> +
> +
> + drm_err(&sdev->dev, "% power-domains count:%d\n", __func__, sdev->pwr_dom_count);
> + if (sdev->pwr_dom_count <= 1)
> + return;
> +
> + for (i = sdev->pwr_dom_count - 1; i >= 0; i--) {
> + if (!sdev->pwr_dom_links[i])
> + device_link_del(sdev->pwr_dom_links[i]);
> + if (!IS_ERR_OR_NULL(sdev->pwr_dom_devs[i]))
> + dev_pm_domain_detach(sdev->pwr_dom_devs[i], true);
> + }
> +}
> +
> +static int simpledrm_device_attach_genpd(struct simpledrm_device *sdev)
> +{
> + struct device *dev = sdev->dev.dev;
> + int i;
> +
> + sdev->pwr_dom_count = of_count_phandle_with_args(dev->of_node, "power-domains",
> + "#power-domain-cells");
> + /*
> + * Single power-domain devices are handled by driver core nothing to do
> + * here. The same for device nodes without "power-domains" property.
> + */
> + if (sdev->pwr_dom_count <= 1)
> + return 0;
> +
> + sdev->pwr_dom_devs = devm_kcalloc(dev, sdev->pwr_dom_count,
> + sizeof(*sdev->pwr_dom_devs),
> + GFP_KERNEL);
> + if (!sdev->pwr_dom_devs)
> + return -ENOMEM;
> +
> + sdev->pwr_dom_links = devm_kcalloc(dev, sdev->pwr_dom_count,
> + sizeof(*sdev->pwr_dom_links),
> + GFP_KERNEL);
> + if (!sdev->pwr_dom_links)
> + return -ENOMEM;
> +
> + for (i = 0; i < sdev->pwr_dom_count; i++) {
> + sdev->pwr_dom_devs[i] = dev_pm_domain_attach_by_id(dev, i);
> + if (IS_ERR(sdev->pwr_dom_devs[i])) {
> + int ret = PTR_ERR(sdev->pwr_dom_devs[i]);
> + if (ret == -EPROBE_DEFER) {
> + simpledrm_device_detach_genpd(sdev);
> + return PTR_ERR(sdev->pwr_dom_devs[i]);
> + }
> + drm_err(&sdev->dev,
> + "pm_domain_attach_by_id(%u) failed: %d\n", i, ret);
> + }
> +
sdev->pwr_dom_devs[i] can be an ERR_PTR here.
Maybe a break or a continue missing after drm_err() above?
CJ
> + sdev->pwr_dom_links[i] = device_link_add(dev,
> + sdev->pwr_dom_devs[i],
> + DL_FLAG_STATELESS |
> + DL_FLAG_PM_RUNTIME |
> + DL_FLAG_RPM_ACTIVE);
> + if (!sdev->pwr_dom_links[i])
> + drm_err(&sdev->dev, "failed to link power-domain %u\n", i);
> + }
> +
> + return devm_add_action_or_reset(dev, simpledrm_device_detach_genpd, sdev);
> +}
> +#else
> +static int simpledrm_device_attach_genpd(struct simpledrm_device *sdev)
> +{
> + return 0;
> +}
> +#endif
> +
> /*
> * Modesetting
> */
> @@ -651,6 +754,9 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
> if (ret)
> return ERR_PTR(ret);
> ret = simpledrm_device_init_regulators(sdev);
> + if (ret)
> + return ERR_PTR(ret);
> + ret = simpledrm_device_attach_genpd(sdev);
> if (ret)
> return ERR_PTR(ret);
>
>
> ---
> base-commit: 15d30b46573d75f5cb58cfacded8ebab9c76a2b0
> change-id: 20230910-simpledrm-multiple-power-domains-f41efa6ad9bc
>
> Best regards,
Powered by blists - more mailing lists