Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH v2 03/23] OMAPDSS: output: Add set/unset device ops for omap_dss_output
From: Tomi Valkeinen @ 2012-08-31 12:28 UTC (permalink / raw)
  To: Archit Taneja; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <5040ACE1.8080502@ti.com>

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

On Fri, 2012-08-31 at 17:54 +0530, Archit Taneja wrote:
> On Friday 31 August 2012 05:33 PM, Tomi Valkeinen wrote:

> > I don't think there's need for this indirection. We should use function
> > pointers only when the func pointer may lead to different functions.
> > Here we'll always have just one function, dss_output_set_device. We can
> > as well call the function directly.
> 
> Okay. I understand that. But in general, don't func pointers prevent us 
> from exporting more symbols?

Yes. But I'm not sure if there's any real downside to exporting, as long
as the names are prefixed properly so that there are no name clashes.

> > I know we have similar func pointers for ovls/mgrs currently, but I
> > don't think they are good either. They are a relic from the time we
> > supported "virtual" overlays and managers, and thus could have different
> > implementations for the operations.
> 
> Oh okay, I guess you mean the L4/sDMA updates for DSI command mode.

Yep.

 Tomi


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

^ permalink raw reply

* Re: [PATCH v2 03/23] OMAPDSS: output: Add set/unset device ops for omap_dss_output
From: Archit Taneja @ 2012-08-31 12:36 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346414621.18766.13.camel@lappyti>

On Friday 31 August 2012 05:33 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
>> An output entity represented by the struct omap_dss_output connects to a
>> omap_dss_device entity. Add functions to set or unset an output's device. This
>> is similar to how managers and devices were connected previously. An output can
>> connect to a device without being connected to a manager. However, the output
>> needs to eventually connect to a manager so that the connected panel can be
>> enabled.
>>
>> Keep the omap_overlay_manager pointer in omap_dss_device for now to prevent
>> breaking things. This will be removed later when outputs are supported
>> completely.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>>   drivers/video/omap2/dss/output.c |   67 ++++++++++++++++++++++++++++++++++++++
>>   include/video/omapdss.h          |    5 +++
>>   2 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/video/omap2/dss/output.c b/drivers/video/omap2/dss/output.c
>> index 7d81be5..abc3aa2 100644
>> --- a/drivers/video/omap2/dss/output.c
>> +++ b/drivers/video/omap2/dss/output.c
>> @@ -24,9 +24,76 @@
>>   #include "dss.h"
>>
>>   static LIST_HEAD(output_list);
>> +static DEFINE_MUTEX(output_lock);
>> +
>> +static int dss_output_set_device(struct omap_dss_output *out,
>> +		struct omap_dss_device *dssdev)
>> +{
>> +	int r;
>> +
>> +	mutex_lock(&output_lock);
>> +
>> +	if (out->device) {
>> +		DSSERR("output already has device %s connected to it\n",
>> +			out->device->name);
>> +		r = -EINVAL;
>> +		goto err;
>> +	}
>> +
>> +	if (out->type != dssdev->type) {
>> +		DSSERR("output type and display type don't match\n");
>> +		r = -EINVAL;
>> +		goto err;
>> +	}
>> +
>> +	out->device = dssdev;
>> +	dssdev->output = out;
>> +
>> +	mutex_unlock(&output_lock);
>> +
>> +	return 0;
>> +err:
>> +	mutex_unlock(&output_lock);
>> +
>> +	return r;
>> +}
>> +
>> +static int dss_output_unset_device(struct omap_dss_output *out)
>> +{
>> +	int r;
>> +
>> +	mutex_lock(&output_lock);
>> +
>> +	if (!out->device) {
>> +		DSSERR("output doesn't have a device connected to it\n");
>> +		r = -EINVAL;
>> +		goto err;
>> +	}
>> +
>> +	if (out->device->state != OMAP_DSS_DISPLAY_DISABLED) {
>> +		DSSERR("device %s is not disabled, cannot unset device\n",
>> +				out->device->name);
>> +		r = -EINVAL;
>> +		goto err;
>> +	}
>> +
>> +	out->device->output = NULL;
>> +	out->device = NULL;
>> +
>> +	mutex_unlock(&output_lock);
>> +
>> +	return 0;
>> +err:
>> +	mutex_unlock(&output_lock);
>> +
>> +	return r;
>> +}
>>
>>   void dss_register_output(struct omap_dss_output *out)
>>   {
>> +	out->set_device = &dss_output_set_device;
>> +	out->unset_device = &dss_output_unset_device;
>> +
>>   	list_add_tail(&out->list, &output_list);
>>   }
>
> I don't think there's need for this indirection. We should use function
> pointers only when the func pointer may lead to different functions.
> Here we'll always have just one function, dss_output_set_device. We can
> as well call the function directly.

Okay. I understand that. But in general, don't func pointers prevent us 
from exporting more symbols?

>
> I know we have similar func pointers for ovls/mgrs currently, but I
> don't think they are good either. They are a relic from the time we
> supported "virtual" overlays and managers, and thus could have different
> implementations for the operations.

Oh okay, I guess you mean the L4/sDMA updates for DSI command mode.

Archit


^ permalink raw reply

* Re: [PATCH v2 06/23] OMAP_VOUT: Remove manager->device references
From: Archit Taneja @ 2012-08-31 12:46 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: rob, linux-omap, linux-fbdev, Vaibhav Hiremath
In-Reply-To: <1346415118.18766.15.camel@lappyti>

On Friday 31 August 2012 05:41 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
>> With the introduction of output entities, managers will now connect to outputs.
>> Use the helper op for managers named get_device. This will abstract away the
>> information on how to get the device from an overlay manager.
>>
>> Using the helper function will reduce the number of pointer dereferences a user
>> of OMAPDSS needs to do and reduce risk of a NULL dereference.
>
> Almost all the uses here seem to be getting the dssdev from an overlay.
> Would it make sense to implement get_device for an ovl? That would
> reduce all the ovl->manager ? ovl->manager->get_device(ovl->manager) :
> NULL; code to ovl->get_device(ovl).

That make sense.

We would still need mgr->get_device() though, as in some places, dss and 
drm try to get the display from the manager.

Archit


^ permalink raw reply

* [PATCH] da8xx-fb: enable LCDC if FB is unblanked
From: Manjunathappa, Prakash @ 2012-08-31 13:12 UTC (permalink / raw)
  To: linux-fbdev

Check for FB_BLANK_UNBLANK before enabling LCDC from PM resume
and DVFS. Without this, behavior in case of below used case is
not be not expected,
1) Issue FB_BLANK, LCDC disabled.
2) Do suspend/resume cycle or DVFS.
3) Do not expect LCDC to get enabled at this point.

Re-enabling of LCDC is done when FB_BLANK_UNBLANK is issued.

Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Applies on top of fbdev-next of Florian Tobias Schandinat's tree.

 drivers/video/da8xx-fb.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 761c8d1..1273354 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -962,7 +962,8 @@ static int lcd_da8xx_cpufreq_transition(struct notifier_block *nb,
 			par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
 			lcd_disable_raster();
 			lcd_calc_clk_divider(par);
-			lcd_enable_raster();
+			if (par->blank = FB_BLANK_UNBLANK)
+				lcd_enable_raster();
 		}
 	}
 
@@ -1486,10 +1487,12 @@ static int fb_resume(struct platform_device *dev)
 
 	console_lock();
 	clk_enable(par->lcdc_clk);
-	lcd_enable_raster();
+	if (par->blank = FB_BLANK_UNBLANK) {
+		lcd_enable_raster();
 
-	if (par->panel_power_ctrl)
-		par->panel_power_ctrl(1);
+		if (par->panel_power_ctrl)
+			par->panel_power_ctrl(1);
+	}
 
 	fb_set_suspend(info, 0);
 	console_unlock();
-- 
1.7.1


^ permalink raw reply related

* [PATCH] video: mbxfb: Include linux/io.h instead of asm/io.h
From: Axel Lin @ 2012-08-31 13:39 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Arnd Bergmann, Mathieu Poirier, Damien Cassou, linux-fbdev,
	linux-kernel

This fixes below build error:

  CC [M]  drivers/video/mbx/mbxfb.o
drivers/video/mbx/mbxfb.c: In function 'mbxfb_probe':
drivers/video/mbx/mbxfb.c:942:2: error: implicit declaration of function 'devm_ioremap_nocache' [-Werror=implicit-function-declaration]
drivers/video/mbx/mbxfb.c:942:22: warning: assignment makes pointer from integer without a cast [enabled by default]
drivers/video/mbx/mbxfb.c:952:21: warning: assignment makes pointer from integer without a cast [enabled by default]
cc1: some warnings being treated as errors

Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
 drivers/video/mbx/mbxfb.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/video/mbx/mbxfb.c b/drivers/video/mbx/mbxfb.c
