Re: [PATCH v3 4/9] OMAP4 : DSS2 : HDMI: HDMI driver header file addition

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

On Fri, 2011-03-04 at 01:48 -0600, K, Mythri P wrote:
> Adding the hdmi interface driver header file (hdmi.h) to the dss driver.
> Register and timing declaration to be used by the corresponding c file
> is added in this file.
> 
> Signed-off-by: Mythri P K <mythripk@ti.com>
> ---
>  drivers/video/omap2/dss/hdmi.h |  444 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 444 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/video/omap2/dss/hdmi.h

<snip>

> +struct hdmi_video_format {
> +       enum hdmi_packing_mode  packing_mode;
> +       u32                     y_res;  /* Line per panel */
> +       u32                     x_res;  /* pixel per line */
> +};
> +
> +struct hdmi_video_interface {
> +       int     vsp;    /* Vsync polarity */
> +       int     hsp;    /* Hsync polarity */
> +       int     interlacing;
> +       int     tm;     /* Timing mode */
> +};
> +
> +struct hdmi_video_timing {
> +       u32     hbp;
> +       u32     hfp;
> +       u32     hsw;
> +       u32     vbp;
> +       u32     vfp;
> +       u32     vsw;
> +};
> +
> +struct hdmi_config {
> +       u16     ppl;    /* pixel per line */
> +       u16     lpp;    /* line per panel */
> +       u32     pixel_clock;
> +       u16     hsw;    /* Horizontal sync pulse width */
> +       u16     hfp;    /* Horizontal front porch */
> +       u16     hbp;    /* Horizontal back porch */
> +       u16     vsw;    /* Vertical sync pulse width */
> +       u16     vfp;    /* Vertical front porch */
> +       u16     vbp;    /* Vertical back porch */
> +       u16     interlace;
> +       u16     h_pol;
> +       u16     v_pol;
> +       u16     hdmi_dvi;
> +       u16     video_format;
> +};

I haven't really read through the hdmi code (as it doesn't apply to my
tree), but it looks quite strange to have multiple structs with
overlapping information. And then there's also the struct
omap_video_timings, which contains similar stuff.

So, could the hdmi_config use hdmi_video_timing, hdmi_video_interface
and hdmi_video_format structs? Can omap_video_timings be used here?
Should omap_video_timings be modified somehow to suit HDMI better?

> +
> +struct hdmi_cm {
> +       int     code;
> +       int     mode;
> +};
> +
> +struct hdmi_irq_vector {
> +       u8      pll_recal;
> +       u8      pll_unlock;
> +       u8      pll_lock;
> +       u8      phy_disconnect;
> +       u8      phy_connect;
> +       u8      phy_short_5v;
> +       u8      video_end_fr;
> +       u8      video_vsync;
> +       u8      fifo_sample_req;
> +       u8      fifo_overflow;
> +       u8      fifo_underflow;
> +       u8      ocp_timeout;
> +       u8      core;
> +};

This struct is not used.

 Tomi