This is a continuation of the series below:
http://marc.info/?l=linux-omap&m4381744304672&w=2
This series tries to pass output specific parameters from panel driver to the
connected output. These are basically parameters which can't really be shared
across different outputs. An attempt has been made to have similar looking
functions for setting DSI videomode timings and RFBI timings. But all the other
parameters in the series take output specific arguments.
After this series, the interface drivers look more independent of
omap_dss_device. It'll now be easier to add outputs as another DSS entity, and
pass them rather than passing omap_dss_device from panel drivers to
output/interface drivers.
git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git pass_output_specific
Tested on 4430SDP and 3430SDP
Archit Taneja (6):
OMAPDSS: DSI: Maintain copy of operation mode in driver data
OMAPDSS: DSI: Rename dsi_videomode_data to dsi_videomode_timings
OMAPDSS: DSI: Maintain copy of video mode timings in driver data
OMAPDSS: RFBI: Maitain copy of rfbi timings in driver data
OMAPDSS: VENC: Maintain copy of venc type in driver data
OMAPDSS: VENC: Maintian copy of video output polarity in private data
drivers/video/omap2/displays/panel-n8x0.c | 2 +
drivers/video/omap2/displays/panel-taal.c | 1 +
drivers/video/omap2/dss/dsi.c | 94 ++++++++++++++++++++---------
drivers/video/omap2/dss/dss.h | 4 ++
drivers/video/omap2/dss/rfbi.c | 14 ++++-
drivers/video/omap2/dss/venc.c | 28 ++++++++-
drivers/video/omap2/dss/venc_panel.c | 8 +++
include/video/omapdss.h | 10 ++-
8 files changed, 126 insertions(+), 35 deletions(-)
--
1.7.9.5
The DSI driver currently relies on the omap_dss_device struct to know the mode
of operation of the DSI protocol(command or video mode). This makes the DSI
interface driver dependent on the omap_dss_device struct.
Make the DSI driver data maintain it's own operation mode field. The panel
driver is expected to call omapdss_dsi_set_operation_mode() before the interface
is enabled.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-taal.c | 1 +
drivers/video/omap2/dss/dsi.c | 42 +++++++++++++++++++++--------
include/video/omapdss.h | 2 ++
3 files changed, 34 insertions(+), 11 deletions(-)
@@ -3909,7 +3911,7 @@ static int dsi_proto_config(struct omap_dss_device *dssdev)dsi_config_vp_num_line_buffers(dssdev);-if(dssdev->panel.dsi_mode=OMAP_DSS_DSI_VIDEO_MODE){+if(dsi->mode=OMAP_DSS_DSI_VIDEO_MODE){dsi_config_vp_sync_events(dssdev);dsi_config_blanking_modes(dssdev);dsi_config_cmd_mode_interleaving(dssdev);
@@ -3988,7 +3990,7 @@ static void dsi_proto_timings(struct omap_dss_device *dssdev)DSSDBG("enter_hs_mode_lat %u, exit_hs_mode_lat %u\n",enter_hs_mode_lat,exit_hs_mode_lat);-if(dssdev->panel.dsi_mode=OMAP_DSS_DSI_VIDEO_MODE){+if(dsi->mode=OMAP_DSS_DSI_VIDEO_MODE){/* TODO: Implement a video mode check_timings function */inthsa=dssdev->panel.dsi_vm_data.hsa;inthfp=dssdev->panel.dsi_vm_data.hfp;
@@ -4113,7 +4115,7 @@ int dsi_enable_video_output(struct omap_dss_device *dssdev, int channel)u16word_count;intr;-if(dssdev->panel.dsi_mode=OMAP_DSS_DSI_VIDEO_MODE){+if(dsi->mode=OMAP_DSS_DSI_VIDEO_MODE){switch(dsi->pix_fmt){caseOMAP_DSS_DSI_FMT_RGB888:data_type=MIPI_DSI_PACKED_PIXEL_STREAM_24;
@@ -4149,7 +4151,7 @@ int dsi_enable_video_output(struct omap_dss_device *dssdev, int channel)r=dss_mgr_enable(dssdev->manager);if(r){-if(dssdev->panel.dsi_mode=OMAP_DSS_DSI_VIDEO_MODE){+if(dsi->mode=OMAP_DSS_DSI_VIDEO_MODE){dsi_if_enable(dsidev,false);dsi_vc_enable(dsidev,channel,false);}
The struct omap_dss_dsi_videomode_data holds fields which need to be configured
for DSI to operate in video mode. Rename the struct to dsi_videomode_timings.
One reason to do this is because most of the fields in the struct are timings
related. The other reason is to create a generic op for output specific
timings. This generic op can be considered as a way to set custom or private
timings for the output.
In the case of OMAP, DSI and RFBI require some more timings apart from the
regular DISPC timings. The structs omap_dss_videomode_timings and rfbi_timings
can be considered as these output specific timings respectively.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/dss/dsi.c | 32 ++++++++++++++++----------------
include/video/omapdss.h | 4 ++--
2 files changed, 18 insertions(+), 18 deletions(-)
@@ -2704,7 +2704,7 @@ void omapdss_dsi_vc_enable_hs(struct omap_dss_device *dssdev, int channel,dsi_force_tx_stop_mode_io(dsidev);/* start the DDR clock by sending a NULL packet */-if(dssdev->panel.dsi_vm_data.ddr_clk_always_on&&enable)+if(dssdev->panel.dsi_vm_timings.ddr_clk_always_on&&enable)dsi_vc_send_null(dssdev,channel);}EXPORT_SYMBOL(omapdss_dsi_vc_enable_hs);
The DSI driver currently relies on the omap_dss_device struct to receive the
video mode timings requested by the panel driver. This makes the DSI interface
driver dependent on the omap_dss_device struct.
Make the DSI driver data maintain it's own video mode timings field. The panel
driver is expected to call omapdss_dsi_set_videomode_timings() to configure the
video mode timings before the interface is enabled. The function takes in a
void pointer rather than a pointer to omap_dss_dsi_videomode_timings struct.
This is because this function will finally be an output op shared across
different outputs to set custom or private timings.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/dss/dsi.c | 52 ++++++++++++++++++++++++++++-------------
include/video/omapdss.h | 2 ++
2 files changed, 38 insertions(+), 16 deletions(-)
@@ -2704,7 +2706,7 @@ void omapdss_dsi_vc_enable_hs(struct omap_dss_device *dssdev, int channel,dsi_force_tx_stop_mode_io(dsidev);/* start the DDR clock by sending a NULL packet */-if(dssdev->panel.dsi_vm_timings.ddr_clk_always_on&&enable)+if(dsi->vm_timings.ddr_clk_always_on&&enable)dsi_vc_send_null(dssdev,channel);}EXPORT_SYMBOL(omapdss_dsi_vc_enable_hs);
The RFBI driver currently relies on the omap_dss_device struct to receive the
rfbi specific timings requested by the panel driver. This makes the RFBI
interface driver dependent on the omap_dss_device struct.
Make the RFBI driver data maintain it's own rfbi specific timings field. The
panel driver is expected to call omapdss_rfbi_set_interface_timings() to
configure the rfbi timings before the interface is enabled. The function takes
a void pointer rather than a pointer to rfbi_timings struct. This is because
this function will finally be an output op shared across different outputs to
set custom or private timings.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-n8x0.c | 2 ++
drivers/video/omap2/dss/rfbi.c | 14 +++++++++++---
include/video/omapdss.h | 2 ++
3 files changed, 15 insertions(+), 3 deletions(-)
The VENC driver currently relies on the omap_dss_device struct to configure the
venc type. This makes the VENC interface driver dependent on the omap_dss_device
struct.
Make the VENC driver data maintain it's own 'venc type' field. A panel driver
is expected to call omapdss_venc_set_type() before enabling the interface or
changing the type via display sysfs attributes.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/dss/dss.h | 2 ++
drivers/video/omap2/dss/venc.c | 15 +++++++++++++--
drivers/video/omap2/dss/venc_panel.c | 2 ++
3 files changed, 17 insertions(+), 2 deletions(-)
The VENC driver currently relies on the omap_dss_device struct to configure the
video output polarity. This makes the VENC interface driver dependent on the
omap_dss_device struct.
Make the VENC driver data maintain it's own polarity field. A panel driver
is expected to call omapdss_venc_set_vid_out_polarity() before enabling the
interface.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/dss/dss.h | 2 ++
drivers/video/omap2/dss/venc.c | 13 ++++++++++++-
drivers/video/omap2/dss/venc_panel.c | 6 ++++++
3 files changed, 20 insertions(+), 1 deletion(-)
From: Tomi Valkeinen <hidden> Date: 2012-08-16 11:19:24
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
quoted hunk
The DSI driver currently relies on the omap_dss_device struct to know the mode
of operation of the DSI protocol(command or video mode). This makes the DSI
interface driver dependent on the omap_dss_device struct.
Make the DSI driver data maintain it's own operation mode field. The panel
driver is expected to call omapdss_dsi_set_operation_mode() before the interface
is enabled.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-taal.c | 1 +
drivers/video/omap2/dss/dsi.c | 42 +++++++++++++++++++++--------
include/video/omapdss.h | 2 ++
3 files changed, 34 insertions(+), 11 deletions(-)
@@ -1063,6 +1063,7 @@ static int taal_power_on(struct omap_dss_device *dssdev)omapdss_dsi_set_size(dssdev,dssdev->panel.timings.x_res,dssdev->panel.timings.y_res);omapdss_dsi_set_pixel_format(dssdev,dssdev->panel.dsi_pix_fmt);+omapdss_dsi_set_operation_mode(dssdev,dssdev->panel.dsi_mode);
Taal is always in cmd mode. Shouldn't we just use a hardcoded value
here?
Actually I think the same goes for the pix_fmt above. It's always the
same for Taal.
Tomi
From: Tomi Valkeinen <hidden> Date: 2012-08-16 11:31:06
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
The DSI driver currently relies on the omap_dss_device struct to receive the
video mode timings requested by the panel driver. This makes the DSI interface
driver dependent on the omap_dss_device struct.
Make the DSI driver data maintain it's own video mode timings field. The panel
driver is expected to call omapdss_dsi_set_videomode_timings() to configure the
video mode timings before the interface is enabled. The function takes in a
void pointer rather than a pointer to omap_dss_dsi_videomode_timings struct.
This is because this function will finally be an output op shared across
different outputs to set custom or private timings.
I don't think the function should take a void * in any case. If we want
to share the function, it should take a struct that perhaps contains an
union of rfbi and dsi timings.
But I'm not sure if there's any benefit for that...
So do you see us having just one set_timings, which would take either
the normal video timings, rfbi timings or dsi timings?
Tomi
From: Tomi Valkeinen <hidden> Date: 2012-08-16 11:38:38
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
quoted hunk
The VENC driver currently relies on the omap_dss_device struct to configure the
video output polarity. This makes the VENC interface driver dependent on the
omap_dss_device struct.
Make the VENC driver data maintain it's own polarity field. A panel driver
is expected to call omapdss_venc_set_vid_out_polarity() before enabling the
interface.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/dss/dss.h | 2 ++
drivers/video/omap2/dss/venc.c | 13 ++++++++++++-
drivers/video/omap2/dss/venc_panel.c | 6 ++++++
3 files changed, 20 insertions(+), 1 deletion(-)
Are you sure this is correct? I know practically nothing about analog
TV, but the TRM doesn't seem to say much about that bit, except it can
be used to "invert the video output". It doesn't say there's an
active/inactive level for the signal.
Tomi
On Thursday 16 August 2012 05:01 PM, Tomi Valkeinen wrote:
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
quoted
The DSI driver currently relies on the omap_dss_device struct to receive the
video mode timings requested by the panel driver. This makes the DSI interface
driver dependent on the omap_dss_device struct.
Make the DSI driver data maintain it's own video mode timings field. The panel
driver is expected to call omapdss_dsi_set_videomode_timings() to configure the
video mode timings before the interface is enabled. The function takes in a
void pointer rather than a pointer to omap_dss_dsi_videomode_timings struct.
This is because this function will finally be an output op shared across
different outputs to set custom or private timings.
I don't think the function should take a void * in any case. If we want
to share the function, it should take a struct that perhaps contains an
union of rfbi and dsi timings.
But I'm not sure if there's any benefit for that...
So do you see us having just one set_timings, which would take either
the normal video timings, rfbi timings or dsi timings?
I thought of having 2 timing ops, one is a standard "modeline like"
set_timings(), and the other a vague-ish set_custom_timings(). For
us(OMAP), we need to use it for DSI videomode and RFBI, we may reduce
that to only RFBI later if we calculate for DSI timings automatically.
For these extra timings to be consistent across SoCs, we would need to
align to get a common struct of some sort, which could then have unions
as you said. For now, I thought having a void pointer might suffice.
Archit
From: Tomi Valkeinen <hidden> Date: 2012-08-16 12:14:17
On Thu, 2012-08-16 at 17:27 +0530, Archit Taneja wrote:
On Thursday 16 August 2012 05:01 PM, Tomi Valkeinen wrote:
quoted
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
quoted
The DSI driver currently relies on the omap_dss_device struct to receive the
video mode timings requested by the panel driver. This makes the DSI interface
driver dependent on the omap_dss_device struct.
Make the DSI driver data maintain it's own video mode timings field. The panel
driver is expected to call omapdss_dsi_set_videomode_timings() to configure the
video mode timings before the interface is enabled. The function takes in a
void pointer rather than a pointer to omap_dss_dsi_videomode_timings struct.
This is because this function will finally be an output op shared across
different outputs to set custom or private timings.
I don't think the function should take a void * in any case. If we want
to share the function, it should take a struct that perhaps contains an
union of rfbi and dsi timings.
But I'm not sure if there's any benefit for that...
So do you see us having just one set_timings, which would take either
the normal video timings, rfbi timings or dsi timings?
I thought of having 2 timing ops, one is a standard "modeline like"
set_timings(), and the other a vague-ish set_custom_timings(). For
us(OMAP), we need to use it for DSI videomode and RFBI, we may reduce
that to only RFBI later if we calculate for DSI timings automatically.
For these extra timings to be consistent across SoCs, we would need to
align to get a common struct of some sort, which could then have unions
as you said. For now, I thought having a void pointer might suffice.
I would only use void * when it's a must, i.e. when really anything can
be passed as a parameter. In our case, I think having just two separate
functions is best.
I think the timings in videomode and rfbi structs should be SoC
independent even now, at least more or less. But you're right, it should
be verified. And any names in the structs should be according to DBI or
DSI spec, not OMAP. But this is for later.
Tomi
On Thursday 16 August 2012 04:49 PM, Tomi Valkeinen wrote:
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
quoted
The DSI driver currently relies on the omap_dss_device struct to know the mode
of operation of the DSI protocol(command or video mode). This makes the DSI
interface driver dependent on the omap_dss_device struct.
Make the DSI driver data maintain it's own operation mode field. The panel
driver is expected to call omapdss_dsi_set_operation_mode() before the interface
is enabled.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-taal.c | 1 +
drivers/video/omap2/dss/dsi.c | 42 +++++++++++++++++++++--------
include/video/omapdss.h | 2 ++
3 files changed, 34 insertions(+), 11 deletions(-)
@@ -1063,6 +1063,7 @@ static int taal_power_on(struct omap_dss_device *dssdev)omapdss_dsi_set_size(dssdev,dssdev->panel.timings.x_res,dssdev->panel.timings.y_res);omapdss_dsi_set_pixel_format(dssdev,dssdev->panel.dsi_pix_fmt);+omapdss_dsi_set_operation_mode(dssdev,dssdev->panel.dsi_mode);
Taal is always in cmd mode. Shouldn't we just use a hardcoded value
here?
Actually I think the same goes for the pix_fmt above. It's always the
same for Taal.
Yes, I'll do this. I guess we could gradually remove some of these
fields from omap_dss_device? Can't we? I mean, if there are no panels
which can ever support command and video mode together, then we can just
leave it to the driver to hold this information.
Archit
From: Tomi Valkeinen <hidden> Date: 2012-08-16 12:23:35
On Thu, 2012-08-16 at 17:41 +0530, Archit Taneja wrote:
On Thursday 16 August 2012 04:49 PM, Tomi Valkeinen wrote:
quoted
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
quoted
The DSI driver currently relies on the omap_dss_device struct to know the mode
of operation of the DSI protocol(command or video mode). This makes the DSI
interface driver dependent on the omap_dss_device struct.
Make the DSI driver data maintain it's own operation mode field. The panel
driver is expected to call omapdss_dsi_set_operation_mode() before the interface
is enabled.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-taal.c | 1 +
drivers/video/omap2/dss/dsi.c | 42 +++++++++++++++++++++--------
include/video/omapdss.h | 2 ++
3 files changed, 34 insertions(+), 11 deletions(-)
@@ -1063,6 +1063,7 @@ static int taal_power_on(struct omap_dss_device *dssdev)omapdss_dsi_set_size(dssdev,dssdev->panel.timings.x_res,dssdev->panel.timings.y_res);omapdss_dsi_set_pixel_format(dssdev,dssdev->panel.dsi_pix_fmt);+omapdss_dsi_set_operation_mode(dssdev,dssdev->panel.dsi_mode);
Taal is always in cmd mode. Shouldn't we just use a hardcoded value
here?
Actually I think the same goes for the pix_fmt above. It's always the
same for Taal.
Yes, I'll do this. I guess we could gradually remove some of these
fields from omap_dss_device? Can't we? I mean, if there are no panels
which can ever support command and video mode together, then we can just
leave it to the driver to hold this information.
Yes, we can remove them. Even if there was a panel taht supports both
cmd and video mode, it would be the panel driver's job to handle it, and
any parameters passed to the driver would happen via the panel's own
platform data, not via dssdev.
Tomi
On Thursday 16 August 2012 05:08 PM, Tomi Valkeinen wrote:
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
quoted
The VENC driver currently relies on the omap_dss_device struct to configure the
video output polarity. This makes the VENC interface driver dependent on the
omap_dss_device struct.
Make the VENC driver data maintain it's own polarity field. A panel driver
is expected to call omapdss_venc_set_vid_out_polarity() before enabling the
interface.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/dss/dss.h | 2 ++
drivers/video/omap2/dss/venc.c | 13 ++++++++++++-
drivers/video/omap2/dss/venc_panel.c | 6 ++++++
3 files changed, 20 insertions(+), 1 deletion(-)
Are you sure this is correct? I know practically nothing about analog
TV, but the TRM doesn't seem to say much about that bit, except it can
be used to "invert the video output". It doesn't say there's an
active/inactive level for the signal.
Well, the code works :), I'm also not totally sure about whether it
should be represented as a 2-level signal, it seemed like it would be
nicer to give it a signal level rather than keeping it as a bool, which
could vary for other SoC's?
I am considering not to pass this via the panel driver for now, I don't
know if all VENC IPs needs to do this, maybe it's okay to leave this
dssdev reference for now?
Archit
From: Tomi Valkeinen <hidden> Date: 2012-08-16 13:09:54
On Thu, 2012-08-16 at 17:57 +0530, Archit Taneja wrote:
On Thursday 16 August 2012 05:08 PM, Tomi Valkeinen wrote:
quoted
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
quoted
The VENC driver currently relies on the omap_dss_device struct to configure the
video output polarity. This makes the VENC interface driver dependent on the
omap_dss_device struct.
Make the VENC driver data maintain it's own polarity field. A panel driver
is expected to call omapdss_venc_set_vid_out_polarity() before enabling the
interface.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/dss/dss.h | 2 ++
drivers/video/omap2/dss/venc.c | 13 ++++++++++++-
drivers/video/omap2/dss/venc_panel.c | 6 ++++++
3 files changed, 20 insertions(+), 1 deletion(-)
Are you sure this is correct? I know practically nothing about analog
TV, but the TRM doesn't seem to say much about that bit, except it can
be used to "invert the video output". It doesn't say there's an
active/inactive level for the signal.
Well, the code works :), I'm also not totally sure about whether it
You could put there APPLE and ORANGE enum values, and it'd still work =)
should be represented as a 2-level signal, it seemed like it would be
nicer to give it a signal level rather than keeping it as a bool, which
could vary for other SoC's?
I am considering not to pass this via the panel driver for now, I don't
know if all VENC IPs needs to do this, maybe it's okay to leave this
dssdev reference for now?
I don't know if other VENC IPs support this or not, but the TRM refers
to external amplifier, so it sounds like a thing that would exist on
other IPs also.
Tomi