| Message ID | 20200625120011.16168-7-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Support simple-framebuffer devices and firmware fbs | expand |
Hi Thomas, On Thu, Jun 25, 2020 at 2:00 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > Make sure required hardware clocks are enabled while the firmware > framebuffer is in use. > > The basic code has been taken from the simplefb driver and adapted > to DRM. Clocks are released automatically via devres helpers. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Thanks for your patch! > --- a/drivers/gpu/drm/tiny/simplekms.c > +++ b/drivers/gpu/drm/tiny/simplekms.c > @@ -210,6 +218,103 @@ static struct simplekms_device *simplekms_device_of_dev(struct drm_device *dev) > return container_of(dev, struct simplekms_device, dev); > } > > +/* > + * Hardware > + */ > + > +#if defined CONFIG_OF && defined CONFIG_COMMON_CLK > +/* > + * Clock handling code. > + * > + * Here we handle the clocks property of our "simple-framebuffer" dt node. > + * This is necessary so that we can make sure that any clocks needed by > + * the display engine that the bootloader set up for us (and for which it > + * provided a simplefb dt node), stay up, for the life of the simplefb > + * driver. > + * > + * When the driver unloads, we cleanly disable, and then release the clocks. > + * > + * 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 simplefb, and the clock definitions 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 simplekms_device_release_clocks(void *res) > +{ > + struct simplekms_device *sdev = simplekms_device_of_dev(res); > + unsigned int i; > + > + for (i = 0; i < sdev->clk_count; ++i) { > + if (sdev->clks[i]) { > + clk_disable_unprepare(sdev->clks[i]); > + clk_put(sdev->clks[i]); > + } > + } > +} > + > +static int simplekms_device_init_clocks(struct simplekms_device *sdev) > +{ > + struct drm_device *dev = &sdev->dev; > + struct platform_device *pdev = sdev->pdev; > + struct device_node *of_node = pdev->dev.of_node; > + struct clk *clock; > + unsigned int i; > + int ret; > + > + if (dev_get_platdata(&pdev->dev) || !of_node) > + return 0; > + > + sdev->clk_count = of_clk_get_parent_count(of_node); > + if (!sdev->clk_count) > + return 0; > + > + sdev->clks = drmm_kzalloc(dev, sdev->clk_count * sizeof(sdev->clks[0]), > + GFP_KERNEL); > + if (!sdev->clks) > + return -ENOMEM; > + > + for (i = 0; i < sdev->clk_count; ++i) { > + clock = of_clk_get(of_node, i); clk_bulk_get_all()? Gr{oetje,eeting}s, Geert
On Thu, Jun 25, 2020 at 03:34:05PM +0200, Geert Uytterhoeven wrote: > Hi Thomas, > > On Thu, Jun 25, 2020 at 2:00 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Make sure required hardware clocks are enabled while the firmware > > framebuffer is in use. > > > > The basic code has been taken from the simplefb driver and adapted > > to DRM. Clocks are released automatically via devres helpers. > > > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > Thanks for your patch! > > > --- a/drivers/gpu/drm/tiny/simplekms.c > > +++ b/drivers/gpu/drm/tiny/simplekms.c > > > @@ -210,6 +218,103 @@ static struct simplekms_device *simplekms_device_of_dev(struct drm_device *dev) > > return container_of(dev, struct simplekms_device, dev); > > } > > > > +/* > > + * Hardware > > + */ > > + > > +#if defined CONFIG_OF && defined CONFIG_COMMON_CLK > > +/* > > + * Clock handling code. > > + * > > + * Here we handle the clocks property of our "simple-framebuffer" dt node. > > + * This is necessary so that we can make sure that any clocks needed by > > + * the display engine that the bootloader set up for us (and for which it > > + * provided a simplefb dt node), stay up, for the life of the simplefb > > + * driver. > > + * > > + * When the driver unloads, we cleanly disable, and then release the clocks. > > + * > > + * 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 simplefb, and the clock definitions 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 simplekms_device_release_clocks(void *res) > > +{ > > + struct simplekms_device *sdev = simplekms_device_of_dev(res); > > + unsigned int i; > > + > > + for (i = 0; i < sdev->clk_count; ++i) { > > + if (sdev->clks[i]) { > > + clk_disable_unprepare(sdev->clks[i]); > > + clk_put(sdev->clks[i]); > > + } > > + } > > +} > > + > > +static int simplekms_device_init_clocks(struct simplekms_device *sdev) > > +{ > > + struct drm_device *dev = &sdev->dev; > > + struct platform_device *pdev = sdev->pdev; > > + struct device_node *of_node = pdev->dev.of_node; > > + struct clk *clock; > > + unsigned int i; > > + int ret; > > + > > + if (dev_get_platdata(&pdev->dev) || !of_node) > > + return 0; > > + > > + sdev->clk_count = of_clk_get_parent_count(of_node); > > + if (!sdev->clk_count) > > + return 0; > > + > > + sdev->clks = drmm_kzalloc(dev, sdev->clk_count * sizeof(sdev->clks[0]), > > + GFP_KERNEL); > > + if (!sdev->clks) > > + return -ENOMEM; > > + > > + for (i = 0; i < sdev->clk_count; ++i) { > > + clock = of_clk_get(of_node, i); > > clk_bulk_get_all()? Plus then you can use devm_clk_bulk_get_all, even less code I think. -Daniel > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
diff --git a/drivers/gpu/drm/tiny/simplekms.c b/drivers/gpu/drm/tiny/simplekms.c index 87636307aa4f..aca186decb48 100644 --- a/drivers/gpu/drm/tiny/simplekms.c +++ b/drivers/gpu/drm/tiny/simplekms.c @@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only +#include <linux/clk.h> +#include <linux/of_clk.h> #include <linux/platform_data/simplefb.h> #include <linux/platform_device.h> @@ -191,6 +193,12 @@ struct simplekms_device { struct drm_device dev; struct platform_device *pdev; + /* clocks */ +#if defined CONFIG_OF && defined CONFIG_COMMON_CLK + unsigned int clk_count; + struct clk **clks; +#endif + /* simplefb settings */ struct drm_display_mode mode; const struct drm_format_info *format; @@ -210,6 +218,103 @@ static struct simplekms_device *simplekms_device_of_dev(struct drm_device *dev) return container_of(dev, struct simplekms_device, dev); } +/* + * Hardware + */ + +#if defined CONFIG_OF && defined CONFIG_COMMON_CLK +/* + * Clock handling code. + * + * Here we handle the clocks property of our "simple-framebuffer" dt node. + * This is necessary so that we can make sure that any clocks needed by + * the display engine that the bootloader set up for us (and for which it + * provided a simplefb dt node), stay up, for the life of the simplefb + * driver. + * + * When the driver unloads, we cleanly disable, and then release the clocks. + * + * 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 simplefb, and the clock definitions 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 simplekms_device_release_clocks(void *res) +{ + struct simplekms_device *sdev = simplekms_device_of_dev(res); + unsigned int i; + + for (i = 0; i < sdev->clk_count; ++i) { + if (sdev->clks[i]) { + clk_disable_unprepare(sdev->clks[i]); + clk_put(sdev->clks[i]); + } + } +} + +static int simplekms_device_init_clocks(struct simplekms_device *sdev) +{ + struct drm_device *dev = &sdev->dev; + struct platform_device *pdev = sdev->pdev; + struct device_node *of_node = pdev->dev.of_node; + struct clk *clock; + unsigned int i; + int ret; + + if (dev_get_platdata(&pdev->dev) || !of_node) + return 0; + + sdev->clk_count = of_clk_get_parent_count(of_node); + if (!sdev->clk_count) + return 0; + + sdev->clks = drmm_kzalloc(dev, sdev->clk_count * sizeof(sdev->clks[0]), + GFP_KERNEL); + if (!sdev->clks) + return -ENOMEM; + + for (i = 0; i < sdev->clk_count; ++i) { + clock = of_clk_get(of_node, i); + if (IS_ERR(clock)) { + ret = PTR_ERR(clock); + if (ret == -EPROBE_DEFER) + goto err; + drm_err(dev, "clock %u not found: %d\n", i, ret); + continue; + } + ret = clk_prepare_enable(clock); + if (ret) { + drm_err(dev, "failed to enable clock %u: %d\n", + i, ret); + clk_put(clock); + } + sdev->clks[i] = clock; + } + + return devm_add_action_or_reset(&pdev->dev, + simplekms_device_release_clocks, + sdev); + +err: + while (i) { + --i; + if (sdev->clks[i]) { + clk_disable_unprepare(sdev->clks[i]); + clk_put(sdev->clks[i]); + } + } + return ret; +} +#else +static int simplekms_device_init_clocks(struct simplekms_device *sdev) +{ + return 0; +} +#endif + /* * Simplefb settings */ @@ -505,6 +610,9 @@ simplekms_device_create(struct drm_driver *drv, struct platform_device *pdev) return ERR_CAST(sdev); sdev->pdev = pdev; + ret = simplekms_device_init_clocks(sdev); + if (ret) + return ERR_PTR(ret); ret = simplekms_device_init_fb(sdev); if (ret) return ERR_PTR(ret);
Make sure required hardware clocks are enabled while the firmware framebuffer is in use. The basic code has been taken from the simplefb driver and adapted to DRM. Clocks are released automatically via devres helpers. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/tiny/simplekms.c | 108 +++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+)