drivers/gpu/drm/tiny/simpledrm.c | 106 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+)
From: Janne Grunau <j@jannau.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@jannau.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_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,
--
Janne Grunau <j@jannau.net>Hi Janne,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 15d30b46573d75f5cb58cfacded8ebab9c76a2b0]
url: https://github.com/intel-lab-lkp/linux/commits/Janne-Grunau-via-B4-Relay/drm-simpledrm-Add-support-for-multiple-power-domains/20230911-004026
base: 15d30b46573d75f5cb58cfacded8ebab9c76a2b0
patch link: https://lore.kernel.org/r/20230910-simpledrm-multiple-power-domains-v1-1-f8718aefc685%40jannau.net
patch subject: [PATCH] drm/simpledrm: Add support for multiple "power-domains"
config: arm64-randconfig-r003-20230912 (https://download.01.org/0day-ci/archive/20230912/202309122212.MetCn4UK-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230912/202309122212.MetCn4UK-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309122212.MetCn4UK-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/tiny/simpledrm.c:506:24: warning: flag ' ' results in undefined behavior with 'p' conversion specifier [-Wformat]
506 | drm_err(&sdev->dev, "% power-domains count:%d\n", __func__, sdev->pwr_dom_count);
| ~^~
include/drm/drm_print.h:469:39: note: expanded from macro 'drm_err'
469 | __drm_printk((drm), err,, "*ERROR* " fmt, ##__VA_ARGS__)
| ^~~
include/drm/drm_print.h:456:41: note: expanded from macro '__drm_printk'
456 | dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
| ^~~
include/linux/dev_printk.h:144:57: note: expanded from macro 'dev_err'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~
include/linux/dev_printk.h:19:22: note: expanded from macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
include/linux/dev_printk.h:110:16: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
1 warning generated.
vim +506 drivers/gpu/drm/tiny/simpledrm.c
477
478 #if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
479 /*
480 * Generic power domain handling code.
481 *
482 * Here we handle the power-domains properties of our "simple-framebuffer"
483 * dt node. This is only necessary if there is more than one power-domain.
484 * A single power-domains is handled automatically by the driver core. Multiple
485 * power-domains have to be handled by drivers since the driver core can't know
486 * the correct power sequencing. Power sequencing is not an issue for simpledrm
487 * since the bootloader has put the power domains already in the correct state.
488 * simpledrm has only to ensure they remain active for its lifetime.
489 *
490 * When the driver unloads, we detach from the power-domains.
491 *
492 * We only complain about errors here, no action is taken as the most likely
493 * error can only happen due to a mismatch between the bootloader which set
494 * up the "simple-framebuffer" dt node, and the PM domain providers in the
495 * device tree. Chances are that there are no adverse effects, and if there are,
496 * a clean teardown of the fb probe will not help us much either. So just
497 * complain and carry on, and hope that the user actually gets a working fb at
498 * the end of things.
499 */
500 static void simpledrm_device_detach_genpd(void *res)
501 {
502 int i;
503 struct simpledrm_device *sdev = /*(struct simpledrm_device *)*/res;
504
505
> 506 drm_err(&sdev->dev, "% power-domains count:%d\n", __func__, sdev->pwr_dom_count);
507 if (sdev->pwr_dom_count <= 1)
508 return;
509
510 for (i = sdev->pwr_dom_count - 1; i >= 0; i--) {
511 if (!sdev->pwr_dom_links[i])
512 device_link_del(sdev->pwr_dom_links[i]);
513 if (!IS_ERR_OR_NULL(sdev->pwr_dom_devs[i]))
514 dev_pm_domain_detach(sdev->pwr_dom_devs[i], true);
515 }
516 }
517
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi
Am 10.09.23 um 18:39 schrieb Janne Grunau via B4 Relay:
> From: Janne Grunau <j@jannau.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@jannau.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 anything, drm_dbg()
> + 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);
The driver's not really failing to initialize AFAICT. CAlling drm_warn()
might be more appropriate.
> + }
> +
> + 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);
Also drm_warn() ?
Best regards
Thomas
> + }
> +
> + 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,
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
On 2023-09-11 14:26:10 +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 10.09.23 um 18:39 schrieb Janne Grunau via B4 Relay:
> > From: Janne Grunau <j@jannau.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@jannau.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 anything, drm_dbg()
see my own reply, was never supposed to be there, removed locally
>
> > + 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);
>
> The driver's not really failing to initialize AFAICT. CAlling drm_warn()
> might be more appropriate.
copied from simpledrm_device_init_regulators() but I agree that
drm_warn() is more appropiate. change locally for v2.
> > + }
> > +
> > + 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);
>
> Also drm_warn() ?
changed
Thanks,
Janne
On Mon, 11 Sep 2023, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi
>
> Am 10.09.23 um 18:39 schrieb Janne Grunau via B4 Relay:
>> From: Janne Grunau <j@jannau.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@jannau.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 anything, drm_dbg()
Drive-by comment, drm_dbg() already prints the function, there's no need
to use __func__.
BR,
Jani.
>
>> + 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);
>
> The driver's not really failing to initialize AFAICT. CAlling drm_warn()
> might be more appropriate.
>
>> + }
>> +
>> + 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);
>
> Also drm_warn() ?
>
> Best regards
> Thomas
>
>> + }
>> +
>> + 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,
--
Jani Nikula, Intel Open Source Graphics Center
Le 10/09/2023 à 18:39, Janne Grunau via B4 Relay a écrit :
> From: Janne Grunau <j@jannau.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@jannau.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,
On 2023-09-10 22:16:05 +0200, Christophe JAILLET wrote:
> Le 10/09/2023 à 18:39, Janne Grunau via B4 Relay a écrit :
> > From: Janne Grunau <j@jannau.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@jannau.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?
yes, a continue is missing, added, locally for v2.
thanks
Janne
On 2023-09-10 18:39:39 +0200, Janne Grunau via B4 Relay wrote:
> From: Janne Grunau <j@jannau.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@jannau.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;
commented cast, removed locally for v2
> +
> +
> + drm_err(&sdev->dev, "% power-domains count:%d\n", __func__, sdev->pwr_dom_count);
broken log statement as pointed out by kernel test robot, not ment to be
included in this change. removed locally for v2
> + 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_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);
wrong format specifier for int, fixed locally for v2
Janne
Hi Janne,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 15d30b46573d75f5cb58cfacded8ebab9c76a2b0]
url: https://github.com/intel-lab-lkp/linux/commits/Janne-Grunau-via-B4-Relay/drm-simpledrm-Add-support-for-multiple-power-domains/20230911-004026
base: 15d30b46573d75f5cb58cfacded8ebab9c76a2b0
patch link: https://lore.kernel.org/r/20230910-simpledrm-multiple-power-domains-v1-1-f8718aefc685%40jannau.net
patch subject: [PATCH] drm/simpledrm: Add support for multiple "power-domains"
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20230911/202309110206.wDXP9YXl-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230911/202309110206.wDXP9YXl-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309110206.wDXP9YXl-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/acpi.h:14,
from include/linux/i2c.h:13,
from include/uapi/linux/fb.h:6,
from include/linux/fb.h:7,
from include/linux/platform_data/simplefb.h:12,
from drivers/gpu/drm/tiny/simpledrm.c:7:
drivers/gpu/drm/tiny/simpledrm.c: In function 'simpledrm_device_detach_genpd':
>> include/drm/drm_print.h:456:39: warning: ' ' flag used with '%p' gnu_printf format [-Wformat=]
456 | dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
| ^~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
include/drm/drm_print.h:456:9: note: in expansion of macro 'dev_err'
456 | dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
| ^~~~
include/drm/drm_print.h:469:9: note: in expansion of macro '__drm_printk'
469 | __drm_printk((drm), err,, "*ERROR* " fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~
drivers/gpu/drm/tiny/simpledrm.c:506:9: note: in expansion of macro 'drm_err'
506 | drm_err(&sdev->dev, "% power-domains count:%d\n", __func__, sdev->pwr_dom_count);
| ^~~~~~~
vim +456 include/drm/drm_print.h
e820f52577b14c6 Jim Cromie 2022-09-11 416
02c9656b2f0d699 Haneen Mohammed 2017-10-17 417 /**
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 418 * DRM_DEV_DEBUG() - Debug output for generic drm code
02c9656b2f0d699 Haneen Mohammed 2017-10-17 419 *
306589856399e18 Douglas Anderson 2021-09-21 420 * NOTE: this is deprecated in favor of drm_dbg_core().
306589856399e18 Douglas Anderson 2021-09-21 421 *
091756bbb1a9613 Haneen Mohammed 2017-10-17 422 * @dev: device pointer
091756bbb1a9613 Haneen Mohammed 2017-10-17 423 * @fmt: printf() like format string.
02c9656b2f0d699 Haneen Mohammed 2017-10-17 424 */
db87086492581c8 Joe Perches 2018-03-16 425 #define DRM_DEV_DEBUG(dev, fmt, ...) \
db87086492581c8 Joe Perches 2018-03-16 426 drm_dev_dbg(dev, DRM_UT_CORE, fmt, ##__VA_ARGS__)
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 427 /**
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 428 * DRM_DEV_DEBUG_DRIVER() - Debug output for vendor specific part of the driver
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 429 *
306589856399e18 Douglas Anderson 2021-09-21 430 * NOTE: this is deprecated in favor of drm_dbg() or dev_dbg().
306589856399e18 Douglas Anderson 2021-09-21 431 *
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 432 * @dev: device pointer
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 433 * @fmt: printf() like format string.
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 434 */
db87086492581c8 Joe Perches 2018-03-16 435 #define DRM_DEV_DEBUG_DRIVER(dev, fmt, ...) \
db87086492581c8 Joe Perches 2018-03-16 436 drm_dev_dbg(dev, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 437 /**
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 438 * DRM_DEV_DEBUG_KMS() - Debug output for modesetting code
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 439 *
306589856399e18 Douglas Anderson 2021-09-21 440 * NOTE: this is deprecated in favor of drm_dbg_kms().
306589856399e18 Douglas Anderson 2021-09-21 441 *
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 442 * @dev: device pointer
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 443 * @fmt: printf() like format string.
b52817e9de06a3a Mauro Carvalho Chehab 2020-10-27 444 */
db87086492581c8 Joe Perches 2018-03-16 445 #define DRM_DEV_DEBUG_KMS(dev, fmt, ...) \
db87086492581c8 Joe Perches 2018-03-16 446 drm_dev_dbg(dev, DRM_UT_KMS, fmt, ##__VA_ARGS__)
a18b21929453af7 Lyude Paul 2018-07-16 447
fb6c7ab8718eb25 Jani Nikula 2019-12-10 448 /*
fb6c7ab8718eb25 Jani Nikula 2019-12-10 449 * struct drm_device based logging
fb6c7ab8718eb25 Jani Nikula 2019-12-10 450 *
fb6c7ab8718eb25 Jani Nikula 2019-12-10 451 * Prefer drm_device based logging over device or prink based logging.
fb6c7ab8718eb25 Jani Nikula 2019-12-10 452 */
fb6c7ab8718eb25 Jani Nikula 2019-12-10 453
fb6c7ab8718eb25 Jani Nikula 2019-12-10 454 /* Helper for struct drm_device based logging. */
fb6c7ab8718eb25 Jani Nikula 2019-12-10 455 #define __drm_printk(drm, level, type, fmt, ...) \
fb6c7ab8718eb25 Jani Nikula 2019-12-10 @456 dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
fb6c7ab8718eb25 Jani Nikula 2019-12-10 457
fb6c7ab8718eb25 Jani Nikula 2019-12-10 458
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2026 Red Hat, Inc.