| Message ID | 20230910-simpledrm-multiple-power-domains-v1-1-f8718aefc685@jannau.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/simpledrm: Add support for multiple "power-domains" | expand |
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
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
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,
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,
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,
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-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
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
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);