index 9229acf..6563e50 100644
--- a/drivers/video/mbx/mbxfb.c
+++ b/drivers/video/mbx/mbxfb.c
@@ -26,8 +26,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/uaccess.h>
-
-#include <asm/io.h>
+#include <linux/io.h>
 
 #include <video/mbxfb.h>
 
-- 
1.7.9.5





^ permalink raw reply related

* Re: [PATCH v2 10/23] OMAPDSS: DPI: Pass omap_dss_output within the driver
From: Tomi Valkeinen @ 2012-08-31 13:48 UTC (permalink / raw)
  To: Archit Taneja; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346326845-16583-11-git-send-email-archit@ti.com>

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

On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
> When a panel driver calls a DPI function, it passes the omap_dss_device
> pointer, this pointer currently propagates within the DPI driver to configure
> the interface.
> 
> Extract the omap_dss_output pointer from omap_dss_device received from the panel
> driver, pass the output pointer to DPI functions local to the driver to
> configure the interface, these functions no longer need omap_dss_device since
> the driver now maintains a copy of output parameters.
> 
> Replace dssdev->manager references with out->manager references as only these
> will be valid later.
> 
> With the addition of outputs. There is a possibility that an omap_dss_device
> isn't connected to an output, or a manager isn't connected to an output yet.
> Ensure that the DPI interface functions proceed only if the output is non NULL.

I agree with the direction of this and the similar patches for other
outputs, but I think we should leave these out for now, at least most of
the code here.

So ultimately we'll have the output drivers taking the omap_dss_output
as an argument, and we don't need dssdev anymore. But we can't do that
yet, and now that you mix both approaches I think the end result makes
the code a bit more confusing, and it would be changed again later when
dssdev can be removed.

What I mean is that, for example, display_enable takes dssdev as an
argument. Then you extract output from that, and pass output to some
other functions. Then these functions again extract dssdev from the
output.

This feels like extra churn that doesn't really help anything, and will
be changed later when the functions get output as an argument. So I
propose to keep passing dssdev around until we've removed the
dependencies to dssdev, and we (probably) get the output directly as an
argument to the functions. Then we can clean them up properly at one go.

The dssdev->manager parts still need to be changed to out->manager, of
course, but I think those are minority of the changes here and the
following patches.

 Tomi


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

^ permalink raw reply

* Re: [PATCH v2 10/23] OMAPDSS: DPI: Pass omap_dss_output within the driver
From: Archit Taneja @ 2012-08-31 13:59 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346420930.7508.9.camel@lappyti>

On Friday 31 August 2012 07:18 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
>> When a panel driver calls a DPI function, it passes the omap_dss_device
>> pointer, this pointer currently propagates within the DPI driver to configure
>> the interface.
>>
>> Extract the omap_dss_output pointer from omap_dss_device received from the panel
>> driver, pass the output pointer to DPI functions local to the driver to
>> configure the interface, these functions no longer need omap_dss_device since
>> the driver now maintains a copy of output parameters.
>>
>> Replace dssdev->manager references with out->manager references as only these
>> will be valid later.
>>
>> With the addition of outputs. There is a possibility that an omap_dss_device
>> isn't connected to an output, or a manager isn't connected to an output yet.
>> Ensure that the DPI interface functions proceed only if the output is non NULL.
>
> I agree with the direction of this and the similar patches for other
> outputs, but I think we should leave these out for now, at least most of
> the code here.
>
> So ultimately we'll have the output drivers taking the omap_dss_output
> as an argument, and we don't need dssdev anymore. But we can't do that
> yet, and now that you mix both approaches I think the end result makes
> the code a bit more confusing, and it would be changed again later when
> dssdev can be removed.
>
> What I mean is that, for example, display_enable takes dssdev as an
> argument. Then you extract output from that, and pass output to some
> other functions. Then these functions again extract dssdev from the
> output.
>
> This feels like extra churn that doesn't really help anything, and will
> be changed later when the functions get output as an argument. So I
> propose to keep passing dssdev around until we've removed the
> dependencies to dssdev, and we (probably) get the output directly as an
> argument to the functions. Then we can clean them up properly at one go.
>
> The dssdev->manager parts still need to be changed to out->manager, of
> course, but I think those are minority of the changes here and the
> following patches.

Ok. Yes, I guess if we start from here, we would need to do unnecessary 
changes once we completely switch to outputs.

I'll change these output patches such that, only the dssdev->manager 
references are fixed to dssdev->output->manager.

Archit


^ permalink raw reply

* Re: [PATCH v2 09/23] OMAPDSS: Create links between managers, outputs and devices
From: Tomi Valkeinen @ 2012-08-31 14:10 UTC (permalink / raw)
  To: Archit Taneja; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346326845-16583-10-git-send-email-archit@ti.com>

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

On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
> Links between DSS entities are made in dss_recheck_connections when a new panel
> is probed. Rewrite the code in dss_recheck_connections to link managers to
> outputs, and outputs to devices.
> 
> The fields in omap_dss_device struct gives information on which output and
> manager to connect to. The desired manager and output pointers are retrieved and
> prepared(existing outputs/devices unset, if default display)) to form the
> desired links. The output is linked to the device, and then the manager to the
> output. If a probed device's required manager isn't free, the required output
> is still connected to the device so that it's easier to use the panel in the
> future.

I've been pondering this one, and I can't come to a conclusion. The
recheck_connections is just so ugly that I'd like to get rid of it =). I
guess this patch doesn't make it any more ugly, so we can include it in
the patch series.

And as I mentioned earlier, I think we should get rid of the
OMAP_DISPLAY_TYPE_* enum, as it's kinda extra now. But doing that would
require changing all board files. That's not out of the question, but I
think we should first make sure we know what we are doing with the board
files so we don't go back and forth there.

Just gathering my thoughts:

When we define a panel in DT/board file, we should also define the
output instance, because that's part of the hardware design. But there
are no hardcoded links between mgrs and outputs, so that doesn't belong
to the DT/board file. For example, omap4460's DSI1 can take input from
LCD1 or LCD2.

So who could/should make the decision which mgr to use... Well, I don't
know on what basis the decision can be made, but I still think omapdss
can't make good decisions on that, so probably the whole "chain" should
be configured in the omapfb/omapdrm level.

 Tomi


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

^ permalink raw reply

* RE: [PATCH] da8xx-fb: enable LCDC if FB is unblanked
From: Manjunathappa, Prakash @ 2012-08-31 14:15 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1346418020-31779-1-git-send-email-prakash.pm@ti.com>

Hi,

On Fri, Aug 31, 2012 at 18:30:20, Manjunathappa, Prakash wrote:
> Check for FB_BLANK_UNBLANK before enabling LCDC from PM resume
> and DVFS. Without this, behavior in case of below used case is
> not be not expected,
> 1) Issue FB_BLANK, LCDC disabled.
> 2) Do suspend/resume cycle or DVFS.
> 3) Do not expect LCDC to get enabled at this point.
> 
> Re-enabling of LCDC is done when FB_BLANK_UNBLANK is issued.

I will reword this and send across next version.

> 
> Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> ---
> Applies on top of fbdev-next of Florian Tobias Schandinat's tree.
> 
>  drivers/video/da8xx-fb.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 761c8d1..1273354 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -962,7 +962,8 @@ static int lcd_da8xx_cpufreq_transition(struct notifier_block *nb,
>  			par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
>  			lcd_disable_raster();
>  			lcd_calc_clk_divider(par);
> -			lcd_enable_raster();
> +			if (par->blank = FB_BLANK_UNBLANK)
> +				lcd_enable_raster();
>  		}
>  	}
>  
> @@ -1486,10 +1487,12 @@ static int fb_resume(struct platform_device *dev)
>  
>  	console_lock();
>  	clk_enable(par->lcdc_clk);
> -	lcd_enable_raster();
> +	if (par->blank = FB_BLANK_UNBLANK) {
> +		lcd_enable_raster();
>  
> -	if (par->panel_power_ctrl)
> -		par->panel_power_ctrl(1);
> +		if (par->panel_power_ctrl)
> +			par->panel_power_ctrl(1);
> +	}
>  
>  	fb_set_suspend(info, 0);
>  	console_unlock();
> -- 
> 1.7.1
> 
> 


^ permalink raw reply

* Re: [PATCH v2 15/23] OMAPDSS: RFBI: Add dssdev pointers as arguments to all exported functions
From: Tomi Valkeinen @ 2012-08-31 14:20 UTC (permalink / raw)
  To: Archit Taneja; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346326845-16583-16-git-send-email-archit@ti.com>

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

On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
> All functions of an interface driver used by a panel driver should have an
> omap_dss_device pointer as an argument. This may not be needed by some of the
> interfaces now as driver data is globally visible in 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 dssdev arguments from functions used by panel drivers which currently miss
> it. This will come to use when the RFBI functions retrieve the driver data
> in the correct manner.

This and the similar patch for HDMI could probably also be left out for
now. Again I agree that this is correct direction, but this is not
really needed (right?) for output work or writeback. And we'll
eventually just change these parameters again.

The motivation for this patch was probably to have common format for the
output driver's functions, so that you can use func pointers in an ops
struct?

Let's delay that work until the common panel framework gets a bit more
solid.

Sorry if I'm saying "leave this patch out" for most of the patches =). I
just want to avoid extra churn, going back and forth with the code. The
most important things now are to get the output work in a state that WB
can be used, and on the other hand to remove the dssdev dependencies so
that at some point we can remove dssdev totally.

 Tomi


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

^ permalink raw reply

* Re: [PATCH v2 20/23] OMAPDSS: MANAGER: Update display sysfs store
From: Tomi Valkeinen @ 2012-08-31 14:30 UTC (permalink / raw)
  To: Archit Taneja; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346326845-16583-21-git-send-email-archit@ti.com>

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

On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
> 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;
>  		}
>  

Hmm, I think this is a bit broken. If I read this right, the unlinking
removes both mgr-output link and the output-dssdev link. But the linking
part only sets up the mgr-output link.

So if there's a very simple configuration with one display, if you
unlink it via sysfs you can't link it back again.

The store function gets the mgr and the dssdev as arguments. I think
this could be changed so that the function would "guess" the needed
output. Well, not so much a guess, because a dssdev can only be
connected to one output, defined by the HW design. That is basically
what the recheck_connections does, we could use the same method here.

Then this sysfs file should work just like before.

 Tomi


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

^ permalink raw reply

* [PATCH v2] da8xx-fb: enable LCDC if FB is unblanked
From: Manjunathappa, Prakash @ 2012-08-31 14:30 UTC (permalink / raw)
  To: linux-fbdev

It is expected that LCDC to continue to be disabled after
resume if it is blanked before suspend. This is also true
for DVFS. But it is observed that LCDC being enabled after
suspend/resume cycle or DVFS.

Correcting it by having check for FB_BLANK_UNBLANK before
enabling.

Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Applies on top of fbdev-next of Florian Tobias Schandinat's tree.
Since v1:
Re-written commit message.

 drivers/video/da8xx-fb.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 761c8d1..1273354 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -962,7 +962,8 @@ static int lcd_da8xx_cpufreq_transition(struct notifier_block *nb,
 			par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
 			lcd_disable_raster();
 			lcd_calc_clk_divider(par);
-			lcd_enable_raster();
+			if (par->blank = FB_BLANK_UNBLANK)
+				lcd_enable_raster();
 		}
 	}
 
@@ -1486,10 +1487,12 @@ static int fb_resume(struct platform_device *dev)
 
 	console_lock();
 	clk_enable(par->lcdc_clk);
-	lcd_enable_raster();
+	if (par->blank = FB_BLANK_UNBLANK) {
+		lcd_enable_raster();
 
-	if (par->panel_power_ctrl)
-		par->panel_power_ctrl(1);
+		if (par->panel_power_ctrl)
+			par->panel_power_ctrl(1);
+	}
 
 	fb_set_suspend(info, 0);
 	console_unlock();
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH v2 09/23] OMAPDSS: Create links between managers, outputs and devices
From: Archit Taneja @ 2012-08-31 14:36 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346422242.7508.18.camel@lappyti>

On Friday 31 August 2012 07:40 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
>> Links between DSS entities are made in dss_recheck_connections when a new panel
>> is probed. Rewrite the code in dss_recheck_connections to link managers to
>> outputs, and outputs to devices.
>>
>> The fields in omap_dss_device struct gives information on which output and
>> manager to connect to. The desired manager and output pointers are retrieved and
>> prepared(existing outputs/devices unset, if default display)) to form the
>> desired links. The output is linked to the device, and then the manager to the
>> output. If a probed device's required manager isn't free, the required output
>> is still connected to the device so that it's easier to use the panel in the
>> future.
>
> I've been pondering this one, and I can't come to a conclusion. The
> recheck_connections is just so ugly that I'd like to get rid of it =). I
> guess this patch doesn't make it any more ugly, so we can include it in
> the patch series.
>
> And as I mentioned earlier, I think we should get rid of the
> OMAP_DISPLAY_TYPE_* enum, as it's kinda extra now. But doing that would
> require changing all board files. That's not out of the question, but I
> think we should first make sure we know what we are doing with the board
> files so we don't go back and forth there.

Yes, I didn't remove it for the same reason.

>
> Just gathering my thoughts:
>
> When we define a panel in DT/board file, we should also define the
> output instance, because that's part of the hardware design. But there

It's a part of hardware design if the panel can't be detached. But yes, 
even for detachable panels, we can assume that the panel would 
eventually connect to that output.

> are no hardcoded links between mgrs and outputs, so that doesn't belong
> to the DT/board file. For example, omap4460's DSI1 can take input from
> LCD1 or LCD2.

Right, so we don't need an equivalent of the dssdev->channel field in DT 
info. As you said, we need the output instance, is that why you were 
sceptical about it being defined as an enum in previous patches? 
Probably we could define output instances in DT as a pair of instance 
number and instance type {number, type}. It would be sort of hard to 
play around with those within OMAPDSS though.

>
> So who could/should make the decision which mgr to use... Well, I don't
> know on what basis the decision can be made, but I still think omapdss
> can't make good decisions on that, so probably the whole "chain" should
> be configured in the omapfb/omapdrm level.

Which manager to chose could be simply picking up the next free manager 
which can connect to that output. omapfb/drm can take care of that.

Archit


^ permalink raw reply

* Re: [PATCH v2 15/23] OMAPDSS: RFBI: Add dssdev pointers as arguments to all exported functions
From: Archit Taneja @ 2012-08-31 14:42 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346422820.16067.21.camel@deskari>

On Friday 31 August 2012 07:50 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
>> All functions of an interface driver used by a panel driver should have an
>> omap_dss_device pointer as an argument. This may not be needed by some of the
>> interfaces now as driver data is globally visible in 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 dssdev arguments from functions used by panel drivers which currently miss
>> it. This will come to use when the RFBI functions retrieve the driver data
>> in the correct manner.
>
> This and the similar patch for HDMI could probably also be left out for
> now. Again I agree that this is correct direction, but this is not
> really needed (right?) for output work or writeback. And we'll
> eventually just change these parameters again.
>
> The motivation for this patch was probably to have common format for the
> output driver's functions, so that you can use func pointers in an ops
> struct?

Yes, or the fact that we need the function to pass something related to 
the output to configure it. Things work now since we just have one 
instance of hdmi/rfbi, and that we have a global struct from which we 
can get the required info. We definitely need to pass something to these 
functions, whether we should pass the panel, or the output itself isn't 
clear yet.

>
> Let's delay that work until the common panel framework gets a bit more
> solid.

I get your point. We might need to replace the dssdevs with outputs (or 
something similar) in the future. Hence it would lead to churn.

>
> Sorry if I'm saying "leave this patch out" for most of the patches =). I
> just want to avoid extra churn, going back and forth with the code. The
> most important things now are to get the output work in a state that WB
> can be used, and on the other hand to remove the dssdev dependencies so
> that at some point we can remove dssdev totally.

That's okay. If we keep this stuff, it'll be us who have to change it 
later :)

Archit

^ permalink raw reply

* Re: [PATCH v2 09/23] OMAPDSS: Create links between managers, outputs and devices
From: Tomi Valkeinen @ 2012-08-31 14:45 UTC (permalink / raw)
  To: Archit Taneja; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <5040C907.5090000@ti.com>

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

On Fri, 2012-08-31 at 19:54 +0530, Archit Taneja wrote:
> On Friday 31 August 2012 07:40 PM, Tomi Valkeinen wrote:
> > On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
> >> Links between DSS entities are made in dss_recheck_connections when a new panel
> >> is probed. Rewrite the code in dss_recheck_connections to link managers to
> >> outputs, and outputs to devices.
> >>
> >> The fields in omap_dss_device struct gives information on which output and
> >> manager to connect to. The desired manager and output pointers are retrieved and
> >> prepared(existing outputs/devices unset, if default display)) to form the
> >> desired links. The output is linked to the device, and then the manager to the
> >> output. If a probed device's required manager isn't free, the required output
> >> is still connected to the device so that it's easier to use the panel in the
> >> future.
> >
> > I've been pondering this one, and I can't come to a conclusion. The
> > recheck_connections is just so ugly that I'd like to get rid of it =). I
> > guess this patch doesn't make it any more ugly, so we can include it in
> > the patch series.
> >
> > And as I mentioned earlier, I think we should get rid of the
> > OMAP_DISPLAY_TYPE_* enum, as it's kinda extra now. But doing that would
> > require changing all board files. That's not out of the question, but I
> > think we should first make sure we know what we are doing with the board
> > files so we don't go back and forth there.
> 
> Yes, I didn't remove it for the same reason.
> 
> >
> > Just gathering my thoughts:
> >
> > When we define a panel in DT/board file, we should also define the
> > output instance, because that's part of the hardware design. But there
> 
> It's a part of hardware design if the panel can't be detached. But yes, 
> even for detachable panels, we can assume that the panel would 
> eventually connect to that output.
> 
> > are no hardcoded links between mgrs and outputs, so that doesn't belong
> > to the DT/board file. For example, omap4460's DSI1 can take input from
> > LCD1 or LCD2.
> 
> Right, so we don't need an equivalent of the dssdev->channel field in DT 
> info. As you said, we need the output instance, is that why you were 

What I'm planning for DT is a direct link to the output instance.
Something like this (not correct, just to give the idea):

dss {
	dpi {
	};
};

i2c {
	/* an i2c controlled panel */
	my-panel {
		video-bus = <&dpi>;
	};
};

Then my-panel can get the handle from video-bus, and it'll get the
output device directly, without need for any IDs or such.

> sceptical about it being defined as an enum in previous patches? 
> Probably we could define output instances in DT as a pair of instance 
> number and instance type {number, type}. It would be sort of hard to 
> play around with those within OMAPDSS though.

Well, optimally we wouldn't need to know about display types or output
instances, at least in most of the cases. We'd just have a bunch of
managers, and a bunch of outputs, and rules how these can be connected.
And the panel devices would have a link to the output it's connect in HW
level.

There are probably some cases where we need some kind of ID, so that we
can handle the DSI PLL's and such. And we need to setup the rules above
somewhere, and perhaps that code needs IDs to set it up. I'm not sure.

And, well, if we don't have IDs, the rules above need to be some kind of
lists of pointers. Perhaps having a bit mask is just simpler, as we'll
never have too many output instances.

So I'm not really against having the enum. It just would've been neat to
have the output type and instance number encoded into this enum, so that
it'd be easy to extract either one. But that kinda ruins the possibility
to use it in a bit mask.

> > So who could/should make the decision which mgr to use... Well, I don't
> > know on what basis the decision can be made, but I still think omapdss
> > can't make good decisions on that, so probably the whole "chain" should
> > be configured in the omapfb/omapdrm level.
> 
> Which manager to chose could be simply picking up the next free manager 
> which can connect to that output. omapfb/drm can take care of that.

That's a good default implementation, but not quite perfect. If there
are two displays, often the other display (well, output) can only be
connected to one particular manager, whereas the other one could use two
managers. And if both try to use the same manager, especially if the one
with two options is selected first, the other one is left without a mgr.

 Tomi


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

^ permalink raw reply

* Re: [PATCH v2 20/23] OMAPDSS: MANAGER: Update display sysfs store
From: Archit Taneja @ 2012-08-31 14:53 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346423449.16067.27.camel@deskari>

On Friday 31 August 2012 08:00 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
>> 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;
>>   		}
>>
>
> Hmm, I think this is a bit broken. If I read this right, the unlinking
> removes both mgr-output link and the output-dssdev link. But the linking
> part only sets up the mgr-output link.
>
> So if there's a very simple configuration with one display, if you
> unlink it via sysfs you can't link it back again.

Ah, right. You might need to reinsert the display module again to get 
the output-display link again. Which is bad. Or have a sysfs file for 
setting manager output. Which is something we want to stay away from anyway.

>
> The store function gets the mgr and the dssdev as arguments. I think
> this could be changed so that the function would "guess" the needed
> output. Well, not so much a guess, because a dssdev can only be
> connected to one output, defined by the HW design. That is basically
> what the recheck_connections does, we could use the same method here.
>
> Then this sysfs file should work just like before.

That's a good idea, we could reuse the code in recheck_connections a 
bit. I wanted to stay away from the guessing. But I think we need to do 
it if a simple unlink-link of a display itself fails.

Archit


^ permalink raw reply

* Re: [PATCH v2 09/23] OMAPDSS: Create links between managers, outputs and devices
From: Tomi Valkeinen @ 2012-08-31 15:08 UTC (permalink / raw)
  To: Archit Taneja; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346424344.16067.39.camel@deskari>

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

On Fri, 2012-08-31 at 17:45 +0300, Tomi Valkeinen wrote:

> So I'm not really against having the enum. It just would've been neat to
> have the output type and instance number encoded into this enum, so that
> it'd be easy to extract either one. But that kinda ruins the possibility
> to use it in a bit mask.

Hmm, this is just a non-finished idea (and it's late Friday... =) that
came to my mind:

I had a brief look at TRMs for different OMAPs (omap4/5/6), and it looks
like the maximum output options for one manager is 4 (LCD2 on omap4460).
Usually there are just 2 or 3.

So, we could trust that the above holds and do this so that we use a u32
field to hold four 8 bit output options. In 8 bits we can combine two
values from 0 to 15. 15 should be more than enough for output display
types and output instances.

Then with helpers like these:

/* second DSI instance */
#define DSI1 ((1 << 4) | DISPLAY_TYPE_DSI)
/* first DPI instance */
#define DPI0 ((0 << 4) | DISPLAY_TYPE_DPI)

an example output options for a manager that can be connected to DSI1
and DPI0 could be:

(DSI1 << 8) | (DPI0)

This could be parsed with a for loop, going though the 4 bytes of the
u32 value. This would allow us to avoid managing a real list, and we
could extract the instance and the type from the value. Of course we
could also extend the field to u64 to hold 8 outputs if need be.

Whether this would be worth it compared to simple bitmask, I'm not sure
=). Depends how much we need to extract the ID and type.

 Tomi


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

^ permalink raw reply

* Re: [PATCHv2 1/4] video: mmp display subsystem
From: Laurent Pinchart @ 2012-09-01  6:25 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1346293222-13692-1-git-send-email-zzhu3@marvell.com>

Hi Zhou,

On Thursday 30 August 2012 10:20:22 Zhou Zhu wrote:
> Added mmp display subsystem to support Marvell MMP display controllers.
> 
> This subsystem contains 4 parts:
> --fb folder
> --core.c
> --hw folder
> --panel folder
> 
> 1. fb folder contains implementation of fb.
> fb get path and ovly from common interface and operates on these structures.
> 
> 2. core.c provides common interface for a hardware abstraction.
> Major parts of this interface are:
> a) Path: path is a output device connected to a panel or HDMI TV.
> Main operations of the path is set/get timing/output color.
> fb operates output device through path structure.
> b) Ovly: Ovly is a buffer shown on the path.
> Ovly describes frame buffer and its source/destination size, offset, input
> color, buffer address, z-order, and so on.
> Each fb device maps to one ovly.
> 
> 3. hw folder contains implementation of hardware operations defined by
> core.c. It registers paths for fb use.
> 
> 4. panel folder contains implementation of panels.
> It's connected to path. Panel drivers would also regiester panels and linked
> to path when probe.

You might want to look at the generic panel framework RFC that I have posted 
recently (http://lwn.net/Articles/512363/).

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH 3/5] drivers/video/jz4740_fb.c: use devm_ functions
From: Lars-Peter Clausen @ 2012-09-02 15:19 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Damien Cassou, kernel-janitors, linux-fbdev, linux-kernel
In-Reply-To: <5036955F.6000806@gmx.de>

On 08/23/2012 10:41 PM, Florian Tobias Schandinat wrote:
> On 08/03/2012 03:40 PM, Damien Cassou wrote:
>> From: Damien Cassou <damien.cassou@lifl.fr>
>>
>> The various devm_ functions allocate memory that is released when a driver
>> detaches. This patch uses these functions for data that is allocated in the
>> probe function of a platform device and is only freed in the remove function.
>>
>> Signed-off-by: Damien Cassou <damien.cassou@lifl.fr>
> 
> Applied.
> 
> 
> Thanks,
> 
> Florian Tobias Schandinat
> 
>>
>> ---
>>  drivers/video/jz4740_fb.c |   22 ++++++----------------
>>  1 file changed, 6 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
>> index de36693..7669770 100644
>> --- a/drivers/video/jz4740_fb.c
>> +++ b/drivers/video/jz4740_fb.c
>> @@ -659,25 +659,25 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
>>  	jzfb->pdata = pdata;
>>  	jzfb->mem = mem;
>>
>> -	jzfb->ldclk = clk_get(&pdev->dev, "lcd");
>> +	jzfb->ldclk = devm_clk_get(&pdev->dev, "lcd");

I guess I'm a bit late, but we do not have devm_clk_get on jz4740 (yet), so
this patch breaks linking for this driver in next. I'll to to have this added
for the next release, but if I do not succeed we'll have to revert part of this
patch.

Also the driver does not include #include <linux/io.h>, since it is required
for devm_ioremap compilation is also broken. This one is easy to fix though,
will send a follow-up patch.

- Lars

^ permalink raw reply

* [RFC PATCH] OMAPDSS: DISPC: Fix IRQ unregister race
From: Dimitar Dimitrov @ 2012-09-02 19:12 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Dimitar Dimitrov

Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
panics due to corrupted completion structure while executing
dispc_irq_wait_handler(). Excerpt from kernel log:

  Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
  Unable to handle kernel paging request at virtual address 00400130
  ...
  PC is at 0xebf205bc
  LR is at __wake_up_common+0x54/0x94
  ...
  (__wake_up_common+0x0/0x94)
  (complete+0x0/0x60)
  (dispc_irq_wait_handler.36902+0x0/0x14)
  (omap_dispc_irq_handler+0x0/0x354)
  (handle_irq_event_percpu+0x0/0x188)
  (handle_irq_event+0x0/0x64)
  (handle_fasteoi_irq+0x0/0x10c)
  (generic_handle_irq+0x0/0x48)
  (asm_do_IRQ+0x0/0xc0)

DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
unregister_isr() and DISPC IRQ might be running in parallel on different
CPUs. So there is a chance that a callback is executed even though it
has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
completion on stack, the dispc_irq_wait_handler() callback might try to
access a completion structure that is invalid. This leads to crashes and
hangs.

Solution is to divide unregister calls into two sets:
  1. Non-strict unregistering of callbacks. Callbacks could safely be
     executed after unregistering them. This is the case with unregister
     calls from the IRQ handler itself.
  2. Strict (synchronized) unregistering. Callbacks are not allowed
     after unregistering. This is the case with completion waiting.

The above solution should satisfy one of the original intentions of the
driver: callbacks should be able to unregister themselves.

Signed-off-by: Dimitar Dimitrov <dinuxbg@gmail.com>
---

WARNING: This bug is quite old. The patch has been tested on v3.0. No testing
has been done after rebasing to v3.6. Hence the RFC tag. Hopefully someone
will beat me in testing with latest linux-omap/master.

 drivers/media/video/omap/omap_vout.c |    4 +--
 drivers/staging/omapdrm/omap_plane.c |    2 +-
 drivers/video/omap2/dss/apply.c      |    2 +-
 drivers/video/omap2/dss/dispc.c      |   47 +++++++++++++++++++++++++++++-----
 drivers/video/omap2/dss/dsi.c        |    4 +--
 drivers/video/omap2/dss/rfbi.c       |    2 +-
 include/video/omapdss.h              |    3 ++-
 7 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/drivers/media/video/omap/omap_vout.c b/drivers/media/video/omap/omap_vout.c
index 88cf9d9..d06896e 100644
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -976,7 +976,7 @@ static int omap_vout_release(struct file *file)
 
 		mask = DISPC_IRQ_VSYNC | DISPC_IRQ_EVSYNC_EVEN |
 			DISPC_IRQ_EVSYNC_ODD | DISPC_IRQ_VSYNC2;
-		omap_dispc_unregister_isr(omap_vout_isr, vout, mask);
+		omap_dispc_unregister_isr_sync(omap_vout_isr, vout, mask);
 		vout->streaming = 0;
 
 		videobuf_streamoff(q);
@@ -1722,7 +1722,7 @@ static int vidioc_streamoff(struct file *file, void *fh, enum v4l2_buf_type i)
 	mask = DISPC_IRQ_VSYNC | DISPC_IRQ_EVSYNC_EVEN | DISPC_IRQ_EVSYNC_ODD
 		| DISPC_IRQ_VSYNC2;
 
-	omap_dispc_unregister_isr(omap_vout_isr, vout, mask);
+	omap_dispc_unregister_isr_sync(omap_vout_isr, vout, mask);
 
 	for (j = 0; j < ovid->num_overlays; j++) {
 		struct omap_overlay *ovl = ovid->overlays[j];
diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c
index 7997be7..8d8aa5b 100644
--- a/drivers/staging/omapdrm/omap_plane.c
+++ b/drivers/staging/omapdrm/omap_plane.c
@@ -82,7 +82,7 @@ static void dispc_isr(void *arg, uint32_t mask)
 	struct omap_plane *omap_plane = to_omap_plane(plane);
 	struct omap_drm_private *priv = plane->dev->dev_private;
 
-	omap_dispc_unregister_isr(dispc_isr, plane,
+	omap_dispc_unregister_isr_nosync(dispc_isr, plane,
 			id2irq[omap_plane->ovl->id]);
 
 	queue_work(priv->wq, &omap_plane->work);
diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 0fefc68..9386834 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -847,7 +847,7 @@ static void dss_unregister_vsync_isr(void)
 	for (i = 0; i < num_mgrs; ++i)
 		mask |= dispc_mgr_get_framedone_irq(i);
 
-	r = omap_dispc_unregister_isr(dss_apply_irq_handler, NULL, mask);
+	r = omap_dispc_unregister_isr_nosync(dss_apply_irq_handler, NULL, mask);
 	WARN_ON(r);
 
 	dss_data.irq_enabled = false;
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 5b289c5..23d5881 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2351,7 +2351,7 @@ static void dispc_mgr_enable_lcd_out(enum omap_channel channel, bool enable)
 					msecs_to_jiffies(100)))
 			DSSERR("timeout waiting for FRAME DONE\n");
 
-		r = omap_dispc_unregister_isr(dispc_disable_isr,
+		r = omap_dispc_unregister_isr_sync(dispc_disable_isr,
 				&frame_done_completion, irq);
 
 		if (r)
@@ -2420,8 +2420,8 @@ static void dispc_mgr_enable_digit_out(bool enable)
 					enable ? "start" : "stop");
 	}
 
-	r = omap_dispc_unregister_isr(dispc_disable_isr, &frame_done_completion,
-			irq_mask);
+	r = omap_dispc_unregister_isr_sync(dispc_disable_isr,
+			&frame_done_completion, irq_mask);
 	if (r)
 		DSSERR("failed to unregister %x isr\n", irq_mask);
 
@@ -3319,7 +3319,8 @@ err:
 }
 EXPORT_SYMBOL(omap_dispc_register_isr);
 
-int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
+/* WARNING: callback might be executed even after this function returns! */
+int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask)
 {
 	int i;
 	unsigned long flags;
@@ -3351,7 +3352,37 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
 
 	return ret;
 }
-EXPORT_SYMBOL(omap_dispc_unregister_isr);
+EXPORT_SYMBOL(omap_dispc_unregister_isr_nosync);
+
+/*
+ * Ensure that callback <isr> will NOT be executed after this function
+ * returns. Must be called from sleepable context, though!
+ */
+int omap_dispc_unregister_isr_sync(omap_dispc_isr_t isr, void *arg, u32 mask)
+{
+	int ret;
+
+	ret = omap_dispc_unregister_isr_nosync(isr, arg, mask);
+
+	/* Task context is not really needed. But if we're called from atomic
+	 * context, it is probably from DISPC IRQ, where we will deadlock.
+	 * So use might_sleep() to catch potential deadlocks.
+	 */
+	might_sleep();
+
+#if defined(CONFIG_SMP)
+	/* DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
+	 * unregister_isr() and DISPC IRQ might be running in parallel on
+	 * different CPUs. So there is a chance that a callback is executed
+	 * even though it has been unregistered. Add a barrier, in order to
+	 * ensure that after returning from this function, the new DISPC IRQ
+	 * will use an updated callback array, and NOT its cached copy.
+	 */
+	synchronize_irq(dispc.irq);
+#endif
+
+	return ret;
+}
 
 #ifdef DEBUG
 static void print_irq_status(u32 status)
@@ -3566,7 +3597,8 @@ int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout)
 
 	timeout = wait_for_completion_timeout(&completion, timeout);
 
-	omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
+	omap_dispc_unregister_isr_sync(dispc_irq_wait_handler, &completion,
+			irqmask);
 
 	if (timeout = 0)
 		return -ETIMEDOUT;
@@ -3597,7 +3629,8 @@ int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
 	timeout = wait_for_completion_interruptible_timeout(&completion,
 			timeout);
 
-	omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
+	omap_dispc_unregister_isr_sync(dispc_irq_wait_handler, &completion,
+			irqmask);
 
 	if (timeout = 0)
 		return -ETIMEDOUT;
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index b07e886..46f6c6c 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -4430,7 +4430,7 @@ static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
 	return 0;
 err1:
 	if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_CMD_MODE)
-		omap_dispc_unregister_isr(dsi_framedone_irq_callback,
+		omap_dispc_unregister_isr_sync(dsi_framedone_irq_callback,
 			(void *) dssdev, irq);
 err:
 	return r;
@@ -4443,7 +4443,7 @@ static void dsi_display_uninit_dispc(struct omap_dss_device *dssdev)
 
 		irq = dispc_mgr_get_framedone_irq(dssdev->manager->id);
 
-		omap_dispc_unregister_isr(dsi_framedone_irq_callback,
+		omap_dispc_unregister_isr_sync(dsi_framedone_irq_callback,
 			(void *) dssdev, irq);
 	}
 }
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index 7c08742..31d167c 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -930,7 +930,7 @@ EXPORT_SYMBOL(omapdss_rfbi_display_enable);
 
 void omapdss_rfbi_display_disable(struct omap_dss_device *dssdev)
 {
-	omap_dispc_unregister_isr(framedone_callback, NULL,
+	omap_dispc_unregister_isr_sync(framedone_callback, NULL,
 			DISPC_IRQ_FRAMEDONE);
 	omap_dss_stop_device(dssdev);
 
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index a6267a2..2287368 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -707,7 +707,8 @@ void omapdss_default_get_timings(struct omap_dss_device *dssdev,
 
 typedef void (*omap_dispc_isr_t) (void *arg, u32 mask);
 int omap_dispc_register_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
-int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
+int omap_dispc_unregister_isr_sync(omap_dispc_isr_t isr, void *arg, u32 mask);
+int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask);
 
 int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout);
 int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
-- 
1.7.10.4


^ permalink raw reply related

* Re: [PATCH] video: s3c2410fb: Use pr_* and dev_* instead of printk
From: Jingoo Han @ 2012-09-03  0:54 UTC (permalink / raw)
  To: linux-fbdev

On Friday, August 31, 2012 8:50 PM Sachin Kamat wrote:
> 
> printk calls are replaced by pr_* and dev_* calls to silence
> checkpatch warnings.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/video/s3c2410fb.c |   18 ++++++++++--------
>  1 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/video/s3c2410fb.c b/drivers/video/s3c2410fb.c
> index 77f34c6..a54e3f9 100644
> --- a/drivers/video/s3c2410fb.c
> +++ b/drivers/video/s3c2410fb.c
> @@ -11,6 +11,8 @@
>   * Driver based on skeletonfb.c, sa1100fb.c and others.
>  */
> 
> +#define pr_fmt(fmt) "s3c2410fb: " fmt
> +

Hi Sachin Kamat,

How about using KBUILD_MODNAME instead of "s3c2410fb: "?
It is because prefix is the same with module name.

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt


Best regards,
Jingoo Han

>  #include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/err.h>
> @@ -48,7 +50,7 @@ static int debug	= 1;
>  static int debug;
>  #endif
> 
> -#define dprintk(msg...)	if (debug) printk(KERN_DEBUG "s3c2410fb: " msg);
> +#define dprintk(msg...)	if (debug) pr_debug(msg);
> 
>  /* useful functions */
> 
> @@ -598,11 +600,11 @@ static int s3c2410fb_debug_store(struct device *dev,
>  	if (strnicmp(buf, "on", 2) = 0 ||
>  	    strnicmp(buf, "1", 1) = 0) {
>  		debug = 1;
> -		printk(KERN_DEBUG "s3c2410fb: Debug On");
> +		dev_dbg(dev, "s3c2410fb: Debug On");
>  	} else if (strnicmp(buf, "off", 3) = 0 ||
>  		   strnicmp(buf, "0", 1) = 0) {
>  		debug = 0;
> -		printk(KERN_DEBUG "s3c2410fb: Debug Off");
> +		dev_dbg(dev, "s3c2410fb: Debug Off");
>  	} else {
>  		return -EINVAL;
>  	}
> @@ -921,7 +923,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
> 
>  	info->clk = clk_get(NULL, "lcd");
>  	if (IS_ERR(info->clk)) {
> -		printk(KERN_ERR "failed to get lcd clock source\n");
> +		dev_err(&pdev->dev, "failed to get lcd clock source\n");
>  		ret = PTR_ERR(info->clk);
>  		goto release_irq;
>  	}
> @@ -947,7 +949,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
>  	/* Initialize video memory */
>  	ret = s3c2410fb_map_video_memory(fbinfo);
>  	if (ret) {
> -		printk(KERN_ERR "Failed to allocate video RAM: %d\n", ret);
> +		dev_err(&pdev->dev, "Failed to allocate video RAM: %d\n", ret);
>  		ret = -ENOMEM;
>  		goto release_clock;
>  	}
> @@ -970,7 +972,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
> 
>  	ret = register_framebuffer(fbinfo);
>  	if (ret < 0) {
> -		printk(KERN_ERR "Failed to register framebuffer device: %d\n",
> +		dev_err(&pdev->dev, "Failed to register framebuffer device: %d\n",
>  			ret);
>  		goto free_cpufreq;
>  	}
> @@ -978,9 +980,9 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
>  	/* create device files */
>  	ret = device_create_file(&pdev->dev, &dev_attr_debug);
>  	if (ret)
> -		printk(KERN_ERR "failed to add debug attribute\n");
> +		dev_err(&pdev->dev, "failed to add debug attribute\n");
> 
> -	printk(KERN_INFO "fb%d: %s frame buffer device\n",
> +	dev_info(&pdev->dev, "fb%d: %s frame buffer device\n",
>  		fbinfo->node, fbinfo->fix.id);
> 
>  	return 0;
> --
> 1.7.4.1
> 
> --
> 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

* [PATCH] video: exynos_dp: replace link_status with link_align to check channel equalization
From: Jingoo Han @ 2012-09-03  8:50 UTC (permalink / raw)
  To: linux-fbdev

To check channel equalization, the value of LANE_ALIGN_STATUS_UPDATED is
necessary in exynos_dp_channel_eq_ok(). Also, link_align includes this value.
However, link_status does not include this value, so it makes the problem
that channel equalization is failed during link training.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index f57c915..cdc1398 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -587,7 +587,7 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
 			dp->link_train.training_lane[lane] = training_lane;
 		}
 
-		if (exynos_dp_channel_eq_ok(link_status, lane_count) = 0) {
+		if (exynos_dp_channel_eq_ok(link_align, lane_count) = 0) {
 			/* traing pattern Set to Normal */
 			exynos_dp_training_pattern_dis(dp);
 
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH v2 09/23] OMAPDSS: Create links between managers, outputs and devices
From: Tomi Valkeinen @ 2012-09-03  9:35 UTC (permalink / raw)
  To: Archit Taneja; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <504477CD.10901@ti.com>

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

On Mon, 2012-09-03 at 14:56 +0530, Archit Taneja wrote:
> On Friday 31 August 2012 08:38 PM, Tomi Valkeinen wrote:
> > On Fri, 2012-08-31 at 17:45 +0300, Tomi Valkeinen wrote:
> >
> >> So I'm not really against having the enum. It just would've been neat to
> >> have the output type and instance number encoded into this enum, so that
> >> it'd be easy to extract either one. But that kinda ruins the possibility
> >> to use it in a bit mask.
> >
> > Hmm, this is just a non-finished idea (and it's late Friday... =) that
> > came to my mind:
> >
> > I had a brief look at TRMs for different OMAPs (omap4/5/6), and it looks
> > like the maximum output options for one manager is 4 (LCD2 on omap4460).
> > Usually there are just 2 or 3.
> >
> > So, we could trust that the above holds and do this so that we use a u32
> > field to hold four 8 bit output options. In 8 bits we can combine two
> > values from 0 to 15. 15 should be more than enough for output display
> > types and output instances.
> >
> > Then with helpers like these:
> >
> > /* second DSI instance */
> > #define DSI1 ((1 << 4) | DISPLAY_TYPE_DSI)
> > /* first DPI instance */
> > #define DPI0 ((0 << 4) | DISPLAY_TYPE_DPI)
> 
> Okay, so DISPLAY_TYPE_DSI / DPI can't be a number left shifted any 
> more(that would limit us to have only 4 types of output display types), 
> like we have right now, they would need to be 0, 1, 2, 3 and so on. So 
> with this approach, we could extract the instance number quickly, but we 
> may not be able to check if the manager supports the output display type 
> in constant time.

True, it wouldn't be constant. But it'd be checking between one to four
bytes, so I think it's quite fast =).

> > an example output options for a manager that can be connected to DSI1
> > and DPI0 could be:
> >
> > (DSI1 << 8) | (DPI0)
> >
> > This could be parsed with a for loop, going though the 4 bytes of the
> > u32 value. This would allow us to avoid managing a real list, and we
> > could extract the instance and the type from the value. Of course we
> > could also extend the field to u64 to hold 8 outputs if need be.
> 
> Probably we could have a u64, and 2 bytes for each output, and a bit 
> mask for both output instance and output display type. We could have 
> display type enums as:
> 
> DISPLAY_TYPE_DPI	1 << 5
> DISPLAY_TYPE_DSI	1 << 6
> DISPLAY_TYPE_VENC	1 << 7
> DISPLAY_TYPE_HDMI	1 << 8
> DISPLAY_TYPE_SDI	1 << 9
> DISPLAY_TYPE_RFBI	1 << 10
> ...
> 
> >
> > Whether this would be worth it compared to simple bitmask, I'm not sure
> > =). Depends how much we need to extract the ID and type.
> 
> Anyway, I don't think I'll try to implement this in this series. I'll 
> stick to the output instance enums for now.

Sure. And as I said, I'm not sure if it's worth it. We currently have
multiple instances only for DSI, and only two instances there. And I
think we need to handle the instance number only in a few places. It's
not too much to do:

if (type == DSI1) ... else if (type == DSI2) ...

 Tomi


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

^ permalink raw reply

* Re: [PATCH v2 09/23] OMAPDSS: Create links between managers, outputs and devices
From: Archit Taneja @ 2012-09-03  9:38 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: rob, linux-omap, linux-fbdev
In-Reply-To: <1346425698.16067.52.camel@deskari>

On Friday 31 August 2012 08:38 PM, Tomi Valkeinen wrote:
> On Fri, 2012-08-31 at 17:45 +0300, Tomi Valkeinen wrote:
>
>> So I'm not really against having the enum. It just would've been neat to
>> have the output type and instance number encoded into this enum, so that
>> it'd be easy to extract either one. But that kinda ruins the possibility
>> to use it in a bit mask.
>
> Hmm, this is just a non-finished idea (and it's late Friday... =) that
> came to my mind:
>
> I had a brief look at TRMs for different OMAPs (omap4/5/6), and it looks
> like the maximum output options for one manager is 4 (LCD2 on omap4460).
> Usually there are just 2 or 3.
>
> So, we could trust that the above holds and do this so that we use a u32
> field to hold four 8 bit output options. In 8 bits we can combine two
> values from 0 to 15. 15 should be more than enough for output display
> types and output instances.
>
> Then with helpers like these:
>
> /* second DSI instance */
> #define DSI1 ((1 << 4) | DISPLAY_TYPE_DSI)
> /* first DPI instance */
> #define DPI0 ((0 << 4) | DISPLAY_TYPE_DPI)

Okay, so DISPLAY_TYPE_DSI / DPI can't be a number left shifted any 
more(that would limit us to have only 4 types of output display types), 
like we have right now, they would need to be 0, 1, 2, 3 and so on. So 
with this approach, we could extract the instance number quickly, but we 
may not be able to check if the manager supports the output display type 
in constant time.

>
> an example output options for a manager that can be connected to DSI1
> and DPI0 could be:
>
> (DSI1 << 8) | (DPI0)
>
> This could be parsed with a for loop, going though the 4 bytes of the
> u32 value. This would allow us to avoid managing a real list, and we
> could extract the instance and the type from the value. Of course we
> could also extend the field to u64 to hold 8 outputs if need be.

Probably we could have a u64, and 2 bytes for each output, and a bit 
mask for both output instance and output display type. We could have 
display type enums as:

DISPLAY_TYPE_DPI	1 << 5
DISPLAY_TYPE_DSI	1 << 6
DISPLAY_TYPE_VENC	1 << 7
DISPLAY_TYPE_HDMI	1 << 8
DISPLAY_TYPE_SDI	1 << 9
DISPLAY_TYPE_RFBI	1 << 10
...

>
> Whether this would be worth it compared to simple bitmask, I'm not sure
> =). Depends how much we need to extract the ID and type.

Anyway, I don't think I'll try to implement this in this series. I'll 
stick to the output instance enums for now.

Archit


^ permalink raw reply

* [PATCH] fbdev: move EXPORT_SYMBOL statements to the functions
From: Jingoo Han @ 2012-09-04  1:35 UTC (permalink / raw)
  To: linux-fbdev

This patch moves the EXPORT_SYMBOL statements to the functions
they belong to.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/fbcmap.c |   17 +++++------------
 drivers/video/fbcvt.c  |    1 +
 drivers/video/fbmem.c  |   26 ++++++++++++++------------
 drivers/video/fbmon.c  |   22 +++++++++++++++-------
 drivers/video/modedb.c |   20 +++++++++-----------
 5 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c
index 5c3960d..8b25bcd 100644
--- a/drivers/video/fbcmap.c
+++ b/drivers/video/fbcmap.c
@@ -132,6 +132,7 @@ int fb_alloc_cmap(struct fb_cmap *cmap, int len, int transp)
 {
 	return fb_alloc_cmap_gfp(cmap, len, transp, GFP_ATOMIC);
 }
+EXPORT_SYMBOL(fb_alloc_cmap);
 
 /**
  *      fb_dealloc_cmap - deallocate a colormap
@@ -152,6 +153,7 @@ void fb_dealloc_cmap(struct fb_cmap *cmap)
 	cmap->red = cmap->green = cmap->blue = cmap->transp = NULL;
 	cmap->len = 0;
 }
+EXPORT_SYMBOL(fb_dealloc_cmap);
 
 /**
  *	fb_copy_cmap - copy a colormap
@@ -184,6 +186,7 @@ int fb_copy_cmap(const struct fb_cmap *from, struct fb_cmap *to)
 		memcpy(to->transp+tooff, from->transp+fromoff, size);
 	return 0;
 }
+EXPORT_SYMBOL(fb_copy_cmap);
 
 int fb_cmap_to_user(const struct fb_cmap *from, struct fb_cmap_user *to)
 {
@@ -259,6 +262,7 @@ int fb_set_cmap(struct fb_cmap *cmap, struct fb_info *info)
 
 	return rc;
 }
+EXPORT_SYMBOL(fb_set_cmap);
 
 int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info)
 {
@@ -319,7 +323,7 @@ const struct fb_cmap *fb_default_cmap(int len)
 	return &default_8_colors;
     return &default_16_colors;
 }
-
+EXPORT_SYMBOL(fb_default_cmap);
 
 /**
  *	fb_invert_cmaps - invert all defaults colormaps
@@ -353,15 +357,4 @@ void fb_invert_cmaps(void)
 	blue16[i] = ~blue16[i];
     }
 }
-
-
-    /*
-     *  Visible symbols for modules
-     */
-
-EXPORT_SYMBOL(fb_alloc_cmap);
-EXPORT_SYMBOL(fb_dealloc_cmap);
-EXPORT_SYMBOL(fb_copy_cmap);
-EXPORT_SYMBOL(fb_set_cmap);
-EXPORT_SYMBOL(fb_default_cmap);
 EXPORT_SYMBOL(fb_invert_cmaps);
diff --git a/drivers/video/fbcvt.c b/drivers/video/fbcvt.c
index 7cb715d..319b5c4 100644
--- a/drivers/video/fbcvt.c
+++ b/drivers/video/fbcvt.c
@@ -377,3 +377,4 @@ int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb)
 
 	return 0;
 }
+EXPORT_SYMBOL(fb_find_mode_cvt);
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 3ff0105..e11ac80 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -43,8 +43,12 @@
 #define FBPIXMAPSIZE	(1024 * 8)
 
 static DEFINE_MUTEX(registration_lock);
+
 struct fb_info *registered_fb[FB_MAX] __read_mostly;
+EXPORT_SYMBOL(registered_fb);
+
 int num_registered_fb __read_mostly;
+EXPORT_SYMBOL(num_registered_fb);
 
 static struct fb_info *get_fb_info(unsigned int idx)
 {
@@ -182,6 +186,7 @@ char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size
 
 	return addr;
 }
+EXPORT_SYMBOL(fb_get_buffer_offset);
 
 #ifdef CONFIG_LOGO
 
@@ -665,9 +670,11 @@ int fb_show_logo(struct fb_info *info, int rotate)
 
 	return y;
 }
+EXPORT_SYMBOL(fb_show_logo);
 #else
 int fb_prepare_logo(struct fb_info *info, int rotate) { return 0; }
 int fb_show_logo(struct fb_info *info, int rotate) { return 0; }
+EXPORT_SYMBOL(fb_show_logo);
 #endif /* CONFIG_LOGO */
 
 static void *fb_seq_start(struct seq_file *m, loff_t *pos)
@@ -909,6 +916,7 @@ fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var)
 		info->var.vmode &= ~FB_VMODE_YWRAP;
 	return 0;
 }
