RE: [PATCH v4 3/7] OMAP: DSS2: Introduce omap_channel as a omap_dss_device parameter
Hi,
Tomi Valkeinen wrote:
On Tue, 2010-11-16 at 12:22 +0100, ext Taneja, Archit wrote:
Hi,
Tomi Valkeinen wrote:
Hi,
[snip]
diff --git a/arch/arm/plat-omap/include/plat/display.h
b/arch/arm/plat-omap/include/plat/display.h
index 586944d..3e6eec1 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -434,6 +434,7 @@ struct omap_dss_device {
struct omap_overlay_manager *manager;
enum omap_dss_display_state state;
+ enum omap_channel channel;
Isn't this the same as dssdev-manager-id?
Yes, this channel stuff is a bit confusing, even the enum omap_channel
is badly named (should at least have dss in it). But
omap_overlay_manager should represent the output pipe, so I don't
think there's need for channel field in dss_device.
The panel somehow needs to tell which manager it is connected to. We
went with with the way of declaring a new member 'channel' and setting
that parameter up in the board file.
The current way to relate the manager and device is done by checking
the dssdev-type in dss_recheck_connections() and then calling set_device()
This is not sufficient any more since two of the managers can support
similar types of displays.
So in the channel approach the following happens:
-mgr-id's are initialized at bootup.
-devices and managers are connected using 'channel'.
-mgr-id automatically becomes equal to channel.
Can we set something like dssdev-manager-id in the board file itself?
Right, now I see.
The dss_recheck_connections() felt like a hack when I wrote it =).
Clearly we need some way to define in the board file which
panel connects to which manager.
It wasn't needed probably for OMAP3 since all non-venc panels connect
to LCD and venc to VENC type.
Now that I think of it, what channel would we mention if the panel is
used in dsi l4 update mode or dma mode? It should have no manager at all.
Perhaps move the channel-field up, just below enum
omap_display_type type. The lines in the beginning of
omap_dss_device are things which define the interface etc,
and later there are miscallaneous dynamic things. And this belongs to the
former.
Now, if we have the channel defined in this way in the
omap_dss_device, I'm wondering if:
1) the manager pointer is needed at all, as the channel tells which manager
it is? 2) if we keep the manager pointer (as a helper shortcut),
should the code use generally use dssdev-channel or dssdev-manager-id?
I think manager-id makes more sense considering your logic below.
3) having this channel field requires a change to every board
file, because the channel has to be defined?
Yes, that is something that has to be done for all 'DIGIT'
panels across all boards.
And answering to myself, I guess the manager pointer is
needed, because
DSS2 supports (at least in theory) multiple panels in the
same interface (not at the same time, of course). This means
that we could have to panels, with the same interface and
channel, but only one would be in use. So the one in use
should have manager pointing to the correct manager, and the
other would have manager NULL.
Yes, passing dssdev-channel would make things worse if 2 panels
are connected to same interface.
And thus perhaps using dssdev-channel only for connecting
the right manager to right panel, and using
dssdev-manager-id for other uses would be the best? The
values would of course be the same, but at least we'd get a
crash if somehow an unconnected display is being used for configuration.
Even in the current kernel on 3430sdp, the board file adds sharp_ls, venc and
generic panels
If I boot up with venc as default display, mgr0 has dvi as its display and mgr1
has tv.
So if I try to enable sharp_ls panel I get a crash when we first try to access
dssdev-manager
in dpi_display_enable(). We should have a way to prevent enabling a panel if it
isn't connected
to a manager. We should also have a way to allow a panel to have no channel at
all (something like
OMAP_DSS_CHANNEL_NONE) if we are using something like dsi l4 update.
I won't add this extra channel now though, we need to think of a better way
later.
Does this make sense?
Yes, it does, I'll make these changes, and more if you can think of any.
Archit--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 3/7] OMAP: DSS2: Introduce omap_channel as a omap_dss_device parameter
Hi,
Tomi Valkeinen wrote:
Hi,
Tomi Valkeinen wrote:
On Tue, 2010-11-16 at 12:22 +0100, ext Taneja, Archit wrote:
Hi,
Tomi Valkeinen wrote:
Hi,
[snip]
diff --git a/arch/arm/plat-omap/include/plat/display.h
b/arch/arm/plat-omap/include/plat/display.h
index 586944d..3e6eec1 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -434,6 +434,7 @@ struct omap_dss_device {
struct omap_overlay_manager *manager;
enum omap_dss_display_state state;
+ enum omap_channel channel;
Isn't this the same as dssdev-manager-id?
Yes, this channel stuff is a bit confusing, even the enum omap_channel
is badly named (should at least have dss in it). But
omap_overlay_manager should represent the output pipe, so I don't
think there's need for channel field in dss_device.
The panel somehow needs to tell which manager it is connected to. We
went with with the way of declaring a new member 'channel' and
setting that parameter up in the board file.
The current way to relate the manager and device is done by checking
the dssdev-type in dss_recheck_connections() and then calling set_device()
This is not sufficient any more since two of the managers can support
similar types of displays.
So in the channel approach the following happens:
-mgr-id's are initialized at bootup.
-devices and managers are connected using 'channel'.
-mgr-id automatically becomes equal to channel.
Can we set something like dssdev-manager-id in the board file itself?
Right, now I see.
The dss_recheck_connections() felt like a hack when I wrote it =).
Clearly we need some way to define in the board file which panel connects to
which manager.
It wasn't needed probably for OMAP3 since all non-venc panels
connect to LCD and venc to VENC type.
Now that I think of it, what channel would we mention if the
panel is used in dsi l4 update mode or dma mode? It should
have no manager at all.
Perhaps move the channel-field up, just below enum omap_display_type
type. The lines in the beginning of omap_dss_device are things which
define the interface etc, and later there are miscallaneous dynamic
things. And this belongs to the former.
Now, if we have the channel defined in this way in the
omap_dss_device, I'm wondering if:
1) the manager pointer is needed at all, as the channel tells which
manager it is? 2) if we keep the manager pointer (as a helper
shortcut), should the code use generally use
dssdev-channel or dssdev-manager-id?
I think manager-id makes more sense considering your logic below.
3) having this channel field requires a change to every board file,
because the channel has to be defined?
Yes, that is something that has to be done for all 'DIGIT'
panels across all boards.
And answering to myself, I guess the manager pointer is needed, because
DSS2 supports (at least in theory) multiple panels in the same
interface (not at the same time, of course). This means that we could
have to panels, with the same interface and channel, but only one
would be in use. So the one in use should have manager pointing to the
correct manager, and the other would have manager NULL.
Yes, passing dssdev-channel would make things worse if 2
panels are connected to same interface.
And thus perhaps using dssdev-channel only for connecting the right
manager to right panel, and using
dssdev-manager-id for other uses would be the best? The
values would of course be the same, but at least we'd get a crash if
somehow an unconnected display is being used for configuration.
Even in the current kernel on 3430sdp, the board file adds
sharp_ls, venc and generic panels If I boot up with venc as
default display, mgr0 has dvi as its display and mgr1 has tv.
So if I try to enable sharp_ls panel I get a crash when we
first try to access dssdev-manager in dpi_display_enable().
We should have a way to prevent enabling a panel if it isn't
connected to a manager. We should also have a way to allow a
panel to have no channel at all (something like
OMAP_DSS_CHANNEL_NONE) if we are using something like dsi l4 update.
I won't add this extra channel now though, we need to think
of a better way later.
Also, do you think when a display unsets a manager and sets a new manager
the channel should be changed? Or should it purely be a one time thing
for choosing the correct manager and not use it anywhere else?
Thanks,
Archit
Does this make sense?
Yes, it does, I'll make these changes, and more if you can
think of any.
Archit--
To unsubscribe from this list: send the line unsubscribe
linux-omap in the body of a message to
majord...@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH v4 3/7] OMAP: DSS2: Introduce omap_channel as a omap_dss_device parameter
Hi,
On Mon, 2010-11-08 at 13:24 +0100, ext Archit Taneja wrote:
From: Sumit Semwal sumit.sem...@ti.com
A new member 'channel' is introduced in omap_dss_device structure to determine
which channel the panel uses. The dispc functions used by interface drivers
(dsi, sdi etc) will use this member to differentiate between the 2 channels.
The following dispc functions are changed to incorporate channel as an
argument:
-dispc_enable_fifohandcheck()
-dispc_set_lcd_size()
-dispc_set_parallel_interface_mode()
-dispc_set_tft_data_lines()
-dispc_set_lcd_display_type()
-dispc_set_lcd_timings()
The calls to these functions from the interface drivers have been changed to
incorporate the new channel argument.
Signed-off-by: Sumit Semwal sumit.sem...@ti.com
Signed-off-by: Mukund Mittal mmit...@ti.com
Signed-off-by: Samreen samr...@ti.com
---
arch/arm/plat-omap/include/plat/display.h |1 +
drivers/video/omap2/dss/dispc.c | 49
-
drivers/video/omap2/dss/dpi.c | 11 +++---
drivers/video/omap2/dss/dsi.c | 13 ---
drivers/video/omap2/dss/dss.h | 19 ++-
drivers/video/omap2/dss/rfbi.c| 19 ++-
drivers/video/omap2/dss/sdi.c | 14 +---
7 files changed, 70 insertions(+), 56 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/display.h
b/arch/arm/plat-omap/include/plat/display.h
index 586944d..3e6eec1 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -434,6 +434,7 @@ struct omap_dss_device {
struct omap_overlay_manager *manager;
enum omap_dss_display_state state;
+ enum omap_channel channel;
Isn't this the same as dssdev-manager-id?
Yes, this channel stuff is a bit confusing, even the enum omap_channel
is badly named (should at least have dss in it). But
omap_overlay_manager should represent the output pipe, so I don't think
there's need for channel field in dss_device.
I think in many places we could even pass omap_overlay_manager pointer
around, instead of omap_channel. However, for low level dispc functions
it's perhaps cleaner to pass the channel ID though.
Tomi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 3/7] OMAP: DSS2: Introduce omap_channel as a omap_dss_device parameter
Hi,
Tomi Valkeinen wrote:
Hi,
[snip]
diff --git a/arch/arm/plat-omap/include/plat/display.h
b/arch/arm/plat-omap/include/plat/display.h
index 586944d..3e6eec1 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -434,6 +434,7 @@ struct omap_dss_device {
struct omap_overlay_manager *manager;
enum omap_dss_display_state state;
+ enum omap_channel channel;
Isn't this the same as dssdev-manager-id?
Yes, this channel stuff is a bit confusing, even the enum omap_channel
is badly named (should at least have dss in it). But
omap_overlay_manager should represent the output pipe, so I
don't think there's need for channel field in dss_device.
The panel somehow needs to tell which manager it is connected to. We went with
with the way of declaring a new member 'channel' and setting that parameter up
in the
board file.
The current way to relate the manager and device is done by checking the
dssdev-type
in dss_recheck_connections() and then calling set_device()
This is not sufficient any more since two of the managers can support similar
types of
displays.
So in the channel approach the following happens:
-mgr-id's are initialized at bootup.
-devices and managers are connected using 'channel'.
-mgr-id automatically becomes equal to channel.
Can we set something like dssdev-manager-id in the board file itself?
Archit
I think in many places we could even pass
omap_overlay_manager pointer around, instead of omap_channel.
However, for low level dispc functions it's perhaps cleaner
to pass the channel ID though.
Tomi--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 3/7] OMAP: DSS2: Introduce omap_channel as a omap_dss_device parameter
On Tue, 2010-11-16 at 12:22 +0100, ext Taneja, Archit wrote:
Hi,
Tomi Valkeinen wrote:
Hi,
[snip]
diff --git a/arch/arm/plat-omap/include/plat/display.h
b/arch/arm/plat-omap/include/plat/display.h
index 586944d..3e6eec1 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -434,6 +434,7 @@ struct omap_dss_device {
struct omap_overlay_manager *manager;
enum omap_dss_display_state state;
+ enum omap_channel channel;
Isn't this the same as dssdev-manager-id?
Yes, this channel stuff is a bit confusing, even the enum omap_channel
is badly named (should at least have dss in it). But
omap_overlay_manager should represent the output pipe, so I
don't think there's need for channel field in dss_device.
The panel somehow needs to tell which manager it is connected to. We went with
with the way of declaring a new member 'channel' and setting that parameter
up in the
board file.
The current way to relate the manager and device is done by checking the
dssdev-type
in dss_recheck_connections() and then calling set_device()
This is not sufficient any more since two of the managers can support similar
types of
displays.
So in the channel approach the following happens:
-mgr-id's are initialized at bootup.
-devices and managers are connected using 'channel'.
-mgr-id automatically becomes equal to channel.
Can we set something like dssdev-manager-id in the board file itself?
Right, now I see.
The dss_recheck_connections() felt like a hack when I wrote it =).
Clearly we need some way to define in the board file which panel
connects to which manager.
Perhaps move the channel-field up, just below enum omap_display_type
type. The lines in the beginning of omap_dss_device are things which
define the interface etc, and later there are miscallaneous dynamic
things. And this belongs to the former.
Now, if we have the channel defined in this way in the omap_dss_device,
I'm wondering if:
1) the manager pointer is needed at all, as the channel tells which
manager it is?
2) if we keep the manager pointer (as a helper shortcut), should the
code use generally use dssdev-channel or dssdev-manager-id?
3) having this channel field requires a change to every board file,
because the channel has to be defined?
And answering to myself, I guess the manager pointer is needed, because
DSS2 supports (at least in theory) multiple panels in the same interface
(not at the same time, of course). This means that we could have to
panels, with the same interface and channel, but only one would be in
use. So the one in use should have manager pointing to the correct
manager, and the other would have manager NULL.
And thus perhaps using dssdev-channel only for connecting the right
manager to right panel, and using dssdev-manager-id for other uses
would be the best? The values would of course be the same, but at least
we'd get a crash if somehow an unconnected display is being used for
configuration.
Does this make sense?
Tomi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
