* Re: [PATCH 2/3] mx3fb: support pan display with fb_set_var
From: Florian Tobias Schandinat @ 2012-07-02 6:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1339376810-8247-2-git-send-email-Ying.Liu@freescale.com>
On 06/11/2012 01:06 AM, Liu Ying wrote:
> Users may call FBIOPUT_VSCREENINFO ioctrl to do pan display.
> This ioctrl relies on fb_set_var() to do the job. fb_set_var()
> calls the custom fb_set_par() method and then calls the custom
> fb_pan_display() method. Before calling the custom fb_pan_display()
> method, info->var is already updated from the new *var in fb_set_var().
> And, the custom fb_pan_display() method checks if xoffset and yoffset
> in info->var and the new *var are different before doing actual panning,
> which prevents the panning from happening within fb_set_var() context.
> This patch caches the current var info locally in mx3fb driver so that
> pan display with fb_set_var is supported.
>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
Applied patches 2 and 3 of this series.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/mx3fb.c | 33 ++++++++++++++++++++++++++-------
> 1 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index d53db60..2dd11c4 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -268,7 +268,7 @@ struct mx3fb_info {
> dma_cookie_t cookie;
> struct scatterlist sg[2];
>
> - u32 sync; /* preserve var->sync flags */
> + struct fb_var_screeninfo cur_var; /* current var info */
> };
>
> static void mx3fb_dma_done(void *);
> @@ -723,7 +723,7 @@ static void mx3fb_dma_done(void *arg)
>
> static int __set_par(struct fb_info *fbi, bool lock)
> {
> - u32 mem_len;
> + u32 mem_len, cur_xoffset, cur_yoffset;
> struct ipu_di_signal_cfg sig_cfg;
> enum ipu_panel mode = IPU_PANEL_TFT;
> struct mx3fb_info *mx3_fbi = fbi->par;
> @@ -805,8 +805,25 @@ static int __set_par(struct fb_info *fbi, bool lock)
> video->out_height = fbi->var.yres;
> video->out_stride = fbi->var.xres_virtual;
>
> - if (mx3_fbi->blank = FB_BLANK_UNBLANK)
> + if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
> sdc_enable_channel(mx3_fbi);
> + /*
> + * sg[0] points to fb smem_start address
> + * and is actually active in controller.
> + */
> + mx3_fbi->cur_var.xoffset = 0;
> + mx3_fbi->cur_var.yoffset = 0;
> + }
> +
> + /*
> + * Preserve xoffset and yoffest in case they are
> + * inactive in controller as fb is blanked.
> + */
> + cur_xoffset = mx3_fbi->cur_var.xoffset;
> + cur_yoffset = mx3_fbi->cur_var.yoffset;
> + mx3_fbi->cur_var = fbi->var;
> + mx3_fbi->cur_var.xoffset = cur_xoffset;
> + mx3_fbi->cur_var.yoffset = cur_yoffset;
>
> return 0;
> }
> @@ -926,8 +943,8 @@ static int mx3fb_check_var(struct fb_var_screeninfo *var, struct fb_info *fbi)
> var->grayscale = 0;
>
> /* Preserve sync flags */
> - var->sync |= mx3_fbi->sync;
> - mx3_fbi->sync |= var->sync;
> + var->sync |= mx3_fbi->cur_var.sync;
> + mx3_fbi->cur_var.sync |= var->sync;
>
> return 0;
> }
> @@ -1073,8 +1090,8 @@ static int mx3fb_pan_display(struct fb_var_screeninfo *var,
> return -EINVAL;
> }
>
> - if (fbi->var.xoffset = var->xoffset &&
> - fbi->var.yoffset = var->yoffset)
> + if (mx3_fbi->cur_var.xoffset = var->xoffset &&
> + mx3_fbi->cur_var.yoffset = var->yoffset)
> return 0; /* No change, do nothing */
>
> y_bottom = var->yoffset;
> @@ -1157,6 +1174,8 @@ static int mx3fb_pan_display(struct fb_var_screeninfo *var,
> else
> fbi->var.vmode &= ~FB_VMODE_YWRAP;
>
> + mx3_fbi->cur_var = fbi->var;
> +
> mutex_unlock(&mx3_fbi->mutex);
>
> dev_dbg(fbi->device, "Update complete\n");
^ permalink raw reply
* Re: [PATCH] da8xx-fb: Rounding FB size to satisfy SGX buffer requirements
From: Florian Tobias Schandinat @ 2012-07-02 6:37 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1337753191-14802-1-git-send-email-aditya.n@ti.com>
On 06/12/2012 09:34 AM, Nellutla, Aditya wrote:
> Florian,
>
> I didn't receive any review comments for the patch below. So could you please merge the patch?
As far as I can see there is no feasible alternative so I applied this
patch. If anyone happens to have problems with the larger memory
requirements caused by this, I guess we could make this behavior depend
on a Kconfig symbol.
Best regards,
Florian Tobias Schandinat
>
> Regards,
> Aditya
>
> -----Original Message-----
> From: Nellutla, Aditya
> Sent: Monday, June 04, 2012 11:33 AM
> To: Nellutla, Aditya; linux-fbdev@vger.kernel.org
> Cc: Hiremath, Vaibhav
> Subject: RE: [PATCH] da8xx-fb: Rounding FB size to satisfy SGX buffer requirements
>
> All,
> Did you get chance to review this patch? Request to give your feedback as early as possible.
>
> Regards,
> Aditya
>
> -----Original Message-----
> From: Nellutla, Aditya
> Sent: Wednesday, May 23, 2012 11:37 AM
> To: linux-fbdev@vger.kernel.org
> Cc: Nellutla, Aditya
> Subject: [PATCH] da8xx-fb: Rounding FB size to satisfy SGX buffer requirements
>
> In the real time use-case when SGX is used for rendering to FB buffers it has been
> observed that, the available memory from framebuffer driver is not sufficient for
> SGX under certain cases (like 16-bit WVGA resolution). SGX requires 2 swap buffers
> with each of the buffers aligned to lcm(line_length, PAGE_SIZE).
>
> Inorder to satisfy this requirement, we have two options,
>
> - Increase number of FB buffers (LCD_NUM_BUFFERS) to 3. This is not
> recommended as we end up wasting huge memory in most of the cases.
>
> - Align FB buffers to lcm(line_length, PAGE_SIZE).This ensures framebuffer
> size is increased to satisfy SGX requirements keeping alignment intact.
>
> This patch makes sure that FB allocates buffers aligned to above formula.
>
> Signed-off-by: Aditya Nellutla <aditya.n@ti.com>
> ---
> drivers/video/da8xx-fb.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 47118c7..2f24c19 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -31,6 +31,7 @@
> #include <linux/cpufreq.h>
> #include <linux/console.h>
> #include <linux/slab.h>
> +#include <linux/lcm.h>
> #include <video/da8xx-fb.h>
> #include <asm/div64.h>
>
> @@ -1114,6 +1115,7 @@ static int __devinit fb_probe(struct platform_device *device)
> struct da8xx_fb_par *par;
> resource_size_t len;
> int ret, i;
> + unsigned long ulcm;
>
> if (fb_pdata = NULL) {
> dev_err(&device->dev, "Can not get platform data\n");
> @@ -1209,7 +1211,8 @@ static int __devinit fb_probe(struct platform_device *device)
>
> /* allocate frame buffer */
> par->vram_size = lcdc_info->width * lcdc_info->height * lcd_cfg->bpp;
> - par->vram_size = PAGE_ALIGN(par->vram_size/8);
> + ulcm = lcm((lcdc_info->width * lcd_cfg->bpp)/8, PAGE_SIZE);
> + par->vram_size = roundup(par->vram_size/8, ulcm);
> par->vram_size = par->vram_size * LCD_NUM_BUFFERS;
>
> par->vram_virt = dma_alloc_coherent(NULL,
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Thierry Reding @ 2012-07-02 6:46 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Mitch Bradley
In-Reply-To: <4FF116F0.5070602-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3872 bytes --]
On Mon, Jul 02, 2012 at 12:35:12PM +0900, Alexandre Courbot wrote:
> On 07/01/2012 03:37 AM, Thierry Reding wrote:>> + ret =
> of_get_named_gpio(node, "enable-gpios", 0);
> >> + if (ret >= 0) {
> >> + data->enable_gpio = of_get_named_gpio(node, "enable-gpios", 0);
> >
> > Can't you just reuse the value of ret here?
>
> Yes, definitely.
>
> >> + pb->enable_gpio = -EINVAL;
> >
> > Perhaps initialize this to -1? Assigning standard error codes to a GPIO
> > doesn't make much sense.
>
> Documentation/gpio.txt states the following:
>
> "If you want to initialize a structure with an invalid GPIO number, use
> some negative number (perhaps "-EINVAL"); that will never be valid."
>
> gpio_is_valid() seems to be happy with any negative value, but
> -EINVAL seems to be a convention here.
I would have thought something like -1 would be good enough to represent
an invalid GPIO, but if there's already a convention then we should
follow it.
> >> + /* optional GPIO that enables/disables the backlight */
> >> + int enable_gpio;
> >> + /* 0 (default initialization value) is a valid GPIO number.
> Make use of
> >> + * control gpio explicit to avoid bad surprises. */
> >> + bool use_enable_gpio;
> >
> > It's a shame we have to add workarounds like this...
>
> Yeah, I hate that too. :/ I see nothing better to do unfortunately.
>
> Other remarks from Stephen made me realize that this patch has two
> major flaws:
>
> 1) The GPIO and regulator are fixed, optional entites ; this should
> cover most cases but is not very flexible.
> 2) Some (most?) backlight specify timings between turning power
> on/enabling PWM/enabling backlight. Even the order of things may be
> different. This patch totally ignores that.
>
> So instead of having fixed "power-supply" and "enable-gpio"
> properties, how about having properties describing the power-on and
> power-off sequences which odd cells alternate between phandles to
> regulators/gpios/pwm and delays in microseconds before continuing
> the sequence. For example:
>
> power-on = <&pwm 2 5000000
> 10000
> &backlight_reg
> 0
> &gpio 28 0>;
> power-off = <&gpio 28 0
> 0
> &backlight_reg
> 10000
> &pwm 2 0>;
>
> Here the power-on sequence would translate to, power on the second
> PWM with a duty-cycle of 5ms, wait 10ms, then enable the backlight
> regulator and GPIO 28 without delay. Power-off is the exact
> opposite. The nice thing with this scheme is that you can reorder
> the sequence at will and support the weirdest setups.
I generally like the idea. I'm Cc'ing the devicetree-discuss mailing
list, let's see what people there think about it. I've also added Mitch
Bradley who already helped a lot with the initial binding.
> What I don't know (device tree newbie here!) is:
> 1) Is it legal to refer the same phandle several times in the same node?
> 2) Is it ok to mix phandles of different types with integer values?
> The DT above compiled, but can you e.g. resolve a regulator phandle
> in the middle of a property?
I believe these shouldn't be a problem.
> 3) Can you "guess" the type of a phandle before parsing it? Here the
> first phandle is a GPIO, but it could as well be the regulator. Do
> we have means to know that in the driver code?
That isn't possible. But you could for instance use a cell to specify
the type of the following phandle.
> Sorry for the long explanation, but I really wonder if doing this is
> possible at all. If it is, then I think that's the way to do for
> backlight initialization.
OTOH we basically end up describing a code sequence in the DT. But as
you mention above sometimes the hardware requires specific timing
parameters so this might actually be a kind of valid sequence to
describe in the DT.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Alexandre Courbot @ 2012-07-02 7:18 UTC (permalink / raw)
To: Thierry Reding
Cc: Stephen Warren,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Mitch Bradley
In-Reply-To: <20120702064624.GA8683-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
On 07/02/2012 03:46 PM, Thierry Reding wrote:
>> So instead of having fixed "power-supply" and "enable-gpio"
>> properties, how about having properties describing the power-on and
>> power-off sequences which odd cells alternate between phandles to
>> regulators/gpios/pwm and delays in microseconds before continuing
>> the sequence. For example:
>>
>> power-on = <&pwm 2 5000000
>> 10000
>> &backlight_reg
>> 0
>> &gpio 28 0>;
>> power-off = <&gpio 28 0
>> 0
>> &backlight_reg
>> 10000
>> &pwm 2 0>;
>>
>> Here the power-on sequence would translate to, power on the second
>> PWM with a duty-cycle of 5ms, wait 10ms, then enable the backlight
>> regulator and GPIO 28 without delay. Power-off is the exact
>> opposite. The nice thing with this scheme is that you can reorder
>> the sequence at will and support the weirdest setups.
>
> I generally like the idea. I'm Cc'ing the devicetree-discuss mailing
> list, let's see what people there think about it. I've also added Mitch
> Bradley who already helped a lot with the initial binding.
Cool, thanks. I am aware that this idea probably needs to be refined,
but ideally we would end up with something as simple as this example.
>> What I don't know (device tree newbie here!) is:
>> 1) Is it legal to refer the same phandle several times in the same node?
>> 2) Is it ok to mix phandles of different types with integer values?
>> The DT above compiled, but can you e.g. resolve a regulator phandle
>> in the middle of a property?
>
> I believe these shouldn't be a problem.
Nice, I'll try to see how I can parse it then.
>> 3) Can you "guess" the type of a phandle before parsing it? Here the
>> first phandle is a GPIO, but it could as well be the regulator. Do
>> we have means to know that in the driver code?
>
> That isn't possible. But you could for instance use a cell to specify
> the type of the following phandle.
That sounds reasonnable, although we would also need to bind values to
phandle types. That would make the DT a little bit more cryptic,
although nothing too bad I think.
>> Sorry for the long explanation, but I really wonder if doing this is
>> possible at all. If it is, then I think that's the way to do for
>> backlight initialization.
>
> OTOH we basically end up describing a code sequence in the DT. But as
> you mention above sometimes the hardware requires specific timing
> parameters so this might actually be a kind of valid sequence to
> describe in the DT.
Yes, the power on/power off sequences are part of the board/panel
specification - they are a hardware description and thus fits within the
purpose of the device tree IMHO. One could argue that initialization
sequences are usually coded inside drivers, but that only works when the
driver has a single initialization sequence to handle. With
pwm-backlight we try to handle something much more general, and so far
these sequences were performed by board-specific functions.
All in all, adding timings & ordering is not so different from the
original patch which accepted one optional regulator and GPIO.
Also, if someone knows of anything else besides PWM/GPIO/regulators that
may need to be controlled by a backlight power sequence, please let me know.
Alex.
^ permalink raw reply
* RE: [PATCH] da8xx-fb: add missing FB_BLANK operations
From: Hiremath, Vaibhav @ 2012-07-02 7:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1341213073-2986-1-git-send-email-yegorslists@googlemail.com>
On Mon, Jul 02, 2012 at 12:41:13, yegorslists@googlemail.com wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
>
> add FB_BLANK_NORMAL, FB_BLANK_VSYNC_SUSPEND and FB_BLANK_HSYNC_SUSPEND
> modes (copy drivers/video/omap2/omapfb/omapfb-main.c implementation).
> Otherwise X-server will complain about invalid parameter.
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
> drivers/video/da8xx-fb.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 29577bf..9879edc 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -987,6 +987,9 @@ static int cfb_blank(int blank, struct fb_info *info)
>
> lcd_enable_raster();
> break;
> + case FB_BLANK_NORMAL:
> + case FB_BLANK_VSYNC_SUSPEND:
> + case FB_BLANK_HSYNC_SUSPEND:
> case FB_BLANK_POWERDOWN:
> if (par->panel_power_ctrl)
> par->panel_power_ctrl(0);
+ linux-fbdev & Florian
You should always send Fbdev patches to linux-fbdev list.
Thanks,
Vaibhav
^ permalink raw reply
* Re: [PATCH] video/smscufx: fix line counting in fb_write
From: Alexander Holler @ 2012-07-02 21:25 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: linux-kernel, linux-fbdev
In-Reply-To: <4FAFAD7B.6090207@gmx.de>
Hi Florian,
sorry for the late answer.
Am 13.05.2012 14:47, schrieb Florian Tobias Schandinat:
> Hi Alexander,
>
> On 04/21/2012 11:26 AM, Alexander Holler wrote:
>> Hello,
>>
>> as for the patch for udlfb, I forgot to mention that this is a candidate
>> for all stable trees 3.2 and above.
>>
>> Btw., the address of the maintainer doesn't seem to be valid anymore.
>
> it is better to cc me on patches to the framebuffer subsystem for such
> cases. I don't have much free time so it's rare that I come around to
> dig in the mailing list.
>
>>
>> Regards,
>>
>> Alexander
>>
>> Am 21.04.2012 00:11, schrieb Alexander Holler:
>>> Line 0 and 1 were both written to line 0 (on the display) and all
>>> subsequent
>>> lines had an offset of -1. The result was that the last line on the
>>> display
>>> was never overwritten by writes to /dev/fbN.
>>>
>>> The origin of this bug seems to have been udlfb.
>>>
>>> Signed-off-by: Alexander Holler<holler@ahsoftware.de>
>>
>> Cc: stable@vger.kernel.org
>
> Patch looks good to me but can be made simpler.
>
>>
>>> ---
>>> drivers/video/smscufx.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/video/smscufx.c b/drivers/video/smscufx.c
>>> index ccbfef5..1e1e2d2 100644
>>> --- a/drivers/video/smscufx.c
>>> +++ b/drivers/video/smscufx.c
>>> @@ -904,7 +904,7 @@ static ssize_t ufx_ops_write(struct fb_info *info,
>>> const char __user *buf,
>>> result = fb_sys_write(info, buf, count, ppos);
>>>
>>> if (result> 0) {
>>> - int start = max((int)(offset / info->fix.line_length) - 1, 0);
>>> + int start = max((int)(offset / info->fix.line_length), 0);
>
> the cast to int as well as the max is superfluous without the -1 as the
> value can no longer be negative.
I had that impression too, but I wanted to change as less as possible,
so I didn't had the need to check types and (their) sizes for possible
overflows or such. I was lazy and just wanted to fix that one bug. ;)
>
>>> int lines = min((u32)((result / info->fix.line_length) + 1),
>>> (u32)info->var.yres);
>>>
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH 0/2] simple device tree support for mxsfb
From: Shawn Guo @ 2012-07-03 7:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1340701379-8619-1-git-send-email-shawn.guo@linaro.org>
Hi Florian,
On Tue, Jun 26, 2012 at 05:02:57PM +0800, Shawn Guo wrote:
> It adds a simple device tree support for mxsfb driver.
>
> Shawn Guo (2):
> video: mxsfb: move mxsfb.h into include/linux
> video: mxsfb: add simple device tree probe
>
I just queued these two patches on mxs arch tree. Please let me know
if you see problem with that.
Regards,
Shawn
> Documentation/devicetree/bindings/fb/mxsfb.txt | 21 +++++++
> arch/arm/mach-mxs/devices-mx23.h | 2 +-
> arch/arm/mach-mxs/devices-mx28.h | 2 +-
> arch/arm/mach-mxs/devices/platform-mxsfb.c | 2 +-
> drivers/video/mxsfb.c | 62 +++++++++++++++-----
> .../include/mach => include/linux}/mxsfb.h | 6 +-
> 6 files changed, 75 insertions(+), 20 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/fb/mxsfb.txt
> rename {arch/arm/mach-mxs/include/mach => include/linux}/mxsfb.h (95%)
>
> --
> 1.7.5.4
>
>
^ permalink raw reply
* [PATCH v2] of: Add videomode helper
From: Sascha Hauer @ 2012-07-04 7:56 UTC (permalink / raw)
To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
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.
+
+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;
+ };
+
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 */
--
1.7.10
^ permalink raw reply related
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Sascha Hauer @ 2012-07-04 10:48 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Thierry Reding, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1340976167-27298-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Hi Alexandre,
On Fri, Jun 29, 2012 at 10:22:47PM +0900, Alexandre Courbot wrote:
> @@ -221,8 +263,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> }
> }
>
> - dev_dbg(&pdev->dev, "got pwm for backlight\n");
> -
> /*
> * The DT case will set the pwm_period_ns field to 0 and store the
> * period, parsed from the DT, in the PWM device. For the non-DT case,
> @@ -231,6 +271,22 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> if (data->pwm_period_ns > 0)
> pwm_set_period(pb->pwm, data->pwm_period_ns);
>
> +
> + 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.
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-04 12:26 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: <20120704104840.GJ24458-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
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, so that
probe could reallocate them the next time it is called. Apparently that
was wrong. Do you know what would the right approach be in this case?
Does the kernel preserve the device structure and its associated data
between the two calls to probe? If so, I could just check whether the
private data has already been constructed to know which state we are in
and continue from there.
Thanks,
Alex.
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Mark Brown @ 2012-07-04 12:27 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: <4FF43692.2040805-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Wed, Jul 04, 2012 at 09:26:58PM +0900, Alex Courbot wrote:
> On 07/04/2012 07:48 PM, Sascha Hauer wrote:>> +
> > 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, so that
> probe could reallocate them the next time it is called. Apparently
> that was wrong. Do you know what would the right approach be in this
> case? Does the kernel preserve the device structure and its
> associated data between the two calls to probe? If so, I could just
> check whether the private data has already been constructed to know
> which state we are in and continue from there.
I'd really expect the devm_ functions to behave as you expect - if they
don't currently we should probably fix them so that they do otherwise
we're going to be in for a lot of surprises and probe deferral becomes a
lot more cumbersome to use.
^ permalink raw reply
* [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
* [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 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
* 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
* 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: 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: 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
* [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
* [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 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
* 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
* 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 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: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
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