Re: [PATCH 17/17] OMAPDSS: DSI: Remove redundant fields in omap_dss_dsi_videomode_data
On Tue, 2012-06-26 at 15:06 +0530, Archit Taneja wrote:
The struct omap_dss_dsi_videomode_data holds polaritiy/logic level information
of the DISPC video port signals DE, HSYNC and VSYNC. This information already
exists in the omap_video_timings struct.
Use the fields in omap_video_timings to program VP_DE_POL, VP_HSYNC_POL and
VP_VSYNC_POL in DSI_CTRL. Remove the redundant fields in
omap_dss_dsi_videomode_data.
Signed-off-by: Archit Taneja arc...@ti.com
---
drivers/video/omap2/dss/dsi.c | 11 ---
include/video/omapdss.h |3 ---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 061bf53..3844430 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -3628,13 +3628,18 @@ 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);
- int de_pol = dssdev-panel.dsi_vm_data.vp_de_pol;
- int hsync_pol = dssdev-panel.dsi_vm_data.vp_hsync_pol;
- int vsync_pol = dssdev-panel.dsi_vm_data.vp_vsync_pol;
+ int de_pol, hsync_pol, vsync_pol;
+ int de_level = dssdev-panel.timings.de_level;
+ int hsync_level = dssdev-panel.timings.hsync_level;
+ int vsync_level = dssdev-panel.timings.vsync_level;
bool vsync_end = dssdev-panel.dsi_vm_data.vp_vsync_end;
bool hsync_end = dssdev-panel.dsi_vm_data.vp_hsync_end;
u32 r;
+ de_pol = de_level == OMAPDSS_SIG_ACTIVE_HIGH ? 1 : 0;
+ hsync_pol = hsync_level == OMAPDSS_SIG_ACTIVE_HIGH ? 1 : 0;
+ vsync_pol = vsync_level == OMAPDSS_SIG_ACTIVE_HIGH ? 1 : 0;
+
r = dsi_read_reg(dsidev, DSI_CTRL);
r = FLD_MOD(r, de_pol, 9, 9); /* VP_DE_POL */
r = FLD_MOD(r, hsync_pol, 10, 10); /* VP_HSYNC_POL */
This patch makes the code cleaner, but I find this DSI sync code a bit
strange.
The sync polarities between DISPC and DSI do not matter elsewhere, they
do not affect the DSI output, so why do we have them in the panel data?
Why doesn't dsi.c just use some hardcoded values for these.
Tomi
signature.asc
Description: This is a digitally signed message part
Re: [PATCH 17/17] OMAPDSS: DSI: Remove redundant fields in omap_dss_dsi_videomode_data
On Wednesday 27 June 2012 05:35 PM, Tomi Valkeinen wrote:
On Tue, 2012-06-26 at 15:06 +0530, Archit Taneja wrote:
The struct omap_dss_dsi_videomode_data holds polaritiy/logic level information
of the DISPC video port signals DE, HSYNC and VSYNC. This information already
exists in the omap_video_timings struct.
Use the fields in omap_video_timings to program VP_DE_POL, VP_HSYNC_POL and
VP_VSYNC_POL in DSI_CTRL. Remove the redundant fields in
omap_dss_dsi_videomode_data.
Signed-off-by: Archit Tanejaarc...@ti.com
---
drivers/video/omap2/dss/dsi.c | 11 ---
include/video/omapdss.h |3 ---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 061bf53..3844430 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -3628,13 +3628,18 @@ 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);
- int de_pol = dssdev-panel.dsi_vm_data.vp_de_pol;
- int hsync_pol = dssdev-panel.dsi_vm_data.vp_hsync_pol;
- int vsync_pol = dssdev-panel.dsi_vm_data.vp_vsync_pol;
+ int de_pol, hsync_pol, vsync_pol;
+ int de_level = dssdev-panel.timings.de_level;
+ int hsync_level = dssdev-panel.timings.hsync_level;
+ int vsync_level = dssdev-panel.timings.vsync_level;
bool vsync_end = dssdev-panel.dsi_vm_data.vp_vsync_end;
bool hsync_end = dssdev-panel.dsi_vm_data.vp_hsync_end;
u32 r;
+ de_pol = de_level == OMAPDSS_SIG_ACTIVE_HIGH ? 1 : 0;
+ hsync_pol = hsync_level == OMAPDSS_SIG_ACTIVE_HIGH ? 1 : 0;
+ vsync_pol = vsync_level == OMAPDSS_SIG_ACTIVE_HIGH ? 1 : 0;
+
r = dsi_read_reg(dsidev, DSI_CTRL);
r = FLD_MOD(r, de_pol, 9, 9); /* VP_DE_POL */
r = FLD_MOD(r, hsync_pol, 10, 10); /* VP_HSYNC_POL */
This patch makes the code cleaner, but I find this DSI sync code a bit
strange.
The sync polarities between DISPC and DSI do not matter elsewhere, they
do not affect the DSI output, so why do we have them in the panel data?
Why doesn't dsi.c just use some hardcoded values for these.
Ok, are you saying that unlike DPI, where a panel may request for some
different polarities. There is no such need for DSI panels, and we can
set the polarities of DISPC and DSI always to active high(or any other
combination)?
Well, we are doing that indirectly in a way, a DSI panel driver would
populate a omap_video_timings struct, and would leave hsync_level,
vsync_level and de_level empty(i.e, the default values). This would be
passed to the DSI driver, and the timings would be applied to DISPC. The
function above would just pick up the same default values and program to
the DSI registers.
What we could do is ignore these fields in the omap_video_timings when
passed from the panel driver to DSI driver, and always use a fixed value
for them, and this way we can use the same fixed values for DSI too. Do
you think that is better?
Archit
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/17] OMAPDSS: DSI: Remove redundant fields in omap_dss_dsi_videomode_data
On Wed, 2012-06-27 at 17:48 +0530, Archit Taneja wrote: On Wednesday 27 June 2012 05:35 PM, Tomi Valkeinen wrote: The sync polarities between DISPC and DSI do not matter elsewhere, they do not affect the DSI output, so why do we have them in the panel data? Why doesn't dsi.c just use some hardcoded values for these. Ok, are you saying that unlike DPI, where a panel may request for some different polarities. There is no such need for DSI panels, and we can set the polarities of DISPC and DSI always to active high(or any other combination)? Yes. There are no sync polarities in DSI bus, there are only sync packets. So afaik, the polarities used here matter only for DISPC and DSI communication. And there the only thing that matters is that both DISPC and DSI have the same configuration for the polarities, so that the communication works. Well, we are doing that indirectly in a way, a DSI panel driver would populate a omap_video_timings struct, and would leave hsync_level, vsync_level and de_level empty(i.e, the default values). This would be passed to the DSI driver, and the timings would be applied to DISPC. The function above would just pick up the same default values and program to the DSI registers. What we could do is ignore these fields in the omap_video_timings when passed from the panel driver to DSI driver, and always use a fixed value for them, and this way we can use the same fixed values for DSI too. Do you think that is better? I think that is clearer. Optimally we wouldn't even have a video timings struct for DSI panels, the kind that contains sync polarities and such, but a separate timings struct that contains stuff relevant for DSI. But for now I think we should just ignore the extra values in video timings. Tomi signature.asc Description: This is a digitally signed message part
[PATCH 17/17] OMAPDSS: DSI: Remove redundant fields in omap_dss_dsi_videomode_data
The struct omap_dss_dsi_videomode_data holds polaritiy/logic level information
of the DISPC video port signals DE, HSYNC and VSYNC. This information already
exists in the omap_video_timings struct.
Use the fields in omap_video_timings to program VP_DE_POL, VP_HSYNC_POL and
VP_VSYNC_POL in DSI_CTRL. Remove the redundant fields in
omap_dss_dsi_videomode_data.
Signed-off-by: Archit Taneja arc...@ti.com
---
drivers/video/omap2/dss/dsi.c | 11 ---
include/video/omapdss.h |3 ---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 061bf53..3844430 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -3628,13 +3628,18 @@ 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);
- int de_pol = dssdev-panel.dsi_vm_data.vp_de_pol;
- int hsync_pol = dssdev-panel.dsi_vm_data.vp_hsync_pol;
- int vsync_pol = dssdev-panel.dsi_vm_data.vp_vsync_pol;
+ int de_pol, hsync_pol, vsync_pol;
+ int de_level = dssdev-panel.timings.de_level;
+ int hsync_level = dssdev-panel.timings.hsync_level;
+ int vsync_level = dssdev-panel.timings.vsync_level;
bool vsync_end = dssdev-panel.dsi_vm_data.vp_vsync_end;
bool hsync_end = dssdev-panel.dsi_vm_data.vp_hsync_end;
u32 r;
+ de_pol = de_level == OMAPDSS_SIG_ACTIVE_HIGH ? 1 : 0;
+ hsync_pol = hsync_level == OMAPDSS_SIG_ACTIVE_HIGH ? 1 : 0;
+ vsync_pol = vsync_level == OMAPDSS_SIG_ACTIVE_HIGH ? 1 : 0;
+
r = dsi_read_reg(dsidev, DSI_CTRL);
r = FLD_MOD(r, de_pol, 9, 9); /* VP_DE_POL */
r = FLD_MOD(r, hsync_pol, 10, 10); /* VP_HSYNC_POL */
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 1498bb3..f627ad9 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -255,9 +255,6 @@ struct omap_dss_dsi_videomode_data {
int hfp_blanking_mode;
/* Video port sync events */
- int vp_de_pol;
- int vp_hsync_pol;
- int vp_vsync_pol;
bool vp_vsync_end;
bool vp_hsync_end;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
