Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] backlight: Add LMS501KF03 LCD panel driver
From: Sachin Kamat @ 2012-04-19  7:52 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1334558406-22576-1-git-send-email-sachin.kamat@linaro.org>

On 18/04/2012, Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> On 04/16/2012 06:40 AM, Sachin Kamat wrote:
>> LMS501KF03 is a 480x800 LCD module with brightness control.
>> The driver uses 3-wired SPI inteface.
>>
>> Signed-off-by: Ilho Lee <Ilho215.lee@samsung.com>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>
>> diff --git a/drivers/video/backlight/lms501kf03.c
>> b/drivers/video/backlight/lms501kf03.c
>> new file mode 100644
>> index 0000000..3dc85d4
>> --- /dev/null
>> +++ b/drivers/video/backlight/lms501kf03.c
>> +static int lms501kf03_ldi_enable(struct lms501kf03 *lcd)
>> +{
>> +	int ret, i;
>> +	const unsigned short *init_seq[] = {
>> +		SEQ_DISPLAY_ON,
>> +	};
>
> Is this array expected to grow at some point?
> Otherwise I'd suggest to get rid of it and simplify the code below.

OK. I will simplify the code.

>
>> +
>> +	for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
>> +		ret = lms501kf03_panel_send_sequence(lcd, init_seq[i]);
>> +		if (ret)
>> +			break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int lms501kf03_ldi_disable(struct lms501kf03 *lcd)
>> +{
>> +	int ret, i;
>> +
>> +	const unsigned short *init_seq[] = {
>> +		SEQ_DISPLAY_OFF,
>> +	};
>
> dito

OK.

>
>> +
>> +	for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
>> +		ret = lms501kf03_panel_send_sequence(lcd, init_seq[i]);
>> +		if (ret)
>> +			break;
>> +	}
>> +
>> +	return ret;
>> +}
>
>

Thank you for reviewing the code. I will re-send the patch with above
suggested changes.

> Best regards,
>
> Florian Tobias Schandinat
>


-- 
With warm regards,
Sachin

^ permalink raw reply

* Re: [PATCH 4/6] OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
From: Tomi Valkeinen @ 2012-04-19  6:37 UTC (permalink / raw)
  To: Archit Taneja; +Cc: Archit Taneja, linux-omap, linux-fbdev
In-Reply-To: <4F8FAD01.3090406@ti.com>

[-- Attachment #1: Type: text/plain, Size: 3139 bytes --]

On Thu, 2012-04-19 at 11:43 +0530, Archit Taneja wrote:
> On Wednesday 18 April 2012 08:28 PM, Tomi Valkeinen wrote:
> > On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
> >> DISPC manager size and DISPC manager blanking parameters(for LCD managers)
> >> follow the shadow register programming model. Currently, they are programmed
> >> directly by the interface drivers.
> >>
> >> Make timings(omap_video_timing struct) an overlay_manager_info member, they are
> >> now programmed via the apply mechanism used for programming shadow registers.
> >>
> >> The interface driver now call the function dss_mgr_set_timings() which applies
> >> the new timing parameters, rather than directly writing to DISPC registers.
> >
> > I don't think that works correctly. The omap_overlay_manager_info is
> > supposed to be set with set_manager_info() by the user of omapdss, to
> > configure the manager's features. The timings are not supposed to be set
> > via that mechanism, but with dssdev->set_timings().
> >
> > This is similar to the info and extra_info for overlay. info has stuff
> > that omapdss doesn't change, it just uses what the user gives.
> > extra_info, on the other hand, has omapdss private stuff that the user
> > does not see. Timings are clearly private stuff in this sense, because
> > they are set via dssdev->set_timings().
> >
> > One reason for this is the programming model we use. If the user of
> > omapdss does get_info() for two overlays, changes the infos, and then
> > calls set_info() for both overlays and finally apply() for the manager,
> > we don't do any locking there because omapdss presumes the info is
> > handled by one user. If, say, the dpi.c would change the info and call
> > apply at the same time, the configuration could go badly wrong.
> 
> I think I get your point. So even though get_info() and set_info() fn's 
> are spinlock protected, if there are 2 users setting the info, it 
> doesn't mean that the info they finally written is correct. Is this 
> example the same thing as what you mean ? :
> 
> In order of time:
> 
> -user 1 gets an overlay's info
> 
> -user 2 gets an overlay's info
> 
> -user 1 modifies and sets overlay info
> 
> -user 2 sets overlay info without the knowledge of what user 1 did.
> 
> So even though we ensure these events happen sequentially, we don't 
> protect the info across multiple users.

Yes. The spinlocks ensure that the info is "whole", so we don't get a
few fields from user1 and a few fields from user2. But they don't
protect us from the case you described above.

For that we would need a "dss lock" that the user would acquire before
using get_info and set_info. But I don't want to go to that direction,
because we really only support one user anyway.

The problem in this particular case is that omapdss itself becomes
another user if it uses get_info & set_info. And that can be easily
avoided by splitting the configuration into public (the "info") and
internal ("extra_info"). The users of omapdss never touch the
extra_info, and omapdss never touches the info.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 4/6] OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
From: Archit Taneja @ 2012-04-19  6:25 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Archit Taneja, linux-omap, linux-fbdev
In-Reply-To: <1334761105.2027.67.camel@deskari>

On Wednesday 18 April 2012 08:28 PM, Tomi Valkeinen wrote:
> On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
>> DISPC manager size and DISPC manager blanking parameters(for LCD managers)
>> follow the shadow register programming model. Currently, they are programmed
>> directly by the interface drivers.
>>
>> Make timings(omap_video_timing struct) an overlay_manager_info member, they are
>> now programmed via the apply mechanism used for programming shadow registers.
>>
>> The interface driver now call the function dss_mgr_set_timings() which applies
>> the new timing parameters, rather than directly writing to DISPC registers.
>
> I don't think that works correctly. The omap_overlay_manager_info is
> supposed to be set with set_manager_info() by the user of omapdss, to
> configure the manager's features. The timings are not supposed to be set
> via that mechanism, but with dssdev->set_timings().
>
> This is similar to the info and extra_info for overlay. info has stuff
> that omapdss doesn't change, it just uses what the user gives.
> extra_info, on the other hand, has omapdss private stuff that the user
> does not see. Timings are clearly private stuff in this sense, because
> they are set via dssdev->set_timings().
>
> One reason for this is the programming model we use. If the user of
> omapdss does get_info() for two overlays, changes the infos, and then
> calls set_info() for both overlays and finally apply() for the manager,
> we don't do any locking there because omapdss presumes the info is
> handled by one user. If, say, the dpi.c would change the info and call
> apply at the same time, the configuration could go badly wrong.

I think I get your point. So even though get_info() and set_info() fn's 
are spinlock protected, if there are 2 users setting the info, it 
doesn't mean that the info they finally written is correct. Is this 
example the same thing as what you mean ? :

In order of time:

-user 1 gets an overlay's info

-user 2 gets an overlay's info

-user 1 modifies and sets overlay info

-user 2 sets overlay info without the knowledge of what user 1 did.

So even though we ensure these events happen sequentially, we don't 
protect the info across multiple users.

>
> So I think what should be done is to add similar "extra" flags and code
> to mgr_priv_data that we have for ovl_priv_data, and add
> omap_video_timings to mgr_priv_data (the same way as we have, say,
> fifo_low for ovl_priv_data).
>
> And then add similar function to dss_ovl_write_regs_extra() for manager,
> and a function like dss_apply_ovl_fifo_thresholds() for timings. And
> finally a non-static function to set the timings (used by dpi.c etc),
> which calls the similar function to dss_apply_ovl_fifo_thresholds(), and
> dss_write_regs() and dss_set_go_bits().

Okay, I'll work on it along these lines.

>
>   Tomi
>


^ permalink raw reply

* RE: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
From: Julia Lawall @ 2012-04-19  5:14 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Ryan Mallon', 'Florian Tobias Schandinat',
	kernel-janitors, linux-fbdev, linux-kernel,
	'Julia Lawall', joe
In-Reply-To: <000401cd1dd2$5f8ee3b0$1eacab10$%han@samsung.com>

I don't know thw impact of the ./.  I use the options

--nokeywords --nogit-fallback --subsystem --norolestats -f

julia

On Thu, 19 Apr 2012, Jingoo Han wrote:

>> -----Original Message-----
>> From: Ryan Mallon
>> Sent: Thursday, April 19, 2012 9:16 AM
>> To: Julia Lawall
>> Cc: Florian Tobias Schandinat; kernel-janitors@vger.kernel.org; linux-fbdev@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
>>
>> On 19/04/12 05:37, Julia Lawall wrote:
>>
>>> From: Julia Lawall <Julia.Lawall@lip6.fr>
>>>
>>> There were two problems in this code: failure of the setup function should
>>> free locally allocated resources like other nearby failures, and the test
>>> if (&info->cmap) can never be false.  To generally clean things up, this
>>> patch reorders the error handling code at the failed label and adds labels
>>> so that the conditionals are not necessary.
>>>
>>> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>>
>> Reviewed-by: Ryan Mallon <rmallon@gmail.com>
>>
>> Oddly, scripts/get_maintainer.pl on this file doesn't return me, even
>> though, according to git blame, I am the author of 90% of the commits.
>> Should I have an entry in the MAINTAINERS file, or is
>> scripts/get_maintainer.pl not working properly?
>
> There are optional differences in using scripts/get_maintainer.pl.
> If you use './' ahead of file path, you will see your name.
>
> Without './' ahead of 'drivers/video/ep93xx-fb.c':
> ./scripts/get_maintainer.pl --file drivers/video/ep93xx-fb.c
> Florian Tobias Schandinat <FlorianSchandinat@gmx.de> (maintainer:FRAMEBUFFER LAYER)
> linux-fbdev@vger.kernel.org (open list:FRAMEBUFFER LAYER)
> linux-kernel@vger.kernel.org (open list)
>
> With './' ahead of 'drivers/video/ep93xx-fb.c':
> ./scripts/get_maintainer.pl --file ./drivers/video/ep93xx-fb.c
> Ryan Mallon <rmallon@gmail.com> (commit_signer:2/3g%)
> Paul Gortmaker <paul.gortmaker@windriver.com> (commit_signer:1/33%)
> H Hartley Sweeten <hsweeten@visionengravers.com> (commit_signer:1/33%)
> Jesper Juhl <jj@chaosbits.net> (commit_signer:1/33%)
> Jiri Kosina <jkosina@suse.cz> (commit_signer:1/33%)
> linux-kernel@vger.kernel.org (open list)
>
>
> Best regards,
> Jingoo Han
>
>>
>> ~Ryan
>>
>>> ---
>>> Not tested.
>>>
>>>  drivers/video/ep93xx-fb.c |   32 +++++++++++++++++---------------
>>>  1 file changed, 17 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/video/ep93xx-fb.c b/drivers/video/ep93xx-fb.c
>>> index f8babbe..345d962 100644
>>> --- a/drivers/video/ep93xx-fb.c
>>> +++ b/drivers/video/ep93xx-fb.c
>>> @@ -507,16 +507,16 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>>>
>>>  	err = fb_alloc_cmap(&info->cmap, 256, 0);
>>>  	if (err)
>>> -		goto failed;
>>> +		goto failed_cmap;
>>>
>>>  	err = ep93xxfb_alloc_videomem(info);
>>>  	if (err)
>>> -		goto failed;
>>> +		goto failed_videomem;
>>>
>>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>  	if (!res) {
>>>  		err = -ENXIO;
>>> -		goto failed;
>>> +		goto failed_resource;
>>>  	}
>>>
>>>  	/*
>>> @@ -532,7 +532,7 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>>>  	fbi->mmio_base = ioremap(res->start, resource_size(res));
>>>  	if (!fbi->mmio_base) {
>>>  		err = -ENXIO;
>>> -		goto failed;
>>> +		goto failed_resource;
>>>  	}
>>>
>>>  	strcpy(info->fix.id, pdev->name);
>>> @@ -553,24 +553,24 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>>>  	if (err = 0) {
>>>  		dev_err(info->dev, "No suitable video mode found\n");
>>>  		err = -EINVAL;
>>> -		goto failed;
>>> +		goto failed_mode;
>>>  	}
>>>
>>>  	if (mach_info->setup) {
>>>  		err = mach_info->setup(pdev);
>>>  		if (err)
>>> -			return err;
>>> +			goto failed_mode;
>>>  	}
>>>
>>>  	err = ep93xxfb_check_var(&info->var, info);
>>>  	if (err)
>>> -		goto failed;
>>> +		goto failed_check;
>>>
>>>  	fbi->clk = clk_get(info->dev, NULL);
>>>  	if (IS_ERR(fbi->clk)) {
>>>  		err = PTR_ERR(fbi->clk);
>>>  		fbi->clk = NULL;
>>> -		goto failed;
>>> +		goto failed_check;
>>>  	}
>>>
>>>  	ep93xxfb_set_par(info);
>>> @@ -585,15 +585,17 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>>>  	return 0;
>>>
>>>  failed:
>>> -	if (fbi->clk)
>>> -		clk_put(fbi->clk);
>>> -	if (fbi->mmio_base)
>>> -		iounmap(fbi->mmio_base);
>>> -	ep93xxfb_dealloc_videomem(info);
>>> -	if (&info->cmap)
>>> -		fb_dealloc_cmap(&info->cmap);
>>> +	clk_put(fbi->clk);
>>> +failed_check:
>>>  	if (fbi->mach_info->teardown)
>>>  		fbi->mach_info->teardown(pdev);
>>> +failed_mode:
>>> +	iounmap(fbi->mmio_base);
>>> +failed_resource:
>>> +	ep93xxfb_dealloc_videomem(info);
>>> +failed_videomem:
>>> +	fb_dealloc_cmap(&info->cmap);
>>> +failed_cmap:
>>>  	kfree(info);
>>>  	platform_set_drvdata(pdev, NULL);
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH] blackfin: fix compile error in bfin-lq035q1-fb.c
From: Mike Frysinger @ 2012-04-19  2:49 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1334803401-26780-1-git-send-email-paul.gortmaker@windriver.com>

[-- Attachment #1: Type: Text/Plain, Size: 447 bytes --]

On Wednesday 18 April 2012 22:43:21 Paul Gortmaker wrote:
> This file has an implicit dependency on GPIO stuff, showing
> up as the following build failure:
> 
> drivers/video/bfin-lq035q1-fb.c:369:6: error: 'GPIOF_OUT_INIT_LOW'
> undeclared
> 
> Other more global bfin build issues prevent an automated bisect, but
> it really doesn't matter - simply add in the appropriate header.

Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH] blackfin: fix compile error in bfin-lq035q1-fb.c
From: Paul Gortmaker @ 2012-04-19  2:43 UTC (permalink / raw)
  To: linux-fbdev

This file has an implicit dependency on GPIO stuff, showing
up as the following build failure:

drivers/video/bfin-lq035q1-fb.c:369:6: error: 'GPIOF_OUT_INIT_LOW' undeclared

Other more global bfin build issues prevent an automated bisect, but
it really doesn't matter - simply add in the appropriate header.

Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

diff --git a/drivers/video/bfin-lq035q1-fb.c b/drivers/video/bfin-lq035q1-fb.c
index 86922ac..353c02f 100644
--- a/drivers/video/bfin-lq035q1-fb.c
+++ b/drivers/video/bfin-lq035q1-fb.c
@@ -13,6 +13,7 @@
 #include <linux/errno.h>
 #include <linux/string.h>
 #include <linux/fb.h>
+#include <linux/gpio.h>
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/types.h>
-- 
1.7.9.1


^ permalink raw reply related

* RE: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
From: Jingoo Han @ 2012-04-19  2:16 UTC (permalink / raw)
  To: 'Ryan Mallon'
  Cc: 'Florian Tobias Schandinat', kernel-janitors, linux-fbdev,
	linux-kernel, 'Julia Lawall', 'Jingoo Han'
In-Reply-To: <4F8F5938.2000207@gmail.com>

> -----Original Message-----
> From: Ryan Mallon
> Sent: Thursday, April 19, 2012 9:16 AM
> To: Julia Lawall
> Cc: Florian Tobias Schandinat; kernel-janitors@vger.kernel.org; linux-fbdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
> 
> On 19/04/12 05:37, Julia Lawall wrote:
> 
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > There were two problems in this code: failure of the setup function should
> > free locally allocated resources like other nearby failures, and the test
> > if (&info->cmap) can never be false.  To generally clean things up, this
> > patch reorders the error handling code at the failed label and adds labels
> > so that the conditionals are not necessary.
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Reviewed-by: Ryan Mallon <rmallon@gmail.com>
> 
> Oddly, scripts/get_maintainer.pl on this file doesn't return me, even
> though, according to git blame, I am the author of 90% of the commits.
> Should I have an entry in the MAINTAINERS file, or is
> scripts/get_maintainer.pl not working properly?

There are optional differences in using scripts/get_maintainer.pl.
If you use './' ahead of file path, you will see your name.

Without './' ahead of 'drivers/video/ep93xx-fb.c':
./scripts/get_maintainer.pl --file drivers/video/ep93xx-fb.c
Florian Tobias Schandinat <FlorianSchandinat@gmx.de> (maintainer:FRAMEBUFFER LAYER)
linux-fbdev@vger.kernel.org (open list:FRAMEBUFFER LAYER)
linux-kernel@vger.kernel.org (open list)

With './' ahead of 'drivers/video/ep93xx-fb.c':
./scripts/get_maintainer.pl --file ./drivers/video/ep93xx-fb.c
Ryan Mallon <rmallon@gmail.com> (commit_signer:2/3g%)
Paul Gortmaker <paul.gortmaker@windriver.com> (commit_signer:1/33%)
H Hartley Sweeten <hsweeten@visionengravers.com> (commit_signer:1/33%)
Jesper Juhl <jj@chaosbits.net> (commit_signer:1/33%)
Jiri Kosina <jkosina@suse.cz> (commit_signer:1/33%)
linux-kernel@vger.kernel.org (open list)


Best regards,
Jingoo Han

> 
> ~Ryan
> 
> > ---
> > Not tested.
> >
> >  drivers/video/ep93xx-fb.c |   32 +++++++++++++++++---------------
> >  1 file changed, 17 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/video/ep93xx-fb.c b/drivers/video/ep93xx-fb.c
> > index f8babbe..345d962 100644
> > --- a/drivers/video/ep93xx-fb.c
> > +++ b/drivers/video/ep93xx-fb.c
> > @@ -507,16 +507,16 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
> >
> >  	err = fb_alloc_cmap(&info->cmap, 256, 0);
> >  	if (err)
> > -		goto failed;
> > +		goto failed_cmap;
> >
> >  	err = ep93xxfb_alloc_videomem(info);
> >  	if (err)
> > -		goto failed;
> > +		goto failed_videomem;
> >
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	if (!res) {
> >  		err = -ENXIO;
> > -		goto failed;
> > +		goto failed_resource;
> >  	}
> >
> >  	/*
> > @@ -532,7 +532,7 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
> >  	fbi->mmio_base = ioremap(res->start, resource_size(res));
> >  	if (!fbi->mmio_base) {
> >  		err = -ENXIO;
> > -		goto failed;
> > +		goto failed_resource;
> >  	}
> >
> >  	strcpy(info->fix.id, pdev->name);
> > @@ -553,24 +553,24 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
> >  	if (err = 0) {
> >  		dev_err(info->dev, "No suitable video mode found\n");
> >  		err = -EINVAL;
> > -		goto failed;
> > +		goto failed_mode;
> >  	}
> >
> >  	if (mach_info->setup) {
> >  		err = mach_info->setup(pdev);
> >  		if (err)
> > -			return err;
> > +			goto failed_mode;
> >  	}
> >
> >  	err = ep93xxfb_check_var(&info->var, info);
> >  	if (err)
> > -		goto failed;
> > +		goto failed_check;
> >
> >  	fbi->clk = clk_get(info->dev, NULL);
> >  	if (IS_ERR(fbi->clk)) {
> >  		err = PTR_ERR(fbi->clk);
> >  		fbi->clk = NULL;
> > -		goto failed;
> > +		goto failed_check;
> >  	}
> >
> >  	ep93xxfb_set_par(info);
> > @@ -585,15 +585,17 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
> >  	return 0;
> >
> >  failed:
> > -	if (fbi->clk)
> > -		clk_put(fbi->clk);
> > -	if (fbi->mmio_base)
> > -		iounmap(fbi->mmio_base);
> > -	ep93xxfb_dealloc_videomem(info);
> > -	if (&info->cmap)
> > -		fb_dealloc_cmap(&info->cmap);
> > +	clk_put(fbi->clk);
> > +failed_check:
> >  	if (fbi->mach_info->teardown)
> >  		fbi->mach_info->teardown(pdev);
> > +failed_mode:
> > +	iounmap(fbi->mmio_base);
> > +failed_resource:
> > +	ep93xxfb_dealloc_videomem(info);
> > +failed_videomem:
> > +	fb_dealloc_cmap(&info->cmap);
> > +failed_cmap:
> >  	kfree(info);
> >  	platform_set_drvdata(pdev, NULL);
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* Re: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
From: Ryan Mallon @ 2012-04-19  0:15 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Florian Tobias Schandinat, kernel-janitors, linux-fbdev,
	linux-kernel
In-Reply-To: <1334777828-3557-1-git-send-email-Julia.Lawall@lip6.fr>

On 19/04/12 05:37, Julia Lawall wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> There were two problems in this code: failure of the setup function should
> free locally allocated resources like other nearby failures, and the test
> if (&info->cmap) can never be false.  To generally clean things up, this
> patch reorders the error handling code at the failed label and adds labels
> so that the conditionals are not necessary.
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Reviewed-by: Ryan Mallon <rmallon@gmail.com>

Oddly, scripts/get_maintainer.pl on this file doesn't return me, even
though, according to git blame, I am the author of 90% of the commits.
Should I have an entry in the MAINTAINERS file, or is
scripts/get_maintainer.pl not working properly?

~Ryan

> ---
> Not tested.
> 
>  drivers/video/ep93xx-fb.c |   32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/video/ep93xx-fb.c b/drivers/video/ep93xx-fb.c
> index f8babbe..345d962 100644
> --- a/drivers/video/ep93xx-fb.c
> +++ b/drivers/video/ep93xx-fb.c
> @@ -507,16 +507,16 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  
>  	err = fb_alloc_cmap(&info->cmap, 256, 0);
>  	if (err)
> -		goto failed;
> +		goto failed_cmap;
>  
>  	err = ep93xxfb_alloc_videomem(info);
>  	if (err)
> -		goto failed;
> +		goto failed_videomem;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {
>  		err = -ENXIO;
> -		goto failed;
> +		goto failed_resource;
>  	}
>  
>  	/*
> @@ -532,7 +532,7 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  	fbi->mmio_base = ioremap(res->start, resource_size(res));
>  	if (!fbi->mmio_base) {
>  		err = -ENXIO;
> -		goto failed;
> +		goto failed_resource;
>  	}
>  
>  	strcpy(info->fix.id, pdev->name);
> @@ -553,24 +553,24 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  	if (err = 0) {
>  		dev_err(info->dev, "No suitable video mode found\n");
>  		err = -EINVAL;
> -		goto failed;
> +		goto failed_mode;
>  	}
>  
>  	if (mach_info->setup) {
>  		err = mach_info->setup(pdev);
>  		if (err)
> -			return err;
> +			goto failed_mode;
>  	}
>  
>  	err = ep93xxfb_check_var(&info->var, info);
>  	if (err)
> -		goto failed;
> +		goto failed_check;
>  
>  	fbi->clk = clk_get(info->dev, NULL);
>  	if (IS_ERR(fbi->clk)) {
>  		err = PTR_ERR(fbi->clk);
>  		fbi->clk = NULL;
> -		goto failed;
> +		goto failed_check;
>  	}
>  
>  	ep93xxfb_set_par(info);
> @@ -585,15 +585,17 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  	return 0;
>  
>  failed:
> -	if (fbi->clk)
> -		clk_put(fbi->clk);
> -	if (fbi->mmio_base)
> -		iounmap(fbi->mmio_base);
> -	ep93xxfb_dealloc_videomem(info);
> -	if (&info->cmap)
> -		fb_dealloc_cmap(&info->cmap);
> +	clk_put(fbi->clk);
> +failed_check:
>  	if (fbi->mach_info->teardown)
>  		fbi->mach_info->teardown(pdev);
> +failed_mode:
> +	iounmap(fbi->mmio_base);
> +failed_resource:
> +	ep93xxfb_dealloc_videomem(info);
> +failed_videomem:
> +	fb_dealloc_cmap(&info->cmap);
> +failed_cmap:
>  	kfree(info);
>  	platform_set_drvdata(pdev, NULL);
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



^ permalink raw reply

* Re: [PATCH v1] ARM: i.mx: mx3fb: add overlay support
From: Guennadi Liakhovetski @ 2012-04-18 22:40 UTC (permalink / raw)
  To: Alex Gershgorin
  Cc: Florian Tobias Schandinat, s.hauer, laurent.pinchart, linux-fbdev,
	linux-media
In-Reply-To: <1334770715-31064-1-git-send-email-alexg@meprolight.com>

Hi Alex

Thanks for reviving, fixing and submitting this code!

On Wed, 18 Apr 2012, Alex Gershgorin wrote:

> This patch is based on the original version submitted by Guennadi Liakhovetski,
> the patch initializes overlay channel, adds ioctl for configuring
> transparency of the overlay and graphics planes, CONFIG_FB_MX3_OVERLAY
> is also supported.
> 
> In case that CONFIG_FB_MX3_OVERLAY is not defined, mx3fb is completely
> backward compatible.
> 
> Blend mode, only global alpha blending has been tested.
> 
> Signed-off-by: Alex Gershgorin <alexg@meprolight.com>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Thanks for the credit (;-)), but no, putting my Sob after yours means, 
that I took your patch and forwarded it on to the next maintainer, which 
is clearly not the case here:-) The original i.MX31 framebuffer overlay 
code from my old patches also clearly wasn't written by me, since I didn't 
have a chance to test it. So, if you like, you can try to trace back 
original authors of that code and ask them, how they want to be credited 
here, otherwise just mentioning, that this work is based on some earlier 
patch series "i.MX31: dmaengine and framebuffer drivers" from 2008 by ... 
should be enough.

I don't think I can review this patch in sufficient depth, just a couple 
of minor comments below

> ---
> 
> Applies to v3.4-rc3
> ---
>  drivers/video/Kconfig |    7 +
>  drivers/video/mx3fb.c |  318 ++++++++++++++++++++++++++++++++++++++++++++-----
>  include/linux/mxcfb.h |   93 ++++++++++++++
>  3 files changed, 388 insertions(+), 30 deletions(-)
>  create mode 100644 include/linux/mxcfb.h
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index a290be5..acbfccc 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2368,6 +2368,13 @@ config FB_MX3
>  	  far only synchronous displays are supported. If you plan to use
>  	  an LCD display with your i.MX31 system, say Y here.
>  
> +config FB_MX3_OVERLAY
> +	bool "MX3 Overlay support"
> +	default n
> +	depends on FB_MX3
> +	---help---
> +	  Say Y here to enable overlay support
> +
>  config FB_BROADSHEET
>  	tristate "E-Ink Broadsheet/Epson S1D13521 controller support"
>  	depends on FB
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index eec0d7b..0fb8a72 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -26,6 +26,7 @@
>  #include <linux/console.h>
>  #include <linux/clk.h>
>  #include <linux/mutex.h>
> +#include <linux/mxcfb.h>
>  
>  #include <mach/dma.h>
>  #include <mach/hardware.h>
> @@ -238,6 +239,7 @@ static const struct fb_videomode mx3fb_modedb[] = {
>  
>  struct mx3fb_data {
>  	struct fb_info		*fbi;
> +	struct fb_info		*fbi_ovl;
>  	int			backlight_level;
>  	void __iomem		*reg_base;
>  	spinlock_t		lock;
> @@ -246,6 +248,9 @@ struct mx3fb_data {
>  	uint32_t		h_start_width;
>  	uint32_t		v_start_width;
>  	enum disp_data_mapping	disp_data_fmt;
> +
> +	/* IDMAC / dmaengine interface */
> +	struct idmac_channel	*idmac_channel[2];	/* We need 2 channels */
>  };
>  
>  struct dma_chan_request {
> @@ -272,6 +277,17 @@ struct mx3fb_info {
>  	u32				sync;	/* preserve var->sync flags */
>  };
>  
> +/* Allocated overlay buffer */
> +struct mx3fb_alloc_list {
> +	struct list_head	list;
> +	dma_addr_t		phy_addr;
> +	void			*cpu_addr;
> +	size_t			size;
> +};
> +
> +/* A list of overlay buffers */
> +static LIST_HEAD(fb_alloc_list);

Static variables are evil:-) Which you prove below by protecting this 
global list-head by a per-device mutex. No, I have no idea whether anyone 
ever comes up with an SoC with multiple instances of this device, but at 
least this seems inconsistent to me.

> +
>  static void mx3fb_dma_done(void *);
>  
>  /* Used fb-mode and bpp. Can be set on kernel command line, therefore file-static. */
> @@ -303,7 +319,11 @@ static void sdc_fb_init(struct mx3fb_info *fbi)
>  	struct mx3fb_data *mx3fb = fbi->mx3fb;
>  	uint32_t reg;
>  
> -	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
> +	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF) & ~SDC_COM_GWSEL;
> +
> +	/* Also enable foreground for overlay graphic window is foreground */
> +	if (mx3fb->fbi_ovl && fbi = mx3fb->fbi_ovl->par)
> +		reg |= (SDC_COM_FG_EN | SDC_COM_GWSEL);

Superfluous parenthesis.

