[PATCH 0/6] OMAPDSS: Pass output specific parameters from panel driver to output

[PATCH 0/6] OMAPDSS: Pass output specific parameters from panel driver to output

[Archit Taneja]

This is a continuation of the series below:

http://marc.info/?l=linux-omap&m\x134381744304672&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

[PATCH 1/6] OMAPDSS: DSI: Maintain copy of operation mode in driver data

[Archit Taneja]

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 <archit@ti.com>
---
 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(-)

diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
index d220f19..649247f 100644
--- a/drivers/video/omap2/displays/panel-taal.c
+++ b/drivers/video/omap2/displays/panel-taal.c
@@ -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);
 
 	r = omapdss_dsi_display_enable(dssdev);
 	if (r) {
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 8f94cb80..c10c8cb 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -335,6 +335,7 @@ struct dsi_data {
 	struct dss_lcd_mgr_config mgr_config;
 	struct omap_video_timings timings;
 	enum omap_dss_dsi_pixel_format pix_fmt;
+	enum omap_dss_dsi_mode mode;
 };
 
 struct dsi_packet_sent_handler_data {
@@ -2362,7 +2363,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 
 	dsi_cio_timings(dsidev);
 
-	if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_VIDEO_MODE) {
+	if (dsi->mode = OMAP_DSS_DSI_VIDEO_MODE) {
 		/* DDR_CLK_ALWAYS_ON */
 		REG_FLD_MOD(dsidev, DSI_CLK_CTRL,
 			dssdev->panel.dsi_vm_data.ddr_clk_always_on, 13, 13);
@@ -3609,9 +3610,10 @@ static void dsi_set_hs_tx_timeout(struct platform_device *dsidev,
 static void dsi_config_vp_num_line_buffers(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	int num_line_buffers;
 
-	if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_VIDEO_MODE) {
+	if (dsi->mode = OMAP_DSS_DSI_VIDEO_MODE) {
 		struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 		int bpp = dsi_get_pixel_size(dsi->pix_fmt);
 		unsigned line_buf_size = dsi_get_line_buf_size(dsidev);
@@ -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 */
 		int hsa = dssdev->panel.dsi_vm_data.hsa;
 		int hfp = dssdev->panel.dsi_vm_data.hfp;
@@ -4113,7 +4115,7 @@ int dsi_enable_video_output(struct omap_dss_device *dssdev, int channel)
 	u16 word_count;
 	int r;
 
-	if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_VIDEO_MODE) {
+	if (dsi->mode = OMAP_DSS_DSI_VIDEO_MODE) {
 		switch (dsi->pix_fmt) {
 		case OMAP_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);
 		}
@@ -4164,8 +4166,9 @@ EXPORT_SYMBOL(dsi_enable_video_output);
 void dsi_disable_video_output(struct omap_dss_device *dssdev, int channel)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 
-	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);
 
@@ -4379,7 +4382,7 @@ static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
 	int r;
 	u32 irq = 0;
 
-	if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_CMD_MODE) {
+	if (dsi->mode = OMAP_DSS_DSI_CMD_MODE) {
 		dsi->timings.hsw = 1;
 		dsi->timings.hfp = 1;
 		dsi->timings.hbp = 1;
@@ -4429,7 +4432,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)
+	if (dsi->mode = OMAP_DSS_DSI_CMD_MODE)
 		omap_dispc_unregister_isr(dsi_framedone_irq_callback,
 			(void *) dssdev, irq);
 err:
@@ -4438,7 +4441,10 @@ err:
 
 static void dsi_display_uninit_dispc(struct omap_dss_device *dssdev)
 {
-	if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_CMD_MODE) {
+	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
+
+	if (dsi->mode = OMAP_DSS_DSI_CMD_MODE) {
 		u32 irq;
 
 		irq = dispc_mgr_get_framedone_irq(dssdev->manager->id);
@@ -4695,6 +4701,20 @@ void omapdss_dsi_set_pixel_format(struct omap_dss_device *dssdev,
 }
 EXPORT_SYMBOL(omapdss_dsi_set_pixel_format);
 
+void omapdss_dsi_set_operation_mode(struct omap_dss_device *dssdev,
+		enum omap_dss_dsi_mode mode)
+{
+	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
+
+	mutex_lock(&dsi->lock);
+
+	dsi->mode = mode;
+
+	mutex_unlock(&dsi->lock);
+}
+EXPORT_SYMBOL(omapdss_dsi_set_operation_mode);
+
 static int __init dsi_init_display(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
@@ -4702,7 +4722,7 @@ static int __init dsi_init_display(struct omap_dss_device *dssdev)
 
 	DSSDBG("DSI init\n");
 
-	if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_CMD_MODE) {
+	if (dsi->mode = OMAP_DSS_DSI_CMD_MODE) {
 		dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE |
 			OMAP_DSS_DISPLAY_CAP_TEAR_ELIM;
 	}
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 88ac6e8..ef14ac5 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -724,6 +724,8 @@ void omapdss_dsi_set_timings(struct omap_dss_device *dssdev,
 void omapdss_dsi_set_size(struct omap_dss_device *dssdev, u16 w, u16 h);
 void omapdss_dsi_set_pixel_format(struct omap_dss_device *dssdev,
 		enum omap_dss_dsi_pixel_format fmt);
+void omapdss_dsi_set_operation_mode(struct omap_dss_device *dssdev,
+		enum omap_dss_dsi_mode mode);
 
 int omap_dsi_update(struct omap_dss_device *dssdev, int channel,
 		void (*callback)(int, void *), void *data);
-- 
1.7.9.5

Re: [PATCH 1/6] OMAPDSS: DSI: Maintain copy of operation mode in driver data

[Tomi Valkeinen]

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

On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
> 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 <archit@ti.com>
> ---
>  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(-)
> 
> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
> index d220f19..649247f 100644
> --- a/drivers/video/omap2/displays/panel-taal.c
> +++ b/drivers/video/omap2/displays/panel-taal.c
> @@ -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


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

Re: [PATCH 1/6] OMAPDSS: DSI: Maintain copy of operation mode in driver data

[Archit Taneja]

On Thursday 16 August 2012 04:49 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
>> 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 <archit@ti.com>
>> ---
>>   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(-)
>>
>> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
>> index d220f19..649247f 100644
>> --- a/drivers/video/omap2/displays/panel-taal.c
>> +++ b/drivers/video/omap2/displays/panel-taal.c
>> @@ -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

Re: [PATCH 1/6] OMAPDSS: DSI: Maintain copy of operation mode in driver data

[Tomi Valkeinen]

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

On Thu, 2012-08-16 at 17:41 +0530, Archit Taneja wrote:
> On Thursday 16 August 2012 04:49 PM, Tomi Valkeinen wrote:
> > On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
> >> 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 <archit@ti.com>
> >> ---
> >>   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(-)
> >>
> >> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
> >> index d220f19..649247f 100644
> >> --- a/drivers/video/omap2/displays/panel-taal.c
> >> +++ b/drivers/video/omap2/displays/panel-taal.c
> >> @@ -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


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

[PATCH 2/6] OMAPDSS: DSI: Rename dsi_videomode_data to dsi_videomode_timings

[Archit Taneja]

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 <archit@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   32 ++++++++++++++++----------------
 include/video/omapdss.h       |    4 ++--
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index c10c8cb..62549f6 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -2366,7 +2366,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 	if (dsi->mode = OMAP_DSS_DSI_VIDEO_MODE) {
 		/* DDR_CLK_ALWAYS_ON */
 		REG_FLD_MOD(dsidev, DSI_CLK_CTRL,
-			dssdev->panel.dsi_vm_data.ddr_clk_always_on, 13, 13);
+			dssdev->panel.dsi_vm_timings.ddr_clk_always_on, 13, 13);
 	}
 
 	dsi->ulps_enabled = false;
@@ -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);
@@ -3638,8 +3638,8 @@ static void dsi_config_vp_num_line_buffers(struct omap_dss_device *dssdev)
 static void dsi_config_vp_sync_events(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
-	bool vsync_end = dssdev->panel.dsi_vm_data.vp_vsync_end;
-	bool hsync_end = dssdev->panel.dsi_vm_data.vp_hsync_end;
+	bool vsync_end = dssdev->panel.dsi_vm_timings.vp_vsync_end;
+	bool hsync_end = dssdev->panel.dsi_vm_timings.vp_hsync_end;
 	u32 r;
 
 	r = dsi_read_reg(dsidev, DSI_CTRL);
@@ -3656,10 +3656,10 @@ static void dsi_config_vp_sync_events(struct omap_dss_device *dssdev)
 static void dsi_config_blanking_modes(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
-	int blanking_mode = dssdev->panel.dsi_vm_data.blanking_mode;
-	int hfp_blanking_mode = dssdev->panel.dsi_vm_data.hfp_blanking_mode;
-	int hbp_blanking_mode = dssdev->panel.dsi_vm_data.hbp_blanking_mode;
-	int hsa_blanking_mode = dssdev->panel.dsi_vm_data.hsa_blanking_mode;
+	int blanking_mode = dssdev->panel.dsi_vm_timings.blanking_mode;
+	int hfp_blanking_mode = dssdev->panel.dsi_vm_timings.hfp_blanking_mode;
+	int hbp_blanking_mode = dssdev->panel.dsi_vm_timings.hbp_blanking_mode;
+	int hsa_blanking_mode = dssdev->panel.dsi_vm_timings.hsa_blanking_mode;
 	u32 r;
 
 	/*
@@ -3992,14 +3992,14 @@ static void dsi_proto_timings(struct omap_dss_device *dssdev)
 
 	 if (dsi->mode = OMAP_DSS_DSI_VIDEO_MODE) {
 		/* TODO: Implement a video mode check_timings function */
-		int hsa = dssdev->panel.dsi_vm_data.hsa;
-		int hfp = dssdev->panel.dsi_vm_data.hfp;
-		int hbp = dssdev->panel.dsi_vm_data.hbp;
-		int vsa = dssdev->panel.dsi_vm_data.vsa;
-		int vfp = dssdev->panel.dsi_vm_data.vfp;
-		int vbp = dssdev->panel.dsi_vm_data.vbp;
-		int window_sync = dssdev->panel.dsi_vm_data.window_sync;
-		bool hsync_end = dssdev->panel.dsi_vm_data.vp_hsync_end;
+		int hsa = dssdev->panel.dsi_vm_timings.hsa;
+		int hfp = dssdev->panel.dsi_vm_timings.hfp;
+		int hbp = dssdev->panel.dsi_vm_timings.hbp;
+		int vsa = dssdev->panel.dsi_vm_timings.vsa;
+		int vfp = dssdev->panel.dsi_vm_timings.vfp;
+		int vbp = dssdev->panel.dsi_vm_timings.vbp;
+		int window_sync = dssdev->panel.dsi_vm_timings.window_sync;
+		bool hsync_end = dssdev->panel.dsi_vm_timings.vp_hsync_end;
 		struct omap_video_timings *timings = &dsi->timings;
 		int bpp = dsi_get_pixel_size(dsi->pix_fmt);
 		int tl, t_he, width_bytes;
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index ef14ac5..363235c 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -243,7 +243,7 @@ void rfbi_bus_unlock(void);
 
 /* DSI */
 
-struct omap_dss_dsi_videomode_data {
+struct omap_dss_dsi_videomode_timings {
 	/* DSI video mode blanking data */
 	/* Unit: byte clock cycles */
 	u16 hsa;
@@ -564,7 +564,7 @@ struct omap_dss_device {
 
 		enum omap_dss_dsi_pixel_format dsi_pix_fmt;
 		enum omap_dss_dsi_mode dsi_mode;
-		struct omap_dss_dsi_videomode_data dsi_vm_data;
+		struct omap_dss_dsi_videomode_timings dsi_vm_timings;
 	} panel;
 
 	struct {
-- 
1.7.9.5

[PATCH 3/6] OMAPDSS: DSI: Maintain copy of video mode timings in driver data

[Archit Taneja]

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 <archit@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   52 ++++++++++++++++++++++++++++-------------
 include/video/omapdss.h       |    2 ++
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 62549f6..6c2c746 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -336,6 +336,7 @@ struct dsi_data {
 	struct omap_video_timings timings;
 	enum omap_dss_dsi_pixel_format pix_fmt;
 	enum omap_dss_dsi_mode mode;
+	struct omap_dss_dsi_videomode_timings vm_timings;
 };
 
 struct dsi_packet_sent_handler_data {
@@ -2366,7 +2367,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 	if (dsi->mode = OMAP_DSS_DSI_VIDEO_MODE) {
 		/* DDR_CLK_ALWAYS_ON */
 		REG_FLD_MOD(dsidev, DSI_CLK_CTRL,
-			dssdev->panel.dsi_vm_timings.ddr_clk_always_on, 13, 13);
+			dsi->vm_timings.ddr_clk_always_on, 13, 13);
 	}
 
 	dsi->ulps_enabled = false;
@@ -2688,6 +2689,7 @@ void omapdss_dsi_vc_enable_hs(struct omap_dss_device *dssdev, int channel,
 		bool enable)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 
 	DSSDBG("dsi_vc_enable_hs(%d, %d)\n", channel, enable);
 
@@ -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);
@@ -3638,8 +3640,9 @@ static void dsi_config_vp_num_line_buffers(struct omap_dss_device *dssdev)
 static void dsi_config_vp_sync_events(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
-	bool vsync_end = dssdev->panel.dsi_vm_timings.vp_vsync_end;
-	bool hsync_end = dssdev->panel.dsi_vm_timings.vp_hsync_end;
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
+	bool vsync_end = dsi->vm_timings.vp_vsync_end;
+	bool hsync_end = dsi->vm_timings.vp_hsync_end;
 	u32 r;
 
 	r = dsi_read_reg(dsidev, DSI_CTRL);
@@ -3656,10 +3659,11 @@ static void dsi_config_vp_sync_events(struct omap_dss_device *dssdev)
 static void dsi_config_blanking_modes(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
-	int blanking_mode = dssdev->panel.dsi_vm_timings.blanking_mode;
-	int hfp_blanking_mode = dssdev->panel.dsi_vm_timings.hfp_blanking_mode;
-	int hbp_blanking_mode = dssdev->panel.dsi_vm_timings.hbp_blanking_mode;
-	int hsa_blanking_mode = dssdev->panel.dsi_vm_timings.hsa_blanking_mode;
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
+	int blanking_mode = dsi->vm_timings.blanking_mode;
+	int hfp_blanking_mode = dsi->vm_timings.hfp_blanking_mode;
+	int hbp_blanking_mode = dsi->vm_timings.hbp_blanking_mode;
+	int hsa_blanking_mode = dsi->vm_timings.hsa_blanking_mode;
 	u32 r;
 
 	/*
@@ -3992,14 +3996,14 @@ static void dsi_proto_timings(struct omap_dss_device *dssdev)
 
 	 if (dsi->mode = OMAP_DSS_DSI_VIDEO_MODE) {
 		/* TODO: Implement a video mode check_timings function */
-		int hsa = dssdev->panel.dsi_vm_timings.hsa;
-		int hfp = dssdev->panel.dsi_vm_timings.hfp;
-		int hbp = dssdev->panel.dsi_vm_timings.hbp;
-		int vsa = dssdev->panel.dsi_vm_timings.vsa;
-		int vfp = dssdev->panel.dsi_vm_timings.vfp;
-		int vbp = dssdev->panel.dsi_vm_timings.vbp;
-		int window_sync = dssdev->panel.dsi_vm_timings.window_sync;
-		bool hsync_end = dssdev->panel.dsi_vm_timings.vp_hsync_end;
+		int hsa = dsi->vm_timings.hsa;
+		int hfp = dsi->vm_timings.hfp;
+		int hbp = dsi->vm_timings.hbp;
+		int vsa = dsi->vm_timings.vsa;
+		int vfp = dsi->vm_timings.vfp;
+		int vbp = dsi->vm_timings.vbp;
+		int window_sync = dsi->vm_timings.window_sync;
+		bool hsync_end = dsi->vm_timings.vp_hsync_end;
 		struct omap_video_timings *timings = &dsi->timings;
 		int bpp = dsi_get_pixel_size(dsi->pix_fmt);
 		int tl, t_he, width_bytes;
@@ -4715,6 +4719,22 @@ void omapdss_dsi_set_operation_mode(struct omap_dss_device *dssdev,
 }
 EXPORT_SYMBOL(omapdss_dsi_set_operation_mode);
 
+void omapdss_dsi_set_videomode_timings(struct omap_dss_device *dssdev,
+		void *timings)
+{
+	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
+	struct omap_dss_dsi_videomode_timings *vm_timings +			(struct omap_dss_dsi_videomode_timings *) timings;
+
+	mutex_lock(&dsi->lock);
+
+	dsi->vm_timings = *vm_timings;
+
+	mutex_unlock(&dsi->lock);
+}
+EXPORT_SYMBOL(omapdss_dsi_set_videomode_timings);
+
 static int __init dsi_init_display(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 363235c..faac986 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -726,6 +726,8 @@ void omapdss_dsi_set_pixel_format(struct omap_dss_device *dssdev,
 		enum omap_dss_dsi_pixel_format fmt);
 void omapdss_dsi_set_operation_mode(struct omap_dss_device *dssdev,
 		enum omap_dss_dsi_mode mode);
+void omapdss_dsi_set_videomode_timings(struct omap_dss_device *dssdev,
+		void *timings);
 
 int omap_dsi_update(struct omap_dss_device *dssdev, int channel,
 		void (*callback)(int, void *), void *data);
-- 
1.7.9.5

Re: [PATCH 3/6] OMAPDSS: DSI: Maintain copy of video mode timings in driver data

[Tomi Valkeinen]

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

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


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

Re: [PATCH 3/6] OMAPDSS: DSI: Maintain copy of video mode timings in driver data

[Archit Taneja]

On Thursday 16 August 2012 05:01 PM, Tomi Valkeinen wrote:
> 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?

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

Re: [PATCH 3/6] OMAPDSS: DSI: Maintain copy of video mode timings in driver data

[Tomi Valkeinen]

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

On Thu, 2012-08-16 at 17:27 +0530, Archit Taneja wrote:
> On Thursday 16 August 2012 05:01 PM, Tomi Valkeinen wrote:
> > 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?
> 
> 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


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

[PATCH 4/6] OMAPDSS: RFBI: Maitain copy of rfbi timings in driver data

[Archit Taneja]

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 <archit@ti.com>
---
 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(-)

diff --git a/drivers/video/omap2/displays/panel-n8x0.c b/drivers/video/omap2/displays/panel-n8x0.c
index 3ffd987..14adc1d 100644
--- a/drivers/video/omap2/displays/panel-n8x0.c
+++ b/drivers/video/omap2/displays/panel-n8x0.c
@@ -307,6 +307,8 @@ static int n8x0_panel_power_on(struct omap_dss_device *dssdev)
 		dssdev->panel.timings.y_res);
 	omapdss_rfbi_set_pixel_size(dssdev, dssdev->ctrl.pixel_size);
 	omapdss_rfbi_set_data_lines(dssdev, dssdev->phy.rfbi.data_lines);
+	omapdss_rfbi_set_interface_timings(dssdev,
+		(void *) &dssdev->ctrl.rfbi_timings);
 
 	r = omapdss_rfbi_display_enable(dssdev);
 	if (r)
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index 698d5b9..b5b0bc6 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -115,6 +115,7 @@ static struct {
 	struct omap_video_timings timings;
 	int pixel_size;
 	int data_lines;
+	struct rfbi_timings intf_timings;
 } rfbi;
 
 static inline void rfbi_write_reg(const struct rfbi_reg idx, u32 val)
@@ -799,6 +800,15 @@ void omapdss_rfbi_set_data_lines(struct omap_dss_device *dssdev, int data_lines)
 }
 EXPORT_SYMBOL(omapdss_rfbi_set_data_lines);
 
+void omapdss_rfbi_set_interface_timings(struct omap_dss_device *dssdev,
+		void *timings)
+{
+	struct rfbi_timings *t = (struct rfbi_timings *) timings;
+
+	rfbi.intf_timings = *t;
+}
+EXPORT_SYMBOL(omapdss_rfbi_set_interface_timings);
+
 static void rfbi_dump_regs(struct seq_file *s)
 {
 #define DUMPREG(r) seq_printf(s, "%-35s %08x\n", #r, rfbi_read_reg(r))
@@ -907,9 +917,7 @@ int omapdss_rfbi_display_enable(struct omap_dss_device *dssdev)
 	rfbi_configure(dssdev->phy.rfbi.channel, rfbi.pixel_size,
 			rfbi.data_lines);
 
-	rfbi_set_timings(dssdev->phy.rfbi.channel,
-			 &dssdev->ctrl.rfbi_timings);
-
+	rfbi_set_timings(dssdev->phy.rfbi.channel, &rfbi.intf_timings);
 
 	return 0;
 err1:
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index faac986..91eba93 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -765,5 +765,7 @@ void omapdss_rfbi_set_pixel_size(struct omap_dss_device *dssdev,
 		int pixel_size);
 void omapdss_rfbi_set_data_lines(struct omap_dss_device *dssdev,
 		int data_lines);
+void omapdss_rfbi_set_interface_timings(struct omap_dss_device *dssdev,
+		void *timings);
 
 #endif
-- 
1.7.9.5

[PATCH 5/6] OMAPDSS: VENC: Maintain copy of venc type in driver data

[Archit Taneja]

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 <archit@ti.com>
---
 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(-)

diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index f919dc8..c17d298 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -477,6 +477,8 @@ int omapdss_venc_check_timings(struct omap_dss_device *dssdev,
 		struct omap_video_timings *timings);
 u32 omapdss_venc_get_wss(struct omap_dss_device *dssdev);
 int omapdss_venc_set_wss(struct omap_dss_device *dssdev, u32 wss);
+void omapdss_venc_set_type(struct omap_dss_device *dssdev,
+		enum omap_dss_venc_type type);
 int venc_panel_init(void);
 void venc_panel_exit(void);
 
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index d96025e..2d90fcf 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -302,6 +302,7 @@ static struct {
 	struct clk	*tv_dac_clk;
 
 	struct omap_video_timings timings;
+	enum omap_dss_venc_type type;
 } venc;
 
 static inline void venc_write_reg(int idx, u32 val)
@@ -436,12 +437,12 @@ static int venc_power_on(struct omap_dss_device *dssdev)
 	venc_reset();
 	venc_write_config(venc_timings_to_config(&venc.timings));
 
-	dss_set_venc_output(dssdev->phy.venc.type);
+	dss_set_venc_output(venc.type);
 	dss_set_dac_pwrdn_bgz(1);
 
 	l = 0;
 
-	if (dssdev->phy.venc.type = OMAP_DSS_VENC_TYPE_COMPOSITE)
+	if (venc.type = OMAP_DSS_VENC_TYPE_COMPOSITE)
 		l |= 1 << 1;
 	else /* S-Video */
 		l |= (1 << 0) | (1 << 2);
@@ -628,6 +629,16 @@ err:
 	return r;
 }
 
+void omapdss_venc_set_type(struct omap_dss_device *dssdev,
+		enum omap_dss_venc_type type)
+{
+	mutex_lock(&venc.venc_lock);
+
+	venc.type = type;
+
+	mutex_unlock(&venc.venc_lock);
+}
+
 static int __init venc_init_display(struct omap_dss_device *dssdev)
 {
 	DSSDBG("init_display\n");
diff --git a/drivers/video/omap2/dss/venc_panel.c b/drivers/video/omap2/dss/venc_panel.c
index 8096369..ef21361 100644
--- a/drivers/video/omap2/dss/venc_panel.c
+++ b/drivers/video/omap2/dss/venc_panel.c
@@ -68,6 +68,7 @@ static ssize_t display_output_type_store(struct device *dev,
 
 	if (dssdev->phy.venc.type != new_type) {
 		dssdev->phy.venc.type = new_type;
+		omapdss_venc_set_type(dssdev, new_type);
 		if (dssdev->state = OMAP_DSS_DISPLAY_ACTIVE) {
 			omapdss_venc_display_disable(dssdev);
 			omapdss_venc_display_enable(dssdev);
@@ -128,6 +129,7 @@ static int venc_panel_enable(struct omap_dss_device *dssdev)
 	}
 
 	omapdss_venc_set_timings(dssdev, &dssdev->panel.timings);
+	omapdss_venc_set_type(dssdev, dssdev->phy.venc.type);
 
 	r = omapdss_venc_display_enable(dssdev);
 	if (r)
-- 
1.7.9.5

[PATCH 6/6] OMAPDSS: VENC: Maintian copy of video output polarity in private data

[Archit Taneja]

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 <archit@ti.com>
---
 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(-)

diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index c17d298..b2cf5530 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -479,6 +479,8 @@ u32 omapdss_venc_get_wss(struct omap_dss_device *dssdev);
 int omapdss_venc_set_wss(struct omap_dss_device *dssdev, u32 wss);
 void omapdss_venc_set_type(struct omap_dss_device *dssdev,
 		enum omap_dss_venc_type type);
+void omapdss_venc_set_vid_out_polarity(struct omap_dss_device *dssdev,
+		enum omap_dss_signal_level vid_out_pol);
 int venc_panel_init(void);
 void venc_panel_exit(void);
 
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 2d90fcf..8cb372f 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -303,6 +303,7 @@ static struct {
 
 	struct omap_video_timings timings;
 	enum omap_dss_venc_type type;
+	enum omap_dss_signal_level vid_out_pol;
 } venc;
 
 static inline void venc_write_reg(int idx, u32 val)
@@ -447,7 +448,7 @@ static int venc_power_on(struct omap_dss_device *dssdev)
 	else /* S-Video */
 		l |= (1 << 0) | (1 << 2);
 
-	if (dssdev->phy.venc.invert_polarity = false)
+	if (venc.vid_out_pol = OMAPDSS_SIG_ACTIVE_HIGH)
 		l |= 1 << 3;
 
 	venc_write_reg(VENC_OUTPUT_CONTROL, l);
@@ -639,6 +640,16 @@ void omapdss_venc_set_type(struct omap_dss_device *dssdev,
 	mutex_unlock(&venc.venc_lock);
 }
 
+void omapdss_venc_set_vid_out_polarity(struct omap_dss_device *dssdev,
+		enum omap_dss_signal_level vid_out_pol)
+{
+	mutex_lock(&venc.venc_lock);
+
+	venc.vid_out_pol = vid_out_pol;
+
+	mutex_unlock(&venc.venc_lock);
+}
+
 static int __init venc_init_display(struct omap_dss_device *dssdev)
 {
 	DSSDBG("init_display\n");
diff --git a/drivers/video/omap2/dss/venc_panel.c b/drivers/video/omap2/dss/venc_panel.c
index ef21361..a8117d0 100644
--- a/drivers/video/omap2/dss/venc_panel.c
+++ b/drivers/video/omap2/dss/venc_panel.c
@@ -118,6 +118,7 @@ static void venc_panel_remove(struct omap_dss_device *dssdev)
 static int venc_panel_enable(struct omap_dss_device *dssdev)
 {
 	int r;
+	enum omap_dss_signal_level vid_out_pol;
 
 	dev_dbg(&dssdev->dev, "venc_panel_enable\n");
 
@@ -131,6 +132,11 @@ static int venc_panel_enable(struct omap_dss_device *dssdev)
 	omapdss_venc_set_timings(dssdev, &dssdev->panel.timings);
 	omapdss_venc_set_type(dssdev, dssdev->phy.venc.type);
 
+	vid_out_pol = dssdev->phy.venc.invert_polarity ?
+			OMAPDSS_SIG_ACTIVE_LOW : OMAPDSS_SIG_ACTIVE_HIGH;
+
+	omapdss_venc_set_vid_out_polarity(dssdev, vid_out_pol);
+
 	r = omapdss_venc_display_enable(dssdev);
 	if (r)
 		goto err;
-- 
1.7.9.5

Re: [PATCH 6/6] OMAPDSS: VENC: Maintian copy of video output polarity in private data

[Tomi Valkeinen]

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

On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
> 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 <archit@ti.com>
> ---
>  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(-)
> 
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index c17d298..b2cf5530 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -479,6 +479,8 @@ u32 omapdss_venc_get_wss(struct omap_dss_device *dssdev);
>  int omapdss_venc_set_wss(struct omap_dss_device *dssdev, u32 wss);
>  void omapdss_venc_set_type(struct omap_dss_device *dssdev,
>  		enum omap_dss_venc_type type);
> +void omapdss_venc_set_vid_out_polarity(struct omap_dss_device *dssdev,
> +		enum omap_dss_signal_level vid_out_pol);
>  int venc_panel_init(void);
>  void venc_panel_exit(void);
>  
> diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
> index 2d90fcf..8cb372f 100644
> --- a/drivers/video/omap2/dss/venc.c
> +++ b/drivers/video/omap2/dss/venc.c
> @@ -303,6 +303,7 @@ static struct {
>  
>  	struct omap_video_timings timings;
>  	enum omap_dss_venc_type type;
> +	enum omap_dss_signal_level vid_out_pol;
>  } venc;
>  
>  static inline void venc_write_reg(int idx, u32 val)
> @@ -447,7 +448,7 @@ static int venc_power_on(struct omap_dss_device *dssdev)
>  	else /* S-Video */
>  		l |= (1 << 0) | (1 << 2);
>  
> -	if (dssdev->phy.venc.invert_polarity == false)
> +	if (venc.vid_out_pol == OMAPDSS_SIG_ACTIVE_HIGH)
>  		l |= 1 << 3;

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


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

Re: [PATCH 6/6] OMAPDSS: VENC: Maintian copy of video output polarity in private data

[Archit Taneja]

On Thursday 16 August 2012 05:08 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
>> 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 <archit@ti.com>
>> ---
>>   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(-)
>>
>> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
>> index c17d298..b2cf5530 100644
>> --- a/drivers/video/omap2/dss/dss.h
>> +++ b/drivers/video/omap2/dss/dss.h
>> @@ -479,6 +479,8 @@ u32 omapdss_venc_get_wss(struct omap_dss_device *dssdev);
>>   int omapdss_venc_set_wss(struct omap_dss_device *dssdev, u32 wss);
>>   void omapdss_venc_set_type(struct omap_dss_device *dssdev,
>>   		enum omap_dss_venc_type type);
>> +void omapdss_venc_set_vid_out_polarity(struct omap_dss_device *dssdev,
>> +		enum omap_dss_signal_level vid_out_pol);
>>   int venc_panel_init(void);
>>   void venc_panel_exit(void);
>>
>> diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
>> index 2d90fcf..8cb372f 100644
>> --- a/drivers/video/omap2/dss/venc.c
>> +++ b/drivers/video/omap2/dss/venc.c
>> @@ -303,6 +303,7 @@ static struct {
>>
>>   	struct omap_video_timings timings;
>>   	enum omap_dss_venc_type type;
>> +	enum omap_dss_signal_level vid_out_pol;
>>   } venc;
>>
>>   static inline void venc_write_reg(int idx, u32 val)
>> @@ -447,7 +448,7 @@ static int venc_power_on(struct omap_dss_device *dssdev)
>>   	else /* S-Video */
>>   		l |= (1 << 0) | (1 << 2);
>>
>> -	if (dssdev->phy.venc.invert_polarity = false)
>> +	if (venc.vid_out_pol = OMAPDSS_SIG_ACTIVE_HIGH)
>>   		l |= 1 << 3;
>
> 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

>
>   Tomi
>

Re: [PATCH 6/6] OMAPDSS: VENC: Maintian copy of video output polarity in private data

[Tomi Valkeinen]

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

On Thu, 2012-08-16 at 17:57 +0530, Archit Taneja wrote:
> On Thursday 16 August 2012 05:08 PM, Tomi Valkeinen wrote:
> > On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
> >> 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 <archit@ti.com>
> >> ---
> >>   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(-)
> >>
> >> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> >> index c17d298..b2cf5530 100644
> >> --- a/drivers/video/omap2/dss/dss.h
> >> +++ b/drivers/video/omap2/dss/dss.h
> >> @@ -479,6 +479,8 @@ u32 omapdss_venc_get_wss(struct omap_dss_device *dssdev);
> >>   int omapdss_venc_set_wss(struct omap_dss_device *dssdev, u32 wss);
> >>   void omapdss_venc_set_type(struct omap_dss_device *dssdev,
> >>   		enum omap_dss_venc_type type);
> >> +void omapdss_venc_set_vid_out_polarity(struct omap_dss_device *dssdev,
> >> +		enum omap_dss_signal_level vid_out_pol);
> >>   int venc_panel_init(void);
> >>   void venc_panel_exit(void);
> >>
> >> diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
> >> index 2d90fcf..8cb372f 100644
> >> --- a/drivers/video/omap2/dss/venc.c
> >> +++ b/drivers/video/omap2/dss/venc.c
> >> @@ -303,6 +303,7 @@ static struct {
> >>
> >>   	struct omap_video_timings timings;
> >>   	enum omap_dss_venc_type type;
> >> +	enum omap_dss_signal_level vid_out_pol;
> >>   } venc;
> >>
> >>   static inline void venc_write_reg(int idx, u32 val)
> >> @@ -447,7 +448,7 @@ static int venc_power_on(struct omap_dss_device *dssdev)
> >>   	else /* S-Video */
> >>   		l |= (1 << 0) | (1 << 2);
> >>
> >> -	if (dssdev->phy.venc.invert_polarity == false)
> >> +	if (venc.vid_out_pol == OMAPDSS_SIG_ACTIVE_HIGH)
> >>   		l |= 1 << 3;
> >
> > 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?

It may be really a bool. TRM says "This may be used to
correct for inversion in an external video amplifier". I don't think
there are any digital on/off signals in analog video output, so I'm
guessing it's really inverting (some parts of) the analog signal. If so,
a boolean invert field sounds correct to me.

Actually, check this out:

http://books.google.fi/books?id=P6BlcWaizHUC&pg=PT81&lpg=PT81&dq=composite+video+polarity&source=bl&ots=-gsl0Exv5R&sig=gMylEnoV9ozRwM4RX2iQFqhRpP8&hl=en&sa=X&ei=0u8sUIe3KYXh4QTf9YDQBA&redir_esc=y#v=onepage&q=composite%20video%20polarity&f=false

A monster url, here's a tinyurl version: http://tinyurl.com/clceb6t

2.16 section there shows signal polarities. I'm not sure if it's the
same one that we're discussing, but sounds like it.

I don't think ACTIVE_LOW/HIGH fits into that kind of polarity. Perhaps a
bool is not quite right for it either, as I'm not sure there's a
"normal" polarity. But I'd go forward with out current bool, and fix it
when somebody who understands this stuff tells us what to do =).

> 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


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

[PATCH 0/3] OMAPDSS: Miscellaneous cleanup patches

[Archit Taneja]

These are minor cleanup patches which will be useful for the future series on
outputs. It's harder to add output entities in DSS when there are more
omap_dss_device references in the driver. The first 2 reduces that a bit. The
third patch just removes some left over fields from omap_dss_device

Refenence Tree:

git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git 1-misc-clean-for-outputs

Archit Taneja (3):
  OMAPDSS: DSI: Pass dsi platform device wherever possible
  OMAPDSS: APPLY: Remove omap_dss_device references in wait_for_go
    functions
  OMAPDSS: Remove unnecessary acb/acbi pin fields from omap_dss_device

 drivers/video/omap2/dss/apply.c |   32 +++++++++-----
 drivers/video/omap2/dss/dsi.c   |   89 +++++++++++++++++----------------------
 include/video/omapdss.h         |    4 --
 3 files changed, 60 insertions(+), 65 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] OMAPDSS: DSI: Pass dsi platform device wherever possible

[Archit Taneja]

Many of the DSI functions receive the connected panel's omap_dss_device pointer
as an argument. The platform device pointer is then derived via omap_dss_device
pointers.

Most of these functions don't really require omap_dss_device pointer anymore
since we now keep copies of parameters in the driver data which were previously
available only via omap_dss_device. Replace the arguments with platform device
pointers for such functions.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   89 ++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 50 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 96d0024..659b6cd 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -2014,9 +2014,8 @@ static unsigned dsi_get_line_buf_size(struct platform_device *dsidev)
 	}
 }
 
-static int dsi_set_lane_config(struct omap_dss_device *dssdev)
+static int dsi_set_lane_config(struct platform_device *dsidev)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	static const u8 offsets[] = { 0, 4, 8, 12, 16 };
 	static const enum dsi_lane_function functions[] = {
@@ -2151,10 +2150,9 @@ static void dsi_cio_timings(struct platform_device *dsidev)
 }
 
 /* lane masks have lane 0 at lsb. mask_p for positive lines, n for negative */
-static void dsi_cio_enable_lane_override(struct omap_dss_device *dssdev,
+static void dsi_cio_enable_lane_override(struct platform_device *dsidev,
 		unsigned mask_p, unsigned mask_n)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	int i;
 	u32 l;
@@ -2201,9 +2199,8 @@ static void dsi_cio_disable_lane_override(struct platform_device *dsidev)
 	REG_FLD_MOD(dsidev, DSI_DSIPHY_CFG10, 0, 22, 17);
 }
 
-static int dsi_cio_wait_tx_clk_esc_reset(struct omap_dss_device *dssdev)
+static int dsi_cio_wait_tx_clk_esc_reset(struct platform_device *dsidev)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	int t, i;
 	bool in_use[DSI_MAX_NR_LANES];
@@ -2251,9 +2248,8 @@ static int dsi_cio_wait_tx_clk_esc_reset(struct omap_dss_device *dssdev)
 }
 
 /* return bitmask of enabled lanes, lane0 being the lsb */
-static unsigned dsi_get_lane_mask(struct omap_dss_device *dssdev)
+static unsigned dsi_get_lane_mask(struct platform_device *dsidev)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	unsigned mask = 0;
 	int i;
@@ -2266,16 +2262,15 @@ static unsigned dsi_get_lane_mask(struct omap_dss_device *dssdev)
 	return mask;
 }
 
-static int dsi_cio_init(struct omap_dss_device *dssdev)
+static int dsi_cio_init(struct platform_device *dsidev)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	int r;
 	u32 l;
 
 	DSSDBGF();
 
-	r = dss_dsi_enable_pads(dsi->module_id, dsi_get_lane_mask(dssdev));
+	r = dss_dsi_enable_pads(dsi->module_id, dsi_get_lane_mask(dsidev));
 	if (r)
 		return r;
 
@@ -2292,7 +2287,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 		goto err_scp_clk_dom;
 	}
 
-	r = dsi_set_lane_config(dssdev);
+	r = dsi_set_lane_config(dsidev);
 	if (r)
 		goto err_scp_clk_dom;
 
@@ -2327,7 +2322,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 			mask_p |= 1 << i;
 		}
 
-		dsi_cio_enable_lane_override(dssdev, mask_p, 0);
+		dsi_cio_enable_lane_override(dsidev, mask_p, 0);
 	}
 
 	r = dsi_cio_power(dsidev, DSI_COMPLEXIO_POWER_ON);
@@ -2344,7 +2339,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 	dsi_if_enable(dsidev, false);
 	REG_FLD_MOD(dsidev, DSI_CLK_CTRL, 1, 20, 20); /* LP_CLK_ENABLE */
 
-	r = dsi_cio_wait_tx_clk_esc_reset(dssdev);
+	r = dsi_cio_wait_tx_clk_esc_reset(dsidev);
 	if (r)
 		goto err_tx_clk_esc_rst;
 
@@ -2385,13 +2380,12 @@ err_cio_pwr:
 		dsi_cio_disable_lane_override(dsidev);
 err_scp_clk_dom:
 	dsi_disable_scp_clk(dsidev);
-	dss_dsi_disable_pads(dsi->module_id, dsi_get_lane_mask(dssdev));
+	dss_dsi_disable_pads(dsi->module_id, dsi_get_lane_mask(dsidev));
 	return r;
 }
 
-static void dsi_cio_uninit(struct omap_dss_device *dssdev)
+static void dsi_cio_uninit(struct platform_device *dsidev)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 
 	/* DDR_CLK_ALWAYS_ON */
@@ -2399,7 +2393,7 @@ static void dsi_cio_uninit(struct omap_dss_device *dssdev)
 
 	dsi_cio_power(dsidev, DSI_COMPLEXIO_POWER_OFF);
 	dsi_disable_scp_clk(dsidev);
-	dss_dsi_disable_pads(dsi->module_id, dsi_get_lane_mask(dssdev));
+	dss_dsi_disable_pads(dsi->module_id, dsi_get_lane_mask(dsidev));
 }
 
 static void dsi_config_tx_fifo(struct platform_device *dsidev,
@@ -2992,10 +2986,9 @@ int dsi_vc_send_null(struct omap_dss_device *dssdev, int channel)
 }
 EXPORT_SYMBOL(dsi_vc_send_null);
 
-static int dsi_vc_write_nosync_common(struct omap_dss_device *dssdev,
+static int dsi_vc_write_nosync_common(struct platform_device *dsidev,
 		int channel, u8 *data, int len, enum dss_dsi_content_type type)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	int r;
 
 	if (len = 0) {
@@ -3026,7 +3019,9 @@ static int dsi_vc_write_nosync_common(struct omap_dss_device *dssdev,
 int dsi_vc_dcs_write_nosync(struct omap_dss_device *dssdev, int channel,
 		u8 *data, int len)
 {
-	return dsi_vc_write_nosync_common(dssdev, channel, data, len,
+	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+
+	return dsi_vc_write_nosync_common(dsidev, channel, data, len,
 			DSS_DSI_CONTENT_DCS);
 }
 EXPORT_SYMBOL(dsi_vc_dcs_write_nosync);
@@ -3034,7 +3029,9 @@ EXPORT_SYMBOL(dsi_vc_dcs_write_nosync);
 int dsi_vc_generic_write_nosync(struct omap_dss_device *dssdev, int channel,
 		u8 *data, int len)
 {
-	return dsi_vc_write_nosync_common(dssdev, channel, data, len,
+	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+
+	return dsi_vc_write_nosync_common(dsidev, channel, data, len,
 			DSS_DSI_CONTENT_GENERIC);
 }
 EXPORT_SYMBOL(dsi_vc_generic_write_nosync);
@@ -3045,7 +3042,7 @@ static int dsi_vc_write_common(struct omap_dss_device *dssdev, int channel,
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	int r;
 
-	r = dsi_vc_write_nosync_common(dssdev, channel, data, len, type);
+	r = dsi_vc_write_nosync_common(dsidev, channel, data, len, type);
 	if (r)
 		goto err;
 
@@ -3123,10 +3120,9 @@ int dsi_vc_generic_write_2(struct omap_dss_device *dssdev, int channel,
 }
 EXPORT_SYMBOL(dsi_vc_generic_write_2);
 
-static int dsi_vc_dcs_send_read_request(struct omap_dss_device *dssdev,
+static int dsi_vc_dcs_send_read_request(struct platform_device *dsidev,
 		int channel, u8 dcs_cmd)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	int r;
 
@@ -3144,10 +3140,9 @@ static int dsi_vc_dcs_send_read_request(struct omap_dss_device *dssdev,
 	return 0;
 }
 
-static int dsi_vc_generic_send_read_request(struct omap_dss_device *dssdev,
+static int dsi_vc_generic_send_read_request(struct platform_device *dsidev,
 		int channel, u8 *reqdata, int reqlen)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	u16 data;
 	u8 data_type;
@@ -3296,7 +3291,7 @@ int dsi_vc_dcs_read(struct omap_dss_device *dssdev, int channel, u8 dcs_cmd,
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	int r;
 
-	r = dsi_vc_dcs_send_read_request(dssdev, channel, dcs_cmd);
+	r = dsi_vc_dcs_send_read_request(dsidev, channel, dcs_cmd);
 	if (r)
 		goto err;
 
@@ -3327,7 +3322,7 @@ static int dsi_vc_generic_read(struct omap_dss_device *dssdev, int channel,
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	int r;
 
-	r = dsi_vc_generic_send_read_request(dssdev, channel, reqdata, reqlen);
+	r = dsi_vc_generic_send_read_request(dsidev, channel, reqdata, reqlen);
 	if (r)
 		return r;
 
@@ -3609,14 +3604,12 @@ static void dsi_set_hs_tx_timeout(struct platform_device *dsidev,
 			(total_ticks * 1000) / (fck / 1000 / 1000));
 }
 
-static void dsi_config_vp_num_line_buffers(struct omap_dss_device *dssdev)
+static void dsi_config_vp_num_line_buffers(struct platform_device *dsidev)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	int num_line_buffers;
 
 	if (dsi->mode = OMAP_DSS_DSI_VIDEO_MODE) {
-		struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 		int bpp = dsi_get_pixel_size(dsi->pix_fmt);
 		unsigned line_buf_size = dsi_get_line_buf_size(dsidev);
 		struct omap_video_timings *timings = &dsi->timings;
@@ -3637,9 +3630,8 @@ static void dsi_config_vp_num_line_buffers(struct omap_dss_device *dssdev)
 	REG_FLD_MOD(dsidev, DSI_CTRL, num_line_buffers, 13, 12);
 }
 
-static void dsi_config_vp_sync_events(struct omap_dss_device *dssdev)
+static void dsi_config_vp_sync_events(struct platform_device *dsidev)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	bool vsync_end = dsi->vm_timings.vp_vsync_end;
 	bool hsync_end = dsi->vm_timings.vp_hsync_end;
@@ -3656,9 +3648,8 @@ static void dsi_config_vp_sync_events(struct omap_dss_device *dssdev)
 	dsi_write_reg(dsidev, DSI_CTRL, r);
 }
 
-static void dsi_config_blanking_modes(struct omap_dss_device *dssdev)
+static void dsi_config_blanking_modes(struct platform_device *dsidev)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	int blanking_mode = dsi->vm_timings.blanking_mode;
 	int hfp_blanking_mode = dsi->vm_timings.hfp_blanking_mode;
@@ -3913,11 +3904,11 @@ static int dsi_proto_config(struct omap_dss_device *dssdev)
 
 	dsi_write_reg(dsidev, DSI_CTRL, r);
 
-	dsi_config_vp_num_line_buffers(dssdev);
+	dsi_config_vp_num_line_buffers(dsidev);
 
 	if (dsi->mode = OMAP_DSS_DSI_VIDEO_MODE) {
-		dsi_config_vp_sync_events(dssdev);
-		dsi_config_blanking_modes(dssdev);
+		dsi_config_vp_sync_events(dsidev);
+		dsi_config_blanking_modes(dsidev);
 		dsi_config_cmd_mode_interleaving(dssdev);
 	}
 
@@ -3929,9 +3920,8 @@ static int dsi_proto_config(struct omap_dss_device *dssdev)
 	return 0;
 }
 
-static void dsi_proto_timings(struct omap_dss_device *dssdev)
+static void dsi_proto_timings(struct platform_device *dsidev)
 {
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	unsigned tlpx, tclk_zero, tclk_prepare, tclk_trail;
 	unsigned tclk_pre, tclk_post;
@@ -4312,8 +4302,7 @@ static void dsi_framedone_timeout_work_callback(struct work_struct *work)
 
 static void dsi_framedone_irq_callback(void *data, u32 mask)
 {
-	struct omap_dss_device *dssdev = (struct omap_dss_device *) data;
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct platform_device *dsidev = (struct platform_device *) data;
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 
 	/* Note: We get FRAMEDONE when DISPC has finished sending pixels and
@@ -4397,7 +4386,7 @@ static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
 		irq = dispc_mgr_get_framedone_irq(dssdev->manager->id);
 
 		r = omap_dispc_register_isr(dsi_framedone_irq_callback,
-			(void *) dssdev, irq);
+			(void *) dsidev, irq);
 		if (r) {
 			DSSERR("can't get FRAMEDONE irq\n");
 			goto err;
@@ -4438,7 +4427,7 @@ static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
 err1:
 	if (dsi->mode = OMAP_DSS_DSI_CMD_MODE)
 		omap_dispc_unregister_isr(dsi_framedone_irq_callback,
-			(void *) dssdev, irq);
+			(void *) dsidev, irq);
 err:
 	return r;
 }
@@ -4454,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,
-			(void *) dssdev, irq);
+			(void *) dsidev, irq);
 	}
 }
 
@@ -4504,13 +4493,13 @@ static int dsi_display_init_dsi(struct omap_dss_device *dssdev)
 
 	DSSDBG("PLL OK\n");
 
-	r = dsi_cio_init(dssdev);
+	r = dsi_cio_init(dsidev);
 	if (r)
 		goto err2;
 
 	_dsi_print_reset_status(dsidev);
 
-	dsi_proto_timings(dssdev);
+	dsi_proto_timings(dsidev);
 	dsi_set_lp_clk_divisor(dssdev);
 
 	if (1)
@@ -4530,7 +4519,7 @@ static int dsi_display_init_dsi(struct omap_dss_device *dssdev)
 
 	return 0;
 err3:
-	dsi_cio_uninit(dssdev);
+	dsi_cio_uninit(dsidev);
 err2:
 	dss_select_dispc_clk_source(OMAP_DSS_CLK_SRC_FCK);
 	dss_select_dsi_clk_source(dsi->module_id, OMAP_DSS_CLK_SRC_FCK);
@@ -4561,7 +4550,7 @@ static void dsi_display_uninit_dsi(struct omap_dss_device *dssdev,
 	dss_select_dispc_clk_source(OMAP_DSS_CLK_SRC_FCK);
 	dss_select_dsi_clk_source(dsi->module_id, OMAP_DSS_CLK_SRC_FCK);
 	dss_select_lcd_clk_source(dssdev->manager->id, OMAP_DSS_CLK_SRC_FCK);
-	dsi_cio_uninit(dssdev);
+	dsi_cio_uninit(dsidev);
 	dsi_pll_uninit(dsidev, disconnect_lanes);
 }
 
-- 
1.7.9.5

[PATCH 2/3] OMAPDSS: APPLY: Remove omap_dss_device references in wait_for_go functions

[Archit Taneja]

The functions dss_mgr_wait_for_go() and dss_mgr_wait_for_go_ovl() check if there
is an enabled display connected to the manager before trying to see the state of
the GO bit.

The checks related to the display can be replaced by checking the state of the
manager, i.e, whether the manager is enabled or not. This makes more sense than
checking with the connected display as the GO bit behaviour is more connected
with the manager state rather than the display state. A GO bit can only be set
if the manager is enabled. If a manager isn't enabled, we can safely assume that
the GO bit is not set.

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

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 52a5940..74f1a58 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -424,17 +424,23 @@ static void wait_pending_extra_info_updates(void)
 int dss_mgr_wait_for_go(struct omap_overlay_manager *mgr)
 {
 	unsigned long timeout = msecs_to_jiffies(500);
-	struct mgr_priv_data *mp;
+	struct mgr_priv_data *mp = get_mgr_priv(mgr);
 	u32 irq;
+	unsigned long flags;
 	int r;
 	int i;
-	struct omap_dss_device *dssdev = mgr->device;
 
-	if (!dssdev || dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
+	if (mgr_manual_update(mgr))
 		return 0;
 
-	if (mgr_manual_update(mgr))
+	spin_lock_irqsave(&data_lock, flags);
+
+	if (!mp->enabled) {
+		spin_unlock_irqrestore(&data_lock, flags);
 		return 0;
+	}
+
+	spin_unlock_irqrestore(&data_lock, flags);
 
 	r = dispc_runtime_get();
 	if (r)
@@ -442,10 +448,8 @@ int dss_mgr_wait_for_go(struct omap_overlay_manager *mgr)
 
 	irq = dispc_mgr_get_vsync_irq(mgr->id);
 
-	mp = get_mgr_priv(mgr);
 	i = 0;
 	while (1) {
-		unsigned long flags;
 		bool shadow_dirty, dirty;
 
 		spin_lock_irqsave(&data_lock, flags);
@@ -489,21 +493,28 @@ int dss_mgr_wait_for_go_ovl(struct omap_overlay *ovl)
 {
 	unsigned long timeout = msecs_to_jiffies(500);
 	struct ovl_priv_data *op;
-	struct omap_dss_device *dssdev;
+	struct mgr_priv_data *mp;
 	u32 irq;
+	unsigned long flags;
 	int r;
 	int i;
 
 	if (!ovl->manager)
 		return 0;
 
-	dssdev = ovl->manager->device;
+	mp = get_mgr_priv(ovl->manager);
 
-	if (!dssdev || dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
+	if (ovl_manual_update(ovl))
 		return 0;
 
-	if (ovl_manual_update(ovl))
+	spin_lock_irqsave(&data_lock, flags);
+
+	if (!mp->enabled) {
+		spin_unlock_irqrestore(&data_lock, flags);
 		return 0;
+	}
+
+	spin_unlock_irqrestore(&data_lock, flags);
 
 	r = dispc_runtime_get();
 	if (r)
@@ -514,7 +525,6 @@ int dss_mgr_wait_for_go_ovl(struct omap_overlay *ovl)
 	op = get_ovl_priv(ovl);
 	i = 0;
 	while (1) {
-		unsigned long flags;
 		bool shadow_dirty, dirty;
 
 		spin_lock_irqsave(&data_lock, flags);
-- 
1.7.9.5

Re: [PATCH 2/3] OMAPDSS: APPLY: Remove omap_dss_device references in wait_for_go functions

[Tomi Valkeinen]

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

On Fri, 2012-08-17 at 16:19 +0530, Archit Taneja wrote:
> The functions dss_mgr_wait_for_go() and dss_mgr_wait_for_go_ovl() check if there
> is an enabled display connected to the manager before trying to see the state of
> the GO bit.
> 
> The checks related to the display can be replaced by checking the state of the
> manager, i.e, whether the manager is enabled or not. This makes more sense than
> checking with the connected display as the GO bit behaviour is more connected
> with the manager state rather than the display state. A GO bit can only be set
> if the manager is enabled. If a manager isn't enabled, we can safely assume that
> the GO bit is not set.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  drivers/video/omap2/dss/apply.c |   32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index 52a5940..74f1a58 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -424,17 +424,23 @@ static void wait_pending_extra_info_updates(void)
>  int dss_mgr_wait_for_go(struct omap_overlay_manager *mgr)
>  {
>  	unsigned long timeout = msecs_to_jiffies(500);
> -	struct mgr_priv_data *mp;
> +	struct mgr_priv_data *mp = get_mgr_priv(mgr);
>  	u32 irq;
> +	unsigned long flags;
>  	int r;
>  	int i;
> -	struct omap_dss_device *dssdev = mgr->device;
>  
> -	if (!dssdev || dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
> +	if (mgr_manual_update(mgr))

This needs to be inside the spinlock also.

>  		return 0;
>  
> -	if (mgr_manual_update(mgr))
> +	spin_lock_irqsave(&data_lock, flags);
> +
> +	if (!mp->enabled) {
> +		spin_unlock_irqrestore(&data_lock, flags);
>  		return 0;
> +	}
> +
> +	spin_unlock_irqrestore(&data_lock, flags);
>  
>  	r = dispc_runtime_get();
>  	if (r)
> @@ -442,10 +448,8 @@ int dss_mgr_wait_for_go(struct omap_overlay_manager *mgr)
>  
>  	irq = dispc_mgr_get_vsync_irq(mgr->id);
>  
> -	mp = get_mgr_priv(mgr);
>  	i = 0;
>  	while (1) {
> -		unsigned long flags;
>  		bool shadow_dirty, dirty;
>  
>  		spin_lock_irqsave(&data_lock, flags);
> @@ -489,21 +493,28 @@ int dss_mgr_wait_for_go_ovl(struct omap_overlay *ovl)
>  {
>  	unsigned long timeout = msecs_to_jiffies(500);
>  	struct ovl_priv_data *op;
> -	struct omap_dss_device *dssdev;
> +	struct mgr_priv_data *mp;
>  	u32 irq;
> +	unsigned long flags;
>  	int r;
>  	int i;
>  
>  	if (!ovl->manager)
>  		return 0;

And this should be inside spinlock (yes, you didn't change that, but now
that you're changing these... =)
 
> -	dssdev = ovl->manager->device;
> +	mp = get_mgr_priv(ovl->manager);
>  
> -	if (!dssdev || dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
> +	if (ovl_manual_update(ovl))

Inside spinlock here too.

Actually, shouldn't the whole wait_for functions be locked with the
apply mutex? Otherwise the output can be disabled/changed while waiting.

On the other hand, that could be quite a long lock, and I don't see
anything in the code that could really break if the output is disabled
or similar. Perhaps it's fine to just hit the timeout in case something
has been changed. If we add a mutex, we risk breaking something that
currently works =).

 Tomi


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

[PATCH 3/3] OMAPDSS: Remove unnecessary acb/acbi pin fields from omap_dss_device

[Archit Taneja]

Passive matrix support was removed recently. The acb and acbi pin declarations
in omap_dss_device struct weren't removed by accident. Remove these fields
from omap_dss_device.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 include/video/omapdss.h |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index b868123..bc686f4 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -558,10 +558,6 @@ struct omap_dss_device {
 	struct {
 		struct omap_video_timings timings;
 
-		int acbi;	/* ac-bias pin transitions per interrupt */
-		/* Unit: line clocks */
-		int acb;	/* ac-bias pin frequency */
-
 		enum omap_dss_dsi_pixel_format dsi_pix_fmt;
 		enum omap_dss_dsi_mode dsi_mode;
 		struct omap_dss_dsi_videomode_timings dsi_vm_timings;
-- 
1.7.9.5