+EXPORT_SYMBOL(fb_pan_display);
 
 static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
 			 u32 activate)
@@ -1042,6 +1050,7 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
  done:
 	return ret;
 }
+EXPORT_SYMBOL(fb_set_var);
 
 int
 fb_blank(struct fb_info *info, int blank)
@@ -1073,6 +1082,7 @@ fb_blank(struct fb_info *info, int blank)
 
  	return ret;
 }
+EXPORT_SYMBOL(fb_blank);
 
 static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
 			unsigned long arg)
@@ -1734,6 +1744,7 @@ register_framebuffer(struct fb_info *fb_info)
 
 	return ret;
 }
+EXPORT_SYMBOL(register_framebuffer);
 
 /**
  *	unregister_framebuffer - releases a frame buffer device
@@ -1762,6 +1773,7 @@ unregister_framebuffer(struct fb_info *fb_info)
 
 	return ret;
 }
+EXPORT_SYMBOL(unregister_framebuffer);
 
 /**
  *	fb_set_suspend - low level driver signals suspend
@@ -1785,6 +1797,7 @@ void fb_set_suspend(struct fb_info *info, int state)
 		fb_notifier_call_chain(FB_EVENT_RESUME, &event);
 	}
 }
+EXPORT_SYMBOL(fb_set_suspend);
 
 /**
  *	fbmem_init - init frame buffer subsystem
@@ -1904,6 +1917,7 @@ int fb_get_options(char *name, char **option)
 
 	return retval;
 }
+EXPORT_SYMBOL(fb_get_options);
 
 #ifndef MODULE
 /**
@@ -1955,16 +1969,4 @@ __setup("video=", video_setup);
      *  Visible symbols for modules
      */
 
-EXPORT_SYMBOL(register_framebuffer);
-EXPORT_SYMBOL(unregister_framebuffer);
-EXPORT_SYMBOL(num_registered_fb);
-EXPORT_SYMBOL(registered_fb);
-EXPORT_SYMBOL(fb_show_logo);
-EXPORT_SYMBOL(fb_set_var);
-EXPORT_SYMBOL(fb_blank);
-EXPORT_SYMBOL(fb_pan_display);
-EXPORT_SYMBOL(fb_get_buffer_offset);
-EXPORT_SYMBOL(fb_set_suspend);
-EXPORT_SYMBOL(fb_get_options);
-
 MODULE_LICENSE("GPL");
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..9af9202 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -675,6 +675,7 @@ void fb_destroy_modedb(struct fb_videomode *modedb)
 {
 	kfree(modedb);
 }
+EXPORT_SYMBOL(fb_destroy_modedb);
 
 static int fb_get_monitor_limits(unsigned char *edid, struct fb_monspecs *specs)
 {
@@ -917,6 +918,7 @@ int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 	}
 	return 1;
 }
+EXPORT_SYMBOL(fb_parse_edid);
 
 void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 {
@@ -981,6 +983,7 @@ void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 
 	DPRINTK("====================\n");
 }
+EXPORT_SYMBOL(fb_edid_to_monspecs);
 
 /**
  * fb_edid_add_monspecs() - add monitor video modes from E-EDID data
@@ -1065,6 +1068,7 @@ void fb_edid_add_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 	specs->modedb = m;
 	specs->modedb_len = specs->modedb_len + num + svd_n;
 }
+EXPORT_SYMBOL(fb_edid_add_monspecs);
 
 /*
  * VESA Generalized Timing Formula (GTF)
@@ -1373,26 +1377,36 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
 	kfree(timings);
 	return err;
 }
+EXPORT_SYMBOL(fb_get_mode);
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
 	return 1;
 }
+EXPORT_SYMBOL(fb_parse_edid);
+
 void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 {
 	specs = NULL;
 }
+EXPORT_SYMBOL(fb_edid_to_monspecs);
+
 void fb_edid_add_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 {
 }
+EXPORT_SYMBOL(fb_edid_add_monspecs);
+
 void fb_destroy_modedb(struct fb_videomode *modedb)
 {
 }
+EXPORT_SYMBOL(fb_destroy_modedb);
+
 int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var,
 		struct fb_info *info)
 {
 	return -EINVAL;
 }
+EXPORT_SYMBOL(fb_get_mode);
 #endif /* CONFIG_FB_MODE_HELPERS */
 
 /*
@@ -1457,6 +1471,7 @@ int fb_validate_mode(const struct fb_var_screeninfo *var, struct fb_info *info)
 		pixclock < dclkmin || pixclock > dclkmax) ?
 		-EINVAL : 0;
 }
+EXPORT_SYMBOL(fb_validate_mode);
 
 #if defined(CONFIG_FIRMWARE_EDID) && defined(CONFIG_X86)
 
@@ -1489,10 +1504,3 @@ const unsigned char *fb_firmware_edid(struct device *device)
 }
 #endif
 EXPORT_SYMBOL(fb_firmware_edid);
-
-EXPORT_SYMBOL(fb_parse_edid);
-EXPORT_SYMBOL(fb_edid_to_monspecs);
-EXPORT_SYMBOL(fb_edid_add_monspecs);
-EXPORT_SYMBOL(fb_get_mode);
-EXPORT_SYMBOL(fb_validate_mode);
-EXPORT_SYMBOL(fb_destroy_modedb);
diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
index a9a907c..b803677 100644
--- a/drivers/video/modedb.c
+++ b/drivers/video/modedb.c
@@ -765,6 +765,7 @@ done:
 	DPRINTK("No valid mode found\n");
 	return 0;
 }
+EXPORT_SYMBOL(fb_find_mode);
 
 /**
  * fb_var_to_videomode - convert fb_var_screeninfo to fb_videomode
@@ -809,6 +810,7 @@ void fb_var_to_videomode(struct fb_videomode *mode,
 	hfreq = pixclock/htotal;
 	mode->refresh = hfreq/vtotal;
 }
+EXPORT_SYMBOL(fb_var_to_videomode);
 
 /**
  * fb_videomode_to_var - convert fb_videomode to fb_var_screeninfo
@@ -834,6 +836,7 @@ void fb_videomode_to_var(struct fb_var_screeninfo *var,
 	var->sync = mode->sync;
 	var->vmode = mode->vmode & FB_VMODE_MASK;
 }
+EXPORT_SYMBOL(fb_videomode_to_var);
 
 /**
  * fb_mode_is_equal - compare 2 videomodes
@@ -858,6 +861,7 @@ int fb_mode_is_equal(const struct fb_videomode *mode1,
 		mode1->sync         = mode2->sync &&
 		mode1->vmode        = mode2->vmode);
 }
+EXPORT_SYMBOL(fb_mode_is_equal);
 
 /**
  * fb_find_best_mode - find best matching videomode
@@ -903,6 +907,7 @@ const struct fb_videomode *fb_find_best_mode(const struct fb_var_screeninfo *var
 	}
 	return best;
 }
+EXPORT_SYMBOL(fb_find_best_mode);
 
 /**
  * fb_find_nearest_mode - find closest videomode
@@ -945,6 +950,7 @@ const struct fb_videomode *fb_find_nearest_mode(const struct fb_videomode *mode,
 
 	return best;
 }
+EXPORT_SYMBOL(fb_find_nearest_mode);
 
 /**
  * fb_match_mode - find a videomode which exactly matches the timings in var
@@ -970,6 +976,7 @@ const struct fb_videomode *fb_match_mode(const struct fb_var_screeninfo *var,
 	}
 	return NULL;
 }
+EXPORT_SYMBOL(fb_match_mode);
 
 /**
  * fb_add_videomode - adds videomode entry to modelist
@@ -1005,6 +1012,7 @@ int fb_add_videomode(const struct fb_videomode *mode, struct list_head *head)
 	}
 	return 0;
 }
+EXPORT_SYMBOL(fb_add_videomode);
 
 /**
  * fb_delete_videomode - removed videomode entry from modelist
@@ -1064,6 +1072,7 @@ void fb_videomode_to_modelist(const struct fb_videomode *modedb, int num,
 			return;
 	}
 }
+EXPORT_SYMBOL(fb_videomode_to_modelist);
 
 const struct fb_videomode *fb_find_best_display(const struct fb_monspecs *specs,
 					        struct list_head *head)
@@ -1124,14 +1133,3 @@ finished:
 	return best;
 }
 EXPORT_SYMBOL(fb_find_best_display);
-
-EXPORT_SYMBOL(fb_videomode_to_var);
-EXPORT_SYMBOL(fb_var_to_videomode);
-EXPORT_SYMBOL(fb_mode_is_equal);
-EXPORT_SYMBOL(fb_add_videomode);
-EXPORT_SYMBOL(fb_match_mode);
-EXPORT_SYMBOL(fb_find_best_mode);
-EXPORT_SYMBOL(fb_find_nearest_mode);
-EXPORT_SYMBOL(fb_videomode_to_modelist);
-EXPORT_SYMBOL(fb_find_mode);
-EXPORT_SYMBOL(fb_find_mode_cvt);
-- 
1.7.1



^ 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