>  
>  	mx3fb_write_reg(mx3fb, reg | SDC_COM_BG_EN, SDC_COM_CONF);
>  }
> @@ -312,13 +332,24 @@ static void sdc_fb_init(struct mx3fb_info *fbi)
>  static uint32_t sdc_fb_uninit(struct mx3fb_info *fbi)
>  {
>  	struct mx3fb_data *mx3fb = fbi->mx3fb;
> -	uint32_t reg;
> +	uint32_t reg, chan_mask;
>  
>  	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
>  
> -	mx3fb_write_reg(mx3fb, reg & ~SDC_COM_BG_EN, SDC_COM_CONF);
> +	/*
> +	 * Don't we have to automatically disable overlay when disabling
> +	 * background? Attention: cannot test mx3fb->fbi_ovl->par, must
> +	 * test mx3fb->fbi->par, because at the time this function is
> +	 * called for the first time fbi_ovl is not assigned yet.
> +	 */
> +	if (fbi = mx3fb->fbi->par)
> +		chan_mask = SDC_COM_BG_EN;
> +	else
> +		chan_mask = SDC_COM_FG_EN | SDC_COM_GWSEL;
> +
> +	mx3fb_write_reg(mx3fb, reg & ~chan_mask, SDC_COM_CONF);
>  
> -	return reg & SDC_COM_BG_EN;
> +	return reg & chan_mask;
>  }
>  
>  static void sdc_enable_channel(struct mx3fb_info *mx3_fbi)
> @@ -412,13 +443,20 @@ static void sdc_disable_channel(struct mx3fb_info *mx3_fbi)
>  static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel,
>  			      int16_t x_pos, int16_t y_pos)
>  {
> -	if (channel != IDMAC_SDC_0)
> -		return -EINVAL;
> -
>  	x_pos += mx3fb->h_start_width;
>  	y_pos += mx3fb->v_start_width;
>  
> -	mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_BG_POS);
> +	switch (channel) {
> +	case IDMAC_SDC_0:
> +		mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_BG_POS);
> +		break;
> +	case IDMAC_SDC_1:
> +		mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_FG_POS);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -482,14 +520,17 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
>  	mx3fb->h_start_width = h_start_width;
>  	mx3fb->v_start_width = v_start_width;
>  
> +	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
> +
>  	switch (panel) {
>  	case IPU_PANEL_SHARP_TFT:
>  		mx3fb_write_reg(mx3fb, 0x00FD0102L, SDC_SHARP_CONF_1);
>  		mx3fb_write_reg(mx3fb, 0x00F500F4L, SDC_SHARP_CONF_2);
> -		mx3fb_write_reg(mx3fb, SDC_COM_SHARP | SDC_COM_TFT_COLOR, SDC_COM_CONF);
> +		mx3fb_write_reg(mx3fb, reg | SDC_COM_SHARP |
> +				SDC_COM_TFT_COLOR, SDC_COM_CONF);
>  		break;
>  	case IPU_PANEL_TFT:
> -		mx3fb_write_reg(mx3fb, SDC_COM_TFT_COLOR, SDC_COM_CONF);
> +		mx3fb_write_reg(mx3fb, reg | SDC_COM_TFT_COLOR, SDC_COM_CONF);
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -563,13 +604,12 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
>  /**
>   * sdc_set_color_key() - set the transparent color key for SDC graphic plane.
>   * @mx3fb:	mx3fb context.
> - * @channel:	IPU DMAC channel ID.
>   * @enable:	boolean to enable or disable color keyl.
>   * @color_key:	24-bit RGB color to use as transparent color key.
>   * @return:	0 on success or negative error code on failure.
>   */
> -static int sdc_set_color_key(struct mx3fb_data *mx3fb, enum ipu_channel channel,
> -			     bool enable, uint32_t color_key)
> +static int sdc_set_color_key(struct mx3fb_data *mx3fb, bool enable,
> +				uint32_t color_key)
>  {
>  	uint32_t reg, sdc_conf;
>  	unsigned long lock_flags;
> @@ -577,10 +617,6 @@ static int sdc_set_color_key(struct mx3fb_data *mx3fb, enum ipu_channel channel,
>  	spin_lock_irqsave(&mx3fb->lock, lock_flags);
>  
>  	sdc_conf = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
> -	if (channel = IDMAC_SDC_0)
> -		sdc_conf &= ~SDC_COM_GWSEL;
> -	else
> -		sdc_conf |= SDC_COM_GWSEL;
>  
>  	if (enable) {
>  		reg = mx3fb_read_reg(mx3fb, SDC_GW_CTRL) & 0xFF000000L;
> @@ -668,8 +704,12 @@ static int mx3fb_set_fix(struct fb_info *fbi)
>  {
>  	struct fb_fix_screeninfo *fix = &fbi->fix;
>  	struct fb_var_screeninfo *var = &fbi->var;
> +	struct mx3fb_info *mx3_fbi = fbi->par;
>  
> -	strncpy(fix->id, "DISP3 BG", 8);
> +	if (mx3_fbi->ipu_ch = IDMAC_SDC_1)
> +		strncpy(fix->id, "DISP3 FG", 8);
> +	else
> +		strncpy(fix->id, "DISP3 BG", 8);
>  
>  	fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
>  
> @@ -689,13 +729,25 @@ static void mx3fb_dma_done(void *arg)
>  	struct idmac_channel *ichannel = to_idmac_chan(chan);
>  	struct mx3fb_data *mx3fb = ichannel->client;
>  	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
> +	struct mx3fb_info *mx3_fbi_cur;
> +	struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl ? mx3fb->fbi_ovl->par :
> +		NULL;
>  
>  	dev_dbg(mx3fb->dev, "irq %d callback\n", ichannel->eof_irq);
>  
> +	if (ichannel = mx3_fbi->idmac_channel) {
> +		mx3_fbi_cur = mx3_fbi;
> +	} else if (mx3_fbi_ovl && ichannel = mx3_fbi_ovl->idmac_channel) {
> +		mx3_fbi_cur = mx3_fbi_ovl;
> +	} else {
> +		WARN(1, "Cannot identify channel!\n");
> +		return;
> +	}
> +
>  	/* We only need one interrupt, it will be re-enabled as needed */
>  	disable_irq_nosync(ichannel->eof_irq);
>  
> -	complete(&mx3_fbi->flip_cmpl);
> +	complete(&mx3_fbi_cur->flip_cmpl);
>  }
>  
>  static int __set_par(struct fb_info *fbi, bool lock)
> @@ -1151,6 +1203,145 @@ static struct fb_ops mx3fb_ops = {
>  	.fb_blank = mx3fb_blank,
>  };
>  
> +#ifdef CONFIG_FB_MX3_OVERLAY
> +static int mx3fb_blank_ovl(int blank, struct fb_info *fbi)
> +{
> +	struct mx3fb_info *mx3_fbi = fbi->par;
> +
> +	dev_dbg(fbi->device, "ovl blank = %d\n", blank);
> +
> +	if (mx3_fbi->blank = blank)
> +		return 0;
> +
> +	mutex_lock(&mx3_fbi->mutex);
> +	mx3_fbi->blank = blank;
> +
> +	switch (blank) {
> +	case FB_BLANK_POWERDOWN:
> +	case FB_BLANK_VSYNC_SUSPEND:
> +	case FB_BLANK_HSYNC_SUSPEND:
> +	case FB_BLANK_NORMAL:
> +		sdc_disable_channel(mx3_fbi);
> +		break;
> +	case FB_BLANK_UNBLANK:
> +		sdc_enable_channel(mx3_fbi);
> +		break;
> +	}
> +	mutex_unlock(&mx3_fbi->mutex);
> +
> +	return 0;
> +}
> +
> +/*
> + * Function to handle custom ioctls for MX3 framebuffer.
> + *
> + *  @inode	inode struct
> + *  @file	file struct
> + *  @cmd	Ioctl command to handle
> + *  @arg	User pointer to command arguments
> + *  @fbi	framebuffer information pointer
> + */
> +static int mx3fb_ioctl_ovl(struct fb_info *fbi, unsigned int cmd,
> +			   unsigned long arg)
> +{
> +	struct mx3fb_info *mx3_fbi = fbi->par;
> +	struct mx3fb_data *mx3fb = mx3_fbi->mx3fb;
> +	struct mxcfb_gbl_alpha ga;
> +	struct mxcfb_color_key key;
> +	int retval = 0;
> +	int __user *argp = (void __user *)arg;
> +	struct mx3fb_alloc_list *mem;
> +	int size;
> +	unsigned long offset;
> +
> +	switch (cmd) {
> +	case FBIO_ALLOC:
> +		if (get_user(size, argp))
> +			return -EFAULT;
> +
> +		mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> +		if (mem = NULL)
> +			return -ENOMEM;
> +
> +		mem->size = PAGE_ALIGN(size);
> +
> +		mem->cpu_addr = dma_alloc_coherent(fbi->device, size,
> +						   &mem->phy_addr,
> +						   GFP_DMA);
> +		if (mem->cpu_addr = NULL) {
> +			kfree(mem);
> +			return -ENOMEM;
> +		}
> +
> +		mutex_lock(&mx3_fbi->mutex);
> +		list_add(&mem->list, &fb_alloc_list);
> +		mutex_unlock(&mx3_fbi->mutex);

Here. At the very least you'd need a global mutex. Or put the list-head in 
struct mx3fb_info.

> +
> +		dev_dbg(fbi->device, "allocated %d bytes  <at>  0x%08X\n",
> +			mem->size, mem->phy_addr);
> +
> +		if (put_user(mem->phy_addr, argp))
> +			return -EFAULT;
> +
> +		break;
> +	case FBIO_FREE:
> +		if (get_user(offset, argp))
> +			return -EFAULT;
> +
> +		retval = -EINVAL;
> +		mutex_lock(&mx3_fbi->mutex);
> +		list_for_each_entry(mem, &fb_alloc_list, list) {
> +			if (mem->phy_addr = offset) {
> +				list_del(&mem->list);
> +				dma_free_coherent(fbi->device,
> +						  mem->size,
> +						  mem->cpu_addr,
> +						  mem->phy_addr);
> +				kfree(mem);
> +				retval = 0;
> +				break;
> +			}
> +		}
> +		mutex_unlock(&mx3_fbi->mutex);
> +
> +		break;
> +	case MXCFB_SET_GBL_ALPHA:

Are you using these proprietary ioctl()s? If not, I wouldn't implement 
them. If you do need them, maybe it would make sense to add such ioctl()s 
globally for fbdev?

> +		if (copy_from_user(&ga, (void *)arg, sizeof(ga)))
> +			retval = -EFAULT;
> +
> +		sdc_set_global_alpha(mx3fb, (bool)ga.enable, ga.alpha);
> +		dev_dbg(fbi->device, "Set global alpha to %d\n", ga.alpha);
> +
> +		break;
> +	case MXCFB_SET_CLR_KEY:
> +		if (copy_from_user(&key, (void *)arg, sizeof(key)))
> +			retval = -EFAULT;
> +
> +		sdc_set_color_key(mx3fb, (bool)key.enable, key.color_key);
> +		dev_dbg(fbi->device, "Set color key to %d\n", key.color_key);
> +
> +		break;
> +	default:
> +		retval = -EINVAL;
> +	}
> +
> +	return retval;
> +}
> +
> +static struct fb_ops mx3fb_ovl_ops = {
> +	.owner = THIS_MODULE,
> +	.fb_set_par = mx3fb_set_par,
> +	.fb_check_var = mx3fb_check_var,
> +	.fb_setcolreg = mx3fb_setcolreg,
> +	.fb_pan_display = mx3fb_pan_display,
> +	.fb_ioctl = mx3fb_ioctl_ovl,
> +	.fb_fillrect = cfb_fillrect,
> +	.fb_copyarea = cfb_copyarea,
> +	.fb_imageblit = cfb_imageblit,
> +	.fb_blank = mx3fb_blank_ovl,
> +};
> +#endif
> +
>  #ifdef CONFIG_PM
>  /*
>   * Power management hooks.      Note that we won't be called from IRQ context,
> @@ -1164,11 +1355,16 @@ static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
>  {
>  	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
>  	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
> +	struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl->par;
>  
>  	console_lock();
>  	fb_set_suspend(mx3fb->fbi, 1);
> +	fb_set_suspend(mx3fb->fbi_ovl, 1);
>  	console_unlock();
>  
> +	if (mx3_fbi_ovl->blank = FB_BLANK_UNBLANK)
> +		sdc_disable_channel(mx3_fbi_ovl);
> +
>  	if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
>  		sdc_disable_channel(mx3_fbi);
>  		sdc_set_brightness(mx3fb, 0);
> @@ -1184,14 +1380,19 @@ static int mx3fb_resume(struct platform_device *pdev)
>  {
>  	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
>  	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
> +	struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl->par;
>  
>  	if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
>  		sdc_enable_channel(mx3_fbi);
>  		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
>  	}
>  
> +	if (mx3_fbi_ovl->blank = FB_BLANK_UNBLANK)
> +		sdc_enable_channel(mx3_fbi_ovl);
> +
>  	console_lock();
>  	fb_set_suspend(mx3fb->fbi, 0);
> +	fb_set_suspend(mx3fb->fbi_ovl, 0);
>  	console_unlock();
>  
>  	return 0;
> @@ -1333,8 +1534,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  	ichan->client = mx3fb;
>  	irq = ichan->eof_irq;
>  
> -	if (ichan->dma_chan.chan_id != IDMAC_SDC_0)
> -		return -EINVAL;
> +	switch (ichan->dma_chan.chan_id) {
> +	case IDMAC_SDC_0:
>  
>  	fbi = mx3fb_init_fbinfo(dev, &mx3fb_ops);

I would bite the bullet and indent this case block...

>  	if (!fbi)
> @@ -1375,7 +1576,29 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  
>  	sdc_set_brightness(mx3fb, 255);
>  	sdc_set_global_alpha(mx3fb, true, 0xFF);
> -	sdc_set_color_key(mx3fb, IDMAC_SDC_0, false, 0);
> +	sdc_set_color_key(mx3fb, false, 0);
> +
> +	break;
> +#ifdef CONFIG_FB_MX3_OVERLAY
> +	case IDMAC_SDC_1:
> +
> +		/* We know, that background has been allocated already! */
> +		fbi = mx3fb_init_fbinfo(dev, &mx3fb_ovl_ops);
> +		if (!fbi)
> +			return -ENOMEM;
> +
> +		/* Default Y virtual size is 2x panel size */
> +		fbi->var = mx3fb->fbi->var;
> +		/* This shouldn't be necessary, it is already set up above */
> +		fbi->var.yres_virtual = mx3fb->fbi->var.yres * 2;
> +
> +		mx3fb->fbi_ovl = fbi;
> +
> +		break;
> +#endif
> +	default:
> +		return -EINVAL;
> +	}
>  
>  	mx3fbi			= fbi->par;
>  	mx3fbi->idmac_channel	= ichan;
> @@ -1392,9 +1615,13 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  	if (ret < 0)
>  		goto esetpar;
>  
> -	__blank(FB_BLANK_UNBLANK, fbi);
> +	/* Overlay stays blanked by default */
> +	if (ichan->dma_chan.chan_id = IDMAC_SDC_0) {
> +		mx3fb_blank(FB_BLANK_UNBLANK, fbi);
>  
> -	dev_info(dev, "registered, using mode %s\n", fb_mode);
> +		dev_info(dev, "mx3fb: fb registered, using mode %s [%c]\n",
> +		fb_mode, list_empty(&ichan->queue) ? '-' : '+');
> +	}
>  
>  	ret = register_framebuffer(fbi);
>  	if (ret < 0)
> @@ -1492,14 +1719,42 @@ static int mx3fb_probe(struct platform_device *pdev)
>  
>  	mx3fb->backlight_level = 255;
>  
> +	mx3fb->idmac_channel[0] = to_idmac_chan(chan);
> +	mx3fb->idmac_channel[0]->client = mx3fb;
> +
>  	ret = init_fb_chan(mx3fb, to_idmac_chan(chan));
>  	if (ret < 0)
>  		goto eisdc0;
>  
> +#ifdef CONFIG_FB_MX3_OVERLAY
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_SLAVE, mask);
> +	dma_cap_set(DMA_PRIVATE, mask);
> +	rq.id = IDMAC_SDC_1;
> +	chan = dma_request_channel(mask, chan_filter, &rq);
> +	if (!chan) {
> +		ret = -EBUSY;
> +		goto ersdc1;
> +	}
> +
> +	mx3fb->idmac_channel[1] = to_idmac_chan(chan);
> +	mx3fb->idmac_channel[1]->client = mx3fb;
> +
> +	ret = init_fb_chan(mx3fb, to_idmac_chan(chan));
> +	if (ret < 0)
> +		goto eisdc1;
> +#endif
> +
>  	return 0;
>  
> +#ifdef CONFIG_FB_MX3_OVERLAY
> +eisdc1:
> +	dma_release_channel(&mx3fb->idmac_channel[1]->dma_chan);
> +ersdc1:
> +	release_fbi(mx3fb->fbi);
> +#endif
>  eisdc0:
> -	dma_release_channel(chan);
> +	dma_release_channel(&mx3fb->idmac_channel[0]->dma_chan);
>  ersdc0:
>  	dmaengine_put();
>  	iounmap(mx3fb->reg_base);
> @@ -1513,13 +1768,16 @@ static int mx3fb_remove(struct platform_device *dev)
>  {
>  	struct mx3fb_data *mx3fb = platform_get_drvdata(dev);
>  	struct fb_info *fbi = mx3fb->fbi;
> -	struct mx3fb_info *mx3_fbi = fbi->par;
> -	struct dma_chan *chan;
>  
> -	chan = &mx3_fbi->idmac_channel->dma_chan;
> -	release_fbi(fbi);
> +	if (fbi)
> +		release_fbi(fbi);
> +
> +	fbi = mx3fb->fbi_ovl;
> +	if (fbi)
> +		release_fbi(fbi);
>  
> -	dma_release_channel(chan);
> +	dma_release_channel(&mx3fb->idmac_channel[1]->dma_chan);
> +	dma_release_channel(&mx3fb->idmac_channel[0]->dma_chan);
>  	dmaengine_put();
>  
>  	iounmap(mx3fb->reg_base);
> diff --git a/include/linux/mxcfb.h b/include/linux/mxcfb.h
> new file mode 100644
> index 0000000..54b720d
> --- /dev/null
> +++ b/include/linux/mxcfb.h
> @@ -0,0 +1,93 @@
> +/*
> + * File: include/linux/mxcfb.h
> + * Global header file for the MXC Framebuffer
> + *
> + * Copyright 2004-2012 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * The code contained herein is licensed under the GNU Lesser General
> + * Public License.  You may obtain a copy of the GNU Lesser General
> + * Public License Version 2.1 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/lgpl-license.html
> + * http://www.gnu.org/copyleft/lgpl.html
> + */
> +
> +#ifndef __LINUX_MXCFB_H__
> +#define __LINUX_MXCFB_H__
> +
> +#include <linux/fb.h>

Why is this needed here?

> +
> +#define FB_SYNC_OE_LOW_ACT	0x80000000
> +#define FB_SYNC_CLK_LAT_FALL	0x40000000
> +#define FB_SYNC_DATA_INVERT	0x20000000
> +#define FB_SYNC_CLK_IDLE_EN	0x10000000
> +#define FB_SYNC_SHARP_MODE	0x08000000
> +#define FB_SYNC_SWAP_RGB	0x04000000
> +
> +struct mxcfb_gbl_alpha {
> +	int enable;
> +	int alpha;
> +};
> +
> +struct mxcfb_color_key {
> +	int enable;
> +	__u32 color_key;
> +};
> +
> +struct mxcfb_pos {
> +	__u16 x;
> +	__u16 y;
> +};
> +
> +struct mxcfb_gamma {
> +	int enable;
> +	int constk[16];
> +	int slopek[16];
> +};
> +
> +struct mxcfb_rect {
> +	__u32 top;
> +	__u32 left;
> +	__u32 width;
> +	__u32 height;
> +};
> +
> +/*
> + * Structure used to define waveform modes for driver
> + * Needed for driver to perform auto-waveform selection
> + */
> +struct mxcfb_waveform_modes {
> +	int mode_init;
> +	int mode_du;
> +	int mode_gc4;
> +	int mode_gc8;
> +	int mode_gc16;
> +	int mode_gc32;
> +};
> +
> +/* IOCTL commands. */
> +
> +#define MXCFB_WAIT_FOR_VSYNC		_IOW('F', 0x20, u_int32_t)
> +#define MXCFB_SET_GBL_ALPHA		_IOW('F', 0x21, struct mxcfb_gbl_alpha)
> +#define MXCFB_SET_CLR_KEY		_IOW('F', 0x22, struct mxcfb_color_key)
> +#define MXCFB_SET_OVERLAY_POS		_IOWR('F', 0x24, struct mxcfb_pos)
> +#define MXCFB_GET_FB_IPU_CHAN		_IOR('F', 0x25, u_int32_t)
> +#define MXCFB_SET_LOC_ALPHA		_IOWR('F', 0x26, struct mxcfb_loc_alpha)
> +#define MXCFB_SET_LOC_ALP_BUF		_IOW('F', 0x27, unsigned long)
> +#define MXCFB_SET_GAMMA			_IOW('F', 0x28, struct mxcfb_gamma)
> +#define MXCFB_GET_FB_IPU_DI		_IOR('F', 0x29, u_int32_t)
> +#define MXCFB_GET_DIFMT			_IOR('F', 0x2A, u_int32_t)
> +#define MXCFB_GET_FB_BLANK		_IOR('F', 0x2B, u_int32_t)

Please, don't add unused identifiers.

> +
> +#ifdef __KERNEL__
> +
> +enum {
> +	MXCFB_REFRESH_OFF,
> +	MXCFB_REFRESH_AUTO,
> +	MXCFB_REFRESH_PARTIAL,
> +};
> +
> +#endif
> +
> +#endif /* _MXCFB_H */
> +
> -- 
> 1.7.0.4
> 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* RE: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
From: H Hartley Sweeten @ 2012-04-18 20:35 UTC (permalink / raw)
  To: Julia Lawall, Florian Tobias Schandinat
  Cc: kernel-janitors@vger.kernel.org, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1334777828-3557-1-git-send-email-Julia.Lawall@lip6.fr>

On Wednesday, April 18, 2012 12:37 PM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> There were two problems in this code: failure of the setup function should
> free locally allocated resources like other nearby failures, and the test
> if (&info->cmap) can never be false.  To generally clean things up, this
> patch reorders the error handling code at the failed label and adds labels
> so that the conditionals are not necessary.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>

Thanks!

Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>


^ permalink raw reply

* [PATCH] drivers/video/mbx/mbxfb.c: correct ioremap_nocache test
From: Julia Lawall @ 2012-04-18 20:07 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

The test tested not the most recently allocated value but a previously
allocated one.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/video/mbx/mbxfb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/mbx/mbxfb.c b/drivers/video/mbx/mbxfb.c
index 55bf619..ab0a8e5 100644
--- a/drivers/video/mbx/mbxfb.c
+++ b/drivers/video/mbx/mbxfb.c
@@ -950,7 +950,7 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
 
 	mfbi->fb_virt_addr = ioremap_nocache(mfbi->fb_phys_addr,
 					     res_size(mfbi->fb_req));
-	if (!mfbi->reg_virt_addr) {
+	if (!mfbi->fb_virt_addr) {
 		dev_err(&dev->dev, "failed to ioremap frame buffer\n");
 		ret = -EINVAL;
 		goto err4;


^ permalink raw reply related

* [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
From: Julia Lawall @ 2012-04-18 19:37 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

There were two problems in this code: failure of the setup function should
free locally allocated resources like other nearby failures, and the test
if (&info->cmap) can never be false.  To generally clean things up, this
patch reorders the error handling code at the failed label and adds labels
so that the conditionals are not necessary.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Not tested.

 drivers/video/ep93xx-fb.c |   32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/video/ep93xx-fb.c b/drivers/video/ep93xx-fb.c
index f8babbe..345d962 100644
--- a/drivers/video/ep93xx-fb.c
+++ b/drivers/video/ep93xx-fb.c
@@ -507,16 +507,16 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
 
 	err = fb_alloc_cmap(&info->cmap, 256, 0);
 	if (err)
-		goto failed;
+		goto failed_cmap;
 
 	err = ep93xxfb_alloc_videomem(info);
 	if (err)
-		goto failed;
+		goto failed_videomem;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		err = -ENXIO;
-		goto failed;
+		goto failed_resource;
 	}
 
 	/*
@@ -532,7 +532,7 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
 	fbi->mmio_base = ioremap(res->start, resource_size(res));
 	if (!fbi->mmio_base) {
 		err = -ENXIO;
-		goto failed;
+		goto failed_resource;
 	}
 
 	strcpy(info->fix.id, pdev->name);
@@ -553,24 +553,24 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
 	if (err = 0) {
 		dev_err(info->dev, "No suitable video mode found\n");
 		err = -EINVAL;
-		goto failed;
+		goto failed_mode;
 	}
 
 	if (mach_info->setup) {
 		err = mach_info->setup(pdev);
 		if (err)
-			return err;
+			goto failed_mode;
 	}
 
 	err = ep93xxfb_check_var(&info->var, info);
 	if (err)
-		goto failed;
+		goto failed_check;
 
 	fbi->clk = clk_get(info->dev, NULL);
 	if (IS_ERR(fbi->clk)) {
 		err = PTR_ERR(fbi->clk);
 		fbi->clk = NULL;
-		goto failed;
+		goto failed_check;
 	}
 
 	ep93xxfb_set_par(info);
@@ -585,15 +585,17 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
 	return 0;
 
 failed:
-	if (fbi->clk)
-		clk_put(fbi->clk);
-	if (fbi->mmio_base)
-		iounmap(fbi->mmio_base);
-	ep93xxfb_dealloc_videomem(info);
-	if (&info->cmap)
-		fb_dealloc_cmap(&info->cmap);
+	clk_put(fbi->clk);
+failed_check:
 	if (fbi->mach_info->teardown)
 		fbi->mach_info->teardown(pdev);
+failed_mode:
+	iounmap(fbi->mmio_base);
+failed_resource:
+	ep93xxfb_dealloc_videomem(info);
+failed_videomem:
+	fb_dealloc_cmap(&info->cmap);
+failed_cmap:
 	kfree(info);
 	platform_set_drvdata(pdev, NULL);
 


^ permalink raw reply related

* [PATCH v1] ARM: i.mx: mx3fb: add overlay support
From: Alex Gershgorin @ 2012-04-18 17:38 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: g.liakhovetski, s.hauer, laurent.pinchart, linux-fbdev,
	linux-media, Alex Gershgorin

This patch is based on the original version submitted by Guennadi Liakhovetski,
the patch initializes overlay channel, adds ioctl for configuring
transparency of the overlay and graphics planes, CONFIG_FB_MX3_OVERLAY
is also supported.

In case that CONFIG_FB_MX3_OVERLAY is not defined, mx3fb is completely
backward compatible.

Blend mode, only global alpha blending has been tested.

Signed-off-by: Alex Gershgorin <alexg@meprolight.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

Applies to v3.4-rc3
---
 drivers/video/Kconfig |    7 +
 drivers/video/mx3fb.c |  318 ++++++++++++++++++++++++++++++++++++++++++++-----
 include/linux/mxcfb.h |   93 ++++++++++++++
 3 files changed, 388 insertions(+), 30 deletions(-)
 create mode 100644 include/linux/mxcfb.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index a290be5..acbfccc 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2368,6 +2368,13 @@ config FB_MX3
 	  far only synchronous displays are supported. If you plan to use
 	  an LCD display with your i.MX31 system, say Y here.
 
+config FB_MX3_OVERLAY
+	bool "MX3 Overlay support"
+	default n
+	depends on FB_MX3
+	---help---
+	  Say Y here to enable overlay support
+
 config FB_BROADSHEET
 	tristate "E-Ink Broadsheet/Epson S1D13521 controller support"
 	depends on FB
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index eec0d7b..0fb8a72 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -26,6 +26,7 @@
 #include <linux/console.h>
 #include <linux/clk.h>
 #include <linux/mutex.h>
+#include <linux/mxcfb.h>
 
 #include <mach/dma.h>
 #include <mach/hardware.h>
@@ -238,6 +239,7 @@ static const struct fb_videomode mx3fb_modedb[] = {
 
 struct mx3fb_data {
 	struct fb_info		*fbi;
+	struct fb_info		*fbi_ovl;
 	int			backlight_level;
 	void __iomem		*reg_base;
 	spinlock_t		lock;
@@ -246,6 +248,9 @@ struct mx3fb_data {
 	uint32_t		h_start_width;
 	uint32_t		v_start_width;
 	enum disp_data_mapping	disp_data_fmt;
+
+	/* IDMAC / dmaengine interface */
+	struct idmac_channel	*idmac_channel[2];	/* We need 2 channels */
 };
 
 struct dma_chan_request {
@@ -272,6 +277,17 @@ struct mx3fb_info {
 	u32				sync;	/* preserve var->sync flags */
 };
 
+/* Allocated overlay buffer */
+struct mx3fb_alloc_list {
+	struct list_head	list;
+	dma_addr_t		phy_addr;
+	void			*cpu_addr;
+	size_t			size;
+};
+
+/* A list of overlay buffers */
+static LIST_HEAD(fb_alloc_list);
+
 static void mx3fb_dma_done(void *);
 
 /* Used fb-mode and bpp. Can be set on kernel command line, therefore file-static. */
@@ -303,7 +319,11 @@ static void sdc_fb_init(struct mx3fb_info *fbi)
 	struct mx3fb_data *mx3fb = fbi->mx3fb;
 	uint32_t reg;
 
-	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
+	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF) & ~SDC_COM_GWSEL;
+
+	/* Also enable foreground for overlay graphic window is foreground */
+	if (mx3fb->fbi_ovl && fbi = mx3fb->fbi_ovl->par)
+		reg |= (SDC_COM_FG_EN | SDC_COM_GWSEL);
 
 	mx3fb_write_reg(mx3fb, reg | SDC_COM_BG_EN, SDC_COM_CONF);
 }
@@ -312,13 +332,24 @@ static void sdc_fb_init(struct mx3fb_info *fbi)
 static uint32_t sdc_fb_uninit(struct mx3fb_info *fbi)
 {
 	struct mx3fb_data *mx3fb = fbi->mx3fb;
-	uint32_t reg;
+	uint32_t reg, chan_mask;
 
 	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
 
-	mx3fb_write_reg(mx3fb, reg & ~SDC_COM_BG_EN, SDC_COM_CONF);
+	/*
+	 * Don't we have to automatically disable overlay when disabling
+	 * background? Attention: cannot test mx3fb->fbi_ovl->par, must
+	 * test mx3fb->fbi->par, because at the time this function is
+	 * called for the first time fbi_ovl is not assigned yet.
+	 */
+	if (fbi = mx3fb->fbi->par)
+		chan_mask = SDC_COM_BG_EN;
+	else
+		chan_mask = SDC_COM_FG_EN | SDC_COM_GWSEL;
+
+	mx3fb_write_reg(mx3fb, reg & ~chan_mask, SDC_COM_CONF);
 
-	return reg & SDC_COM_BG_EN;
+	return reg & chan_mask;
 }
 
 static void sdc_enable_channel(struct mx3fb_info *mx3_fbi)
@@ -412,13 +443,20 @@ static void sdc_disable_channel(struct mx3fb_info *mx3_fbi)
 static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel,
 			      int16_t x_pos, int16_t y_pos)
 {
-	if (channel != IDMAC_SDC_0)
-		return -EINVAL;
-
 	x_pos += mx3fb->h_start_width;
 	y_pos += mx3fb->v_start_width;
 
-	mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_BG_POS);
+	switch (channel) {
+	case IDMAC_SDC_0:
+		mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_BG_POS);
+		break;
+	case IDMAC_SDC_1:
+		mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_FG_POS);
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -482,14 +520,17 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 	mx3fb->h_start_width = h_start_width;
 	mx3fb->v_start_width = v_start_width;
 
+	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
+
 	switch (panel) {
 	case IPU_PANEL_SHARP_TFT:
 		mx3fb_write_reg(mx3fb, 0x00FD0102L, SDC_SHARP_CONF_1);
 		mx3fb_write_reg(mx3fb, 0x00F500F4L, SDC_SHARP_CONF_2);
-		mx3fb_write_reg(mx3fb, SDC_COM_SHARP | SDC_COM_TFT_COLOR, SDC_COM_CONF);
+		mx3fb_write_reg(mx3fb, reg | SDC_COM_SHARP |
+				SDC_COM_TFT_COLOR, SDC_COM_CONF);
 		break;
 	case IPU_PANEL_TFT:
-		mx3fb_write_reg(mx3fb, SDC_COM_TFT_COLOR, SDC_COM_CONF);
+		mx3fb_write_reg(mx3fb, reg | SDC_COM_TFT_COLOR, SDC_COM_CONF);
 		break;
 	default:
 		return -EINVAL;
@@ -563,13 +604,12 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 /**
  * sdc_set_color_key() - set the transparent color key for SDC graphic plane.
  * @mx3fb:	mx3fb context.
- * @channel:	IPU DMAC channel ID.
  * @enable:	boolean to enable or disable color keyl.
  * @color_key:	24-bit RGB color to use as transparent color key.
  * @return:	0 on success or negative error code on failure.
  */
-static int sdc_set_color_key(struct mx3fb_data *mx3fb, enum ipu_channel channel,
-			     bool enable, uint32_t color_key)
+static int sdc_set_color_key(struct mx3fb_data *mx3fb, bool enable,
+				uint32_t color_key)
 {
 	uint32_t reg, sdc_conf;
 	unsigned long lock_flags;
@@ -577,10 +617,6 @@ static int sdc_set_color_key(struct mx3fb_data *mx3fb, enum ipu_channel channel,
 	spin_lock_irqsave(&mx3fb->lock, lock_flags);
 
 	sdc_conf = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
-	if (channel = IDMAC_SDC_0)
-		sdc_conf &= ~SDC_COM_GWSEL;
-	else
-		sdc_conf |= SDC_COM_GWSEL;
 
 	if (enable) {
 		reg = mx3fb_read_reg(mx3fb, SDC_GW_CTRL) & 0xFF000000L;
@@ -668,8 +704,12 @@ static int mx3fb_set_fix(struct fb_info *fbi)
 {
 	struct fb_fix_screeninfo *fix = &fbi->fix;
 	struct fb_var_screeninfo *var = &fbi->var;
+	struct mx3fb_info *mx3_fbi = fbi->par;
 
-	strncpy(fix->id, "DISP3 BG", 8);
+	if (mx3_fbi->ipu_ch = IDMAC_SDC_1)
+		strncpy(fix->id, "DISP3 FG", 8);
+	else
+		strncpy(fix->id, "DISP3 BG", 8);
 
 	fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
 
@@ -689,13 +729,25 @@ static void mx3fb_dma_done(void *arg)
 	struct idmac_channel *ichannel = to_idmac_chan(chan);
 	struct mx3fb_data *mx3fb = ichannel->client;
 	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
+	struct mx3fb_info *mx3_fbi_cur;
+	struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl ? mx3fb->fbi_ovl->par :
+		NULL;
 
 	dev_dbg(mx3fb->dev, "irq %d callback\n", ichannel->eof_irq);
 
+	if (ichannel = mx3_fbi->idmac_channel) {
+		mx3_fbi_cur = mx3_fbi;
+	} else if (mx3_fbi_ovl && ichannel = mx3_fbi_ovl->idmac_channel) {
+		mx3_fbi_cur = mx3_fbi_ovl;
+	} else {
+		WARN(1, "Cannot identify channel!\n");
+		return;
+	}
+
 	/* We only need one interrupt, it will be re-enabled as needed */
 	disable_irq_nosync(ichannel->eof_irq);
 
-	complete(&mx3_fbi->flip_cmpl);
+	complete(&mx3_fbi_cur->flip_cmpl);
 }
 
 static int __set_par(struct fb_info *fbi, bool lock)
@@ -1151,6 +1203,145 @@ static struct fb_ops mx3fb_ops = {
 	.fb_blank = mx3fb_blank,
 };
 
+#ifdef CONFIG_FB_MX3_OVERLAY
+static int mx3fb_blank_ovl(int blank, struct fb_info *fbi)
+{
+	struct mx3fb_info *mx3_fbi = fbi->par;
+
+	dev_dbg(fbi->device, "ovl blank = %d\n", blank);
+
+	if (mx3_fbi->blank = blank)
+		return 0;
+
+	mutex_lock(&mx3_fbi->mutex);
+	mx3_fbi->blank = blank;
+
+	switch (blank) {
+	case FB_BLANK_POWERDOWN:
+	case FB_BLANK_VSYNC_SUSPEND:
+	case FB_BLANK_HSYNC_SUSPEND:
+	case FB_BLANK_NORMAL:
+		sdc_disable_channel(mx3_fbi);
+		break;
+	case FB_BLANK_UNBLANK:
+		sdc_enable_channel(mx3_fbi);
+		break;
+	}
+	mutex_unlock(&mx3_fbi->mutex);
+
+	return 0;
+}
+
+/*
+ * Function to handle custom ioctls for MX3 framebuffer.
+ *
+ *  @inode	inode struct
+ *  @file	file struct
+ *  @cmd	Ioctl command to handle
+ *  @arg	User pointer to command arguments
+ *  @fbi	framebuffer information pointer
+ */
+static int mx3fb_ioctl_ovl(struct fb_info *fbi, unsigned int cmd,
+			   unsigned long arg)
+{
+	struct mx3fb_info *mx3_fbi = fbi->par;
+	struct mx3fb_data *mx3fb = mx3_fbi->mx3fb;
+	struct mxcfb_gbl_alpha ga;
+	struct mxcfb_color_key key;
+	int retval = 0;
+	int __user *argp = (void __user *)arg;
+	struct mx3fb_alloc_list *mem;
+	int size;
+	unsigned long offset;
+
+	switch (cmd) {
+	case FBIO_ALLOC:
+		if (get_user(size, argp))
+			return -EFAULT;
+
+		mem = kzalloc(sizeof(*mem), GFP_KERNEL);
+		if (mem = NULL)
+			return -ENOMEM;
+
+		mem->size = PAGE_ALIGN(size);
+
+		mem->cpu_addr = dma_alloc_coherent(fbi->device, size,
+						   &mem->phy_addr,
+						   GFP_DMA);
+		if (mem->cpu_addr = NULL) {
+			kfree(mem);
+			return -ENOMEM;
+		}
+
+		mutex_lock(&mx3_fbi->mutex);
+		list_add(&mem->list, &fb_alloc_list);
+		mutex_unlock(&mx3_fbi->mutex);
+
+		dev_dbg(fbi->device, "allocated %d bytes  <at>  0x%08X\n",
+			mem->size, mem->phy_addr);
+
+		if (put_user(mem->phy_addr, argp))
+			return -EFAULT;
+
+		break;
+	case FBIO_FREE:
+		if (get_user(offset, argp))
+			return -EFAULT;
+
+		retval = -EINVAL;
+		mutex_lock(&mx3_fbi->mutex);
+		list_for_each_entry(mem, &fb_alloc_list, list) {
+			if (mem->phy_addr = offset) {
+				list_del(&mem->list);
+				dma_free_coherent(fbi->device,
+						  mem->size,
+						  mem->cpu_addr,
+						  mem->phy_addr);
+				kfree(mem);
+				retval = 0;
+				break;
+			}
+		}
+		mutex_unlock(&mx3_fbi->mutex);
+
+		break;
+	case MXCFB_SET_GBL_ALPHA:
+		if (copy_from_user(&ga, (void *)arg, sizeof(ga)))
+			retval = -EFAULT;
+
+		sdc_set_global_alpha(mx3fb, (bool)ga.enable, ga.alpha);
+		dev_dbg(fbi->device, "Set global alpha to %d\n", ga.alpha);
+
+		break;
+	case MXCFB_SET_CLR_KEY:
+		if (copy_from_user(&key, (void *)arg, sizeof(key)))
+			retval = -EFAULT;
+
+		sdc_set_color_key(mx3fb, (bool)key.enable, key.color_key);
+		dev_dbg(fbi->device, "Set color key to %d\n", key.color_key);
+
+		break;
+	default:
+		retval = -EINVAL;
+	}
+
+	return retval;
+}
+
+static struct fb_ops mx3fb_ovl_ops = {
+	.owner = THIS_MODULE,
+	.fb_set_par = mx3fb_set_par,
+	.fb_check_var = mx3fb_check_var,
+	.fb_setcolreg = mx3fb_setcolreg,
+	.fb_pan_display = mx3fb_pan_display,
+	.fb_ioctl = mx3fb_ioctl_ovl,
+	.fb_fillrect = cfb_fillrect,
+	.fb_copyarea = cfb_copyarea,
+	.fb_imageblit = cfb_imageblit,
+	.fb_blank = mx3fb_blank_ovl,
+};
+#endif
+
 #ifdef CONFIG_PM
 /*
  * Power management hooks.      Note that we won't be called from IRQ context,
@@ -1164,11 +1355,16 @@ static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
 	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
+	struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl->par;
 
 	console_lock();
 	fb_set_suspend(mx3fb->fbi, 1);
+	fb_set_suspend(mx3fb->fbi_ovl, 1);
 	console_unlock();
 
+	if (mx3_fbi_ovl->blank = FB_BLANK_UNBLANK)
+		sdc_disable_channel(mx3_fbi_ovl);
+
 	if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
 		sdc_disable_channel(mx3_fbi);
 		sdc_set_brightness(mx3fb, 0);
@@ -1184,14 +1380,19 @@ static int mx3fb_resume(struct platform_device *pdev)
 {
 	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
 	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
+	struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl->par;
 
 	if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
 		sdc_enable_channel(mx3_fbi);
 		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
 	}
 
+	if (mx3_fbi_ovl->blank = FB_BLANK_UNBLANK)
+		sdc_enable_channel(mx3_fbi_ovl);
+
 	console_lock();
 	fb_set_suspend(mx3fb->fbi, 0);
+	fb_set_suspend(mx3fb->fbi_ovl, 0);
 	console_unlock();
 
 	return 0;
@@ -1333,8 +1534,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	ichan->client = mx3fb;
 	irq = ichan->eof_irq;
 
-	if (ichan->dma_chan.chan_id != IDMAC_SDC_0)
-		return -EINVAL;
+	switch (ichan->dma_chan.chan_id) {
+	case IDMAC_SDC_0:
 
 	fbi = mx3fb_init_fbinfo(dev, &mx3fb_ops);
 	if (!fbi)
@@ -1375,7 +1576,29 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 
 	sdc_set_brightness(mx3fb, 255);
 	sdc_set_global_alpha(mx3fb, true, 0xFF);
-	sdc_set_color_key(mx3fb, IDMAC_SDC_0, false, 0);
+	sdc_set_color_key(mx3fb, false, 0);
+
+	break;
+#ifdef CONFIG_FB_MX3_OVERLAY
+	case IDMAC_SDC_1:
+
+		/* We know, that background has been allocated already! */
+		fbi = mx3fb_init_fbinfo(dev, &mx3fb_ovl_ops);
+		if (!fbi)
+			return -ENOMEM;
+
+		/* Default Y virtual size is 2x panel size */
+		fbi->var = mx3fb->fbi->var;
+		/* This shouldn't be necessary, it is already set up above */
+		fbi->var.yres_virtual = mx3fb->fbi->var.yres * 2;
+
+		mx3fb->fbi_ovl = fbi;
+
+		break;
+#endif
+	default:
+		return -EINVAL;
+	}
 
 	mx3fbi			= fbi->par;
 	mx3fbi->idmac_channel	= ichan;
@@ -1392,9 +1615,13 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	if (ret < 0)
 		goto esetpar;
 
-	__blank(FB_BLANK_UNBLANK, fbi);
+	/* Overlay stays blanked by default */
+	if (ichan->dma_chan.chan_id = IDMAC_SDC_0) {
+		mx3fb_blank(FB_BLANK_UNBLANK, fbi);
 
-	dev_info(dev, "registered, using mode %s\n", fb_mode);
+		dev_info(dev, "mx3fb: fb registered, using mode %s [%c]\n",
+		fb_mode, list_empty(&ichan->queue) ? '-' : '+');
+	}
 
 	ret = register_framebuffer(fbi);
 	if (ret < 0)
@@ -1492,14 +1719,42 @@ static int mx3fb_probe(struct platform_device *pdev)
 
 	mx3fb->backlight_level = 255;
 
+	mx3fb->idmac_channel[0] = to_idmac_chan(chan);
+	mx3fb->idmac_channel[0]->client = mx3fb;
+
 	ret = init_fb_chan(mx3fb, to_idmac_chan(chan));
 	if (ret < 0)
 		goto eisdc0;
 
+#ifdef CONFIG_FB_MX3_OVERLAY
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	dma_cap_set(DMA_PRIVATE, mask);
+	rq.id = IDMAC_SDC_1;
+	chan = dma_request_channel(mask, chan_filter, &rq);
+	if (!chan) {
+		ret = -EBUSY;
+		goto ersdc1;
+	}
+
+	mx3fb->idmac_channel[1] = to_idmac_chan(chan);
+	mx3fb->idmac_channel[1]->client = mx3fb;
+
+	ret = init_fb_chan(mx3fb, to_idmac_chan(chan));
+	if (ret < 0)
+		goto eisdc1;
+#endif
+
 	return 0;
 
+#ifdef CONFIG_FB_MX3_OVERLAY
+eisdc1:
+	dma_release_channel(&mx3fb->idmac_channel[1]->dma_chan);
+ersdc1:
+	release_fbi(mx3fb->fbi);
+#endif
 eisdc0:
-	dma_release_channel(chan);
+	dma_release_channel(&mx3fb->idmac_channel[0]->dma_chan);
 ersdc0:
 	dmaengine_put();
 	iounmap(mx3fb->reg_base);
@@ -1513,13 +1768,16 @@ static int mx3fb_remove(struct platform_device *dev)
 {
 	struct mx3fb_data *mx3fb = platform_get_drvdata(dev);
 	struct fb_info *fbi = mx3fb->fbi;
-	struct mx3fb_info *mx3_fbi = fbi->par;
-	struct dma_chan *chan;
 
-	chan = &mx3_fbi->idmac_channel->dma_chan;
-	release_fbi(fbi);
+	if (fbi)
+		release_fbi(fbi);
+
+	fbi = mx3fb->fbi_ovl;
+	if (fbi)
+		release_fbi(fbi);
 
-	dma_release_channel(chan);
+	dma_release_channel(&mx3fb->idmac_channel[1]->dma_chan);
+	dma_release_channel(&mx3fb->idmac_channel[0]->dma_chan);
 	dmaengine_put();
 
 	iounmap(mx3fb->reg_base);
diff --git a/include/linux/mxcfb.h b/include/linux/mxcfb.h
new file mode 100644
index 0000000..54b720d
--- /dev/null
+++ b/include/linux/mxcfb.h
@@ -0,0 +1,93 @@
+/*
+ * File: include/linux/mxcfb.h
+ * Global header file for the MXC Framebuffer
+ *
+ * Copyright 2004-2012 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * The code contained herein is licensed under the GNU Lesser General
+ * Public License.  You may obtain a copy of the GNU Lesser General
+ * Public License Version 2.1 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/lgpl-license.html
+ * http://www.gnu.org/copyleft/lgpl.html
+ */
+
+#ifndef __LINUX_MXCFB_H__
+#define __LINUX_MXCFB_H__
+
+#include <linux/fb.h>
+
+#define FB_SYNC_OE_LOW_ACT	0x80000000
+#define FB_SYNC_CLK_LAT_FALL	0x40000000
+#define FB_SYNC_DATA_INVERT	0x20000000
+#define FB_SYNC_CLK_IDLE_EN	0x10000000
+#define FB_SYNC_SHARP_MODE	0x08000000
+#define FB_SYNC_SWAP_RGB	0x04000000
+
+struct mxcfb_gbl_alpha {
+	int enable;
+	int alpha;
+};
+
+struct mxcfb_color_key {
+	int enable;
+	__u32 color_key;
+};
+
+struct mxcfb_pos {
+	__u16 x;
+	__u16 y;
+};
+
+struct mxcfb_gamma {
+	int enable;
+	int constk[16];
+	int slopek[16];
+};
+
+struct mxcfb_rect {
+	__u32 top;
+	__u32 left;
+	__u32 width;
+	__u32 height;
+};
+
+/*
+ * Structure used to define waveform modes for driver
+ * Needed for driver to perform auto-waveform selection
+ */
+struct mxcfb_waveform_modes {
+	int mode_init;
+	int mode_du;
+	int mode_gc4;
+	int mode_gc8;
+	int mode_gc16;
+	int mode_gc32;
+};
+
+/* IOCTL commands. */
+
+#define MXCFB_WAIT_FOR_VSYNC		_IOW('F', 0x20, u_int32_t)
+#define MXCFB_SET_GBL_ALPHA		_IOW('F', 0x21, struct mxcfb_gbl_alpha)
+#define MXCFB_SET_CLR_KEY		_IOW('F', 0x22, struct mxcfb_color_key)
+#define MXCFB_SET_OVERLAY_POS		_IOWR('F', 0x24, struct mxcfb_pos)
+#define MXCFB_GET_FB_IPU_CHAN		_IOR('F', 0x25, u_int32_t)
+#define MXCFB_SET_LOC_ALPHA		_IOWR('F', 0x26, struct mxcfb_loc_alpha)
+#define MXCFB_SET_LOC_ALP_BUF		_IOW('F', 0x27, unsigned long)
+#define MXCFB_SET_GAMMA			_IOW('F', 0x28, struct mxcfb_gamma)
+#define MXCFB_GET_FB_IPU_DI		_IOR('F', 0x29, u_int32_t)
+#define MXCFB_GET_DIFMT			_IOR('F', 0x2A, u_int32_t)
+#define MXCFB_GET_FB_BLANK		_IOR('F', 0x2B, u_int32_t)
+
+#ifdef __KERNEL__
+
+enum {
+	MXCFB_REFRESH_OFF,
+	MXCFB_REFRESH_AUTO,
+	MXCFB_REFRESH_PARTIAL,
+};
+
+#endif
+
+#endif /* _MXCFB_H */
+
-- 
1.7.0.4


^ permalink raw reply related

* Re: [RESEND][PATCH 09/10] cobalt_lcdfb: LCD panel framebuffer support for SEAD-3 platform.
From: Florian Tobias Schandinat @ 2012-04-18 16:48 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1334758309-3986-1-git-send-email-sjhill@mips.com>

[Cc'ing Yoichi Yuasa and linux-fbdev]

On 04/18/2012 02:13 PM, Steven J. Hill wrote:
> From: "Steven J. Hill" <sjhill@mips.com>
> 
> Add support for LCD panel on MIPS SEAD-3 development platform.
> 
> Signed-off-by: Douglas Leung <douglas@mips.com>
> Signed-off-by: Chris Dearman <chris@mips.com>
> Signed-off-by: Steven J. Hill <sjhill@mips.com>

Looks good to me. I will apply it in a few days if nobody is against it.


Best regards,

Florian Tobias Schandinat

> ---
>  drivers/video/Kconfig        |    2 +-
>  drivers/video/cobalt_lcdfb.c |   45 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index a8a897a..e921a45 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2210,7 +2210,7 @@ config FB_XILINX
>  
>  config FB_COBALT
>  	tristate "Cobalt server LCD frame buffer support"
> -	depends on FB && MIPS_COBALT
> +	depends on FB && (MIPS_COBALT || MIPS_SEAD3)
>  
>  config FB_SH7760
>  	bool "SH7760/SH7763/SH7720/SH7721 LCDC support"
> diff --git a/drivers/video/cobalt_lcdfb.c b/drivers/video/cobalt_lcdfb.c
> index f56699d..eae46f6 100644
> --- a/drivers/video/cobalt_lcdfb.c
> +++ b/drivers/video/cobalt_lcdfb.c
> @@ -1,7 +1,8 @@
>  /*
> - *  Cobalt server LCD frame buffer driver.
> + *  Cobalt/SEAD3 LCD frame buffer driver.
>   *
>   *  Copyright (C) 2008  Yoichi Yuasa <yuasa@linux-mips.org>
> + *  Copyright (C) 2012  MIPS Technologies, Inc.
>   *
>   *  This program is free software; you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -62,6 +63,7 @@
>  #define LCD_CUR_POS(x)		((x) & LCD_CUR_POS_MASK)
>  #define LCD_TEXT_POS(x)		((x) | LCD_TEXT_MODE)
>  
> +#ifdef CONFIG_MIPS_COBALT
>  static inline void lcd_write_control(struct fb_info *info, u8 control)
>  {
>  	writel((u32)control << 24, info->screen_base);
> @@ -81,6 +83,47 @@ static inline u8 lcd_read_data(struct fb_info *info)
>  {
>  	return readl(info->screen_base + LCD_DATA_REG_OFFSET) >> 24;
>  }
> +#else
> +
> +#define LCD_CTL			0x00
> +#define LCD_DATA		0x08
> +#define CPLD_STATUS		0x10
> +#define CPLD_DATA		0x18
> +
> +static inline void cpld_wait(struct fb_info *info)
> +{
> +	do {
> +	} while (readl(info->screen_base + CPLD_STATUS) & 1);
> +}
> +
> +static inline void lcd_write_control(struct fb_info *info, u8 control)
> +{
> +	cpld_wait(info);
> +	writel(control, info->screen_base + LCD_CTL);
> +}
> +
> +static inline u8 lcd_read_control(struct fb_info *info)
> +{
> +	cpld_wait(info);
> +	readl(info->screen_base + LCD_CTL);
> +	cpld_wait(info);
> +	return readl(info->screen_base + CPLD_DATA) & 0xff;
> +}
> +
> +static inline void lcd_write_data(struct fb_info *info, u8 data)
> +{
> +	cpld_wait(info);
> +	writel(data, info->screen_base + LCD_DATA);
> +}
> +
> +static inline u8 lcd_read_data(struct fb_info *info)
> +{
> +	cpld_wait(info);
> +	readl(info->screen_base + LCD_DATA);
> +	cpld_wait(info);
> +	return readl(info->screen_base + CPLD_DATA) & 0xff;
> +}
> +#endif
>  
>  static int lcd_busy_wait(struct fb_info *info)
>  {


^ permalink raw reply

* [PATCH] i.MX28: Shut down the LCD controller to avoid BootROM sampling bug
From: Marek Vasut @ 2012-04-18 16:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1334695011-8188-1-git-send-email-marex@denx.de>

From: Marek Vasut <marek.vasut@gmail.com>

If there's some traffic on the LCD controller pads, the BootROM has trouble with
sampling the bootmode from these pads when the system is restarted from Linux.
The BootROM usually ends in a loop.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Chen Peter-B29397 <B29397@freescale.com>
Cc: Detlev Zundel <dzu@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: Li Frank-B20596 <B20596@freescale.com>
Cc: Lin Tony-B19295 <B19295@freescale.com>
Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawn.guo@freescale.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Subodh Nijsure <snijsure@grid-net.com>
Cc: Tony Lin <tony.lin@freescale.com>
Cc: Wolfgang Denk <wd@denx.de>
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
Acked-by: Wolfgang Denk <wd@denx.de>
Tested-by: Wolfgang Denk <wd@denx.de>
---
 drivers/video/mxsfb.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 4a89f88..a265356 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -880,6 +880,18 @@ static int __devexit mxsfb_remove(struct platform_device *pdev)
 	return 0;
 }
 
+void mxsfb_shutdown(struct platform_device *pdev)
+{
+	struct fb_info *fb_info = platform_get_drvdata(pdev);
+	struct mxsfb_info *host = to_imxfb_host(fb_info);
+
+	/*
+	 * Force stop the LCD controller as keeping it running during reboot
+	 * might interfere with the BootROM's boot mode pads sampling.
+	 */
+	writel(CTRL_RUN, host->base + LCDC_CTRL + REG_CLR);
+}
+
 static struct platform_device_id mxsfb_devtype[] = {
 	{
 		.name = "imx23-fb",
@@ -896,6 +908,7 @@ MODULE_DEVICE_TABLE(platform, mxsfb_devtype);
 static struct platform_driver mxsfb_driver = {
 	.probe = mxsfb_probe,
 	.remove = __devexit_p(mxsfb_remove),
+	.shutdown = mxsfb_shutdown,
 	.id_table = mxsfb_devtype,
 	.driver = {
 		   .name = DRIVER_NAME,
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH] backlight: Add LMS501KF03 LCD panel driver
From: Florian Tobias Schandinat @ 2012-04-18 16:21 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1334558406-22576-1-git-send-email-sachin.kamat@linaro.org>

On 04/16/2012 06:40 AM, Sachin Kamat wrote:
> LMS501KF03 is a 480x800 LCD module with brightness control.
> The driver uses 3-wired SPI inteface.
> 
> Signed-off-by: Ilho Lee <Ilho215.lee@samsung.com>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---

> diff --git a/drivers/video/backlight/lms501kf03.c b/drivers/video/backlight/lms501kf03.c
> new file mode 100644
> index 0000000..3dc85d4
> --- /dev/null
> +++ b/drivers/video/backlight/lms501kf03.c
> +static int lms501kf03_ldi_enable(struct lms501kf03 *lcd)
> +{
> +	int ret, i;
> +	const unsigned short *init_seq[] = {
> +		SEQ_DISPLAY_ON,
> +	};

Is this array expected to grow at some point?
Otherwise I'd suggest to get rid of it and simplify the code below.

> +
> +	for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
> +		ret = lms501kf03_panel_send_sequence(lcd, init_seq[i]);
> +		if (ret)
> +			break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int lms501kf03_ldi_disable(struct lms501kf03 *lcd)
> +{
> +	int ret, i;
> +
> +	const unsigned short *init_seq[] = {
> +		SEQ_DISPLAY_OFF,
> +	};

dito

> +
> +	for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
> +		ret = lms501kf03_panel_send_sequence(lcd, init_seq[i]);
> +		if (ret)
> +			break;
> +	}
> +
> +	return ret;
> +}


Best regards,

Florian Tobias Schandinat

^ permalink raw reply

* Re: [PATCH] i.MX28: Shut down the LCD controller to avoid bootrom sampling bug
From: Florian Tobias Schandinat @ 2012-04-18 15:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1334695011-8188-1-git-send-email-marex@denx.de>

Hi,

On 04/17/2012 08:36 PM, Marek Vasut wrote:
> From: Marek Vasut <marek.vasut@gmail.com>

please write a commit message that describes the bug you want to fix and
why/how your patch fixes it.


Thanks,

Florian Tobias Schandinat

> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Chen Peter-B29397 <B29397@freescale.com>
> Cc: Detlev Zundel <dzu@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc: Li Frank-B20596 <B20596@freescale.com>
> Cc: Lin Tony-B19295 <B19295@freescale.com>
> Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@freescale.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Subodh Nijsure <snijsure@grid-net.com>
> Cc: Tony Lin <tony.lin@freescale.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Acked-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  drivers/video/mxsfb.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index 4a89f88..a265356 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -880,6 +880,18 @@ static int __devexit mxsfb_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +void mxsfb_shutdown(struct platform_device *pdev)
> +{
> +	struct fb_info *fb_info = platform_get_drvdata(pdev);
> +	struct mxsfb_info *host = to_imxfb_host(fb_info);
> +
> +	/*
> +	 * Force stop the LCD controller as keeping it running during reboot
> +	 * might interfere with the BootROM's boot mode pads sampling.
> +	 */
> +	writel(CTRL_RUN, host->base + LCDC_CTRL + REG_CLR);
> +}
> +
>  static struct platform_device_id mxsfb_devtype[] = {
>  	{
>  		.name = "imx23-fb",
> @@ -896,6 +908,7 @@ MODULE_DEVICE_TABLE(platform, mxsfb_devtype);
>  static struct platform_driver mxsfb_driver = {
>  	.probe = mxsfb_probe,
>  	.remove = __devexit_p(mxsfb_remove),
> +	.shutdown = mxsfb_shutdown,
>  	.id_table = mxsfb_devtype,
>  	.driver = {
>  		   .name = DRIVER_NAME,


^ permalink raw reply

* Re: [PATCH 4/6] OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
From: Tomi Valkeinen @ 2012-04-18 14:58 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1334561027-28569-5-git-send-email-archit@ti.com>

[-- Attachment #1: Type: text/plain, Size: 2174 bytes --]

On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
> DISPC manager size and DISPC manager blanking parameters(for LCD managers)
> follow the shadow register programming model. Currently, they are programmed
> directly by the interface drivers.
> 
> Make timings(omap_video_timing struct) an overlay_manager_info member, they are
> now programmed via the apply mechanism used for programming shadow registers.
> 
> The interface driver now call the function dss_mgr_set_timings() which applies
> the new timing parameters, rather than directly writing to DISPC registers.

I don't think that works correctly. The omap_overlay_manager_info is
supposed to be set with set_manager_info() by the user of omapdss, to
configure the manager's features. The timings are not supposed to be set
via that mechanism, but with dssdev->set_timings().

This is similar to the info and extra_info for overlay. info has stuff
that omapdss doesn't change, it just uses what the user gives.
extra_info, on the other hand, has omapdss private stuff that the user
does not see. Timings are clearly private stuff in this sense, because
they are set via dssdev->set_timings(). 

One reason for this is the programming model we use. If the user of
omapdss does get_info() for two overlays, changes the infos, and then
calls set_info() for both overlays and finally apply() for the manager,
we don't do any locking there because omapdss presumes the info is
handled by one user. If, say, the dpi.c would change the info and call
apply at the same time, the configuration could go badly wrong.

So I think what should be done is to add similar "extra" flags and code
to mgr_priv_data that we have for ovl_priv_data, and add
omap_video_timings to mgr_priv_data (the same way as we have, say,
fifo_low for ovl_priv_data).

And then add similar function to dss_ovl_write_regs_extra() for manager,
and a function like dss_apply_ovl_fifo_thresholds() for timings. And
finally a non-static function to set the timings (used by dpi.c etc),
which calls the similar function to dss_apply_ovl_fifo_thresholds(), and
dss_write_regs() and dss_set_go_bits().

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [RESEND][PATCH 09/10] cobalt_lcdfb: LCD panel framebuffer support for SEAD-3 platform.
From: Steven J. Hill @ 2012-04-18 14:11 UTC (permalink / raw)
  To: linux-fbdev

From: "Steven J. Hill" <sjhill@mips.com>

Add support for LCD panel on MIPS SEAD-3 development platform.

Signed-off-by: Douglas Leung <douglas@mips.com>
Signed-off-by: Chris Dearman <chris@mips.com>
Signed-off-by: Steven J. Hill <sjhill@mips.com>
---
 drivers/video/Kconfig        |    2 +-
 drivers/video/cobalt_lcdfb.c |   45 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index a8a897a..e921a45 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2210,7 +2210,7 @@ config FB_XILINX
 
 config FB_COBALT
 	tristate "Cobalt server LCD frame buffer support"
-	depends on FB && MIPS_COBALT
+	depends on FB && (MIPS_COBALT || MIPS_SEAD3)
 
 config FB_SH7760
 	bool "SH7760/SH7763/SH7720/SH7721 LCDC support"
diff --git a/drivers/video/cobalt_lcdfb.c b/drivers/video/cobalt_lcdfb.c
index f56699d..eae46f6 100644
--- a/drivers/video/cobalt_lcdfb.c
+++ b/drivers/video/cobalt_lcdfb.c
@@ -1,7 +1,8 @@
 /*
- *  Cobalt server LCD frame buffer driver.
+ *  Cobalt/SEAD3 LCD frame buffer driver.
  *
  *  Copyright (C) 2008  Yoichi Yuasa <yuasa@linux-mips.org>
+ *  Copyright (C) 2012  MIPS Technologies, Inc.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -62,6 +63,7 @@
 #define LCD_CUR_POS(x)		((x) & LCD_CUR_POS_MASK)
 #define LCD_TEXT_POS(x)		((x) | LCD_TEXT_MODE)
 
+#ifdef CONFIG_MIPS_COBALT
 static inline void lcd_write_control(struct fb_info *info, u8 control)
 {
 	writel((u32)control << 24, info->screen_base);
@@ -81,6 +83,47 @@ static inline u8 lcd_read_data(struct fb_info *info)
 {
 	return readl(info->screen_base + LCD_DATA_REG_OFFSET) >> 24;
 }
+#else
+
+#define LCD_CTL			0x00
+#define LCD_DATA		0x08
+#define CPLD_STATUS		0x10
+#define CPLD_DATA		0x18
+
+static inline void cpld_wait(struct fb_info *info)
+{
+	do {
+	} while (readl(info->screen_base + CPLD_STATUS) & 1);
+}
+
+static inline void lcd_write_control(struct fb_info *info, u8 control)
+{
+	cpld_wait(info);
+	writel(control, info->screen_base + LCD_CTL);
+}
+
+static inline u8 lcd_read_control(struct fb_info *info)
+{
+	cpld_wait(info);
+	readl(info->screen_base + LCD_CTL);
+	cpld_wait(info);
+	return readl(info->screen_base + CPLD_DATA) & 0xff;
+}
+
+static inline void lcd_write_data(struct fb_info *info, u8 data)
+{
+	cpld_wait(info);
+	writel(data, info->screen_base + LCD_DATA);
+}
+
+static inline u8 lcd_read_data(struct fb_info *info)
+{
+	cpld_wait(info);
+	readl(info->screen_base + LCD_DATA);
+	cpld_wait(info);
+	return readl(info->screen_base + CPLD_DATA) & 0xff;
+}
+#endif
 
 static int lcd_busy_wait(struct fb_info *info)
 {
-- 
1.7.9.6


^ permalink raw reply related

* Re: [PATCH] i.MX28: Shut down the LCD controller to avoid bootrom sampling bug
From: Wolfgang Denk @ 2012-04-18  7:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1334695011-8188-1-git-send-email-marex@denx.de>

Dear Marek,

In message <1334695011-8188-1-git-send-email-marex@denx.de> you wrote:
> From: Marek Vasut <marek.vasut@gmail.com>
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Chen Peter-B29397 <B29397@freescale.com>
> Cc: Detlev Zundel <dzu@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc: Li Frank-B20596 <B20596@freescale.com>
> Cc: Lin Tony-B19295 <B19295@freescale.com>
> Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@freescale.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Subodh Nijsure <snijsure@grid-net.com>
> Cc: Tony Lin <tony.lin@freescale.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Acked-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  drivers/video/mxsfb.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)

Acked-by: Wolfgang Denk <wd@denx.de>
Tested-by: Wolfgang Denk <wd@denx.de>


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
A dog always bit deepest on the veterinary hand.
                                    - Terry Pratchett, _Wyrd Sisters_

^ permalink raw reply

* [PATCH] i.MX28: Shut down the LCD controller to avoid bootrom sampling bug
From: Marek Vasut @ 2012-04-17 20:36 UTC (permalink / raw)
  To: linux-arm-kernel

From: Marek Vasut <marek.vasut@gmail.com>

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Chen Peter-B29397 <B29397@freescale.com>
Cc: Detlev Zundel <dzu@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: Li Frank-B20596 <B20596@freescale.com>
Cc: Lin Tony-B19295 <B19295@freescale.com>
Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawn.guo@freescale.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Subodh Nijsure <snijsure@grid-net.com>
Cc: Tony Lin <tony.lin@freescale.com>
Cc: Wolfgang Denk <wd@denx.de>
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/video/mxsfb.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 4a89f88..a265356 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -880,6 +880,18 @@ static int __devexit mxsfb_remove(struct platform_device *pdev)
 	return 0;
 }
 
+void mxsfb_shutdown(struct platform_device *pdev)
+{
+	struct fb_info *fb_info = platform_get_drvdata(pdev);
+	struct mxsfb_info *host = to_imxfb_host(fb_info);
+
+	/*
+	 * Force stop the LCD controller as keeping it running during reboot
+	 * might interfere with the BootROM's boot mode pads sampling.
+	 */
+	writel(CTRL_RUN, host->base + LCDC_CTRL + REG_CLR);
+}
+
 static struct platform_device_id mxsfb_devtype[] = {
 	{
 		.name = "imx23-fb",
@@ -896,6 +908,7 @@ MODULE_DEVICE_TABLE(platform, mxsfb_devtype);
 static struct platform_driver mxsfb_driver = {
 	.probe = mxsfb_probe,
 	.remove = __devexit_p(mxsfb_remove),
+	.shutdown = mxsfb_shutdown,
 	.id_table = mxsfb_devtype,
 	.driver = {
 		   .name = DRIVER_NAME,
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH] video/via: Convert to kstrtou8_from_user
From: Alexey Dobriyan @ 2012-04-16 16:36 UTC (permalink / raw)
  To: Peter Huewe
  Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel,
	kernel-janitors
In-Reply-To: <1334445886-32448-1-git-send-email-peterhuewe@gmx.de>

On Sun, Apr 15, 2012 at 2:24 AM, Peter Huewe <peterhuewe@gmx.de> wrote:
> This patch replaces the code for getting an number from a
> userspace buffer by a simple call to kstrou8_from_user.
> This makes it easier to read and less error prone.

Initialization with 0 is not necessary then.
Also "count < 1" check could be dropped, since empty string will fail
with EINVAL.
Otherwise OK.

> --- a/drivers/video/via/viafbdev.c
> +++ b/drivers/video/via/viafbdev.c
> @@ -1276,17 +1276,14 @@ static int viafb_dfph_proc_open(struct inode *inode, struct file *file)
>  static ssize_t viafb_dfph_proc_write(struct file *file,
>        const char __user *buffer, size_t count, loff_t *pos)
>  {
> -       char buf[20];
> +       int err;
>        u8 reg_val = 0;
> -       unsigned long length;
>        if (count < 1)
>                return -EINVAL;
> -       length = count > 20 ? 20 : count;
> -       if (copy_from_user(&buf[0], buffer, length))
> -               return -EFAULT;
> -       buf[length - 1] = '\0'; /*Ensure end string */
> -       if (kstrtou8(buf, 0, &reg_val) < 0)
> -               return -EINVAL;
> +       err = kstrtou8_from_user(buffer, count, 0, &reg_val);
> +       if (err)
> +               return err;
> +
>        viafb_write_reg_mask(CR97, VIACR, reg_val, 0x0f);
>        return count;
>  }
> @@ -1316,17 +1313,14 @@ static int viafb_dfpl_proc_open(struct inode *inode, struct file *file)
>  static ssize_t viafb_dfpl_proc_write(struct file *file,
>        const char __user *buffer, size_t count, loff_t *pos)
>  {
> -       char buf[20];
> +       int err;
>        u8 reg_val = 0;
> -       unsigned long length;
>        if (count < 1)
>                return -EINVAL;
> -       length = count > 20 ? 20 : count;
> -       if (copy_from_user(&buf[0], buffer, length))
> -               return -EFAULT;
> -       buf[length - 1] = '\0'; /*Ensure end string */
> -       if (kstrtou8(buf, 0, &reg_val) < 0)
> -               return -EINVAL;
> +       err = kstrtou8_from_user(buffer, count, 0, &reg_val);
> +       if (err)
> +               return err;
> +
>        viafb_write_reg_mask(CR99, VIACR, reg_val, 0x0f);
>        return count;
>  }
> --
> 1.7.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* [PATCH 6/6] OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
From: Archit Taneja @ 2012-04-16  7:35 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1334561027-28569-1-git-send-email-archit@ti.com>

In order to check the validity of overlay and manager info, there was a need to
use the omap_dss_device struct to get the panel resolution. manager_info now
contains the manager size(which should be the panel resolution in most cases).
Hence, we don't need to depend on the display resolution.

manager_info is now passed to dss_ovl_check() to see if the overlay fits within
the manager area.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c   |   24 +++++++++++-------------
 drivers/video/omap2/dss/dss.h     |    5 ++---
 drivers/video/omap2/dss/manager.c |    3 +--
 drivers/video/omap2/dss/overlay.c |   20 +++++++++-----------
 4 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 53a198f..5073009 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -198,7 +198,7 @@ static bool mgr_manual_update(struct omap_overlay_manager *mgr)
 }
 
 static int dss_check_settings_low(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev, bool applying)
+		bool applying)
 {
 	struct omap_overlay_info *oi;
 	struct omap_overlay_manager_info *mi;
@@ -228,26 +228,24 @@ static int dss_check_settings_low(struct omap_overlay_manager *mgr,
 		ois[ovl->id] = oi;
 	}
 
-	return dss_mgr_check(mgr, dssdev, mi, ois);
+	return dss_mgr_check(mgr, mi, ois);
 }
 
 /*
  * check manager and overlay settings using overlay_info from data->info
  */
-static int dss_check_settings(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev)
+static int dss_check_settings(struct omap_overlay_manager *mgr)
 {
-	return dss_check_settings_low(mgr, dssdev, false);
+	return dss_check_settings_low(mgr, false);
 }
 
 /*
  * check manager and overlay settings using overlay_info from ovl->info if
  * dirty and from data->info otherwise
  */
-static int dss_check_settings_apply(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev)
+static int dss_check_settings_apply(struct omap_overlay_manager *mgr)
 {
-	return dss_check_settings_low(mgr, dssdev, true);
+	return dss_check_settings_low(mgr, true);
 }
 
 static bool need_isr(void)
@@ -668,7 +666,7 @@ static void dss_write_regs(void)
 		if (!mp->enabled || mgr_manual_update(mgr) || mp->busy)
 			continue;
 
-		r = dss_check_settings(mgr, mgr->device);
+		r = dss_check_settings(mgr);
 		if (r) {
 			DSSERR("cannot write registers for manager %s: "
 					"illegal configuration\n", mgr->name);
@@ -733,7 +731,7 @@ void dss_mgr_start_update(struct omap_overlay_manager *mgr)
 
 	WARN_ON(mp->updating);
 
-	r = dss_check_settings(mgr, mgr->device);
+	r = dss_check_settings(mgr);
 	if (r) {
 		DSSERR("cannot start manual update: illegal configuration\n");
 		spin_unlock_irqrestore(&data_lock, flags);
@@ -879,7 +877,7 @@ int omap_dss_mgr_apply(struct omap_overlay_manager *mgr)
 
 	spin_lock_irqsave(&data_lock, flags);
 
-	r = dss_check_settings_apply(mgr, mgr->device);
+	r = dss_check_settings_apply(mgr);
 	if (r) {
 		spin_unlock_irqrestore(&data_lock, flags);
 		DSSERR("failed to apply settings: illegal configuration.\n");
@@ -1072,7 +1070,7 @@ int dss_mgr_enable(struct omap_overlay_manager *mgr)
 
 	mp->enabled = true;
 
-	r = dss_check_settings(mgr, mgr->device);
+	r = dss_check_settings(mgr);
 	if (r) {
 		DSSERR("failed to enable manager %d: check_settings failed\n",
 				mgr->id);
@@ -1415,7 +1413,7 @@ int dss_ovl_enable(struct omap_overlay *ovl)
 
 	op->enabling = true;
 
-	r = dss_check_settings(ovl->manager, ovl->manager->device);
+	r = dss_check_settings(ovl->manager);
 	if (r) {
 		DSSERR("failed to enable overlay %d: check_settings failed\n",
 				ovl->id);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 9e47bdf..fb5ca51 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -209,7 +209,6 @@ int dss_mgr_simple_check(struct omap_overlay_manager *mgr,
 int dss_mgr_check_timings(struct omap_overlay_manager *mgr,
 		struct omap_video_timings *timings);
 int dss_mgr_check(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev,
 		struct omap_overlay_manager_info *info,
 		struct omap_overlay_info **overlay_infos);
 int dss_mgr_set_timings(struct omap_overlay_manager *mgr,
@@ -222,8 +221,8 @@ void dss_overlay_setup_dispc_manager(struct omap_overlay_manager *mgr);
 void dss_recheck_connections(struct omap_dss_device *dssdev, bool force);
 int dss_ovl_simple_check(struct omap_overlay *ovl,
 		const struct omap_overlay_info *info);
-int dss_ovl_check(struct omap_overlay *ovl,
-		struct omap_overlay_info *info, struct omap_dss_device *dssdev);
+int dss_ovl_check(struct omap_overlay *ovl, struct omap_overlay_info *info,
+		struct omap_overlay_manager_info *mgr_info);
 
 /* DSS */
 int dss_init_platform_driver(void);
diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index fb61b76..9821015 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -666,7 +666,6 @@ int dss_mgr_check_timings(struct omap_overlay_manager *mgr,
 }
 
 int dss_mgr_check(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev,
 		struct omap_overlay_manager_info *info,
 		struct omap_overlay_info **overlay_infos)
 {
@@ -692,7 +691,7 @@ int dss_mgr_check(struct omap_overlay_manager *mgr,
 		if (oi = NULL)
 			continue;
 
-		r = dss_ovl_check(ovl, oi, dssdev);
+		r = dss_ovl_check(ovl, oi, info);
 		if (r)
 			return r;
 	}
diff --git a/drivers/video/omap2/dss/overlay.c b/drivers/video/omap2/dss/overlay.c
index 6e82181..4436376 100644
--- a/drivers/video/omap2/dss/overlay.c
+++ b/drivers/video/omap2/dss/overlay.c
@@ -631,16 +631,14 @@ int dss_ovl_simple_check(struct omap_overlay *ovl,
 	return 0;
 }
 
-int dss_ovl_check(struct omap_overlay *ovl,
-		struct omap_overlay_info *info, struct omap_dss_device *dssdev)
+int dss_ovl_check(struct omap_overlay *ovl, struct omap_overlay_info *info,
+		struct omap_overlay_manager_info *mgr_info)
 {
 	u16 outw, outh;
-	u16 dw, dh;
+	u16 mgr_width, mgr_height;
 
-	if (dssdev = NULL)
-		return 0;
-
-	dssdev->driver->get_resolution(dssdev, &dw, &dh);
+	mgr_width = mgr_info->timings.x_res;
+	mgr_height = mgr_info->timings.y_res;
 
 	if ((ovl->caps & OMAP_DSS_OVL_CAP_SCALE) = 0) {
 		outw = info->width;
@@ -657,17 +655,17 @@ int dss_ovl_check(struct omap_overlay *ovl,
 			outh = info->out_height;
 	}
 
-	if (dw < info->pos_x + outw) {
+	if (mgr_width < info->pos_x + outw) {
 		DSSERR("overlay %d horizontally not inside the display area "
 				"(%d + %d >= %d)\n",
-				ovl->id, info->pos_x, outw, dw);
+				ovl->id, info->pos_x, outw, mgr_width);
 		return -EINVAL;
 	}
 
-	if (dh < info->pos_y + outh) {
+	if (mgr_height < info->pos_y + outh) {
 		DSSERR("overlay %d vertically not inside the display area "
 				"(%d + %d >= %d)\n",
-				ovl->id, info->pos_y, outh, dh);
+				ovl->id, info->pos_y, outh, mgr_height);
 		return -EINVAL;
 	}
 
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH 5/6] OMAPDSS: MANAGER: Check validity of manager timings
From: Archit Taneja @ 2012-04-16  7:35 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1334561027-28569-1-git-send-email-archit@ti.com>

The manager timings is a member of the overlay_manager_info struct. We can now
check it's validity in dss_mgr_check(). Add a function dss_mgr_check_timings()
which checks if the timings are correct.

Currently, the manager timings are initialized in an interface's enable call.
Since a mgr_apply() can happen before the connected display is enabled, we
initialize the manager timings to a dummy valid value in apply_init().

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c   |   22 ++++++++++++++++++++++
 drivers/video/omap2/dss/dpi.c     |    2 +-
 drivers/video/omap2/dss/dss.h     |    2 ++
 drivers/video/omap2/dss/manager.c |   15 +++++++++++++++
 4 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index b10b3bc..53a198f 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -132,10 +132,32 @@ static struct mgr_priv_data *get_mgr_priv(struct omap_overlay_manager *mgr)
 void dss_apply_init(void)
 {
 	const int num_ovls = dss_feat_get_num_ovls();
+	const int num_mgrs = dss_feat_get_num_ovls();
 	int i;
+	/* Use dummy manager timings during initialization */
+	struct omap_video_timings timings = {
+		.hsw		= 1,
+		.hfp		= 1,
+		.hbp		= 1,
+		.vsw		= 1,
+		.vfp		= 0,
+		.vbp		= 0,
+		.x_res	= dss_feat_get_param_max(FEAT_PARAM_MGR_WIDTH),
+		.y_res	= dss_feat_get_param_max(FEAT_PARAM_MGR_HEIGHT),
+	};
 
 	spin_lock_init(&data_lock);
 
+	for (i = 0; i < num_mgrs; i++) {
+		struct mgr_priv_data *mp;
+
+		mp = &dss_data.mgr_priv_data_array[i];
+
+		mp->info.timings = timings;
+
+		mp->user_info = mp->info;
+	}
+
 	for (i = 0; i < num_ovls; ++i) {
 		struct ovl_priv_data *op;
 
diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index 76e2cae..a1bdbf7 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -314,7 +314,7 @@ int dpi_check_timings(struct omap_dss_device *dssdev,
 	unsigned long pck;
 	struct dispc_clock_info dispc_cinfo;
 
-	if (!dispc_mgr_timings_ok(dssdev->manager->id, timings))
+	if (!dss_mgr_check_timings(dssdev->manager, timings))
 		return -EINVAL;
 
 	if (timings->pixel_clock = 0)
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 0bff325..9e47bdf 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -206,6 +206,8 @@ int dss_init_overlay_managers(struct platform_device *pdev);
 void dss_uninit_overlay_managers(struct platform_device *pdev);
 int dss_mgr_simple_check(struct omap_overlay_manager *mgr,
 		const struct omap_overlay_manager_info *info);
+int dss_mgr_check_timings(struct omap_overlay_manager *mgr,
+		struct omap_video_timings *timings);
 int dss_mgr_check(struct omap_overlay_manager *mgr,
 		struct omap_dss_device *dssdev,
 		struct omap_overlay_manager_info *info,
diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index 2c85988..fb61b76 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -654,6 +654,17 @@ static int dss_mgr_check_zorder(struct omap_overlay_manager *mgr,
 	return 0;
 }
 
+int dss_mgr_check_timings(struct omap_overlay_manager *mgr,
+		struct omap_video_timings *timings)
+{
+	if (!dispc_mgr_timings_ok(mgr->id, timings)) {
+		DSSERR("check_manager: invalid timings\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int dss_mgr_check(struct omap_overlay_manager *mgr,
 		struct omap_dss_device *dssdev,
 		struct omap_overlay_manager_info *info,
@@ -668,6 +679,10 @@ int dss_mgr_check(struct omap_overlay_manager *mgr,
 			return r;
 	}
 
+	r = dss_mgr_check_timings(mgr, &info->timings);
+	if (r)
+		return r;
+
 	list_for_each_entry(ovl, &mgr->overlays, list) {
 		struct omap_overlay_info *oi;
 		int r;
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH 4/6] OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
From: Archit Taneja @ 2012-04-16  7:35 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1334561027-28569-1-git-send-email-archit@ti.com>

DISPC manager size and DISPC manager blanking parameters(for LCD managers)
follow the shadow register programming model. Currently, they are programmed
directly by the interface drivers.

Make timings(omap_video_timing struct) an overlay_manager_info member, they are
now programmed via the apply mechanism used for programming shadow registers.

The interface driver now call the function dss_mgr_set_timings() which applies
the new timing parameters, rather than directly writing to DISPC registers.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c   |    5 ++++-
 drivers/video/omap2/dss/dpi.c     |    4 +++-
 drivers/video/omap2/dss/dsi.c     |   11 +++++++++--
 drivers/video/omap2/dss/dss.h     |    4 ++--
 drivers/video/omap2/dss/hdmi.c    |    5 ++++-
 drivers/video/omap2/dss/manager.c |   21 +++++++++++++++++++++
 drivers/video/omap2/dss/rfbi.c    |    7 +++++--
 drivers/video/omap2/dss/sdi.c     |    5 ++++-
 drivers/video/omap2/dss/venc.c    |   12 +++++++-----
 include/video/omapdss.h           |    2 ++
 10 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 46bcb55..63de49d 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -120,6 +120,8 @@ enum omap_color_component {
 };
 
 static void _omap_dispc_set_irqs(void);
+static void dispc_mgr_set_timings(enum omap_channel channel,
+		struct omap_video_timings *timings);
 
 static inline void dispc_write_reg(const u16 idx, u32 val)
 {
@@ -2211,6 +2213,7 @@ void dispc_mgr_setup(enum omap_channel channel,
 		dispc_mgr_enable_cpr(channel, info->cpr_enable);
 		dispc_mgr_set_cpr_coef(channel, &info->cpr_coefs);
 	}
+	dispc_mgr_set_timings(channel, &info->timings);
 }
 
 void dispc_mgr_set_tft_data_lines(enum omap_channel channel, u8 data_lines)
@@ -2348,7 +2351,7 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
 }
 
 /* change name to mode? */
-void dispc_mgr_set_timings(enum omap_channel channel,
+static void dispc_mgr_set_timings(enum omap_channel channel,
 		struct omap_video_timings *timings)
 {
 	unsigned xtot, ytot;
diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index cec1166..76e2cae 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -156,7 +156,9 @@ static int dpi_set_mode(struct omap_dss_device *dssdev)
 		t->pixel_clock = pck;
 	}
 
-	dispc_mgr_set_timings(dssdev->manager->id, t);
+	r = dss_mgr_set_timings(dssdev->manager, t);
+	if (r)
+		return r;
 
 	return 0;
 }
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index b6cf03c..fbca76c 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -4219,13 +4219,20 @@ static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
 		dispc_mgr_enable_stallmode(dssdev->manager->id, true);
 		dispc_mgr_enable_fifohandcheck(dssdev->manager->id, 1);
 
-		dispc_mgr_set_timings(dssdev->manager->id, &timings);
+		r = dss_mgr_set_timings(dssdev->manager, &timings);
+		if (r) {
+			omap_dispc_unregister_isr(dsi_framedone_irq_callback,
+				(void *) dssdev, irq);
+			return r;
+		}
 	} else {
 		dispc_mgr_enable_stallmode(dssdev->manager->id, false);
 		dispc_mgr_enable_fifohandcheck(dssdev->manager->id, 0);
 
-		dispc_mgr_set_timings(dssdev->manager->id,
+		r = dss_mgr_set_timings(dssdev->manager,
 			&dssdev->panel.timings);
+		if (r)
+			return r;
 	}
 
 		dispc_mgr_set_lcd_display_type(dssdev->manager->id,
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 1dc336b..0bff325 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -210,6 +210,8 @@ int dss_mgr_check(struct omap_overlay_manager *mgr,
 		struct omap_dss_device *dssdev,
 		struct omap_overlay_manager_info *info,
 		struct omap_overlay_info **overlay_infos);
+int dss_mgr_set_timings(struct omap_overlay_manager *mgr,
+		struct omap_video_timings *timings);
 
 /* overlay */
 void dss_init_overlays(struct platform_device *pdev);
@@ -442,8 +444,6 @@ void dispc_mgr_enable_stallmode(enum omap_channel channel, bool enable);
 void dispc_mgr_set_tft_data_lines(enum omap_channel channel, u8 data_lines);
 void dispc_mgr_set_lcd_display_type(enum omap_channel channel,
 		enum omap_lcd_display_type type);
-void dispc_mgr_set_timings(enum omap_channel channel,
-		struct omap_video_timings *timings);
 void dispc_mgr_set_pol_freq(enum omap_channel channel,
 		enum omap_panel_config config, u8 acbi, u8 acb);
 unsigned long dispc_mgr_lclk_rate(enum omap_channel channel);
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 56f6e9c..4563781 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -376,7 +376,9 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)
 	dispc_enable_gamma_table(0);
 
 	/* tv size */
-	dispc_mgr_set_timings(dssdev->manager->id, &dssdev->panel.timings);
+	r = dss_mgr_set_timings(dssdev->manager, &dssdev->panel.timings);
+	if (r)
+		goto err_mgr_set_timings;
 
 	hdmi.ip_data.ops->video_enable(&hdmi.ip_data, 1);
 
@@ -387,6 +389,7 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)
 	return 0;
 
 err_mgr_enable:
+err_mgr_set_timings:
 	hdmi.ip_data.ops->video_enable(&hdmi.ip_data, 0);
 	hdmi.ip_data.ops->phy_disable(&hdmi.ip_data);
 	hdmi.ip_data.ops->pll_disable(&hdmi.ip_data);
diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index e736460..2c85988 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -684,3 +684,24 @@ int dss_mgr_check(struct omap_overlay_manager *mgr,
 
 	return 0;
 }
+
+int dss_mgr_set_timings(struct omap_overlay_manager *mgr,
+		struct omap_video_timings *timings)
+{
+	int r;
+	struct omap_overlay_manager_info info;
+
+	mgr->get_manager_info(mgr, &info);
+
+	info.timings = *timings;
+
+	r = mgr->set_manager_info(mgr, &info);
+	if (r)
+		return r;
+
+	r = mgr->apply(mgr);
+	if (r)
+		return r;
+
+	return 0;
+}
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index a81ffcb..588160a 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -320,7 +320,7 @@ static void rfbi_transfer_area(struct omap_dss_device *dssdev, u16 width,
 
 	DSSDBG("rfbi_transfer_area %dx%d\n", width, height);
 
-	dispc_mgr_set_timings(dssdev->manager->id, &timings);
+	dss_mgr_set_timings(dssdev->manager, &timings);
 
 	dispc_mgr_enable(dssdev->manager->id, true);
 
@@ -776,6 +776,7 @@ int omap_rfbi_prepare_update(struct omap_dss_device *dssdev,
 		u16 *x, u16 *y, u16 *w, u16 *h)
 {
 	u16 dw, dh;
+	int r;
 	struct omap_video_timings timings = {
 		.hsw		= 1,
 		.hfp		= 1,
@@ -804,7 +805,9 @@ int omap_rfbi_prepare_update(struct omap_dss_device *dssdev,
 	if (*w = 0 || *h = 0)
 		return -EINVAL;
 
-	dispc_mgr_set_timings(dssdev->manager->id, &timings);
+	r = dss_mgr_set_timings(dssdev->manager, &timings);
+	if (r)
+		return r;
 
 	return 0;
 }
diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
index 741b834..2e70f46 100644
--- a/drivers/video/omap2/dss/sdi.c
+++ b/drivers/video/omap2/dss/sdi.c
@@ -107,7 +107,9 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
 	}
 
 
-	dispc_mgr_set_timings(dssdev->manager->id, t);
+	r = dss_mgr_set_timings(dssdev->manager, t);
+	if (r)
+		goto err_set_mgr_timings;
 
 	r = dss_set_clock_div(&dss_cinfo);
 	if (r)
@@ -134,6 +136,7 @@ err_mgr_enable:
 err_sdi_enable:
 err_set_dispc_clock_div:
 err_set_dss_clock_div:
+err_set_mgr_timings:
 err_calc_clock_div:
 	dispc_runtime_put();
 err_get_dispc:
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 30bbb63..073b223 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -444,22 +444,24 @@ static int venc_power_on(struct omap_dss_device *dssdev)
 	timings = dssdev->panel.timings;
 	timings.y_res /= 2;
 
-	dispc_mgr_set_timings(dssdev->manager->id, &timings);
+	r = dss_mgr_set_timings(dssdev->manager, &timings);
+	if (r)
+		goto err0;
 
 	r = regulator_enable(venc.vdda_dac_reg);
 	if (r)
-		goto err;
+		goto err1;
 
 	if (dssdev->platform_enable)
 		dssdev->platform_enable(dssdev);
 
 	r = dss_mgr_enable(dssdev->manager);
 	if (r)
-		goto err;
+		goto err1;
 
 	return 0;
 
-err:
+err1:
 	venc_write_reg(VENC_OUTPUT_CONTROL, 0);
 	dss_set_dac_pwrdn_bgz(0);
 
@@ -467,7 +469,7 @@ err:
 		dssdev->platform_disable(dssdev);
 
 	regulator_disable(venc.vdda_dac_reg);
-
+err0:
 	return r;
 }
 
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 5f36ddd..dbc62e8 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -429,6 +429,8 @@ struct omap_overlay_manager_info {
 
 	bool cpr_enable;
 	struct omap_dss_cpr_coefs cpr_coefs;
+
+	struct omap_video_timings timings;
 };
 
 struct omap_overlay_manager {
-- 
1.7.5.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox