* Re: [PATCH v2] of: Add videomode helper
From: Sascha Hauer @ 2012-07-05 16:50 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <3714247.L7zNpJpvuy@avalon>
On Thu, Jul 05, 2012 at 04:08:07PM +0200, Laurent Pinchart wrote:
> Hi Sascha,
>
> Thanks for the patch.
>
> > +++ b/Documentation/devicetree/bindings/video/displaymode
> > @@ -0,0 +1,40 @@
> > +videomode bindings
> > +=========
> > +
> > +Required properties:
> > + - xres, yres: Display resolution
> > + - left-margin, right-margin, hsync-len: Horizontal Display timing
> > parameters + in pixels
> > + upper-margin, lower-margin, vsync-len: Vertical display timing
> > parameters in + lines
> > + - clock: displayclock in Hz
> > +
> > +Optional properties:
> > + - width-mm, height-mm: Display dimensions in mm
>
> I've always had mixed feelings about the physical display dimension being part
> of the display mode. Those are properties of the panel/display instead of the
> mode. Storing them as part of the mode can be convenient, but we then run into
> consistency issues (developers have to remember in which display mode
> instances the values are available, and in which instances they're set to 0
> for instance). If we want to clean this up, this patch would be a good
> occasion.
This sounds like a display node with one or more node subnodes, like:
display {
width_mm = <>;
height_mm = <>;
mode {
xres = <>;
yres = <>;
...
};
};
Is that what you mean or are you thinking of something else?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Stephen Warren @ 2012-07-05 16:03 UTC (permalink / raw)
To: Alex Courbot
Cc: Thierry Reding, Sascha Hauer, Mark Brown,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org
In-Reply-To: <4FF54C6C.1060700@nvidia.com>
On 07/05/2012 02:12 AM, Alex Courbot wrote:
> On 07/05/2012 04:57 PM, Thierry Reding wrote:
>> I agree. Non-DT platforms have always used the callbacks to execute this
>> kind of code. As you've said before there are situations where it isn't
>> just about setting a GPIO or enabling a regulator but it also requires a
>> specific timing. Representing this in the platform data would become
>> tedious.
>
> That will settle the whole issue then.
>
>> So I think for the DT case you can parse the power-on and power-off
>> sequences directly and execute code based on it, while in non-DT cases
>> the init and exit callbacks should be used instead. I think it even
>> makes sense to reuse the platform data's init and exit functions in the
>> DT case and implement the parser/interpreter within those.
>
> It totally makes sense indeed.
I don't agree here. It'd be best if non-DT and DT cases worked as
similarly as possible. Relying on callbacks in one case and
data-parsed-from-DT in the other isn't consistent with that. After all,
in the DT case, you parse some data out of the DT and into some data
structure. In the non-DT case, you can have that data structure passed
in directly using platform data. Now, there's certainly a need to
continue to support callbacks for backwards compatibility, at the very
least temporarily before all clients are converted to the new model, but
requiring different models rather than simply allowing it seems like a
bad idea to me.
^ permalink raw reply
* Re: [PATCH v2] of: Add videomode helper
From: Rob Herring @ 2012-07-05 14:51 UTC (permalink / raw)
To: Sascha Hauer
Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart
In-Reply-To: <1341388595-30672-1-git-send-email-s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 07/04/2012 02:56 AM, Sascha Hauer wrote:
> This patch adds a helper function for parsing videomodes from the devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>
> changes since v1:
> - use hyphens instead of underscores for property names
>
> .../devicetree/bindings/video/displaymode | 40 ++++++++
> drivers/of/Kconfig | 5 +
> drivers/of/Makefile | 1 +
> drivers/of/of_videomode.c | 108 ++++++++++++++++++++
> include/linux/of_videomode.h | 19 ++++
> 5 files changed, 173 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/displaymode
> create mode 100644 drivers/of/of_videomode.c
> create mode 100644 include/linux/of_videomode.h
>
> diff --git a/Documentation/devicetree/bindings/video/displaymode b/Documentation/devicetree/bindings/video/displaymode
> new file mode 100644
> index 0000000..43cc17d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,40 @@
> +videomode bindings
> +=========
> +
> +Required properties:
> + - xres, yres: Display resolution
> + - left-margin, right-margin, hsync-len: Horizontal Display timing parameters
> + in pixels
> + upper-margin, lower-margin, vsync-len: Vertical display timing parameters in
> + lines
> + - clock: displayclock in Hz
> +
> +Optional properties:
> + - width-mm, height-mm: Display dimensions in mm
> + - hsync-active-high (bool): Hsync pulse is active high
> + - vsync-active-high (bool): Vsync pulse is active high
> + - interlaced (bool): This is an interlaced mode
> + - doublescan (bool): This is a doublescan mode
> +
> +There are different ways of describing a display mode. The devicetree representation
> +corresponds to the one used by the Linux Framebuffer framework described here in
> +Documentation/fb/framebuffer.txt. This representation has been chosen because it's
> +the only format which does not allow for inconsistent parameters.Unlike the Framebuffer
> +framework the devicetree has the clock in Hz instead of ps.
This implies you are putting linux settings into DT rather than
describing the h/w. I'm not saying the binding is wrong, but documenting
it this way makes it seem so.
One important piece missing (and IIRC linux doesn't really support) is
defining the pixel format of the interface.
> +Example:
> +
> + display@0 {
> + /* 1920x1080p24 */
> + clock = <52000000>;
Should this use the clock binding? You probably need both constraints
and clock binding though.
Often you don't know the frequency up front and/or have limited control
of the frequency (i.e. integer dividers). Then you have to adjust the
margins to get the desired refresh rate. To do that, you need to know
the ranges of values a panel can support. Perhaps you just assume you
can increase the right-margin and lower-margins as I think you will hit
pixel clock frequency max before any limit on margins.
Rob
> + xres = <1920>;
> + yres = <1080>;
> + left-margin = <25>;
> + right-margin = <25>;
> + hsync-len = <25>;
> + lower-margin = <2>;
> + upper-margin = <2>;
> + vsync-len = <2>;
> + hsync-active-high;
> + };
> +
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index dfba3e6..a3acaa3 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -83,4 +83,9 @@ config OF_MTD
> depends on MTD
> def_bool y
>
> +config OF_VIDEOMODE
> + def_bool y
> + help
> + helper to parse videomodes from the devicetree
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index e027f44..80e6db3 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
> obj-$(CONFIG_OF_PCI) += of_pci.o
> obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
> obj-$(CONFIG_OF_MTD) += of_mtd.o
> +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
> diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
> new file mode 100644
> index 0000000..50d3bd2
> --- /dev/null
> +++ b/drivers/of/of_videomode.c
> @@ -0,0 +1,108 @@
> +/*
> + * OF helpers for parsing display modes
> + *
> + * Copyright (c) 2012 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
> + *
> + * This file is released under the GPLv2
> + */
> +#include <linux/of.h>
> +#include <linux/fb.h>
> +#include <linux/export.h>
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc.h>
> +
> +int of_get_video_mode(struct device_node *np, struct drm_display_mode *dmode,
> + struct fb_videomode *fbmode)
> +{
> + int ret = 0;
> + u32 left_margin, xres, right_margin, hsync_len;
> + u32 upper_margin, yres, lower_margin, vsync_len;
> + u32 width_mm = 0, height_mm = 0;
> + u32 clock;
> + bool hah = false, vah = false, interlaced = false, doublescan = false;
> +
> + if (!np)
> + return -EINVAL;
> +
> + ret |= of_property_read_u32(np, "left-margin", &left_margin);
> + ret |= of_property_read_u32(np, "xres", &xres);
> + ret |= of_property_read_u32(np, "right-margin", &right_margin);
> + ret |= of_property_read_u32(np, "hsync-len", &hsync_len);
> + ret |= of_property_read_u32(np, "upper-margin", &upper_margin);
> + ret |= of_property_read_u32(np, "yres", &yres);
> + ret |= of_property_read_u32(np, "lower-margin", &lower_margin);
> + ret |= of_property_read_u32(np, "vsync-len", &vsync_len);
> + ret |= of_property_read_u32(np, "clock", &clock);
> + if (ret)
> + return -EINVAL;
> +
> + of_property_read_u32(np, "width-mm", &width_mm);
> + of_property_read_u32(np, "height-mm", &height_mm);
> +
> + hah = of_property_read_bool(np, "hsync-active-high");
> + vah = of_property_read_bool(np, "vsync-active-high");
> + interlaced = of_property_read_bool(np, "interlaced");
> + doublescan = of_property_read_bool(np, "doublescan");
> +
> + if (dmode) {
> + memset(dmode, 0, sizeof(*dmode));
> +
> + dmode->hdisplay = xres;
> + dmode->hsync_start = xres + right_margin;
> + dmode->hsync_end = xres + right_margin + hsync_len;
> + dmode->htotal = xres + right_margin + hsync_len + left_margin;
> +
> + dmode->vdisplay = yres;
> + dmode->vsync_start = yres + lower_margin;
> + dmode->vsync_end = yres + lower_margin + vsync_len;
> + dmode->vtotal = yres + lower_margin + vsync_len + upper_margin;
> +
> + dmode->width_mm = width_mm;
> + dmode->height_mm = height_mm;
> +
> + dmode->clock = clock / 1000;
> +
> + if (hah)
> + dmode->flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + dmode->flags |= DRM_MODE_FLAG_NHSYNC;
> + if (vah)
> + dmode->flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + dmode->flags |= DRM_MODE_FLAG_NVSYNC;
> + if (interlaced)
> + dmode->flags |= DRM_MODE_FLAG_INTERLACE;
> + if (doublescan)
> + dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> +
> + drm_mode_set_name(dmode);
> + }
> +
> + if (fbmode) {
> + memset(fbmode, 0, sizeof(*fbmode));
> +
> + fbmode->xres = xres;
> + fbmode->left_margin = left_margin;
> + fbmode->right_margin = right_margin;
> + fbmode->hsync_len = hsync_len;
> +
> + fbmode->yres = yres;
> + fbmode->upper_margin = upper_margin;
> + fbmode->lower_margin = lower_margin;
> + fbmode->vsync_len = vsync_len;
> +
> + fbmode->pixclock = KHZ2PICOS(clock / 1000);
> +
> + if (hah)
> + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> + if (vah)
> + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> + if (interlaced)
> + fbmode->vmode |= FB_VMODE_INTERLACED;
> + if (doublescan)
> + fbmode->vmode |= FB_VMODE_DOUBLE;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_get_video_mode);
> diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
> new file mode 100644
> index 0000000..a988429
> --- /dev/null
> +++ b/include/linux/of_videomode.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright 2012 Sascha Hauer <s.hauer@pengutronix.de>
> + *
> + * OF helpers for videomodes.
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_OF_VIDEOMODE_H
> +#define __LINUX_OF_VIDEOMODE_H
> +
> +struct device_node;
> +struct fb_videomode;
> +struct drm_display_mode;
> +
> +int of_get_video_mode(struct device_node *np, struct drm_display_mode *dmode,
> + struct fb_videomode *fbmode);
> +
> +#endif /* __LINUX_OF_VIDEOMODE_H */
>
^ permalink raw reply
* Re: [PATCH v2] of: Add videomode helper
From: Laurent Pinchart @ 2012-07-05 14:08 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev, devicetree-discuss, dri-devel, Grant Likely, kernel,
Mitch Bradley, Anatolij Gustschin
In-Reply-To: <1341388595-30672-1-git-send-email-s.hauer@pengutronix.de>
Hi Sascha,
Thanks for the patch.
On Wednesday 04 July 2012 09:56:35 Sascha Hauer wrote:
> This patch adds a helper function for parsing videomodes from the
> devicetree. The videomode can be either converted to a struct
> drm_display_mode or a struct fb_videomode.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>
> changes since v1:
> - use hyphens instead of underscores for property names
>
> .../devicetree/bindings/video/displaymode | 40 ++++++++
> drivers/of/Kconfig | 5 +
> drivers/of/Makefile | 1 +
> drivers/of/of_videomode.c | 108 +++++++++++++++++
> include/linux/of_videomode.h | 19 ++++
> 5 files changed, 173 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/displaymode
> create mode 100644 drivers/of/of_videomode.c
> create mode 100644 include/linux/of_videomode.h
>
> diff --git a/Documentation/devicetree/bindings/video/displaymode
> b/Documentation/devicetree/bindings/video/displaymode new file mode 100644
> index 0000000..43cc17d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,40 @@
> +videomode bindings
> +=========
> +
> +Required properties:
> + - xres, yres: Display resolution
> + - left-margin, right-margin, hsync-len: Horizontal Display timing
> parameters + in pixels
> + upper-margin, lower-margin, vsync-len: Vertical display timing
> parameters in + lines
> + - clock: displayclock in Hz
> +
> +Optional properties:
> + - width-mm, height-mm: Display dimensions in mm
I've always had mixed feelings about the physical display dimension being part
of the display mode. Those are properties of the panel/display instead of the
mode. Storing them as part of the mode can be convenient, but we then run into
consistency issues (developers have to remember in which display mode
instances the values are available, and in which instances they're set to 0
for instance). If we want to clean this up, this patch would be a good
occasion.
> + - hsync-active-high (bool): Hsync pulse is active high
> + - vsync-active-high (bool): Vsync pulse is active high
> + - interlaced (bool): This is an interlaced mode
> + - doublescan (bool): This is a doublescan mode
> +
> +There are different ways of describing a display mode. The devicetree
> representation +corresponds to the one used by the Linux Framebuffer
> framework described here in +Documentation/fb/framebuffer.txt. This
> representation has been chosen because it's +the only format which does not
> allow for inconsistent parameters.Unlike the Framebuffer +framework the
> devicetree has the clock in Hz instead of ps.
> +
> +Example:
> +
> + display@0 {
> + /* 1920x1080p24 */
> + clock = <52000000>;
> + xres = <1920>;
> + yres = <1080>;
> + left-margin = <25>;
> + right-margin = <25>;
> + hsync-len = <25>;
> + lower-margin = <2>;
> + upper-margin = <2>;
> + vsync-len = <2>;
> + hsync-active-high;
> + };
> +
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Mark Brown @ 2012-07-05 10:41 UTC (permalink / raw)
To: Sascha Hauer
Cc: Alex Courbot, Thierry Reding, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <20120705080234.GQ30009@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 522 bytes --]
On Thu, Jul 05, 2012 at 10:02:34AM +0200, Sascha Hauer wrote:
> So here the regulator core knows that there is no regulator and never
> will be. All that needs to be done is to make _regulator_get look at
> that value.
> There may be some side effects if we just return ERR_PTR(-ENODEV) when
> regulator_dev_lookup returns -ENODEV. Maybe Mark has some comments to
> this.
I'm concerned about how this is going to interact with all the plans
people have for dynamically loading device tree fragments during
enumeration.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Mark Brown @ 2012-07-05 10:39 UTC (permalink / raw)
To: Alex Courbot
Cc: Sascha Hauer, Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4FF5459F.5090201-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1201 bytes --]
On Thu, Jul 05, 2012 at 04:43:27PM +0900, Alex Courbot wrote:
> One could actually question whether the whole regulator/gpio thing
> should be supported at all with platform data. The platform
> interface can use the function hooks in order to implement whatever
> behavior it wants when the light needs to be powered on and off. The
> reason for introducing optional regulator/gpio parameters is because
> the DT cannot use these. Since I have no plan to remove these
> function hooks, making the regulator/gpio option available in
> platform data might be redundant. Any thought about this?
Well, no - it's also done because even if you're not using device tree
(as on most of the architectures we support...) it's not good to have to
cut'n'paste code everywhere. This means that we want to be able to
provide things like GPIOs and regulators via data which means we have
exactly the same situation as we do with device tree.
> >Right now the regulator core will just return -EPROBE_DEFER in both
> >cases. This could easily be changed in the regulator core.
> Could this be because the regulator core cannot make the difference
> between a not-yet-available regulator and a missing one?
Yes.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Mark Brown @ 2012-07-05 10:37 UTC (permalink / raw)
To: Alex Courbot
Cc: Sascha Hauer, Thierry Reding, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <4FF53368.6090805@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 536 bytes --]
On Thu, Jul 05, 2012 at 03:25:44PM +0900, Alex Courbot wrote:
> On 07/05/2012 03:20 PM, Sascha Hauer wrote:
> >I think the only way doing this is to add a flag to platform_data. I
> >don't know if that's accepted though.
> I thought about just checking if devm_get_regulator returned -ENODEV
> and happily continue if that was the case, assuming no regulator was
> declared.
No, that's really not a good idea - as I keep saying if we really want
to go down that line we should remove all error checking instead, it's
the end result.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Alex Courbot @ 2012-07-05 8:12 UTC (permalink / raw)
To: Thierry Reding
Cc: Sascha Hauer, Mark Brown, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <20120705075714.GA26428@avionic-0098.mockup.avionic-design.de>
On 07/05/2012 04:57 PM, Thierry Reding wrote:
> I agree. Non-DT platforms have always used the callbacks to execute this
> kind of code. As you've said before there are situations where it isn't
> just about setting a GPIO or enabling a regulator but it also requires a
> specific timing. Representing this in the platform data would become
> tedious.
That will settle the whole issue then.
> So I think for the DT case you can parse the power-on and power-off
> sequences directly and execute code based on it, while in non-DT cases
> the init and exit callbacks should be used instead. I think it even
> makes sense to reuse the platform data's init and exit functions in the
> DT case and implement the parser/interpreter within those.
It totally makes sense indeed.
> I case where the regulator comes from a DT it should assume that it will
> become available at some point, so -EPROBE_DEFER is correct. However if
> the DT doesn't even contain the power-supply property, then EPROBE_DEFER
> will never work because there's no regulator to become available.
Indeed. And as Sascha mentionned this could easily be fixed. Guess I can
also submit a patch for that while I am at it.
Alex.
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Sascha Hauer @ 2012-07-05 8:02 UTC (permalink / raw)
To: Alex Courbot
Cc: Mark Brown, Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4FF5459F.5090201-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Thu, Jul 05, 2012 at 04:43:27PM +0900, Alex Courbot wrote:
> On 07/05/2012 03:47 PM, Sascha Hauer wrote:
> >>I thought about just checking if devm_get_regulator returned -ENODEV
> >>and happily continue if that was the case, assuming no regulator was
> >>declared.
> >
> >And that's the problem. The get_regulator won't return -ENODEV. It will
> >return -EPROBE_DEFER which tells you nothing about whether a regulator
> >will ever be available or not.
> >
> >Having a flag in platform data would be fine with me, but I know other
> >people think differently.
> >
> >BTW in devicetree this flag implicitely exists with the power-supply
> >property.
>
> One could actually question whether the whole regulator/gpio thing
> should be supported at all with platform data. The platform
> interface can use the function hooks in order to implement whatever
> behavior it wants when the light needs to be powered on and off. The
> reason for introducing optional regulator/gpio parameters is because
> the DT cannot use these. Since I have no plan to remove these
> function hooks, making the regulator/gpio option available in
> platform data might be redundant. Any thought about this?
sounds good.
>
> >Right now the regulator core will just return -EPROBE_DEFER in both
> >cases. This could easily be changed in the regulator core.
>
> Could this be because the regulator core cannot make the difference
> between a not-yet-available regulator and a missing one?
It could. In regulator_dev_lookup we have:
if (node) {
...
} else {
/*
* If we couldn't even get the node then it's
* not just that the device didn't register
* yet, there's no node and we'll never
* succeed.
*/
*ret = -ENODEV;
}
So here the regulator core knows that there is no regulator and never
will be. All that needs to be done is to make _regulator_get look at
that value.
There may be some side effects if we just return ERR_PTR(-ENODEV) when
regulator_dev_lookup returns -ENODEV. Maybe Mark has some comments to
this.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Thierry Reding @ 2012-07-05 7:57 UTC (permalink / raw)
To: Alex Courbot
Cc: Sascha Hauer, Mark Brown,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4FF5459F.5090201-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2430 bytes --]
On Thu, Jul 05, 2012 at 04:43:27PM +0900, Alex Courbot wrote:
> On 07/05/2012 03:47 PM, Sascha Hauer wrote:
> >>I thought about just checking if devm_get_regulator returned -ENODEV
> >>and happily continue if that was the case, assuming no regulator was
> >>declared.
> >
> >And that's the problem. The get_regulator won't return -ENODEV. It will
> >return -EPROBE_DEFER which tells you nothing about whether a regulator
> >will ever be available or not.
> >
> >Having a flag in platform data would be fine with me, but I know other
> >people think differently.
> >
> >BTW in devicetree this flag implicitely exists with the power-supply
> >property.
>
> One could actually question whether the whole regulator/gpio thing
> should be supported at all with platform data. The platform
> interface can use the function hooks in order to implement whatever
> behavior it wants when the light needs to be powered on and off. The
> reason for introducing optional regulator/gpio parameters is because
> the DT cannot use these. Since I have no plan to remove these
> function hooks, making the regulator/gpio option available in
> platform data might be redundant. Any thought about this?
I agree. Non-DT platforms have always used the callbacks to execute this
kind of code. As you've said before there are situations where it isn't
just about setting a GPIO or enabling a regulator but it also requires a
specific timing. Representing this in the platform data would become
tedious.
So I think for the DT case you can parse the power-on and power-off
sequences directly and execute code based on it, while in non-DT cases
the init and exit callbacks should be used instead. I think it even
makes sense to reuse the platform data's init and exit functions in the
DT case and implement the parser/interpreter within those.
> >Right now the regulator core will just return -EPROBE_DEFER in both
> >cases. This could easily be changed in the regulator core.
>
> Could this be because the regulator core cannot make the difference
> between a not-yet-available regulator and a missing one?
I case where the regulator comes from a DT it should assume that it will
become available at some point, so -EPROBE_DEFER is correct. However if
the DT doesn't even contain the power-supply property, then EPROBE_DEFER
will never work because there's no regulator to become available.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Alex Courbot @ 2012-07-05 7:43 UTC (permalink / raw)
To: Sascha Hauer
Cc: Mark Brown, Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20120705064742.GL30009-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 07/05/2012 03:47 PM, Sascha Hauer wrote:
>> I thought about just checking if devm_get_regulator returned -ENODEV
>> and happily continue if that was the case, assuming no regulator was
>> declared.
>
> And that's the problem. The get_regulator won't return -ENODEV. It will
> return -EPROBE_DEFER which tells you nothing about whether a regulator
> will ever be available or not.
>
> Having a flag in platform data would be fine with me, but I know other
> people think differently.
>
> BTW in devicetree this flag implicitely exists with the power-supply
> property.
One could actually question whether the whole regulator/gpio thing
should be supported at all with platform data. The platform interface
can use the function hooks in order to implement whatever behavior it
wants when the light needs to be powered on and off. The reason for
introducing optional regulator/gpio parameters is because the DT cannot
use these. Since I have no plan to remove these function hooks, making
the regulator/gpio option available in platform data might be redundant.
Any thought about this?
> Right now the regulator core will just return -EPROBE_DEFER in both
> cases. This could easily be changed in the regulator core.
Could this be because the regulator core cannot make the difference
between a not-yet-available regulator and a missing one?
Alex.
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Sascha Hauer @ 2012-07-05 6:47 UTC (permalink / raw)
To: Alex Courbot
Cc: Mark Brown, Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4FF53368.6090805-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Thu, Jul 05, 2012 at 03:25:44PM +0900, Alex Courbot wrote:
> On 07/05/2012 03:20 PM, Sascha Hauer wrote:
> >>Oh, that is a mistake of mine then. Driver probe should continue if
> >>no regulator is declared (but should fail if some other error
> >>occured). I want to maintain backward compatibility with current
> >>users of the driver, so regulator/gpio specification should be
> >>optional.
> >
> >I think the only way doing this is to add a flag to platform_data. I
> >don't know if that's accepted though.
>
> I thought about just checking if devm_get_regulator returned -ENODEV
> and happily continue if that was the case, assuming no regulator was
> declared.
And that's the problem. The get_regulator won't return -ENODEV. It will
return -EPROBE_DEFER which tells you nothing about whether a regulator
will ever be available or not.
Having a flag in platform data would be fine with me, but I know other
people think differently.
BTW in devicetree this flag implicitely exists with the power-supply
property. The regulator core could look if a power-supply property
is given and
- if it is given, a regulator is mandatory and the core either
returns the regulator or -EPROBE_DEFER if it cannot find one.
- If it is not given, there is no regulator and the core could either
return a special error code or a dummy regulator.
Right now the regulator core will just return -EPROBE_DEFER in both
cases. This could easily be changed in the regulator core.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Alex Courbot @ 2012-07-05 6:25 UTC (permalink / raw)
To: Sascha Hauer
Cc: Mark Brown, Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20120705062011.GI30009-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 07/05/2012 03:20 PM, Sascha Hauer wrote:
>> Oh, that is a mistake of mine then. Driver probe should continue if
>> no regulator is declared (but should fail if some other error
>> occured). I want to maintain backward compatibility with current
>> users of the driver, so regulator/gpio specification should be
>> optional.
>
> I think the only way doing this is to add a flag to platform_data. I
> don't know if that's accepted though.
I thought about just checking if devm_get_regulator returned -ENODEV and
happily continue if that was the case, assuming no regulator was declared.
But anyway with the power sequences specification this problem becomes
null, since regulators will have to be explicitly declared anyway. I
might be flamed for putting a parser and interpreter into a backlight
driver, but I'll take my chances. :)
Alex.
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Sascha Hauer @ 2012-07-05 6:20 UTC (permalink / raw)
To: Alex Courbot
Cc: Mark Brown, Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4FF4FDC0.8020405-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Thu, Jul 05, 2012 at 11:36:48AM +0900, Alex Courbot wrote:
> On 07/05/2012 12:24 AM, Mark Brown wrote:
> >On Thu, Jul 05, 2012 at 12:14:39AM +0900, Alex Courbot wrote:
> >>On Wed 04 Jul 2012 10:00:56 PM JST, Sascha Hauer wrote:
> >
> >>>Also please be aware that using a regulator in the pwm backlight will
> >>>instantly break all existing users. That's hardly your fault though.
> >
> >>Sorry, I don't see why. Could you elaborate on this?
> >
> >All existing machines will start failing during probe as they won't be
> >able to find the regulator - you should ideally make sure everyone in
> >mainline gets an appropriate regulator set up.
>
> Oh, that is a mistake of mine then. Driver probe should continue if
> no regulator is declared (but should fail if some other error
> occured). I want to maintain backward compatibility with current
> users of the driver, so regulator/gpio specification should be
> optional.
I think the only way doing this is to add a flag to platform_data. I
don't know if that's accepted though.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Alex Courbot @ 2012-07-05 2:36 UTC (permalink / raw)
To: Mark Brown
Cc: Sascha Hauer, Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20120704152451.GA7333-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
On 07/05/2012 12:24 AM, Mark Brown wrote:
> On Thu, Jul 05, 2012 at 12:14:39AM +0900, Alex Courbot wrote:
>> On Wed 04 Jul 2012 10:00:56 PM JST, Sascha Hauer wrote:
>
>>> Also please be aware that using a regulator in the pwm backlight will
>>> instantly break all existing users. That's hardly your fault though.
>
>> Sorry, I don't see why. Could you elaborate on this?
>
> All existing machines will start failing during probe as they won't be
> able to find the regulator - you should ideally make sure everyone in
> mainline gets an appropriate regulator set up.
Oh, that is a mistake of mine then. Driver probe should continue if no
regulator is declared (but should fail if some other error occured). I
want to maintain backward compatibility with current users of the
driver, so regulator/gpio specification should be optional.
Thanks for all the feedback people - I will come back with a new version
that addresses the points highlighted and also allows power on/off
sequences to be specified in the device tree.
Alex.
^ permalink raw reply
* [PATCH 2/2] fbcon: move bit_putcs() into separate source file
From: David Herrmann @ 2012-07-04 21:48 UTC (permalink / raw)
To: linux-serial
Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel, alan,
gregkh, David Herrmann
In-Reply-To: <1341438514-1786-1-git-send-email-dh.herrmann@googlemail.com>
If we want to use font-draw-operations in other modules than fbcon, we
need to split this function off of fbcon headers and sources. This also
makes bit_putcs() totally independent of vc_* and fbcon_* structures.
As scr_read() cannot be called inside of non-fbcon/vt functions, we need
to assemble the buffer before passing it to fbdraw_font(). This slows down
this operations a little bit but my rough benchmark showed that it didn't
really matter. Anyway, if it does, we can still put it into VT_BUF_HAVE_RW
conditions so platforms that don't use it won't be affected. And for other
platforms we can add the buffer to the vc-struct so we can reuse it.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/video/console/bitblit.c | 127 ++++------------------------------------
drivers/video/console/fbdraw.c | 125 +++++++++++++++++++++++++++++++++++++++
drivers/video/console/fbdraw.h | 4 ++
3 files changed, 139 insertions(+), 117 deletions(-)
diff --git a/drivers/video/console/bitblit.c b/drivers/video/console/bitblit.c
index 6ec2905..c5d897b 100644
--- a/drivers/video/console/bitblit.c
+++ b/drivers/video/console/bitblit.c
@@ -54,132 +54,25 @@ static void bit_clear(struct vc_data *vc, struct fb_info *info, int sy,
info->fbops->fb_fillrect(info, ®ion);
}
-static inline void bit_putcs_aligned(struct vc_data *vc, struct fb_info *info,
- const u16 *s, u32 attr, u32 cnt,
- u32 d_pitch, u32 s_pitch, u32 cellsize,
- struct fb_image *image, u8 *buf, u8 *dst)
-{
- u16 charmask = vc->vc_hi_font_mask ? 0x1ff : 0xff;
- u32 idx = vc->vc_font.width >> 3;
- u8 *src;
-
- while (cnt--) {
- src = vc->vc_font.data + (scr_readw(s++)&
- charmask)*cellsize;
-
- if (attr) {
- fbdraw_update_attr(buf, src, attr, &vc->vc_font);
- src = buf;
- }
-
- if (likely(idx = 1))
- __fb_pad_aligned_buffer(dst, d_pitch, src, idx,
- image->height);
- else
- fb_pad_aligned_buffer(dst, d_pitch, src, idx,
- image->height);
-
- dst += s_pitch;
- }
-
- info->fbops->fb_imageblit(info, image);
-}
-
-static inline void bit_putcs_unaligned(struct vc_data *vc,
- struct fb_info *info, const u16 *s,
- u32 attr, u32 cnt, u32 d_pitch,
- u32 s_pitch, u32 cellsize,
- struct fb_image *image, u8 *buf,
- u8 *dst)
-{
- u16 charmask = vc->vc_hi_font_mask ? 0x1ff : 0xff;
- u32 shift_low = 0, mod = vc->vc_font.width % 8;
- u32 shift_high = 8;
- u32 idx = vc->vc_font.width >> 3;
- u8 *src;
-
- while (cnt--) {
- src = vc->vc_font.data + (scr_readw(s++)&
- charmask)*cellsize;
-
- if (attr) {
- fbdraw_update_attr(buf, src, attr, &vc->vc_font);
- src = buf;
- }
-
- fb_pad_unaligned_buffer(dst, d_pitch, src, idx,
- image->height, shift_high,
- shift_low, mod);
- shift_low += mod;
- dst += (shift_low >= 8) ? s_pitch : s_pitch - 1;
- shift_low &= 7;
- shift_high = 8 - shift_low;
- }
-
- info->fbops->fb_imageblit(info, image);
-
-}
-
static void bit_putcs(struct vc_data *vc, struct fb_info *info,
const unsigned short *s, int count, int yy, int xx,
int fg, int bg)
{
- struct fb_image image;
- u32 width = DIV_ROUND_UP(vc->vc_font.width, 8);
- u32 cellsize = width * vc->vc_font.height;
- u32 maxcnt = info->pixmap.size/cellsize;
- u32 scan_align = info->pixmap.scan_align - 1;
- u32 buf_align = info->pixmap.buf_align - 1;
- u32 mod = vc->vc_font.width % 8, cnt, pitch, size;
+ u16 *buf;
+ int i;
u32 attribute = get_attribute(info, scr_readw(s));
- u8 *dst, *buf = NULL;
- image.fg_color = fg;
- image.bg_color = bg;
- image.dx = xx * vc->vc_font.width;
- image.dy = yy * vc->vc_font.height;
- image.height = vc->vc_font.height;
- image.depth = 1;
+ buf = kmalloc(sizeof(*buf) * count, GFP_KERNEL);
+ if (!buf)
+ return;
- if (attribute) {
- buf = kmalloc(cellsize, GFP_KERNEL);
- if (!buf)
- return;
- }
-
- while (count) {
- if (count > maxcnt)
- cnt = maxcnt;
- else
- cnt = count;
-
- image.width = vc->vc_font.width * cnt;
- pitch = DIV_ROUND_UP(image.width, 8) + scan_align;
- pitch &= ~scan_align;
- size = pitch * image.height + buf_align;
- size &= ~buf_align;
- dst = fb_get_buffer_offset(info, &info->pixmap, size);
- image.data = dst;
-
- if (!mod)
- bit_putcs_aligned(vc, info, s, attribute, cnt, pitch,
- width, cellsize, &image, buf, dst);
- else
- bit_putcs_unaligned(vc, info, s, attribute, cnt,
- pitch, width, cellsize, &image,
- buf, dst);
-
- image.dx += cnt * vc->vc_font.width;
- count -= cnt;
- s += cnt;
- }
+ for (i = 0; i < count; ++i)
+ buf[i] = scr_readw(s++);
- /* buf is always NULL except when in monochrome mode, so in this case
- it's a gain to check buf against NULL even though kfree() handles
- NULL pointers just fine */
- if (unlikely(buf))
- kfree(buf);
+ fbdraw_font(info, &vc->vc_font, vc->vc_hi_font_mask, xx, yy, fg, bg,
+ attribute, buf, count);
+ kfree(buf);
}
static void bit_clear_margins(struct vc_data *vc, struct fb_info *info,
diff --git a/drivers/video/console/fbdraw.c b/drivers/video/console/fbdraw.c
index 65e7003..e2e0661 100644
--- a/drivers/video/console/fbdraw.c
+++ b/drivers/video/console/fbdraw.c
@@ -41,6 +41,131 @@ void fbdraw_update_attr(u8 *dst, const u8 *src, int attribute,
}
EXPORT_SYMBOL(fbdraw_update_attr);
+static inline void bit_putcs_aligned(struct fb_info *info, bool hi_font,
+ struct console_font *font, u32 attribute,
+ u32 d_pitch, u32 s_pitch, u32 cellsize,
+ struct fb_image *image, u8 *buf, u8 *dst,
+ const u16 *chars, size_t cnt)
+{
+ u16 charmask = hi_font ? 0x1ff : 0xff;
+ u32 idx = font->width >> 3;
+ u8 *src;
+
+ while (cnt--) {
+ src = font->data + ((*chars++) & charmask) * cellsize;
+
+ if (attribute) {
+ fbdraw_update_attr(buf, src, attribute, font);
+ src = buf;
+ }
+
+ if (likely(idx = 1))
+ __fb_pad_aligned_buffer(dst, d_pitch, src, idx,
+ image->height);
+ else
+ fb_pad_aligned_buffer(dst, d_pitch, src, idx,
+ image->height);
+
+ dst += s_pitch;
+ }
+
+ info->fbops->fb_imageblit(info, image);
+}
+
+static inline void bit_putcs_unaligned(struct fb_info *info, bool hi_font,
+ struct console_font *font, u32 attribute,
+ u32 d_pitch, u32 s_pitch, u32 cellsize,
+ struct fb_image *image, u8 *buf, u8 *dst,
+ const u16 *chars, size_t cnt)
+{
+ u16 charmask = hi_font ? 0x1ff : 0xff;
+ u32 shift_low = 0, mod = font->width % 8;
+ u32 shift_high = 8;
+ u32 idx = font->width >> 3;
+ u8 *src;
+
+ while (cnt--) {
+ src = font->data + ((*chars++) & charmask) * cellsize;
+
+ if (attribute) {
+ fbdraw_update_attr(buf, src, attribute, font);
+ src = buf;
+ }
+
+ fb_pad_unaligned_buffer(dst, d_pitch, src, idx,
+ image->height, shift_high,
+ shift_low, mod);
+ shift_low += mod;
+ dst += (shift_low >= 8) ? s_pitch : s_pitch - 1;
+ shift_low &= 7;
+ shift_high = 8 - shift_low;
+ }
+
+ info->fbops->fb_imageblit(info, image);
+}
+
+void fbdraw_font(struct fb_info *info, struct console_font *font, bool hi_font,
+ unsigned int xpos, unsigned int ypos, int fg, int bg,
+ u32 attribute, const u16 *chars, size_t count)
+{
+ struct fb_image image;
+ u32 width = DIV_ROUND_UP(font->width, 8);
+ u32 cellsize = width * font->height;
+ u32 maxcnt = info->pixmap.size / cellsize;
+ u32 scan_align = info->pixmap.scan_align - 1;
+ u32 buf_align = info->pixmap.buf_align - 1;
+ u32 mod = font->width % 8, cnt, pitch, size;
+ u8 *dst, *buf = NULL;
+
+ image.fg_color = fg;
+ image.bg_color = bg;
+ image.dx = xpos * font->width;
+ image.dy = ypos * font->height;
+ image.height = font->height;
+ image.depth = 1;
+
+ if (attribute) {
+ buf = kmalloc(cellsize, GFP_KERNEL);
+ if (!buf)
+ return;
+ }
+
+ while (count) {
+ if (count > maxcnt)
+ cnt = maxcnt;
+ else
+ cnt = count;
+
+ image.width = font->width * cnt;
+ pitch = DIV_ROUND_UP(image.width, 8) + scan_align;
+ pitch &= ~scan_align;
+ size = pitch * image.height + buf_align;
+ size &= ~buf_align;
+ dst = fb_get_buffer_offset(info, &info->pixmap, size);
+ image.data = dst;
+
+ if (!mod)
+ bit_putcs_aligned(info, hi_font, font, attribute,
+ pitch, width, cellsize, &image, buf,
+ dst, chars, cnt);
+ else
+ bit_putcs_unaligned(info, hi_font, font, attribute,
+ pitch, width, cellsize, &image, buf,
+ dst, chars, cnt);
+
+ image.dx += cnt * font->width;
+ count -= cnt;
+ chars += cnt;
+ }
+
+ /* buf is always NULL except when in monochrome mode, so in this case
+ it's a gain to check buf against NULL even though kfree() handles
+ NULL pointers just fine */
+ if (unlikely(buf))
+ kfree(buf);
+}
+EXPORT_SYMBOL(fbdraw_font);
+
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("David Herrmann <dh.herrmann@googlemail.com>");
MODULE_DESCRIPTION("Framebuffer helpers for image draw-operations");
diff --git a/drivers/video/console/fbdraw.h b/drivers/video/console/fbdraw.h
index 77edd7f..b9f1ffa 100644
--- a/drivers/video/console/fbdraw.h
+++ b/drivers/video/console/fbdraw.h
@@ -23,4 +23,8 @@
void fbdraw_update_attr(u8 *dst, const u8 *src, int attribute,
struct console_font *font);
+void fbdraw_font(struct fb_info *info, struct console_font *font, bool hi_font,
+ unsigned int xpos, unsigned int ypos, int fg, int bg,
+ u32 attribute, const u16 *chars, size_t count);
+
#endif /* _VIDEO_FBDRAW_H */
--
1.7.11.1
^ permalink raw reply related
* [PATCH 1/2] fbcon: move update_attr() into separate source file
From: David Herrmann @ 2012-07-04 21:48 UTC (permalink / raw)
To: linux-serial
Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel, alan,
gregkh, David Herrmann
In-Reply-To: <1341438514-1786-1-git-send-email-dh.herrmann@googlemail.com>
If we want to use update_attr() independently from fbcon, we need to split
it off from bitblit.c and fbcon.h. Therefore, introduce a new header and
source file (fbdraw.[ch]) which does not depende on vc_* and fbcon_*
structures in any way.
This does not introduce any new code nor does it make the paths deeper,
it simply splits the function off.
The other update_attr() functions (inside the rotation sources) seem
similar but are significantly different and I haven't found a way to merge
them efficiently (which is probably the reason why they are split off
now). Furthermore, I do not intend to use them in the coming code so there
is no need to split them off.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/video/console/Makefile | 3 ++-
drivers/video/console/bitblit.c | 26 +++--------------------
drivers/video/console/fbcon.h | 5 +----
drivers/video/console/fbdraw.c | 46 +++++++++++++++++++++++++++++++++++++++++
drivers/video/console/fbdraw.h | 26 +++++++++++++++++++++++
5 files changed, 78 insertions(+), 28 deletions(-)
create mode 100644 drivers/video/console/fbdraw.c
create mode 100644 drivers/video/console/fbdraw.h
diff --git a/drivers/video/console/Makefile b/drivers/video/console/Makefile
index a862e91..9a52226 100644
--- a/drivers/video/console/Makefile
+++ b/drivers/video/console/Makefile
@@ -25,7 +25,8 @@ obj-$(CONFIG_SGI_NEWPORT_CONSOLE) += newport_con.o font.o
obj-$(CONFIG_STI_CONSOLE) += sticon.o sticore.o font.o
obj-$(CONFIG_VGA_CONSOLE) += vgacon.o
obj-$(CONFIG_MDA_CONSOLE) += mdacon.o
-obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon.o bitblit.o font.o softcursor.o
+obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon.o bitblit.o font.o softcursor.o \
+ fbdraw.o
ifeq ($(CONFIG_FB_TILEBLITTING),y)
obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += tileblit.o
endif
diff --git a/drivers/video/console/bitblit.c b/drivers/video/console/bitblit.c
index 28b1a83..6ec2905 100644
--- a/drivers/video/console/bitblit.c
+++ b/drivers/video/console/bitblit.c
@@ -22,26 +22,6 @@
/*
* Accelerated handlers.
*/
-static void update_attr(u8 *dst, u8 *src, int attribute,
- struct vc_data *vc)
-{
- int i, offset = (vc->vc_font.height < 10) ? 1 : 2;
- int width = DIV_ROUND_UP(vc->vc_font.width, 8);
- unsigned int cellsize = vc->vc_font.height * width;
- u8 c;
-
- offset = cellsize - (offset * width);
- for (i = 0; i < cellsize; i++) {
- c = src[i];
- if (attribute & FBCON_ATTRIBUTE_UNDERLINE && i >= offset)
- c = 0xff;
- if (attribute & FBCON_ATTRIBUTE_BOLD)
- c |= c >> 1;
- if (attribute & FBCON_ATTRIBUTE_REVERSE)
- c = ~c;
- dst[i] = c;
- }
-}
static void bit_bmove(struct vc_data *vc, struct fb_info *info, int sy,
int sx, int dy, int dx, int height, int width)
@@ -88,7 +68,7 @@ static inline void bit_putcs_aligned(struct vc_data *vc, struct fb_info *info,
charmask)*cellsize;
if (attr) {
- update_attr(buf, src, attr, vc);
+ fbdraw_update_attr(buf, src, attr, &vc->vc_font);
src = buf;
}
@@ -123,7 +103,7 @@ static inline void bit_putcs_unaligned(struct vc_data *vc,
charmask)*cellsize;
if (attr) {
- update_attr(buf, src, attr, vc);
+ fbdraw_update_attr(buf, src, attr, &vc->vc_font);
src = buf;
}
@@ -275,7 +255,7 @@ static void bit_cursor(struct vc_data *vc, struct fb_info *info, int mode,
return;
kfree(ops->cursor_data);
ops->cursor_data = dst;
- update_attr(dst, src, attribute, vc);
+ fbdraw_update_attr(dst, src, attribute, &vc->vc_font);
src = dst;
}
diff --git a/drivers/video/console/fbcon.h b/drivers/video/console/fbcon.h
index 6bd2e0c..8623bac 100644
--- a/drivers/video/console/fbcon.h
+++ b/drivers/video/console/fbcon.h
@@ -16,6 +16,7 @@
#include <linux/vt_kern.h>
#include <asm/io.h>
+#include "fbdraw.h"
#define FBCON_FLAGS_INIT 1
#define FBCON_FLAGS_CURSOR_TIMER 2
@@ -219,10 +220,6 @@ extern void fbcon_set_tileops(struct vc_data *vc, struct fb_info *info);
extern void fbcon_set_bitops(struct fbcon_ops *ops);
extern int soft_cursor(struct fb_info *info, struct fb_cursor *cursor);
-#define FBCON_ATTRIBUTE_UNDERLINE 1
-#define FBCON_ATTRIBUTE_REVERSE 2
-#define FBCON_ATTRIBUTE_BOLD 4
-
static inline int real_y(struct display *p, int ypos)
{
int rows = p->vrows;
diff --git a/drivers/video/console/fbdraw.c b/drivers/video/console/fbdraw.c
new file mode 100644
index 0000000..65e7003
--- /dev/null
+++ b/drivers/video/console/fbdraw.c
@@ -0,0 +1,46 @@
+/*
+ * Framebuffer helpers for image draw-operations
+ *
+ * Copyright (c) 2004 Antonino Daplas <adaplas @pol.net>
+ * Copyright (c) 2012 David Herrmann <dh.herrmann@googlemail.com>
+ *
+ * Originally from drivers/video/console/bitblit.c which itself originally is
+ * from the 'accel_*' routines in drivers/video/console/fbcon.c.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive for
+ * more details.
+ */
+
+#include <linux/console.h>
+#include <linux/fb.h>
+#include <linux/kd.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include "fbdraw.h"
+
+void fbdraw_update_attr(u8 *dst, const u8 *src, int attribute,
+ struct console_font *font)
+{
+ int i, offset = (font->height < 10) ? 1 : 2;
+ int width = DIV_ROUND_UP(font->width, 8);
+ unsigned int cellsize = font->height * width;
+ u8 c;
+
+ offset = cellsize - (offset * width);
+ for (i = 0; i < cellsize; i++) {
+ c = src[i];
+ if (attribute & FBCON_ATTRIBUTE_UNDERLINE && i >= offset)
+ c = 0xff;
+ if (attribute & FBCON_ATTRIBUTE_BOLD)
+ c |= c >> 1;
+ if (attribute & FBCON_ATTRIBUTE_REVERSE)
+ c = ~c;
+ dst[i] = c;
+ }
+}
+EXPORT_SYMBOL(fbdraw_update_attr);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("David Herrmann <dh.herrmann@googlemail.com>");
+MODULE_DESCRIPTION("Framebuffer helpers for image draw-operations");
diff --git a/drivers/video/console/fbdraw.h b/drivers/video/console/fbdraw.h
new file mode 100644
index 0000000..77edd7f
--- /dev/null
+++ b/drivers/video/console/fbdraw.h
@@ -0,0 +1,26 @@
+/*
+ * Framebuffer helpers for image draw-operations
+ *
+ * Copyright (c) 2012 David Herrmann <dh.herrmann@googlemail.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive for
+ * more details.
+ */
+
+#ifndef _VIDEO_FBDRAW_H
+#define _VIDEO_FBDRAW_H
+
+#include <linux/console.h>
+#include <linux/fb.h>
+#include <linux/kd.h>
+
+/* fbcon character attributes */
+#define FBCON_ATTRIBUTE_UNDERLINE 1
+#define FBCON_ATTRIBUTE_REVERSE 2
+#define FBCON_ATTRIBUTE_BOLD 4
+
+void fbdraw_update_attr(u8 *dst, const u8 *src, int attribute,
+ struct console_font *font);
+
+#endif /* _VIDEO_FBDRAW_H */
--
1.7.11.1
^ permalink raw reply related
* [PATCH 0/2] Split off bitblit operations for fblog
From: David Herrmann @ 2012-07-04 21:48 UTC (permalink / raw)
To: linux-serial
Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel, alan,
gregkh, David Herrmann
Hi
This is a patchset that moves update_attr() and bit_putcs() into a separate
source file and makes both files not depend on any vc_* or fbcon_* structures
and code. That is, we can now safely use both in the fblog patchset that I sent
last month. Otherwise, we would need to copy both functionalities.
The rotation helpers have very similar functions which do the same but with
rotation in mind. I didn't find an efficient way to merge them so I decided to
leave them as they are. Furthermore, I am not using them so there is no need to
split them off.
I actually don't know who is taking patches for fblog and which trees they are
going through, but if you think I should include the patches in the fblog
patchset instead, I will do so. Otherwise, it would be nice if they could be
applied separately. The second fblog patchset will follow tomorrow.
I also tested both patches on my machine and couldn't notice any performance
breakdowns, but it would be nice if someone else could test them, too. The
second patch mentions this shortly, regarding the additional kmalloc().
Regards
David
David Herrmann (2):
fbcon: move update_attr() into separate source file
fbcon: move bit_putcs() into separate source file
drivers/video/console/Makefile | 3 +-
drivers/video/console/bitblit.c | 149 +++-------------------------------
drivers/video/console/fbcon.h | 5 +-
drivers/video/console/fbdraw.c | 171 ++++++++++++++++++++++++++++++++++++++++
drivers/video/console/fbdraw.h | 30 +++++++
5 files changed, 215 insertions(+), 143 deletions(-)
create mode 100644 drivers/video/console/fbdraw.c
create mode 100644 drivers/video/console/fbdraw.h
--
1.7.11.1
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Sascha Hauer @ 2012-07-04 20:26 UTC (permalink / raw)
To: Alex Courbot
Cc: Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4FF45DDF.9000306-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Thu, Jul 05, 2012 at 12:14:39AM +0900, Alex Courbot wrote:
> On Wed 04 Jul 2012 10:00:56 PM JST, Sascha Hauer wrote:
> >>That's very bad indeed. I assumed that the kernel would free
> >>devm-allocated resources after probe returned -EPROBE_DEFER,
> >
> >It indeed does free devm allocated resources, but neither the gpio nor
> >the pwm are devm allocated.
>
> As far as I can tell the gpio is allocated through devm as well:
>
> > + ret = devm_gpio_request_one(&pdev->dev, data->enable_gpio,
> > + GPIOF_OUT_INIT_HIGH, "backlight_enable");
You're right. For the GPIO it's ok the way it is.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Mark Brown @ 2012-07-04 15:24 UTC (permalink / raw)
To: Alex Courbot
Cc: Sascha Hauer, Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4FF45DDF.9000306-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Thu, Jul 05, 2012 at 12:14:39AM +0900, Alex Courbot wrote:
> On Wed 04 Jul 2012 10:00:56 PM JST, Sascha Hauer wrote:
> >Also please be aware that using a regulator in the pwm backlight will
> >instantly break all existing users. That's hardly your fault though.
> Sorry, I don't see why. Could you elaborate on this?
All existing machines will start failing during probe as they won't be
able to find the regulator - you should ideally make sure everyone in
mainline gets an appropriate regulator set up.
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Alex Courbot @ 2012-07-04 15:14 UTC (permalink / raw)
To: Sascha Hauer
Cc: Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20120704130056.GC30009-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Wed 04 Jul 2012 10:00:56 PM JST, Sascha Hauer wrote:
>> That's very bad indeed. I assumed that the kernel would free
>> devm-allocated resources after probe returned -EPROBE_DEFER,
>
> It indeed does free devm allocated resources, but neither the gpio nor
> the pwm are devm allocated.
As far as I can tell the gpio is allocated through devm as well:
> + ret = devm_gpio_request_one(&pdev->dev, data->enable_gpio,
> + GPIOF_OUT_INIT_HIGH, "backlight_enable");
Thus if it is not reclaimed with probe returns with -EPROBE_DEFER, then
I guess something is going wrong elsewhere. You are right that the PWM
should be freed by the driver thought.
> Also please be aware that using a regulator in the pwm backlight will
> instantly break all existing users. That's hardly your fault though.
Sorry, I don't see why. Could you elaborate on this?
Thanks,
Alex.
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Sascha Hauer @ 2012-07-04 13:00 UTC (permalink / raw)
To: Alex Courbot
Cc: Thierry Reding,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4FF43692.2040805-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Wed, Jul 04, 2012 at 09:26:58PM +0900, Alex Courbot wrote:
> Hi Sascha,
>
> On 07/04/2012 07:48 PM, Sascha Hauer wrote:>> +
> >> + pb->power_reg = devm_regulator_get(&pdev->dev, "power");
> >> + if (IS_ERR(pb->power_reg))
> >> + return PTR_ERR(pb->power_reg);
> >
> > This looses several resources allocated earlier, like the enable gpio
> > and the pwm. This is really bad here since I have no regulator specified
> > and devm_regulator_get returns with -EPROBE_DEFER. Next time the core
> > tries to probe the driver the driver bails out because the gpio and the
> > pwm is already requested.
>
> That's very bad indeed. I assumed that the kernel would free
> devm-allocated resources after probe returned -EPROBE_DEFER,
It indeed does free devm allocated resources, but neither the gpio nor
the pwm are devm allocated.
Also please be aware that using a regulator in the pwm backlight will
instantly break all existing users. That's hardly your fault though.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH 2/2] OMAPDSS: fix warnings if CONFIG_PM_RUNTIME=n
From: Archit Taneja @ 2012-07-04 12:55 UTC (permalink / raw)
To: FlorianSchandinat
Cc: tomi.valkeinen, jaswinder.singh, jw, linux-fbdev, linux-omap,
Grazvydas Ignotas
In-Reply-To: <1341405829-16964-1-git-send-email-archit@ti.com>
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
If runtime PM is not enabled in the kernel config, pm_runtime_get_sync()
will always return 1 and pm_runtime_put_sync() will always return
-ENOSYS. pm_runtime_get_sync() returning 1 presents no problem to the
driver, but -ENOSYS from pm_runtime_put_sync() causes the driver to
print a warning.
One option would be to ignore errors returned by pm_runtime_put_sync()
totally, as they only say that the call was unable to put the hardware
into suspend mode.
However, I chose to ignore the returned -ENOSYS explicitly, and print a
warning for other errors, as I think we should get notified if the HW
failed to go to suspend properly.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jassi Brar <jaswinder.singh@linaro.org>
Cc: Grazvydas Ignotas <notasas@gmail.com>
---
drivers/video/omap2/dss/dispc.c | 2 +-
drivers/video/omap2/dss/dsi.c | 2 +-
drivers/video/omap2/dss/dss.c | 2 +-
drivers/video/omap2/dss/hdmi.c | 2 +-
drivers/video/omap2/dss/rfbi.c | 2 +-
drivers/video/omap2/dss/venc.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 4749ac3..397d4ee 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -384,7 +384,7 @@ void dispc_runtime_put(void)
DSSDBG("dispc_runtime_put\n");
r = pm_runtime_put_sync(&dispc.pdev->dev);
- WARN_ON(r < 0);
+ WARN_ON(r < 0 && r != -ENOSYS);
}
static inline bool dispc_mgr_is_lcd(enum omap_channel channel)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index ca8382d..14ce8cc 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1075,7 +1075,7 @@ void dsi_runtime_put(struct platform_device *dsidev)
DSSDBG("dsi_runtime_put\n");
r = pm_runtime_put_sync(&dsi->pdev->dev);
- WARN_ON(r < 0);
+ WARN_ON(r < 0 && r != -ENOSYS);
}
/* source clock for DSI PLL. this could also be PCLKFREE */
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 7706323..d2b5719 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -731,7 +731,7 @@ static void dss_runtime_put(void)
DSSDBG("dss_runtime_put\n");
r = pm_runtime_put_sync(&dss.pdev->dev);
- WARN_ON(r < 0 && r != -EBUSY);
+ WARN_ON(r < 0 && r != -ENOSYS && r != -EBUSY);
}
/* DEBUGFS */
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 8195c71..26a2430 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -138,7 +138,7 @@ static void hdmi_runtime_put(void)
DSSDBG("hdmi_runtime_put\n");
r = pm_runtime_put_sync(&hdmi.pdev->dev);
- WARN_ON(r < 0);
+ WARN_ON(r < 0 && r != -ENOSYS);
}
static int __init hdmi_init_display(struct omap_dss_device *dssdev)
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index 3d8c206..7985fa1 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -141,7 +141,7 @@ static void rfbi_runtime_put(void)
DSSDBG("rfbi_runtime_put\n");
r = pm_runtime_put_sync(&rfbi.pdev->dev);
- WARN_ON(r < 0);
+ WARN_ON(r < 0 && r != -ENOSYS);
}
void rfbi_bus_lock(void)
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 2b89739..3907c8b 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -402,7 +402,7 @@ static void venc_runtime_put(void)
DSSDBG("venc_runtime_put\n");
r = pm_runtime_put_sync(&venc.pdev->dev);
- WARN_ON(r < 0);
+ WARN_ON(r < 0 && r != -ENOSYS);
}
static const struct venc_config *venc_timings_to_config(
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/2] OMAPDSS: Use PM notifiers for system suspend
From: Archit Taneja @ 2012-07-04 12:55 UTC (permalink / raw)
To: FlorianSchandinat
Cc: tomi.valkeinen, jaswinder.singh, jw, linux-fbdev, linux-omap
In-Reply-To: <1341405829-16964-1-git-send-email-archit@ti.com>
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
The current way how omapdss handles system suspend and resume is that
omapdss device (a platform device, which is not part of the device
hierarchy of the DSS HW devices, like DISPC and DSI, or panels.) uses
the suspend and resume callbacks from platform_driver to handle system
suspend. It does this by disabling all enabled panels on suspend, and
resuming the previously disabled panels on resume.
This presents a few problems.
One is that as omapdss device is not related to the panel devices or the
DSS HW devices, there's no ordering in the suspend process. This means
that suspend could be first ran for DSS HW devices and panels, and only
then for omapdss device. Currently this is not a problem, as DSS HW
devices and panels do not handle suspend.
Another, more pressing problem, is that when suspending or resuming, the
runtime PM functions return -EACCES as runtime PM is disabled during
system suspend. This causes the driver to print warnings, and operations
to fail as they think that they failed to bring up the HW.
This patch changes the omapdss suspend handling to use PM notifiers,
which are called before suspend and after resume. This way we have a
normally functioning system when we are suspending and resuming the
panels.
This patch, I believe, creates a problem that somebody could enable or
disable a panel between PM_SUSPEND_PREPARE and the system suspend, and
similarly the other way around in resume. I choose to ignore the problem
for now, as it sounds rather unlikely, and if it happens, it's not
fatal.
In the long run the system suspend handling of omapdss and panels should
be thought out properly. The current approach feels rather hacky.
Perhaps the panel drivers should handle system suspend, or the users of
omapdss (omapfb, omapdrm) should handle system suspend.
Note that after this patch we could probably revert
0eaf9f52e94f756147dbfe1faf1f77a02378dbf9 (OMAPDSS: use sync versions of
pm_runtime_put). But as I said, this patch may be temporary, so let's
leave the sync version still in place.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reported-by: Jassi Brar <jaswinder.singh@linaro.org>
Tested-by: Jassi Brar <jaswinder.singh@linaro.org>
Tested-by: Joe Woodward <jw@terrafix.co.uk>
---
drivers/video/omap2/dss/core.c | 45 ++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 16 deletions(-)
diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index 5066eee..c35a248 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -32,6 +32,7 @@
#include <linux/io.h>
#include <linux/device.h>
#include <linux/regulator/consumer.h>
+#include <linux/suspend.h>
#include <video/omapdss.h>
@@ -201,6 +202,30 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
#endif /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUG_SUPPORT */
/* PLATFORM DEVICE */
+static int omap_dss_pm_notif(struct notifier_block *b, unsigned long v, void *d)
+{
+ DSSDBG("pm notif %lu\n", v);
+
+ switch (v)
+ {
+ case PM_SUSPEND_PREPARE:
+ DSSDBG("suspending displays\n");
+ return dss_suspend_all_devices();
+
+ case PM_POST_SUSPEND:
+ DSSDBG("resuming displays\n");
+ return dss_resume_all_devices();
+
+ default:
+ return 0;
+ }
+}
+
+static struct notifier_block omap_dss_pm_notif_block +{
+ .notifier_call = omap_dss_pm_notif,
+};
+
static int __init omap_dss_probe(struct platform_device *pdev)
{
struct omap_dss_board_info *pdata = pdev->dev.platform_data;
@@ -224,6 +249,8 @@ static int __init omap_dss_probe(struct platform_device *pdev)
else if (pdata->default_device)
core.default_display_name = pdata->default_device->name;
+ register_pm_notifier(&omap_dss_pm_notif_block);
+
return 0;
err_debugfs:
@@ -233,6 +260,8 @@ err_debugfs:
static int omap_dss_remove(struct platform_device *pdev)
{
+ unregister_pm_notifier(&omap_dss_pm_notif_block);
+
dss_uninitialize_debugfs();
dss_uninit_overlays(pdev);
@@ -247,25 +276,9 @@ static void omap_dss_shutdown(struct platform_device *pdev)
dss_disable_all_devices();
}
-static int omap_dss_suspend(struct platform_device *pdev, pm_message_t state)
-{
- DSSDBG("suspend %d\n", state.event);
-
- return dss_suspend_all_devices();
-}
-
-static int omap_dss_resume(struct platform_device *pdev)
-{
- DSSDBG("resume\n");
-
- return dss_resume_all_devices();
-}
-
static struct platform_driver omap_dss_driver = {
.remove = omap_dss_remove,
.shutdown = omap_dss_shutdown,
- .suspend = omap_dss_suspend,
- .resume = omap_dss_resume,
.driver = {
.name = "omapdss",
.owner = THIS_MODULE,
--
1.7.9.5
^ permalink raw reply related
* [PATCH 0/2] OMAPDSS: PM runtime fixes for 3.5-rc
From: Archit Taneja @ 2012-07-04 12:55 UTC (permalink / raw)
To: FlorianSchandinat
Cc: tomi.valkeinen, jaswinder.singh, jw, linux-fbdev, linux-omap,
Archit Taneja
Hi Florian,
These are 2 fixes which Tomi had intended to push for the coming rcs. He is
currently on vacation and asked me to post the patches on his behalf.
The first patch ensures that system suspend doesn't break on OMAP if DSS is
enabled. The second patch prevents some warning backtraces when the kernel is
not built with PM runtime.
Could you please queue these for the upcoming rc?
Thanks,
Archit
Tomi Valkeinen (2):
OMAPDSS: Use PM notifiers for system suspend
OMAPDSS: fix warnings if CONFIG_PM_RUNTIME=n
drivers/video/omap2/dss/core.c | 45 +++++++++++++++++++++++++--------------
drivers/video/omap2/dss/dispc.c | 2 +-
drivers/video/omap2/dss/dsi.c | 2 +-
drivers/video/omap2/dss/dss.c | 2 +-
drivers/video/omap2/dss/hdmi.c | 2 +-
drivers/video/omap2/dss/rfbi.c | 2 +-
drivers/video/omap2/dss/venc.c | 2 +-
7 files changed, 35 insertions(+), 22 deletions(-)
--
1.7.9.5
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox