Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] fbcon: fix race condition between console lock and cursor timer
From: Peter Zijlstra @ 2012-08-21 12:15 UTC (permalink / raw)
  To: Alan Cox
  Cc: Dave Airlie, linux-fbdev, dri-devel, linux-kernel, Linus,
	Randy Dunlap, Josh Boyer
In-Reply-To: <20120821101505.61c0221a@pyramind.ukuu.org.uk>

On Tue, 2012-08-21 at 10:15 +0100, Alan Cox wrote:
> > So after much tracing with direct netconsole writes (printks
> > under console_lock not so useful), I think I found the race.
> 
> Direct netconsole write would be a useful patch to have mainline I think
> 8) 

could we make that use the earlyprintk infrastructure?

^ permalink raw reply

* Re: [PATCH] fbcon: fix race condition between console lock and cursor timer
From: Peter Zijlstra @ 2012-08-21 12:14 UTC (permalink / raw)
  To: Dave Airlie
  Cc: linux-fbdev, dri-devel, linux-kernel, Linus, Alan Cox,
	Randy Dunlap, Josh Boyer
In-Reply-To: <1345531207-24926-1-git-send-email-airlied@redhat.com>

On Tue, 2012-08-21 at 16:40 +1000, Dave Airlie wrote:
> So after much tracing with direct netconsole writes (printks
> under console_lock not so useful) 

I always use earlyprintk on serial.. 

^ permalink raw reply

* Re: [PATCH V5 1/6] OMAPDSS: DISPC: cleanup cpu_is_xxxx checks
From: Mahapatra, Chandrabhanu @ 2012-08-21 11:32 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345545102.4085.44.camel@deskari>

On Tue, Aug 21, 2012 at 4:01 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Mon, 2012-08-20 at 18:52 +0530, Chandrabhanu Mahapatra wrote:
>> All the cpu_is checks have been moved to dispc_init_features function providing
>> a much more generic and cleaner interface. The OMAP version and revision
>> specific functions and data are initialized by dispc_features structure which is
>> local to dispc.c.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>>  drivers/video/omap2/dss/dispc.c |  433 +++++++++++++++++++++++++--------------
>>  1 file changed, 278 insertions(+), 155 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index ff52702..3fad33a 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -81,6 +81,23 @@ struct dispc_irq_stats {
>>       unsigned irqs[32];
>>  };
>>
>> +struct dispc_features {
>> +     int hp_max;
>> +     int vp_max;
>> +     int sw_max;
>> +     int sw_start;
>> +     int fp_start;
>> +     int bp_start;
>
> Here you could use a bit smaller datatype. u16 should probably be more
> than enough.
>

After looking at the values that these variables take I think hp_max,
vp_max and sw_max can be u16 and the rest three sw_start, fp_start and
bp_start can be u8.

>> +static int __init dispc_init_features(struct device *dev)
>> +{
>> +     struct dispc_features *feat = devm_kzalloc(dev, sizeof(*feat),
>> +                                                             GFP_KERNEL);
>> +     if (!feat) {
>> +             dev_err(dev, "Failed to allocate DISPC Features\n");
>> +             return -ENOMEM;
>> +     }
>> +
>> +     if (cpu_is_omap24xx()) {
>> +             memcpy(feat, &omap24xx_dispc_feats, sizeof(*feat));
>> +     } else if (cpu_is_omap34xx()) {
>> +             if (omap_rev() < OMAP3430_REV_ES3_0)
>> +                     memcpy(feat, &omap34xx_rev1_0_dispc_feats,
>> +                                                     sizeof(*feat));
>> +             else
>> +                     memcpy(feat, &omap34xx_rev3_0_dispc_feats,
>> +                                                     sizeof(*feat));
>> +     } else if (cpu_is_omap44xx()) {
>> +             memcpy(feat, &omap44xx_dispc_feats, sizeof(*feat));
>> +     } else {
>> +             return -ENODEV;
>> +     }
>> +
>> +     dispc.feat = feat;
>> +
>> +     return 0;
>> +}
>
> This becomes much cleaner with something like the following (same could
> be used in dss.c also):
>
>         const struct dispc_features *src;
>         struct dispc_features *dst;
>
>         dst = devm_kzalloc(dev, sizeof(*dst), GFP_KERNEL);
>         if (!dsst) {
>                 dev_err(dev, "Failed to allocate DISPC Features\n");
>                 return -ENOMEM;
>         }
>
>         if (cpu_is_omap24xx()) {
>                 src = &omap24xx_dispc_feats;
>         } else if (cpu_is_omap34xx()) {
>                 if (omap_rev() < OMAP3430_REV_ES3_0)
>                         src = &omap34xx_rev1_0_dispc_feats;
>                 else
>                         src = &omap34xx_rev3_0_dispc_feats;
>         } else if (cpu_is_omap44xx()) {
>                 src = &omap44xx_dispc_feats;
>         } else {
>                 return -ENODEV;
>         }
>
>         memcpy(dst, src, sizeof(*dst));
>
>         dispc.feat = dst;
>
>  Tomi
>

ok, looks cleaner.

-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

^ permalink raw reply

* Re: [PATCH V5 5/6] ARM: OMAP: Disable venc for OMAP4
From: Mahapatra, Chandrabhanu @ 2012-08-21 11:25 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: tony, paul, linux-omap, linux-fbdev
In-Reply-To: <1345545159.4085.45.camel@deskari>

On Tue, Aug 21, 2012 at 4:02 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Mon, 2012-08-20 at 18:54 +0530, Chandrabhanu Mahapatra wrote:
>> This is a alternative to "HACK: OMAP: DSS2: VENC: disable VENC on OMAP4 to
>> prevent crash" (ba02fa37de) by Tomi Valkeinen <tomi.valkeinen@ti.com> to prevent
>> VENC from crashing OMAP4 kernel. This prevents OMAPDSS from initial registration
>> of a device for VENC on OMAP4.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>>  arch/arm/mach-omap2/display.c |    1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
>> index af1ed7d..ee40739 100644
>> --- a/arch/arm/mach-omap2/display.c
>> +++ b/arch/arm/mach-omap2/display.c
>> @@ -95,7 +95,6 @@ static const struct omap_dss_hwmod_data omap4_dss_hwmod_data[] __initdata = {
>>       { "dss_core", "omapdss_dss", -1 },
>>       { "dss_dispc", "omapdss_dispc", -1 },
>>       { "dss_rfbi", "omapdss_rfbi", -1 },
>> -     { "dss_venc", "omapdss_venc", -1 },
>>       { "dss_dsi1", "omapdss_dsi", 0 },
>>       { "dss_dsi2", "omapdss_dsi", 1 },
>>       { "dss_hdmi", "omapdss_hdmi", -1 },
>
> You need to reorder this and the previous patch. If you first remove the
> workaround from the previous patch, the kernel will crash if after that
> patch. So this one should be before the previous patch.
>
>  Tomi
>

Ok.

-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

^ permalink raw reply

* Re: [PATCH V5 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Tomi Valkeinen @ 2012-08-21 11:20 UTC (permalink / raw)
  To: Mahapatra, Chandrabhanu; +Cc: linux-omap, linux-fbdev
In-Reply-To: <CAF0AtAtvyKdW-J97NJ+YNi5pUGQFyxk=B2Eqn_74MZD4dWebEA@mail.gmail.com>

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

On Tue, 2012-08-21 at 16:36 +0530, Mahapatra, Chandrabhanu wrote:
> On Tue, Aug 21, 2012 at 4:05 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Mon, 2012-08-20 at 18:53 +0530, Chandrabhanu Mahapatra wrote:
> >> All the cpu_is checks have been moved to dss_init_features function providing a
> >> much more generic and cleaner interface. The OMAP version and revision specific
> >> initializations in various functions are cleaned and the necessary data are
> >> moved to dss_features structure which is local to dss.c.
> >>
> >> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> >> ---
> >>  drivers/video/omap2/dss/dss.c |  117 ++++++++++++++++++++++++++---------------
> >>  1 file changed, 76 insertions(+), 41 deletions(-)
> >>
> >> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
> >> index 10566ae..f6d3d0a 100644
> >> --- a/drivers/video/omap2/dss/dss.c
> >> +++ b/drivers/video/omap2/dss/dss.c
> >> @@ -31,6 +31,7 @@
> >>  #include <linux/clk.h>
> >>  #include <linux/platform_device.h>
> >>  #include <linux/pm_runtime.h>
> >> +#include <linux/gfp.h>
> >>
> >>  #include <video/omapdss.h>
> >>
> >> @@ -65,6 +66,12 @@ struct dss_reg {
> >>  static int dss_runtime_get(void);
> >>  static void dss_runtime_put(void);
> >>
> >> +struct dss_features {
> >> +     u16 fck_div_max;
> >> +     int dss_fck_multiplier;
> >> +     char *clk_name;
> >
> > const char *
> >
> >  Tomi
> >
> 
> All the values are const. So we have a const pointer dss.feat to hold
> this structure. Is a separate
> const char *clk_name required?

Yes, the values are const, so the pointer itself is const. However, the
chars where the pointer points are not const. So:

feat.clk_name = 1234; // this is rejected by the compiler
feat.clk_name[0] = 0; // this would be accepted

 Tomi




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

^ permalink raw reply

* Re: [PATCH V5 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Mahapatra, Chandrabhanu @ 2012-08-21 11:18 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345545312.4085.46.camel@deskari>

On Tue, Aug 21, 2012 at 4:05 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Mon, 2012-08-20 at 18:53 +0530, Chandrabhanu Mahapatra wrote:
>> All the cpu_is checks have been moved to dss_init_features function providing a
>> much more generic and cleaner interface. The OMAP version and revision specific
>> initializations in various functions are cleaned and the necessary data are
>> moved to dss_features structure which is local to dss.c.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>>  drivers/video/omap2/dss/dss.c |  117 ++++++++++++++++++++++++++---------------
>>  1 file changed, 76 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
>> index 10566ae..f6d3d0a 100644
>> --- a/drivers/video/omap2/dss/dss.c
>> +++ b/drivers/video/omap2/dss/dss.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/clk.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/gfp.h>
>>
>>  #include <video/omapdss.h>
>>
>> @@ -65,6 +66,12 @@ struct dss_reg {
>>  static int dss_runtime_get(void);
>>  static void dss_runtime_put(void);
>>
>> +struct dss_features {
>> +     u16 fck_div_max;
>> +     int dss_fck_multiplier;
>> +     char *clk_name;
>
> const char *
>
>  Tomi
>

All the values are const. So we have a const pointer dss.feat to hold
this structure. Is a separate
const char *clk_name required?

-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

^ permalink raw reply

* Re: [PATCH V5 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Tomi Valkeinen @ 2012-08-21 10:35 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <94a123bbc0a4ecd48fc71275fd5efbe0aac93b87.1345468541.git.cmahapatra@ti.com>

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

On Mon, 2012-08-20 at 18:53 +0530, Chandrabhanu Mahapatra wrote:
> All the cpu_is checks have been moved to dss_init_features function providing a
> much more generic and cleaner interface. The OMAP version and revision specific
> initializations in various functions are cleaned and the necessary data are
> moved to dss_features structure which is local to dss.c.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
>  drivers/video/omap2/dss/dss.c |  117 ++++++++++++++++++++++++++---------------
>  1 file changed, 76 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
> index 10566ae..f6d3d0a 100644
> --- a/drivers/video/omap2/dss/dss.c
> +++ b/drivers/video/omap2/dss/dss.c
> @@ -31,6 +31,7 @@
>  #include <linux/clk.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/gfp.h>
>  
>  #include <video/omapdss.h>
>  
> @@ -65,6 +66,12 @@ struct dss_reg {
>  static int dss_runtime_get(void);
>  static void dss_runtime_put(void);
>  
> +struct dss_features {
> +	u16 fck_div_max;
> +	int dss_fck_multiplier;
> +	char *clk_name;

const char *

 Tomi


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

^ permalink raw reply

* Re: [PATCH V5 5/6] ARM: OMAP: Disable venc for OMAP4
From: Tomi Valkeinen @ 2012-08-21 10:32 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: tony, paul, linux-omap, linux-fbdev
In-Reply-To: <a07a3a216827e830cfa249f4f39ad26e75dc0afd.1345468541.git.cmahapatra@ti.com>

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

On Mon, 2012-08-20 at 18:54 +0530, Chandrabhanu Mahapatra wrote:
> This is a alternative to "HACK: OMAP: DSS2: VENC: disable VENC on OMAP4 to
> prevent crash" (ba02fa37de) by Tomi Valkeinen <tomi.valkeinen@ti.com> to prevent
> VENC from crashing OMAP4 kernel. This prevents OMAPDSS from initial registration
> of a device for VENC on OMAP4.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
>  arch/arm/mach-omap2/display.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> index af1ed7d..ee40739 100644
> --- a/arch/arm/mach-omap2/display.c
> +++ b/arch/arm/mach-omap2/display.c
> @@ -95,7 +95,6 @@ static const struct omap_dss_hwmod_data omap4_dss_hwmod_data[] __initdata = {
>  	{ "dss_core", "omapdss_dss", -1 },
>  	{ "dss_dispc", "omapdss_dispc", -1 },
>  	{ "dss_rfbi", "omapdss_rfbi", -1 },
> -	{ "dss_venc", "omapdss_venc", -1 },
>  	{ "dss_dsi1", "omapdss_dsi", 0 },
>  	{ "dss_dsi2", "omapdss_dsi", 1 },
>  	{ "dss_hdmi", "omapdss_hdmi", -1 },

You need to reorder this and the previous patch. If you first remove the
workaround from the previous patch, the kernel will crash if after that
patch. So this one should be before the previous patch.

 Tomi


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

^ permalink raw reply

* Re: [PATCH V5 1/6] OMAPDSS: DISPC: cleanup cpu_is_xxxx checks
From: Tomi Valkeinen @ 2012-08-21 10:31 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <978118e2c23e2da40ceb740665fd19f86810b58a.1345468541.git.cmahapatra@ti.com>

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

On Mon, 2012-08-20 at 18:52 +0530, Chandrabhanu Mahapatra wrote:
> All the cpu_is checks have been moved to dispc_init_features function providing
> a much more generic and cleaner interface. The OMAP version and revision
> specific functions and data are initialized by dispc_features structure which is
> local to dispc.c.
> 
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c |  433 +++++++++++++++++++++++++--------------
>  1 file changed, 278 insertions(+), 155 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index ff52702..3fad33a 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -81,6 +81,23 @@ struct dispc_irq_stats {
>  	unsigned irqs[32];
>  };
>  
> +struct dispc_features {
> +	int hp_max;
> +	int vp_max;
> +	int sw_max;
> +	int sw_start;
> +	int fp_start;
> +	int bp_start;

Here you could use a bit smaller datatype. u16 should probably be more
than enough.

> +static int __init dispc_init_features(struct device *dev)
> +{
> +	struct dispc_features *feat = devm_kzalloc(dev, sizeof(*feat),
> +								GFP_KERNEL);
> +	if (!feat) {
> +		dev_err(dev, "Failed to allocate DISPC Features\n");
> +		return -ENOMEM;
> +	}
> +
> +	if (cpu_is_omap24xx()) {
> +		memcpy(feat, &omap24xx_dispc_feats, sizeof(*feat));
> +	} else if (cpu_is_omap34xx()) {
> +		if (omap_rev() < OMAP3430_REV_ES3_0)
> +			memcpy(feat, &omap34xx_rev1_0_dispc_feats,
> +							sizeof(*feat));
> +		else
> +			memcpy(feat, &omap34xx_rev3_0_dispc_feats,
> +							sizeof(*feat));
> +	} else if (cpu_is_omap44xx()) {
> +		memcpy(feat, &omap44xx_dispc_feats, sizeof(*feat));
> +	} else {
> +		return -ENODEV;
> +	}
> +
> +	dispc.feat = feat;
> +
> +	return 0;
> +}

This becomes much cleaner with something like the following (same could
be used in dss.c also):

	const struct dispc_features *src;
	struct dispc_features *dst;

	dst = devm_kzalloc(dev, sizeof(*dst), GFP_KERNEL);
	if (!dsst) {
		dev_err(dev, "Failed to allocate DISPC Features\n");
		return -ENOMEM;
	}

	if (cpu_is_omap24xx()) {
		src = &omap24xx_dispc_feats;
	} else if (cpu_is_omap34xx()) {
		if (omap_rev() < OMAP3430_REV_ES3_0)
			src = &omap34xx_rev1_0_dispc_feats;
		else
			src = &omap34xx_rev3_0_dispc_feats;
	} else if (cpu_is_omap44xx()) {
		src = &omap44xx_dispc_feats;
	} else {
		return -ENODEV;
	}

	memcpy(dst, src, sizeof(*dst));

	dispc.feat = dst;

 Tomi


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

^ permalink raw reply

* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-08-21  9:54 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Alex Courbot, Stephen Warren, Simon Glass, Grant Likely,
	Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
	Arnd Bergmann, Leela Krishna Amudala,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20120821091306.GA4819-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>

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

On Tue, 2012-08-21 at 11:13 +0200, Thierry Reding wrote:
> On Tue, Aug 21, 2012 at 11:57:45AM +0300, Tomi Valkeinen wrote:

> > This doesn't mean that we'd have a separate driver for each device. For
> > example, we have a generic panel driver in OMAP, which contains a kind
> > of small panel database. The panel database contains the name of the
> > panel as a key, and panel specific configuration as a value. This
> > configuration could also contain some kind of generic power sequence.
> 
> I see. I do like the idea of it, because it is more straightforward than
> representing the whole sequence in DT. Matching could be done using the
> standard compatible property.

Right.

> However this also means we'll essentially just be moving the board code.

What do you mean "just"? Wasn't the point of the whole "arm board file
mess" to get rid of the code from the board files? If the code in the
board file is device specific code, not board specific, then the driver
of the device is a logical place to place it.

And as I said, I don't have any problems with some kind of generic power
sequences. So the code in the board file could be moved and converted to
use the power sequences, if that is better than just plain c code.

> Being in a central location it would be easier to refactor commonalities
> though.
> 
> > I'd like to require the board developer to only fill in to the DT data
> > what panel he is using, and how it's connected on his board. Not panel's
> > internal functionality.
> 
> The amount of work required by the board developer would largely depend
> on whether support for the panel is already present or not. For new
> panels this would mean that a new driver needs to be written, while
> representing the power sequence in DT might be easier to do, and readily
> available from some datasheet.

That's true.

However, if we already have a generic driver for that type of panel,
(which we would need anyway for the DT based approach), the developer
only needs to add the name of the panel and the data for the power
sequence to the panel driver's database, which is about the same amount
of work as with DT.

So it's really just a question of where to put the data in question, DT
or driver. Both contain the same data, and the data structure may also
be the same. In DT's case it just needs to be parsed first, whereas in
database case you'll enter the data in structs.

 Tomi


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

^ permalink raw reply

* Re: [RFC 0/5] Generic panel framework
From: Laurent Pinchart @ 2012-08-21  9:23 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-fbdev, dri-devel, linux-leds, linux-media, Bryan Wu,
	Richard Purdie, Marcus Lorentzon, Sumit Semwal, Archit Taneja,
	Sebastien Guiriec, Inki Dae, Kyungmin Park
In-Reply-To: <1345528197.15491.8.camel@lappyti>

Hi Tomi,

On Tuesday 21 August 2012 08:49:57 Tomi Valkeinen wrote:
> On Tue, 2012-08-21 at 01:29 +0200, Laurent Pinchart wrote:
> > On Monday 20 August 2012 14:39:30 Tomi Valkeinen wrote:
> > > On Sat, 2012-08-18 at 03:16 +0200, Laurent Pinchart wrote:
> > > > Hi Tomi,
> > > > 
> > > > mipi-dbi-bus might not belong to include/video/panel/ though, as it
> > > > can be used for non-panel devices (at least in theory). The future
> > > > mipi-dsi-bus certainly will.
> > > 
> > > They are both display busses. So while they could be used for anything,
> > > I find it quite unlikely as there are much better alternatives for
> > > generic bus needs.
> > 
> > My point is that they could be used for display devices other than panels.
> > This is especially true for DSI, as there are DSI to HDMI converters.
> > Technically speaking that's also true for DBI, as DBI chips convert from
> > DBI to DPI, but we can group both the DBI-to-DPI chip and the panel in a
> > single panel object.
> 
> Ah, ok. I thought "panels" would include these buffer/converter chips.
> 
> I think we should have one driver for one indivisible hardware entity.
> So if you've got a panel module that contains DBI receiver, buffer
> memory and a DPI panel, let's just have one "DBI panel" driver for it.
> 
> If we get lots of different panel modules containing the same DBI RX IP,
> we could have the DBI IP part as a common library, but still have one
> panel driver per panel module.

Sounds good to me.

> But how do you see the case for separate converter/buffer chips? Are
> they part of the generic panel framework? I guess they kinda have to be.
> On one side they use the "panel" API control the bus they are connected
> to, and on the other they offer an API for the connected panel to use
> the bus they provide.

The DBI/DSI APIs will not be panel-specific (they won't contain any reference 
to "panel") I'm thus thinking of moving them from drivers/video/panel/ to 
drivers/video/.

Furthermore, a DSI-to-HDMI converter chip is not a panel, but needs to expose 
display-related operations in a way similar to panels. I was thus wondering if 
we shouldn't replace the panel structure with some kind of video entity 
structure that would expose operations similar to panels. We could then extend 
that structure with converter-specific operations later. The framework would 
become a bit more generic, while remaining display-specific.

> Did you just mean we should have a separate directory for them, while
> still part of the same framework, or...?

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH] fbcon: fix race condition between console lock and cursor timer
From: Alan Cox @ 2012-08-21  9:15 UTC (permalink / raw)
  To: Dave Airlie
  Cc: linux-fbdev, dri-devel, linux-kernel, Linus, Randy Dunlap,
	Josh Boyer
In-Reply-To: <1345531207-24926-1-git-send-email-airlied@redhat.com>

> So after much tracing with direct netconsole writes (printks
> under console_lock not so useful), I think I found the race.

Direct netconsole write would be a useful patch to have mainline I think
8)

> Hopefully this fixes the problem for anyone seeing vesafb->kms
> driver handoff.

Not really the proper fix but its clear and is probably the best thing to
go in initially with a cc: stable. Can you at least stick a large 

+ /* FIXME: we should sort out the unbind locking instead */

on the patch however.

^ permalink raw reply

* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-08-21  9:13 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Alex Courbot, Stephen Warren, Simon Glass, Grant Likely,
	Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
	Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org
In-Reply-To: <1345539465.4085.22.camel@deskari>

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

On Tue, Aug 21, 2012 at 11:57:45AM +0300, Tomi Valkeinen wrote:
> On Tue, 2012-08-21 at 10:33 +0200, Thierry Reding wrote:
> 
> > I suppose power sequences aren't needed if you have a specific driver
> > for every panel out there. However that also means that you'd have to
> > write drivers for literally every panel that requires support. In the
> > end this will just result in discussion down the road how the common
> > functionality can be refactored and we may end up with power sequences
> > again.
> > 
> > Also as you mentioned, power sequences are useful for a number of other
> > use-cases. Without power sequences you'll have to potentially create
> > extra frameworks tha reimplement parts of the power sequence code for
> > their specific hardware needs.
> 
> Right. I think my main concern is the use of DT data, not power
> sequences as such. I've been going back and forth in my mind with this
> issue with OMAP also.
> 
> The question is: what stuff belongs to DT data and what belongs to the
> kernel? I've been trying to go to the direction where DT is used to
> describe the HW connections of different IP blocks and to pass board
> specific configuration. Everything else is in the driver.
> 
> This doesn't mean that we'd have a separate driver for each device. For
> example, we have a generic panel driver in OMAP, which contains a kind
> of small panel database. The panel database contains the name of the
> panel as a key, and panel specific configuration as a value. This
> configuration could also contain some kind of generic power sequence.

I see. I do like the idea of it, because it is more straightforward than
representing the whole sequence in DT. Matching could be done using the
standard compatible property.

However this also means we'll essentially just be moving the board code.
Being in a central location it would be easier to refactor commonalities
though.

> I'd like to require the board developer to only fill in to the DT data
> what panel he is using, and how it's connected on his board. Not panel's
> internal functionality.

The amount of work required by the board developer would largely depend
on whether support for the panel is already present or not. For new
panels this would mean that a new driver needs to be written, while
representing the power sequence in DT might be easier to do, and readily
available from some datasheet.

> The one benefit I see with DT based approach is that if we have, say,
> 10000 panels, we'd have quite a big database in kernel memory and a
> board may only need one or two of those. But perhaps that could be
> helped with the use of __initdata.

I haven't worked with many different panels, so maybe I can't judge this
too well, but if panel drivers were kept in a central location, then the
number can be reduced by generalizing parts of existing drivers.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-08-21  8:57 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Alex Courbot, Stephen Warren, Simon Glass, Grant Likely,
	Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
	Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org
In-Reply-To: <20120821083329.GA28992@avionic-0098.adnet.avionic-design.de>

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

On Tue, 2012-08-21 at 10:33 +0200, Thierry Reding wrote:

> I suppose power sequences aren't needed if you have a specific driver
> for every panel out there. However that also means that you'd have to
> write drivers for literally every panel that requires support. In the
> end this will just result in discussion down the road how the common
> functionality can be refactored and we may end up with power sequences
> again.
> 
> Also as you mentioned, power sequences are useful for a number of other
> use-cases. Without power sequences you'll have to potentially create
> extra frameworks tha reimplement parts of the power sequence code for
> their specific hardware needs.

Right. I think my main concern is the use of DT data, not power
sequences as such. I've been going back and forth in my mind with this
issue with OMAP also.

The question is: what stuff belongs to DT data and what belongs to the
kernel? I've been trying to go to the direction where DT is used to
describe the HW connections of different IP blocks and to pass board
specific configuration. Everything else is in the driver.

This doesn't mean that we'd have a separate driver for each device. For
example, we have a generic panel driver in OMAP, which contains a kind
of small panel database. The panel database contains the name of the
panel as a key, and panel specific configuration as a value. This
configuration could also contain some kind of generic power sequence.

I'd like to require the board developer to only fill in to the DT data
what panel he is using, and how it's connected on his board. Not panel's
internal functionality.

The one benefit I see with DT based approach is that if we have, say,
10000 panels, we'd have quite a big database in kernel memory and a
board may only need one or two of those. But perhaps that could be
helped with the use of __initdata.

 Tomi


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

^ permalink raw reply

* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-08-21  8:53 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Tomi Valkeinen, Stephen Warren, Simon Glass, Grant Likely,
	Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
	Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org
In-Reply-To: <20120821083329.GA28992@avionic-0098.adnet.avionic-design.de>

On Tuesday 21 August 2012 16:33:30 Thierry Reding wrote:
> I suppose power sequences aren't needed if you have a specific driver
> for every panel out there. However that also means that you'd have to
> write drivers for literally every panel that requires support. In the
> end this will just result in discussion down the road how the common
> functionality can be refactored and we may end up with power sequences
> again.
> 
> Also as you mentioned, power sequences are useful for a number of other
> use-cases. Without power sequences you'll have to potentially create
> extra frameworks tha reimplement parts of the power sequence code for
> their specific hardware needs.

Yes, I can imagine what a mess it would become it we had one driver for every 
panel out there which sole purpose would be to define power sequences over more 
generic drivers. That reassures me about the usefulness of this work.

Another (small) benefit of power sequences over specific drivers is that being 
defined in the DT, they would allow an old kernel to operate a newer device if 
the base driver is the same.

Alex.


^ permalink raw reply

* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-08-21  8:33 UTC (permalink / raw)
  To: Alex Courbot
  Cc: Tomi Valkeinen, Stephen Warren, Simon Glass, Grant Likely,
	Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
	Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org
In-Reply-To: <1562509.b0FYTUZ1D8@percival>

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

On Tue, Aug 21, 2012 at 05:22:12PM +0900, Alex Courbot wrote:
> Hi Tomi,
> 
> On Tuesday 21 August 2012 15:44:29 Tomi Valkeinen wrote:
> > > +Problem
> > > +-------
> > > +One very common board-dependent code is the out-of-driver code that is
> > > used to +turn a device on or off. For instance, SoC boards very commonly
> > > use a GPIO +(abstracted to a regulator or not) to control the power
> > > supply of a backlight, +disabling it when the backlight is not used in
> > > order to save power. The GPIO +that should be used, however, as well as
> > > the exact power sequence that may +also involve other resources, is
> > > board-dependent and thus unknown of the driver. +
> > > +This was previously addressed by having hooks in the device's platform
> > > data that +are called whenever the state of the device might reflect a
> > > power change. This +approach, however, introduces board-dependant code
> > > into the kernel and is not +compatible with the device tree.
> > 
> > I've been having the same problems on OMAP display related code, but the
> > problem has always been fixable by just having the driver to use a
> > common framework to do the job (a framework which may not have existed
> > at the time). The problems have never been board specific in the end,
> > but device specific.
> > 
> > Can you describe your particular HW problem a bit more? In the backlight
> > case, what exactly requires the delays and the sequence you show in the
> > example to enable/disable the backlight?
> 
> In the example, the sequence (including delays) is clearly stated by the 
> backlight specification, which is part of the panel specification. The backlight 
> uses a PWM, which makes it suitable to use the generic PWM backlight driver, 
> but how to turn the backlight on and off is very backlight specific. The power 
> sequences allow to replace the board-specific backlight callbacks by sequences 
> in the DT.
> 
> On the other hand, I saw your discussion with Laurent about the panel 
> framework, and I wonder how this would fit into it. Backlights are typically 
> embedded within panels. Power sequences are a good way to deal with the 
> absence of specific drivers for every panels, since they allow to tailor the 
> behavior of generic drivers to fit particular needs. But if every panel comes 
> with its own driver (which would define the backlight device using the most 
> appropriate driver), then it could just as well perform its backlight's 
> sequences via regular callbacks. In this particular case, there would be no 
> need for power sequences.
> 
> Power sequences are supposed to go beyond backlight drivers and support all 
> sort of devices (I have heard that it could be useful for modems as well), but 
> I wonder how relevant they are when there is a proper driver for a device. I 
> hate to question my own work but now I cannot help but think that a proper 
> driver could do the same job. So what are we trying to achieve with power 
> sequences? Are we trying to avoid a drivers' explosion by keeping some of the 
> specifics out of them? Which approach would be preferable? Are there cases 
> where a dedicated driver could not replace power sequences?

I suppose power sequences aren't needed if you have a specific driver
for every panel out there. However that also means that you'd have to
write drivers for literally every panel that requires support. In the
end this will just result in discussion down the road how the common
functionality can be refactored and we may end up with power sequences
again.

Also as you mentioned, power sequences are useful for a number of other
use-cases. Without power sequences you'll have to potentially create
extra frameworks tha reimplement parts of the power sequence code for
their specific hardware needs.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-08-21  8:22 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
	Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
	Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org
In-Reply-To: <1345535069.4085.7.camel@deskari>

Hi Tomi,

On Tuesday 21 August 2012 15:44:29 Tomi Valkeinen wrote:
> > +Problem
> > +-------
> > +One very common board-dependent code is the out-of-driver code that is
> > used to +turn a device on or off. For instance, SoC boards very commonly
> > use a GPIO +(abstracted to a regulator or not) to control the power
> > supply of a backlight, +disabling it when the backlight is not used in
> > order to save power. The GPIO +that should be used, however, as well as
> > the exact power sequence that may +also involve other resources, is
> > board-dependent and thus unknown of the driver. +
> > +This was previously addressed by having hooks in the device's platform
> > data that +are called whenever the state of the device might reflect a
> > power change. This +approach, however, introduces board-dependant code
> > into the kernel and is not +compatible with the device tree.
> 
> I've been having the same problems on OMAP display related code, but the
> problem has always been fixable by just having the driver to use a
> common framework to do the job (a framework which may not have existed
> at the time). The problems have never been board specific in the end,
> but device specific.
> 
> Can you describe your particular HW problem a bit more? In the backlight
> case, what exactly requires the delays and the sequence you show in the
> example to enable/disable the backlight?

In the example, the sequence (including delays) is clearly stated by the 
backlight specification, which is part of the panel specification. The backlight 
uses a PWM, which makes it suitable to use the generic PWM backlight driver, 
but how to turn the backlight on and off is very backlight specific. The power 
sequences allow to replace the board-specific backlight callbacks by sequences 
in the DT.

On the other hand, I saw your discussion with Laurent about the panel 
framework, and I wonder how this would fit into it. Backlights are typically 
embedded within panels. Power sequences are a good way to deal with the 
absence of specific drivers for every panels, since they allow to tailor the 
behavior of generic drivers to fit particular needs. But if every panel comes 
with its own driver (which would define the backlight device using the most 
appropriate driver), then it could just as well perform its backlight's 
sequences via regular callbacks. In this particular case, there would be no 
need for power sequences.

Power sequences are supposed to go beyond backlight drivers and support all 
sort of devices (I have heard that it could be useful for modems as well), but 
I wonder how relevant they are when there is a proper driver for a device. I 
hate to question my own work but now I cannot help but think that a proper 
driver could do the same job. So what are we trying to achieve with power 
sequences? Are we trying to avoid a drivers' explosion by keeping some of the 
specifics out of them? Which approach would be preferable? Are there cases 
where a dedicated driver could not replace power sequences?

Alex.


^ permalink raw reply

* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-08-21  7:44 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
	Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
	Arnd Bergmann, Leela Krishna Amudala,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1345097337-24170-2-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

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

Hi,

On Thu, 2012-08-16 at 15:08 +0900, Alexandre Courbot wrote:

> +Problem
> +-------
> +One very common board-dependent code is the out-of-driver code that is used to
> +turn a device on or off. For instance, SoC boards very commonly use a GPIO
> +(abstracted to a regulator or not) to control the power supply of a backlight,
> +disabling it when the backlight is not used in order to save power. The GPIO
> +that should be used, however, as well as the exact power sequence that may
> +also involve other resources, is board-dependent and thus unknown of the driver.
> +
> +This was previously addressed by having hooks in the device's platform data that
> +are called whenever the state of the device might reflect a power change. This
> +approach, however, introduces board-dependant code into the kernel and is not
> +compatible with the device tree.

I've been having the same problems on OMAP display related code, but the
problem has always been fixable by just having the driver to use a
common framework to do the job (a framework which may not have existed
at the time). The problems have never been board specific in the end,
but device specific.

Can you describe your particular HW problem a bit more? In the backlight
case, what exactly requires the delays and the sequence you show in the
example to enable/disable the backlight?

 Tomi


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

^ permalink raw reply

* [PATCH] fbcon: fix race condition between console lock and cursor timer
From: Dave Airlie @ 2012-08-21  6:40 UTC (permalink / raw)
  To: linux-fbdev
  Cc: dri-devel, linux-kernel, Linus, Alan Cox, Randy Dunlap,
	Josh Boyer, Dave Airlie

So we've had a fair few reports of fbcon handover breakage between
efi/vesafb and i915 surface recently, so I dedicated a couple of
days to finding the problem.

Essentially the last thing we saw was the conflicting framebuffer
message and that was all.

So after much tracing with direct netconsole writes (printks
under console_lock not so useful), I think I found the race.

Thread A (driver load)    Thread B (timer thread)
  unbind_con_driver ->              |
  bind_con_driver ->                |
  vc->vc_sw->con_deinit ->          |
  fbcon_deinit ->                   |
  console_lock()                    |
      |                             |
      |                       fbcon_flashcursor timer fires
      |                       console_lock() <- blocked for A
      |
      |
fbcon_del_cursor_timer ->
  del_timer_sync
  (BOOM)

Of course because all of this is under the console lock,
we never see anything, also since we also just unbound the active
console guess what we never see anything.

Hopefully this fixes the problem for anyone seeing vesafb->kms
driver handoff.

Signed-off-by: David Airlie <airlied@redhat.com>
---
 drivers/video/console/fbcon.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 2e471c2..f8a79fc 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -372,8 +372,12 @@ static void fb_flashcursor(struct work_struct *work)
 	struct vc_data *vc = NULL;
 	int c;
 	int mode;
+	int ret;
+
+	ret = console_trylock();
+	if (ret = 0)
+		return;
 
-	console_lock();
 	if (ops && ops->currcon != -1)
 		vc = vc_cons[ops->currcon].d;
 
-- 
1.7.10.2


^ permalink raw reply related

* [PATCH 23/23] OMAPDSS: Remove old way of setting manager and device links
From: Archit Taneja @ 2012-08-21  6:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, rob, sumit.semwal, Archit Taneja
In-Reply-To: <1345528711-27801-1-git-send-email-archit@ti.com>

Now that an omap_dss_output can be used to link between managers and devices, we
can remove the old way of setting manager and device links. This involves
removing the device and manager pointers from omap_overlay_manager and
omap_dss_device respectively, and removing the set_device/unset_device ops from
omap_overlay_manager.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c   |   64 -------------------------------------
 drivers/video/omap2/dss/manager.c |    2 --
 include/video/omapdss.h           |    5 ---
 3 files changed, 71 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index d584f0c..0ae70ab 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -1247,70 +1247,6 @@ void dss_mgr_get_info(struct omap_overlay_manager *mgr,
 	spin_unlock_irqrestore(&data_lock, flags);
 }
 
-int dss_mgr_set_device(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev)
-{
-	int r;
-
-	mutex_lock(&apply_lock);
-
-	if (dssdev->manager) {
-		DSSERR("display '%s' already has a manager '%s'\n",
-			       dssdev->name, dssdev->manager->name);
-		r = -EINVAL;
-		goto err;
-	}
-
-	if ((mgr->supported_displays & dssdev->type) = 0) {
-		DSSERR("display '%s' does not support manager '%s'\n",
-			       dssdev->name, mgr->name);
-		r = -EINVAL;
-		goto err;
-	}
-
-	dssdev->manager = mgr;
-	mgr->device = dssdev;
-
-	mutex_unlock(&apply_lock);
-
-	return 0;
-err:
-	mutex_unlock(&apply_lock);
-	return r;
-}
-
-int dss_mgr_unset_device(struct omap_overlay_manager *mgr)
-{
-	int r;
-
-	mutex_lock(&apply_lock);
-
-	if (!mgr->device) {
-		DSSERR("failed to unset display, display not set.\n");
-		r = -EINVAL;
-		goto err;
-	}
-
-	/*
-	 * Don't allow currently enabled displays to have the overlay manager
-	 * pulled out from underneath them
-	 */
-	if (mgr->device->state != OMAP_DSS_DISPLAY_DISABLED) {
-		r = -EINVAL;
-		goto err;
-	}
-
-	mgr->device->manager = NULL;
-	mgr->device = NULL;
-
-	mutex_unlock(&apply_lock);
-
-	return 0;
-err:
-	mutex_unlock(&apply_lock);
-	return r;
-}
-
 int dss_mgr_set_output(struct omap_overlay_manager *mgr,
 		struct omap_dss_output *output)
 {
diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index d14ffc5..953f5ee 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -570,8 +570,6 @@ int dss_init_overlay_managers(struct platform_device *pdev)
 			break;
 		}
 
-		mgr->set_device = &dss_mgr_set_device;
-		mgr->unset_device = &dss_mgr_unset_device;
 		mgr->set_output = &dss_mgr_set_output;
 		mgr->unset_output = &dss_mgr_unset_output;
 		mgr->apply = &omap_dss_mgr_apply;
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index b3faea7..2be7629 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -466,7 +466,6 @@ struct omap_overlay_manager {
 	enum omap_dss_output_id supported_outputs;
 
 	/* dynamic fields */
-	struct omap_dss_device *device;
 	struct omap_dss_output *output;
 
 	/*
@@ -480,9 +479,6 @@ struct omap_overlay_manager {
 	 * interrupt context
 	 */
 
-	int (*set_device)(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev);
-	int (*unset_device)(struct omap_overlay_manager *mgr);
 	int (*set_output)(struct omap_overlay_manager *mgr,
 		struct omap_dss_output *output);
 	int (*unset_output)(struct omap_overlay_manager *mgr);
@@ -634,7 +630,6 @@ struct omap_dss_device {
 
 	enum omap_display_caps caps;
 
-	struct omap_overlay_manager *manager;
 	struct omap_dss_output *output;
 
 	enum omap_dss_display_state state;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 22/23] OMAPDSS: APPLY: Remove omap_dss_device references from dss_ovl_enable/disable
From: Archit Taneja @ 2012-08-21  6:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, rob, sumit.semwal, Archit Taneja
In-Reply-To: <1345528711-27801-1-git-send-email-archit@ti.com>

An overlay isn't allowed to be enabled/disabled if it isn't connected to an
omap_dss_device. This requirement isn't needed any more. An overlay can be
enabled/disabled as long as it has an output connected to it. The output may
not be connected to a device, but we can be assured that the connected
manager's output is in use by an output interface.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 8a05cbc..d584f0c 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -1607,8 +1607,7 @@ int dss_ovl_enable(struct omap_overlay *ovl)
 		goto err1;
 	}
 
-	if (ovl->manager = NULL ||
-			ovl->manager->get_device(ovl->manager) = NULL) {
+	if (ovl->manager = NULL || ovl->manager->output = NULL) {
 		r = -EINVAL;
 		goto err1;
 	}
@@ -1677,8 +1676,7 @@ int dss_ovl_disable(struct omap_overlay *ovl)
 		goto err;
 	}
 
-	if (ovl->manager = NULL ||
-			ovl->manager->get_device(ovl->manager) = NULL) {
+	if (ovl->manager = NULL || ovl->manager->output = NULL) {
 		r = -EINVAL;
 		goto err;
 	}
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 21/23] OMAPDSS: MANAGER: Get device via output
From: Archit Taneja @ 2012-08-21  6:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, rob, sumit.semwal, Archit Taneja
In-Reply-To: <1345528711-27801-1-git-send-email-archit@ti.com>

A manager is not connected to a device directly any more. It first connects
to an output, and then to the display. Update the manager's get_device op to
return the device via the connected output.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/manager.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index d808ce2..d14ffc5 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -509,7 +509,7 @@ static struct kobj_type manager_ktype = {
 
 static inline struct omap_dss_device *dss_mgr_get_device(struct omap_overlay_manager *mgr)
 {
-	return mgr->device;
+	return mgr->output ? mgr->output->device : NULL;
 }
 
 static int dss_mgr_wait_for_vsync(struct omap_overlay_manager *mgr)
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 20/23] OMAPDSS: MANAGER: Update display sysfs store
From: Archit Taneja @ 2012-08-21  6:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, rob, sumit.semwal, Archit Taneja
In-Reply-To: <1345528711-27801-1-git-send-email-archit@ti.com>

The display sysfs attribute's store function needs to be changed with the
introduction of outputs.

Providing a manager to the display isn't enough to create a link now, the
manager needs and output to connect to. A manager's display store file only
has the information of the manager and the desired display, it is not aware
of which output should the manager connect to.

Because of this, a new constraint needs to be set up when setting a display via
this sysfs file. The constraint is that the desired display should already be
connected to an output before calling this sysfs function.

This might break some existing user space stuff which uses sysfs directly. But
in most cases dss_recheck_connections will connect displays to floating outputs.
DSS sysfs files are being planned to be remove anyway, so it's not much of a
harm.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/manager.c |   25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index fd39f66..d808ce2 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -74,18 +74,33 @@ static ssize_t manager_display_store(struct omap_overlay_manager *mgr,
 	if (dssdev)
 		DSSDBG("display %s found\n", dssdev->name);
 
-	if (mgr->get_device(mgr)) {
-		r = mgr->unset_device(mgr);
+	if (mgr->output) {
+		if (mgr->output->device) {
+			r = mgr->output->unset_device(mgr->output);
+			if (r) {
+				goto put_device;
+				DSSERR("failed to unset device from output\n");
+			}
+		}
+
+		r = mgr->unset_output(mgr);
 		if (r) {
-			DSSERR("failed to unset display\n");
+			DSSERR("failed to unset current output\n");
 			goto put_device;
 		}
 	}
 
 	if (dssdev) {
-		r = mgr->set_device(mgr, dssdev);
+		struct omap_dss_output *out = dssdev->output;
+
+		if (!out) {
+			DSSERR("no output connected to device\n");
+			goto put_device;
+		}
+
+		r = mgr->set_output(mgr, out);
 		if (r) {
-			DSSERR("failed to set manager\n");
+			DSSERR("failed to set manager output\n");
 			goto put_device;
 		}
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 19/23] OMAPDSS/OMAPFB: Change dssdev->manager references
From: Archit Taneja @ 2012-08-21  6:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, rob, sumit.semwal, Archit Taneja
In-Reply-To: <1345528711-27801-1-git-send-email-archit@ti.com>

To retrieve the manager pointer via a device, we need to now access it via the
output to which the device is connected. Make this change in the places where
such a reference is made.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/display.c         |   11 +++++++++--
 drivers/video/omap2/omapfb/omapfb-ioctl.c |    4 ++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
index 5bd957e..07fd1c7 100644
--- a/drivers/video/omap2/dss/display.c
+++ b/drivers/video/omap2/dss/display.c
@@ -349,8 +349,15 @@ void dss_uninit_device(struct platform_device *pdev,
 	while ((attr = display_sysfs_attrs[i++]) != NULL)
 		device_remove_file(&dssdev->dev, attr);
 
-	if (dssdev->manager)
-		dssdev->manager->unset_device(dssdev->manager);
+	if (dssdev->output) {
+		if (dssdev->output->manager) {
+			struct omap_overlay_manager *mgr +					dssdev->output->manager;
+
+			mgr->unset_output(mgr);
+		}
+		dssdev->output->unset_device(dssdev->output);
+	}
 }
 
 static int dss_suspend_device(struct device *dev, void *data)
diff --git a/drivers/video/omap2/omapfb/omapfb-ioctl.c b/drivers/video/omap2/omapfb/omapfb-ioctl.c
index c6cf372..25c0097 100644
--- a/drivers/video/omap2/omapfb/omapfb-ioctl.c
+++ b/drivers/video/omap2/omapfb/omapfb-ioctl.c
@@ -786,12 +786,12 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, unsigned long arg)
 
 	case OMAPFB_WAITFORVSYNC:
 		DBG("ioctl WAITFORVSYNC\n");
-		if (!display) {
+		if (!display && !display->output && !display->output->manager) {
 			r = -EINVAL;
 			break;
 		}
 
-		r = display->manager->wait_for_vsync(display->manager);
+		r = display->output->manager->wait_for_vsync(display->output->manager);
 		break;
 
 	case OMAPFB_WAITFORGO:
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 18/23] OMAPDSS: HDMI: Add output pointers as arguments to all functions used by hdmi panel dr
From: Archit Taneja @ 2012-08-21  6:10 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, rob, sumit.semwal, Archit Taneja
In-Reply-To: <1345528711-27801-1-git-send-email-archit@ti.com>

All functions of an output driver used by a panel driver should have an
omap_dss_output pointer as an argument. This is needed as the function would
somehow need to retrieve the output driver's private data. It may not be needed
by some of the outputs for now as driver data is globally visible within them.
The correct way to retrieve driver data is to extract the platform device from
the output, and then extract the driver data from the platform device.

Add output arguments from functions used by panel drivers which currently miss
it. This will come to use when the HDMI functions retrieve the driver data
in the correct manner.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dss.h        |   17 ++++++++-------
 drivers/video/omap2/dss/hdmi.c       |   16 +++++++-------
 drivers/video/omap2/dss/hdmi_panel.c |   38 +++++++++++++++++++++++++---------
 3 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index bfa8b4d..dfbc3d9 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -509,17 +509,18 @@ void omapdss_hdmi_display_set_timing(struct omap_dss_output *out,
 		struct omap_video_timings *timings);
 int omapdss_hdmi_display_check_timing(struct omap_dss_output *out,
 		struct omap_video_timings *timings);
-int omapdss_hdmi_read_edid(u8 *buf, int len);
-bool omapdss_hdmi_detect(void);
+int omapdss_hdmi_read_edid(struct omap_dss_output *out, u8 *buf, int len);
+bool omapdss_hdmi_detect(struct omap_dss_output *out);
 int hdmi_panel_init(void);
 void hdmi_panel_exit(void);
 #ifdef CONFIG_OMAP4_DSS_HDMI_AUDIO
-int hdmi_audio_enable(void);
-void hdmi_audio_disable(void);
-int hdmi_audio_start(void);
-void hdmi_audio_stop(void);
-bool hdmi_mode_has_audio(void);
-int hdmi_audio_config(struct omap_dss_audio *audio);
+int hdmi_audio_enable(struct omap_dss_output *out);
+void hdmi_audio_disable(struct omap_dss_output *out);
+int hdmi_audio_start(struct omap_dss_output *out);
+void hdmi_audio_stop(struct omap_dss_output *out);
+bool hdmi_mode_has_audio(struct omap_dss_output *out);
+int hdmi_audio_config(struct omap_dss_output *out,
+		struct omap_dss_audio *audio);
 #endif
 
 /* RFBI */
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 16b745c..39dfc2b 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -602,7 +602,7 @@ static void hdmi_dump_regs(struct seq_file *s)
 	mutex_unlock(&hdmi.lock);
 }
 
-int omapdss_hdmi_read_edid(u8 *buf, int len)
+int omapdss_hdmi_read_edid(struct omap_dss_output *out, u8 *buf, int len)
 {
 	int r;
 
@@ -619,7 +619,7 @@ int omapdss_hdmi_read_edid(u8 *buf, int len)
 	return r;
 }
 
-bool omapdss_hdmi_detect(void)
+bool omapdss_hdmi_detect(struct omap_dss_output *out)
 {
 	int r;
 
@@ -829,35 +829,35 @@ int hdmi_compute_acr(u32 sample_freq, u32 *n, u32 *cts)
 	return 0;
 }
 
-int hdmi_audio_enable(void)
+int hdmi_audio_enable(struct omap_dss_output *out)
 {
 	DSSDBG("audio_enable\n");
 
 	return hdmi.ip_data.ops->audio_enable(&hdmi.ip_data);
 }
 
-void hdmi_audio_disable(void)
+void hdmi_audio_disable(struct omap_dss_output *out)
 {
 	DSSDBG("audio_disable\n");
 
 	hdmi.ip_data.ops->audio_disable(&hdmi.ip_data);
 }
 
-int hdmi_audio_start(void)
+int hdmi_audio_start(struct omap_dss_output *out)
 {
 	DSSDBG("audio_start\n");
 
 	return hdmi.ip_data.ops->audio_start(&hdmi.ip_data);
 }
 
-void hdmi_audio_stop(void)
+void hdmi_audio_stop(struct omap_dss_output *out)
 {
 	DSSDBG("audio_stop\n");
 
 	hdmi.ip_data.ops->audio_stop(&hdmi.ip_data);
 }
 
-bool hdmi_mode_has_audio(void)
+bool hdmi_mode_has_audio(struct omap_dss_output *out)
 {
 	if (hdmi.ip_data.cfg.cm.mode = HDMI_HDMI)
 		return true;
@@ -865,7 +865,7 @@ bool hdmi_mode_has_audio(void)
 		return false;
 }
 
-int hdmi_audio_config(struct omap_dss_audio *audio)
+int hdmi_audio_config(struct omap_dss_output *out, struct omap_dss_audio *audio)
 {
 	return hdmi.ip_data.ops->audio_config(&hdmi.ip_data, audio);
 }
diff --git a/drivers/video/omap2/dss/hdmi_panel.c b/drivers/video/omap2/dss/hdmi_panel.c
index fb35bbb..532882d 100644
--- a/drivers/video/omap2/dss/hdmi_panel.c
+++ b/drivers/video/omap2/dss/hdmi_panel.c
@@ -78,21 +78,25 @@ static void hdmi_panel_remove(struct omap_dss_device *dssdev)
 #if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
 static int hdmi_panel_audio_enable(struct omap_dss_device *dssdev)
 {
+	struct omap_dss_output *out = dssdev->output;
 	unsigned long flags;
 	int r;
 
+	if (out = NULL)
+		return -ENODEV;
+
 	mutex_lock(&hdmi.lock);
 	spin_lock_irqsave(&hdmi.audio_lock, flags);
 
 	/* enable audio only if the display is active and supports audio */
 	if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE ||
-	    !hdmi_mode_has_audio()) {
+	    !hdmi_mode_has_audio(out)) {
 		DSSERR("audio not supported or display is off\n");
 		r = -EPERM;
 		goto err;
 	}
 
-	r = hdmi_audio_enable();
+	r = hdmi_audio_enable(out);
 
 	if (!r)
 		dssdev->audio_state = OMAP_DSS_AUDIO_ENABLED;
@@ -105,11 +109,12 @@ err:
 
 static void hdmi_panel_audio_disable(struct omap_dss_device *dssdev)
 {
+	struct omap_dss_output *out = dssdev->output;
 	unsigned long flags;
 
 	spin_lock_irqsave(&hdmi.audio_lock, flags);
 
-	hdmi_audio_disable();
+	hdmi_audio_disable(out);
 
 	dssdev->audio_state = OMAP_DSS_AUDIO_DISABLED;
 
@@ -118,9 +123,13 @@ static void hdmi_panel_audio_disable(struct omap_dss_device *dssdev)
 
 static int hdmi_panel_audio_start(struct omap_dss_device *dssdev)
 {
+	struct omap_dss_output *out = dssdev->output;
 	unsigned long flags;
 	int r;
 
+	if (out = NULL)
+		return -ENODEV;
+
 	spin_lock_irqsave(&hdmi.audio_lock, flags);
 	/*
 	 * No need to check the panel state. It was checked when trasitioning
@@ -132,7 +141,7 @@ static int hdmi_panel_audio_start(struct omap_dss_device *dssdev)
 		goto err;
 	}
 
-	r = hdmi_audio_start();
+	r = hdmi_audio_start(out);
 
 	if (!r)
 		dssdev->audio_state = OMAP_DSS_AUDIO_PLAYING;
@@ -144,11 +153,12 @@ err:
 
 static void hdmi_panel_audio_stop(struct omap_dss_device *dssdev)
 {
+	struct omap_dss_output *out = dssdev->output;
 	unsigned long flags;
 
 	spin_lock_irqsave(&hdmi.audio_lock, flags);
 
-	hdmi_audio_stop();
+	hdmi_audio_stop(out);
 	dssdev->audio_state = OMAP_DSS_AUDIO_ENABLED;
 
 	spin_unlock_irqrestore(&hdmi.audio_lock, flags);
@@ -156,14 +166,18 @@ static void hdmi_panel_audio_stop(struct omap_dss_device *dssdev)
 
 static bool hdmi_panel_audio_supported(struct omap_dss_device *dssdev)
 {
+	struct omap_dss_output *out = dssdev->output;
 	bool r = false;
 
+	if (out = NULL)
+		return r;
+
 	mutex_lock(&hdmi.lock);
 
 	if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
 		goto err;
 
-	if (!hdmi_mode_has_audio())
+	if (!hdmi_mode_has_audio(out))
 		goto err;
 
 	r = true;
@@ -175,21 +189,25 @@ err:
 static int hdmi_panel_audio_config(struct omap_dss_device *dssdev,
 		struct omap_dss_audio *audio)
 {
+	struct omap_dss_output *out = dssdev->output;
 	unsigned long flags;
 	int r;
 
+	if (out = NULL)
+		return -ENODEV;
+
 	mutex_lock(&hdmi.lock);
 	spin_lock_irqsave(&hdmi.audio_lock, flags);
 
 	/* config audio only if the display is active and supports audio */
 	if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE ||
-	    !hdmi_mode_has_audio()) {
+	    !hdmi_mode_has_audio(out)) {
 		DSSERR("audio not supported or display is off\n");
 		r = -EPERM;
 		goto err;
 	}
 
-	r = hdmi_audio_config(audio);
+	r = hdmi_audio_config(out, audio);
 
 	if (!r)
 		dssdev->audio_state = OMAP_DSS_AUDIO_CONFIGURED;
@@ -407,7 +425,7 @@ static int hdmi_read_edid(struct omap_dss_device *dssdev, u8 *buf, int len)
 			goto err;
 	}
 
-	r = omapdss_hdmi_read_edid(buf, len);
+	r = omapdss_hdmi_read_edid(out, buf, len);
 
 	if (dssdev->state = OMAP_DSS_DISPLAY_DISABLED ||
 			dssdev->state = OMAP_DSS_DISPLAY_SUSPENDED)
@@ -434,7 +452,7 @@ static bool hdmi_detect(struct omap_dss_device *dssdev)
 			goto err;
 	}
 
-	r = omapdss_hdmi_detect();
+	r = omapdss_hdmi_detect(out);
 
 	if (dssdev->state = OMAP_DSS_DISPLAY_DISABLED ||
 			dssdev->state = OMAP_DSS_DISPLAY_SUSPENDED)
-- 
1.7.9.5


^ 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