from:"K, Mythri P"

Hi,

On Mon, Sep 5, 2011 at 11:03 PM, K, Mythri P mythr...@ti.com wrote:
 Hi,

 On Mon, Sep 5, 2011 at 4:31 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-09-02 at 16:17 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com

 To make the current hdmi DSS driver compatible with future OMAP having
 different IP blocks( A combination of different core, PHY, PLL blocks),
 Add HDMI hdmi functions as a function pointer in dss_features to abstract
 hdmi dss driver IP agnostic, hdmi dss driver  which will now access
 generic functions irrespective of underlying IP.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/dss_features.c |   20 
  drivers/video/omap2/dss/dss_features.h |    3 +++
  drivers/video/omap2/dss/hdmi.c         |   19 ++-
  include/video/omapdss.h                |   23 +++
  include/video/ti_hdmi.h                |    1 +
  5 files changed, 57 insertions(+), 9 deletions(-)

 diff --git a/drivers/video/omap2/dss/dss_features.c 
 b/drivers/video/omap2/dss/dss_features.c
 index b63c5f8..4d50b30 100644
 --- a/drivers/video/omap2/dss/dss_features.c
 +++ b/drivers/video/omap2/dss/dss_features.c
 @@ -429,6 +429,26 @@ static const struct omap_dss_features 
 omap4_dss_features = {
       .burst_size_unit = 16,
  };

 +#if defined(CONFIG_OMAP4_DSS_HDMI)
 +/* HDMI OMAP4 Functions*/
 +static const struct omap_hdmi_ip_ops omap4_hdmi_functions = {
 +
 +     .video_configure        =       ti_hdmi_4xxx_basic_configure,
 +     .phy_enable             =       ti_hdmi_4xxx_phy_enable,
 +     .phy_disable            =       ti_hdmi_4xxx_phy_disable,
 +     .read_edid              =       ti_hdmi_4xxx_read_edid,
 +     .pll_enable             =       ti_hdmi_4xxx_pll_enable,
 +     .pll_disable            =       ti_hdmi_4xxx_pll_disable,
 +     .video_enable           =       ti_hdmi_4xxx_wp_video_start,
 +};
 +
 +void dss_init_hdmi_ip_ops(struct hdmi_ip_data *ip_data)
 +{
 +     if (cpu_is_omap44xx())
 +             ip_data-ops = omap4_hdmi_functions;
 +}
 +#endif
 +
  /* Functions returning values related to a DSS feature */
  int dss_feat_get_num_mgrs(void)
  {
 diff --git a/drivers/video/omap2/dss/dss_features.h 
 b/drivers/video/omap2/dss/dss_features.h
 index 4271e96..8b74f69 100644
 --- a/drivers/video/omap2/dss/dss_features.h
 +++ b/drivers/video/omap2/dss/dss_features.h
 @@ -99,4 +99,7 @@ u32 dss_feat_get_burst_size_unit(void);             /* in 
 bytes */
  bool dss_has_feature(enum dss_feat_id id);
  void dss_feat_get_reg_field(enum dss_feat_reg_field id, u8 *start, u8 
 *end);
  void dss_features_init(void);
 +#if defined(CONFIG_OMAP4_DSS_HDMI)
 +void dss_init_hdmi_ip_ops(struct hdmi_ip_data *ip_data);
 +#endif
  #endif
 diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
 index 1b989b3..cb54925 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -185,6 +185,7 @@ int hdmi_init_display(struct omap_dss_device *dssdev)
  {
       DSSDBG(init_display\n);

 +     dss_init_hdmi_ip_ops(hdmi.hdmi_data);
       return 0;
  }

 @@ -365,7 +366,7 @@ static void hdmi_read_edid(struct omap_video_timings 
 *dp)
       memset(hdmi.edid, 0, HDMI_EDID_MAX_LENGTH);

       if (!hdmi.edid_set)
 -             ret = ti_hdmi_4xxx_read_edid(hdmi.hdmi_data, hdmi.edid,
 +             ret = hdmi.hdmi_data.ops-read_edid(hdmi.hdmi_data, 
 hdmi.edid,
                                               HDMI_EDID_MAX_LENGTH);
       if (!ret) {
               if (!memcmp(hdmi.edid, edid_header, sizeof(edid_header))) {
 @@ -479,16 +480,16 @@ static int hdmi_power_on(struct omap_dss_device 
 *dssdev)

       hdmi_compute_pll(dssdev, phy, hdmi.hdmi_data.pll_data);

 -     ti_hdmi_4xxx_wp_video_start(hdmi.hdmi_data, 0);
 +     hdmi.hdmi_data.ops-video_enable(hdmi.hdmi_data, 0);

       /* config the PLL and PHY hdmi_set_pll_pwrfirst */
 -     r = ti_hdmi_4xxx_pll_enable(hdmi.hdmi_data);
 +     r = hdmi.hdmi_data.ops-pll_enable(hdmi.hdmi_data);
       if (r) {
               DSSDBG(Failed to lock PLL\n);
               goto err;
       }

 -     r = ti_hdmi_4xxx_phy_enable(hdmi.hdmi_data);
 +     r = hdmi.hdmi_data.ops-phy_enable(hdmi.hdmi_data);
       if (r) {
               DSSDBG(Failed to start PHY\n);
               goto err;
 @@ -496,7 +497,7 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)

       hdmi.hdmi_data.cfg.cm.mode = hdmi.mode;
       hdmi.hdmi_data.cfg.cm.code = hdmi.code;
 -     ti_hdmi_4xxx_basic_configure(hdmi.hdmi_data);
 +     hdmi.hdmi_data.ops-video_configure(hdmi.hdmi_data);

       /* Make selection of HDMI in DSS */
       dss_select_hdmi_venc_clk_source(DSS_HDMI_M_PCLK);
 @@ -518,7 +519,7 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)

       dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 1);

 -     ti_hdmi_4xxx_wp_video_start(hdmi.hdmi_data, 1);
 +     hdmi.hdmi_data.ops-video_enable(hdmi.hdmi_data, 1);

       return 0;
  err

Re: [PATCH v3 00/10]HDMI: Split hdmi.c in DSS to seperate OMAP dependent

Hi,

On Mon, Sep 5, 2011 at 6:52 PM, K, Mythri P mythr...@ti.com wrote:
 Hi,

 On Mon, Sep 5, 2011 at 5:45 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-09-02 at 16:17 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com

 HDMI IP block is common between TI OMAP4 Procerssor and Netra processor 
 although
 the Display subsytem is different.Also the IP block in future OMAP may 
 differ
 from the one existing in OMAP4. Thus to reuse the code between these two
 processors , and maintain the multi omap build functionality in DSS.
 HDMI IP dependant code is seperated out from hdmi.c and moved to
 new library file hdmi_ti_4xxx_ip.c.From the DSS dependent HDMI code only the
 function pointer to functionality/features offered by HDMI is called.

 This patch series does the split and also renames hdmi_omap4_panel.c to
 hdmi_panel.c as that file has nothing specific to OMAP4 and can be reused 
 for
 other OMAP family of processors as well.

 This patch series is based on Tomi's LO-DSS2 master branch.

 The kernel doesn't compile at all after enabling
 SND_OMAP_SOC_OMAP4_HDMI. Looks like you didn't move any of the sound
 stuff, but just left them in hdmi.c file.

 I shall check this, may be i should handle some dependency but i didnt
 think there were any direct dependency , i guess you need to enable
 some audio files as well? let me check.

There is a dependency on the audio files on the register writers and i
was not very sure on what would be a genuine API v's what can be a
static function.
Ricardo any comments would you like to post a patch on top of this series ?

thanks and regards,
Mythri.
  Tomi




--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 09/10] OMAP4: DSS2: HDMI: Function pointer approach to call

Hi,

On Tue, Sep 6, 2011 at 3:47 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2011-09-06 at 11:38 +0530, K, Mythri P wrote:
  Did you consider how the code would look if the function pointers
 were
  just included into struct hdmi_ip_data, without any ops struct at
 all?
 
 I tried without ops structure , but then using it in dss_features to
 initialize would be a problem so i have moved it to ti_hdmi.h.

 I'm fine with the ops struct, but I don't see why it would be a problem
 without it. You'd just pass the hdmi_ip_data to dss_features, which
 would initialize it.
well instead of passing the struct ops if the function pointers are
included in ip_data directly, in dss_features each function will have
to be initialized in the dss_init_hdmi_ip_ops function i thought it is
cleaner to initialize the struct :-) ,it just looked cleaner to me
:-). else the entire intialization will come in a if else condition
for CPU's.

Thanks and regards,
Mythri.

  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 00/10]HDMI: Split hdmi.c in DSS to seperate OMAP dependent

Hi,

On Tue, Sep 6, 2011 at 3:09 PM, K, Mythri P mythr...@ti.com wrote:
 Hi,

 On Mon, Sep 5, 2011 at 6:52 PM, K, Mythri P mythr...@ti.com wrote:
 Hi,

 On Mon, Sep 5, 2011 at 5:45 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-09-02 at 16:17 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com

 HDMI IP block is common between TI OMAP4 Procerssor and Netra processor 
 although
 the Display subsytem is different.Also the IP block in future OMAP may 
 differ
 from the one existing in OMAP4. Thus to reuse the code between these two
 processors , and maintain the multi omap build functionality in DSS.
 HDMI IP dependant code is seperated out from hdmi.c and moved to
 new library file hdmi_ti_4xxx_ip.c.From the DSS dependent HDMI code only 
 the
 function pointer to functionality/features offered by HDMI is called.

 This patch series does the split and also renames hdmi_omap4_panel.c to
 hdmi_panel.c as that file has nothing specific to OMAP4 and can be reused 
 for
 other OMAP family of processors as well.

 This patch series is based on Tomi's LO-DSS2 master branch.

 The kernel doesn't compile at all after enabling
 SND_OMAP_SOC_OMAP4_HDMI. Looks like you didn't move any of the sound
 stuff, but just left them in hdmi.c file.

 I shall check this, may be i should handle some dependency but i didnt
 think there were any direct dependency , i guess you need to enable
 some audio files as well? let me check.

 There is a dependency on the audio files on the register writers and i
 was not very sure on what would be a genuine API v's what can be a
 static function.
 Ricardo any comments would you like to post a patch on top of this series ?

I have made a temporary patch to move the IP  dependent audio
functions to ti_hdmi_4xxx_ip.c and i see it compiles fine, later on we
can decide on what could be the API's and move the audio(non IP
dependent ) portion out of  DSS ?
are you ok with that ?

Thanks and regards,
Mythri.
 thanks and regards,
 Mythri.
  Tomi





--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 09/10] OMAP4: DSS2: HDMI: Function pointer approach to call

Hi,

On Tue, Sep 6, 2011 at 6:15 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2011-09-05 at 23:03 +0530, K, Mythri P wrote:
 Hi,

 On Mon, Sep 5, 2011 at 4:31 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Fri, 2011-09-02 at 16:17 +0530, mythr...@ti.com wrote:
  From: Mythri P K mythr...@ti.com
 
  To make the current hdmi DSS driver compatible with future OMAP having
  different IP blocks( A combination of different core, PHY, PLL blocks),
  Add HDMI hdmi functions as a function pointer in dss_features to abstract
  hdmi dss driver IP agnostic, hdmi dss driver  which will now access
  generic functions irrespective of underlying IP.
 

 snip

  diff --git a/include/video/omapdss.h b/include/video/omapdss.h
  index 9398dd3..c8891d1 100644
  --- a/include/video/omapdss.h
  +++ b/include/video/omapdss.h
  @@ -21,6 +21,9 @@
   #include linux/list.h
   #include linux/kobject.h
   #include linux/device.h
  +#if defined(CONFIG_OMAP4_DSS_HDMI)
  +#include video/ti_hdmi.h
  +#endif
 
   #define DISPC_IRQ_FRAMEDONE          (1  0)
   #define DISPC_IRQ_VSYNC                      (1  1)
  @@ -555,6 +558,26 @@ struct omap_dss_driver {
        u32 (*get_wss)(struct omap_dss_device *dssdev);
   };
 
  +#if defined(CONFIG_OMAP4_DSS_HDMI)
  +struct omap_hdmi_ip_ops {
  +
  +     void (*video_configure)(struct hdmi_ip_data *ip_data);
  +
  +     int (*phy_enable)(struct hdmi_ip_data *ip_data);
  +
  +     void (*phy_disable)(struct hdmi_ip_data *ip_data);
  +
  +     int (*read_edid)(struct hdmi_ip_data *ip_data,
  +                     u8 *pedid, u16 max_length);
  +
  +     int (*pll_enable)(struct hdmi_ip_data *ip_data);
  +
  +     void (*pll_disable)(struct hdmi_ip_data *ip_data);
  +
  +     void (*video_enable)(struct hdmi_ip_data *ip_data, bool start);
  +};
  +#endif
  +
 
  Hmm, I don't think omapdss.h is the right place for this struct.
 
  You've made it omap specific, but similar struct will be needed by other
  platforms also, right? So would it better be in ti_hdmi.h, and perhaps
  called ti_hdmi_ip_ops?
 
  And actually, ti_hdmi.h contains struct hdmi_ip_data, which contains
  pointer to struct omap_hdmi_ip_ops, so it's clear something is wrong
  there.
 
  So either the omap_hdmi_ip_ops should be omapdss internal struct, and it
  shouldn't be in struct hdmi_ip_data, or the ops struct should be generic
  one in ti_hdmi.h.
 
  Did you consider how the code would look if the function pointers were
  just included into struct hdmi_ip_data, without any ops struct at all?
 
 I guess moving it to ti_hdmi.h is a good option.. but i would think
 wrapping it in a struct
 would look cleaner ?

 You didn't make this change for the v4?

copy error , I resent the patch. Incorporated the comment.

Thanks and regards,
Mythri.
  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 00/10]HDMI: Split hdmi.c in DSS to seperate OMAP dependent

2011-09-05 Thread K, Mythri P

Hi,

On Mon, Sep 5, 2011 at 5:45 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-09-02 at 16:17 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com

 HDMI IP block is common between TI OMAP4 Procerssor and Netra processor 
 although
 the Display subsytem is different.Also the IP block in future OMAP may differ
 from the one existing in OMAP4. Thus to reuse the code between these two
 processors , and maintain the multi omap build functionality in DSS.
 HDMI IP dependant code is seperated out from hdmi.c and moved to
 new library file hdmi_ti_4xxx_ip.c.From the DSS dependent HDMI code only the
 function pointer to functionality/features offered by HDMI is called.

 This patch series does the split and also renames hdmi_omap4_panel.c to
 hdmi_panel.c as that file has nothing specific to OMAP4 and can be reused for
 other OMAP family of processors as well.

 This patch series is based on Tomi's LO-DSS2 master branch.

 The kernel doesn't compile at all after enabling
 SND_OMAP_SOC_OMAP4_HDMI. Looks like you didn't move any of the sound
 stuff, but just left them in hdmi.c file.

I shall check this, may be i should handle some dependency but i didnt
think there were any direct dependency , i guess you need to enable
some audio files as well? let me check.
  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 09/10] OMAP4: DSS2: HDMI: Function pointer approach to call

2011-09-05 Thread K, Mythri P

Hi,

On Mon, Sep 5, 2011 at 4:31 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-09-02 at 16:17 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com

 To make the current hdmi DSS driver compatible with future OMAP having
 different IP blocks( A combination of different core, PHY, PLL blocks),
 Add HDMI hdmi functions as a function pointer in dss_features to abstract
 hdmi dss driver IP agnostic, hdmi dss driver  which will now access
 generic functions irrespective of underlying IP.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/dss_features.c |   20 
  drivers/video/omap2/dss/dss_features.h |    3 +++
  drivers/video/omap2/dss/hdmi.c         |   19 ++-
  include/video/omapdss.h                |   23 +++
  include/video/ti_hdmi.h                |    1 +
  5 files changed, 57 insertions(+), 9 deletions(-)

 diff --git a/drivers/video/omap2/dss/dss_features.c 
 b/drivers/video/omap2/dss/dss_features.c
 index b63c5f8..4d50b30 100644
 --- a/drivers/video/omap2/dss/dss_features.c
 +++ b/drivers/video/omap2/dss/dss_features.c
 @@ -429,6 +429,26 @@ static const struct omap_dss_features 
 omap4_dss_features = {
       .burst_size_unit = 16,
  };

 +#if defined(CONFIG_OMAP4_DSS_HDMI)
 +/* HDMI OMAP4 Functions*/
 +static const struct omap_hdmi_ip_ops omap4_hdmi_functions = {
 +
 +     .video_configure        =       ti_hdmi_4xxx_basic_configure,
 +     .phy_enable             =       ti_hdmi_4xxx_phy_enable,
 +     .phy_disable            =       ti_hdmi_4xxx_phy_disable,
 +     .read_edid              =       ti_hdmi_4xxx_read_edid,
 +     .pll_enable             =       ti_hdmi_4xxx_pll_enable,
 +     .pll_disable            =       ti_hdmi_4xxx_pll_disable,
 +     .video_enable           =       ti_hdmi_4xxx_wp_video_start,
 +};
 +
 +void dss_init_hdmi_ip_ops(struct hdmi_ip_data *ip_data)
 +{
 +     if (cpu_is_omap44xx())
 +             ip_data-ops = omap4_hdmi_functions;
 +}
 +#endif
 +
  /* Functions returning values related to a DSS feature */
  int dss_feat_get_num_mgrs(void)
  {
 diff --git a/drivers/video/omap2/dss/dss_features.h 
 b/drivers/video/omap2/dss/dss_features.h
 index 4271e96..8b74f69 100644
 --- a/drivers/video/omap2/dss/dss_features.h
 +++ b/drivers/video/omap2/dss/dss_features.h
 @@ -99,4 +99,7 @@ u32 dss_feat_get_burst_size_unit(void);             /* in 
 bytes */
  bool dss_has_feature(enum dss_feat_id id);
  void dss_feat_get_reg_field(enum dss_feat_reg_field id, u8 *start, u8 *end);
  void dss_features_init(void);
 +#if defined(CONFIG_OMAP4_DSS_HDMI)
 +void dss_init_hdmi_ip_ops(struct hdmi_ip_data *ip_data);
 +#endif
  #endif
 diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
 index 1b989b3..cb54925 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -185,6 +185,7 @@ int hdmi_init_display(struct omap_dss_device *dssdev)
  {
       DSSDBG(init_display\n);

 +     dss_init_hdmi_ip_ops(hdmi.hdmi_data);
       return 0;
  }

 @@ -365,7 +366,7 @@ static void hdmi_read_edid(struct omap_video_timings *dp)
       memset(hdmi.edid, 0, HDMI_EDID_MAX_LENGTH);

       if (!hdmi.edid_set)
 -             ret = ti_hdmi_4xxx_read_edid(hdmi.hdmi_data, hdmi.edid,
 +             ret = hdmi.hdmi_data.ops-read_edid(hdmi.hdmi_data, hdmi.edid,
                                               HDMI_EDID_MAX_LENGTH);
       if (!ret) {
               if (!memcmp(hdmi.edid, edid_header, sizeof(edid_header))) {
 @@ -479,16 +480,16 @@ static int hdmi_power_on(struct omap_dss_device 
 *dssdev)

       hdmi_compute_pll(dssdev, phy, hdmi.hdmi_data.pll_data);

 -     ti_hdmi_4xxx_wp_video_start(hdmi.hdmi_data, 0);
 +     hdmi.hdmi_data.ops-video_enable(hdmi.hdmi_data, 0);

       /* config the PLL and PHY hdmi_set_pll_pwrfirst */
 -     r = ti_hdmi_4xxx_pll_enable(hdmi.hdmi_data);
 +     r = hdmi.hdmi_data.ops-pll_enable(hdmi.hdmi_data);
       if (r) {
               DSSDBG(Failed to lock PLL\n);
               goto err;
       }

 -     r = ti_hdmi_4xxx_phy_enable(hdmi.hdmi_data);
 +     r = hdmi.hdmi_data.ops-phy_enable(hdmi.hdmi_data);
       if (r) {
               DSSDBG(Failed to start PHY\n);
               goto err;
 @@ -496,7 +497,7 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)

       hdmi.hdmi_data.cfg.cm.mode = hdmi.mode;
       hdmi.hdmi_data.cfg.cm.code = hdmi.code;
 -     ti_hdmi_4xxx_basic_configure(hdmi.hdmi_data);
 +     hdmi.hdmi_data.ops-video_configure(hdmi.hdmi_data);

       /* Make selection of HDMI in DSS */
       dss_select_hdmi_venc_clk_source(DSS_HDMI_M_PCLK);
 @@ -518,7 +519,7 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)

       dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 1);

 -     ti_hdmi_4xxx_wp_video_start(hdmi.hdmi_data, 1);
 +     hdmi.hdmi_data.ops-video_enable(hdmi.hdmi_data, 1);

       return 0;
  err:
 @@ -530,9 +531,9 @@ static void hdmi_power_off(struct omap_dss_device 
 *dssdev)
  {

Re: [PATCH v3 00/10]HDMI: Split hdmi.c in DSS to seperate OMAP dependent

2011-09-05 Thread K, Mythri P

Hi,

On Mon, Sep 5, 2011 at 1:10 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-09-02 at 16:17 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com

 HDMI IP block is common between TI OMAP4 Procerssor and Netra processor 
 although
 the Display subsytem is different.Also the IP block in future OMAP may differ
 from the one existing in OMAP4. Thus to reuse the code between these two
 processors , and maintain the multi omap build functionality in DSS.
 HDMI IP dependant code is seperated out from hdmi.c and moved to
 new library file hdmi_ti_4xxx_ip.c.From the DSS dependent HDMI code only the
 function pointer to functionality/features offered by HDMI is called.

 This patch series does the split and also renames hdmi_omap4_panel.c to
 hdmi_panel.c as that file has nothing specific to OMAP4 and can be reused for
 other OMAP family of processors as well.

 This patch series is based on Tomi's LO-DSS2 master branch.

 changes since
 V2: Rename certain files/function to have standard format and handle scenario
 when hdmi is disabled.
 V1: Function pointer approach to call the HDMI IP functions from DSS HDMI

I guess i shall change the description , it is changes since , so v2
would be actually changes since v2 to v3.
 No change log for this version. It'd be nice to have it, so it's easier
 to see what changes made into this version.

 I also see this when compiling with sparse:

 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c:166:5: warning: symbol
 'hdmi_set_pll_pwr' was not declared. Should it be static?

I missed running sparse i shall fix it and send out.

  Tomi



Thanks and regards,
Mythri.
--
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 v2 03/10] OMAP4: DSS: HDMI: Use specific HDMI timings structure

Hi,

On Thu, Sep 1, 2011 at 1:44 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2011-08-29 at 11:44 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com

 Define new HDMI timings structure to replace the OMAP DSS timing strucutre in
 hdmi.c to have the HDMI include defintion out of DSS.

 structure and definition typoed. And the point is not to remove hdmi
 stuff from dss, but dss stuff from hdmi. The generic hdmi driver cannot
 use omap dss specific stuff.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/hdmi.c |   22 +++---
  drivers/video/omap2/dss/hdmi.h |   15 ++-
  2 files changed, 33 insertions(+), 4 deletions(-)

 diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
 index 084a47e..ba1ad06 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -537,6 +537,20 @@ static int read_edid(struct hdmi_ip_data *ip_data, u8 
 *pedid, u16 max_length)
       return 0;
  }

 +static void copy_hdmi_to_dss_timings(struct hdmi_video_timings hdmi_timings,
 +                     struct omap_video_timings *timings)

 Pass hdmi_timings as a const pointer.

Ok.

  Tomi



Thanks and regards,
Mythri
--
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 v2 02/10] OMAP4: DSS: HDMI: Move pll and video configuration

Hi,

On Thu, Sep 1, 2011 at 1:57 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2011-08-29 at 11:44 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com

 As the pll and the video configuration info are part of the ip_data those
 structures are moved to the ip_data strtucure.Also the functions are modified
 accordingly to take care of this movement.

 structure typoed, and use a space after period.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/hdmi.c |   34 +++---
  drivers/video/omap2/dss/hdmi.h |   18 ++
  2 files changed, 25 insertions(+), 27 deletions(-)

 diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
 index 544f93e..084a47e 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -59,7 +59,6 @@ static struct {
       u8 edid[HDMI_EDID_MAX_LENGTH];
       u8 edid_set;
       bool custom_set;
 -     struct hdmi_config cfg;

       struct clk *sys_clk;
  } hdmi;
 @@ -230,11 +229,11 @@ int hdmi_init_display(struct omap_dss_device *dssdev)
  }

  static int hdmi_pll_init(struct hdmi_ip_data *ip_data,
 -             enum hdmi_clk_refsel refsel, int dcofreq,
 -             struct hdmi_pll_info *fmt, u16 sd)
 +                     enum hdmi_clk_refsel refsel)
  {
       u32 r;
       void __iomem *pll_base = hdmi_pll_base(ip_data);
 +     struct hdmi_pll_info *fmt = ip_data-pll_data;

       /* PLL start always use manual mode */
       REG_FLD_MOD(pll_base, PLLCTRL_PLL_CONTROL, 0x0, 0, 0);
 @@ -251,9 +250,9 @@ static int hdmi_pll_init(struct hdmi_ip_data *ip_data,
       r = FLD_MOD(r, 0x1, 13, 13); /* PLL_REFEN */
       r = FLD_MOD(r, 0x0, 14, 14); /* PHY_CLKINEN de-assert during locking */

 -     if (dcofreq) {
 +     if (fmt-dcofreq) {
               /* divider programming for frequency beyond 1000Mhz */
 -             REG_FLD_MOD(pll_base, PLLCTRL_CFG3, sd, 17, 10);
 +             REG_FLD_MOD(pll_base, PLLCTRL_CFG3, fmt-regsd, 17, 10);
               r = FLD_MOD(r, 0x4, 3, 1); /* 1000MHz and 2000MHz */
       } else {
               r = FLD_MOD(r, 0x2, 3, 1); /* 500MHz and 1000MHz */
 @@ -379,8 +378,7 @@ static int hdmi_phy_init(struct hdmi_ip_data *ip_data)
       return 0;
  }

 -static int hdmi_pll_program(struct hdmi_ip_data *ip_data,
 -                             struct hdmi_pll_info *fmt)
 +static int hdmi_pll_program(struct hdmi_ip_data *ip_data)
  {
       u16 r = 0;
       enum hdmi_clk_refsel refsel;
 @@ -399,7 +397,7 @@ static int hdmi_pll_program(struct hdmi_ip_data *ip_data,

       refsel = HDMI_REFSEL_SYSCLK;

 -     r = hdmi_pll_init(ip_data, refsel, fmt-dcofreq, fmt, fmt-regsd);
 +     r = hdmi_pll_init(ip_data, refsel);

 I don't think I quite understood why refsel is not part of the pll info.
 And if it has to be hardcoded, you could as well do that in
 hdmi_pll_init().

Ok , yes that is better.
  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/10] OMAP4: DSS: HDMI: Move the common header file

Hi,

On Thu, Sep 1, 2011 at 2:30 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2011-08-29 at 11:44 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com

 Some of the header file definitions of HDMI IP are needed by audio driver 
 thus
 moving the common defintion to more generic Include/video.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/dss.h  |   10 -
  drivers/video/omap2/dss/hdmi.c |    1 +
  drivers/video/omap2/dss/hdmi.h |   53 
  include/video/omaphdmi.h       |   86 
 
  4 files changed, 87 insertions(+), 63 deletions(-)
  create mode 100644 include/video/omaphdmi.h

 As the functions will be renamed to hdmi_ti_4xxx_*, perhaps the header
 file is a bit misnamed.

 Also, please first do the changes/renamings/etc, and only then move the
 finished header file to include/video/ to prevent unnecessary changes in
 include/video.

The Include/video hdmi header file is a generic file that can be used
across 4 ,5 and Netra. Actually even the hdmi_ti_5xxx_ definitions
would come in this header so what do you think it can be named as ? ,
If you have any better name for hdmi_ti_4xxx_ip as well please suggest
would be happy to take it , as it doesn't sound that intuitive to me
as well.

  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 02/10] OMAP4: DSS: HDMI: Move pll and video configuration

Hi,

On Fri, Sep 2, 2011 at 10:43 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-09-02 at 10:41 +0530, K, Mythri P wrote:
 Hi,

 On Thu, Sep 1, 2011 at 1:57 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Mon, 2011-08-29 at 11:44 +0530, mythr...@ti.com wrote:
  From: Mythri P K mythr...@ti.com
 
  As the pll and the video configuration info are part of the ip_data those
  structures are moved to the ip_data strtucure.Also the functions are 
  modified
  accordingly to take care of this movement.
 
  structure typoed, and use a space after period.
 
  Signed-off-by: Mythri P K mythr...@ti.com
  ---
   drivers/video/omap2/dss/hdmi.c |   34 +++---
   drivers/video/omap2/dss/hdmi.h |   18 ++
   2 files changed, 25 insertions(+), 27 deletions(-)

  -static int hdmi_pll_program(struct hdmi_ip_data *ip_data,
  -                             struct hdmi_pll_info *fmt)
  +static int hdmi_pll_program(struct hdmi_ip_data *ip_data)
   {
        u16 r = 0;
        enum hdmi_clk_refsel refsel;
  @@ -399,7 +397,7 @@ static int hdmi_pll_program(struct hdmi_ip_data 
  *ip_data,
 
        refsel = HDMI_REFSEL_SYSCLK;
 
  -     r = hdmi_pll_init(ip_data, refsel, fmt-dcofreq, fmt, fmt-regsd);
  +     r = hdmi_pll_init(ip_data, refsel);
 
  I don't think I quite understood why refsel is not part of the pll info.
  And if it has to be hardcoded, you could as well do that in
  hdmi_pll_init().
 
 Ok , yes that is better.

 So, why refsel is not part of the pll info? The HW doesn't support
 changing it? If so, is it only for OMAPs or for all SoCs?
It is a variable parameter ,H/w supports changing it but it is never tried,
I was considering hard-coding it in the hdmi.c and passing it,
so that there would be no hard-coding in IP functions.

Thanks and regards,
Mythri.

  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/10] OMAP4: DSS: HDMI: Move the common header file

Hi,

On Fri, Sep 2, 2011 at 10:54 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-09-02 at 10:45 +0530, K, Mythri P wrote:
 Hi,

 On Thu, Sep 1, 2011 at 2:30 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Mon, 2011-08-29 at 11:44 +0530, mythr...@ti.com wrote:
  From: Mythri P K mythr...@ti.com
 
  Some of the header file definitions of HDMI IP are needed by audio driver 
  thus
  moving the common defintion to more generic Include/video.
 
  Signed-off-by: Mythri P K mythr...@ti.com
  ---
   drivers/video/omap2/dss/dss.h  |   10 -
   drivers/video/omap2/dss/hdmi.c |    1 +
   drivers/video/omap2/dss/hdmi.h |   53 
   include/video/omaphdmi.h       |   86 
  
   4 files changed, 87 insertions(+), 63 deletions(-)
   create mode 100644 include/video/omaphdmi.h
 
  As the functions will be renamed to hdmi_ti_4xxx_*, perhaps the header
  file is a bit misnamed.
 
  Also, please first do the changes/renamings/etc, and only then move the
  finished header file to include/video/ to prevent unnecessary changes in
  include/video.
 
 The Include/video hdmi header file is a generic file that can be used
 across 4 ,5 and Netra. Actually even the hdmi_ti_5xxx_ definitions
 would come in this header so what do you think it can be named as ? ,
 If you have any better name for hdmi_ti_4xxx_ip as well please suggest
 would be happy to take it , as it doesn't sound that intuitive to me
 as well.

 Well, I think whatever the header name is, it should somehow match the
 functions and the .c file. So if the functions and the .c file in this
 case are hdmi_ti_4xxx, then it'd be logical for the .h file to be named
 similarly.

 So, if it will contain functions for other IPs also, perhaps just leave
 the 4xxx out of it and name it hdmi_ti.h. Then the functions could be
 named hdmi_ti_4xxx_yyy(), and accordingly the .c file can be
 hdmi_ti_4xxx.c.

 Or, perhaps ti_hdmi.h (and funcs  .c files accordingly) would be more
 standard, as it's quite usual to name drivers etc. starting with the
 company name.

Thanks i shall change it to ti_hdmi.h and rename functions accordingly.

Thanks and regards,
Mythri.
  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] HDMI: Split hdmi.c to seperate HDMI IP dependant code from DSS.

2011-07-01 Thread K, Mythri P

Hi Tomi,

On Fri, Jul 1, 2011 at 2:21 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2011-06-30 at 23:16 +0530, K, Mythri P wrote:
 Hi Tomi,

 On Wed, Jun 29, 2011 at 9:51 PM, Tomi Valkeinen tomi.valkei...@ti.com 
 wrote:
  Hi,
 
  On Wed, 2011-06-29 at 19:08 +0530, K, Mythri P wrote:
  Hi Tomi,
 
  As the HDMI PLL , PHY and video blocks are logical blocks it would
  make sense to have the API's for all and the DSS HDMI (interface
  driver - user driver) would make a call to configure this in a
  particular sequence to enable HDMI , in case you the call to be
  generic across OMAPS in future then we should i either have a funtion
  in hdmi.c which will do this sequence and will be  aware of underlying
  IP , Which doesnt appear to be the solution you prefer but then there
  would be a need to have an intermediate file which would take the
  common API call(function pointer) and then arbitrate between different
  IP's based on the make , Is that what you are suggesting ?
 
  I agree that they are separate blocks, and at some level they need to be
  separate with own functions for each. But I don't see why the user needs
  to know about it.
 
  For example, consider OMAP DSI. DSI has protocol, PLL and PHY blocks,
  and the driver has functions to initialize them, use them etc. But the
  user of DSI, in this case panel drivers, do not need to know about it,
  and the API exported to the panel drivers does not contain any functions
  related to PLL or PHY. The panel drivers just enable the DSI driver and
  use it.
 
  So I'm still asking: what benefit does it give to the user that the API
  has functions to handle the blocks separately? There has to be a reason
  for the functions in the API.
 
 It doesnt give any additional benefit but flexibility to change the
 blocks (PHY / PLL) indivudally to use a different one.

 Right. But this API is not the right place to implement that
 flexibility. If there's no benefit for the user of the API to know about
 the details of the HW, it's better hidden. The user just wants to use
 the functionality, not know what lies below.

   And it just occurred to me that perhaps our views of the API are a bit
   different, and that's why we have differing opinions. I see this API as
   something that could be used by OMAP DSS (and equivalent components on
   other SoCs) to use the HDMI HW on OMAP4 and any future OMAPs. And
   perhaps you see this API more as an API to use the current HDMI HW in
   the OMAP4.
 
  Tomi , Yes my Idea of this API is for OMAP and Netra series only , I
  am unable to envision a common HDMI API library , in that case it
  would make more sense to have an intermediate file and have function
  pointer , which would take common API calls like hdmi_enable ,
  hdmi_avi_confg etc and then arbitrate based on the build. Please let
  me know if you have anything else in mind.
 
  I didn't mean a generic HDMI API either. I meant a TI HDMI API,
  currently for OMAP and Netra. But my comments would be valid even if the
  API would be only OMAP DSS internal.
 
 Neither the HDMI DSS driver should be concerned about these PHY/PLL
 internal API's nor should the configuration be done in the  IP driver
 as the IP driver shpuld only provide functionality and should be
 flexible enough as across OMAP4 and netra itself there is a

 What configuration are you referring to? I don't think the user of the
 API (i.e. omapdss) should do any configuration that requires knowledge
 of the underlying HDMI HW.

 possibility of the PHY block being different , so  Now the only
 solution i see is to have a intermediate file , whose job is to
 provide common API function and based on the CPU switch would be aware
 of the IP inside and make appropriate call to IP driver , Does this
 sound like a good approach ?

 I don't see a need for a separate file right now. We have the
 hdmi_ti_4xxx_ip.c file which contains code for the HDMI block as a
 whole, and could well contain the code that implements the API.

if the hdmi_ti_4xxx_ip.c  is handling the configuration then how are
we going to handle a scenario where netra uses a different PHY block ,
you suggest to have a #if in the programming sequence within
hdmi_ti_4xxx_ip.c function ?
Also in future OMAP's when are using the PHY and PLL block from
hdmi_ti_4xxx_ip.c , but a different core/video block from
hdmi_ti_5xxx_ip.c then how  would hdmi_ti_5xxx_ip.c hdmi_enable be
able to call the PHY and PLL configuration functions  from
hdmi_ti_4xxx_ip.c ??

  Tomi






-- 
Thanks and regards,
Mythri.
--
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 0/8] HDMI: Split hdmi.c to seperate HDMI IP dependant code from DSS.

2011-07-01 Thread K, Mythri P

Hi Tomi,

On Fri, Jul 1, 2011 at 5:14 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-07-01 at 14:52 +0530, K, Mythri P wrote:

  I don't see a need for a separate file right now. We have the
  hdmi_ti_4xxx_ip.c file which contains code for the HDMI block as a
  whole, and could well contain the code that implements the API.
 
 if the hdmi_ti_4xxx_ip.c  is handling the configuration then how are
 we going to handle a scenario where netra uses a different PHY block ,

 I wasn't aware that Netra has different HDMI blocks. I understood it's
 the same as on OMAP4.

 So they have a different PHY driver, but want to use PLL and core parts
 of OMAP4 HDMI driver?

That was precisely what i was trying to explain for last few threads :-).
 you suggest to have a #if in the programming sequence within
 hdmi_ti_4xxx_ip.c function ?

 No, we can't use #ifdefs as the same kernel has to work for both SoCs.

 Also in future OMAP's when are using the PHY and PLL block from
 hdmi_ti_4xxx_ip.c , but a different core/video block from
 hdmi_ti_5xxx_ip.c then how  would hdmi_ti_5xxx_ip.c hdmi_enable be
 able to call the PHY and PLL configuration functions  from
 hdmi_ti_4xxx_ip.c ??

 If the different HDMI blocks will be mixed like that, then we need to
 split the blocks to separate files. Otherwise it will get strange if
 OMAP5 HDMI code is calling PLL functions from OMAP4 HDMI code.

 And on top of those drivers/libs we would have a higher level driver/lib
 for the whole HDMI entity, and this higher level driver would implement
 the API being discussed. It would then use the lower level drivers/libs,
 and be used by OMAP/Netra DSS.

Splitting that to multiple would be a file overhead , ie creating a
file to just host 2/3 functions.
Also note it is not OMAP5 HDMI code calling OMAP4 HDMI code , It it no
way in relation to OMAP's as such , so these IP drivers be
provisioning for API's , which the intermediate Arbitration lib / file
which can provide the clean handle to user and can handle the sequence
 as to what IP functions should be called.

  Tomi






-- 
Thanks and regards,
Mythri.
--
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 0/8] HDMI: Split hdmi.c to seperate HDMI IP dependant code from DSS.

2011-06-30 Thread K, Mythri P

Hi Tomi,

On Wed, Jun 29, 2011 at 9:51 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 Hi,

 On Wed, 2011-06-29 at 19:08 +0530, K, Mythri P wrote:
 Hi Tomi,

 As the HDMI PLL , PHY and video blocks are logical blocks it would
 make sense to have the API's for all and the DSS HDMI (interface
 driver - user driver) would make a call to configure this in a
 particular sequence to enable HDMI , in case you the call to be
 generic across OMAPS in future then we should i either have a funtion
 in hdmi.c which will do this sequence and will be  aware of underlying
 IP , Which doesnt appear to be the solution you prefer but then there
 would be a need to have an intermediate file which would take the
 common API call(function pointer) and then arbitrate between different
 IP's based on the make , Is that what you are suggesting ?

 I agree that they are separate blocks, and at some level they need to be
 separate with own functions for each. But I don't see why the user needs
 to know about it.

 For example, consider OMAP DSI. DSI has protocol, PLL and PHY blocks,
 and the driver has functions to initialize them, use them etc. But the
 user of DSI, in this case panel drivers, do not need to know about it,
 and the API exported to the panel drivers does not contain any functions
 related to PLL or PHY. The panel drivers just enable the DSI driver and
 use it.

 So I'm still asking: what benefit does it give to the user that the API
 has functions to handle the blocks separately? There has to be a reason
 for the functions in the API.

It doesnt give any additional benefit but flexibility to change the
blocks (PHY / PLL) indivudally to use a different one.
  And it just occurred to me that perhaps our views of the API are a bit
  different, and that's why we have differing opinions. I see this API as
  something that could be used by OMAP DSS (and equivalent components on
  other SoCs) to use the HDMI HW on OMAP4 and any future OMAPs. And
  perhaps you see this API more as an API to use the current HDMI HW in
  the OMAP4.

 Tomi , Yes my Idea of this API is for OMAP and Netra series only , I
 am unable to envision a common HDMI API library , in that case it
 would make more sense to have an intermediate file and have function
 pointer , which would take common API calls like hdmi_enable ,
 hdmi_avi_confg etc and then arbitrate based on the build. Please let
 me know if you have anything else in mind.

 I didn't mean a generic HDMI API either. I meant a TI HDMI API,
 currently for OMAP and Netra. But my comments would be valid even if the
 API would be only OMAP DSS internal.

Neither the HDMI DSS driver should be concerned about these PHY/PLL
internal API's nor should the configuration be done in the  IP driver
as the IP driver shpuld only provide functionality and should be
flexible enough as across OMAP4 and netra itself there is a
possibility of the PHY block being different , so  Now the only
solution i see is to have a intermediate file , whose job is to
provide common API function and based on the CPU switch would be aware
of the IP inside and make appropriate call to IP driver , Does this
sound like a good approach ?

  Tomi






-- 
Thanks and regards,
Mythri.
--
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 0/8] HDMI: Split hdmi.c to seperate HDMI IP dependant code from DSS.

2011-06-29 Thread K, Mythri P

Hi Tomi,


On Mon, Jun 27, 2011 at 6:28 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2011-06-27 at 11:21 +0530, K, Mythri P wrote:
 Hi Tomi,

 On Thu, Jun 23, 2011 at 6:01 PM, Tomi Valkeinen tomi.valkei...@ti.com 
 wrote:
  On Thu, 2011-06-23 at 17:35 +0530, K, Mythri P wrote:
  Hi Tomi,
 
  On Thu, Jun 23, 2011 at 3:28 PM, Tomi Valkeinen tomi.valkei...@ti.com 
  wrote:
   On Fri, 2011-06-17 at 13:47 +0530, Mythri P K wrote:
   HDMI IP block is common between TI OMAP4 Procerssor and Netra 
   processor although
   the Display subsytem is different. Thus to reuse the code between 
   these two
   processors , HDMI IP dependant code is seperated out from hdmi.c and 
   moved to
   new library file hdmi_ti_4xxx_ip.c which now resides in /drivers/video 
   a more
   generic location out of omap2/dss folder.
  
   This patch series does the split and also renames hdmi_omap4_panel.c to
   hdmi_panel.c as that file has nothing specific to OMAP4 and can be 
   reused for
   other OMAP family of processors as well.
  
   These comments are based on the branch you have, not to particular
   patches:
  
   include/video/hdmi_ti_4xxx_ip.h:
  
   - This file is in a way the most important one, because it's the public
   API of the component. So you should be extra careful with this, and see
   that everything in it makes sense, and it's clear how it is to be used.
  
   - hdmi_core_hdmi_dvi is defined but not used. Should it be in the
   private header?
 
   HDMI_HDMI  and HDMI_DVI are used everywhere in hdmi.c and 
  hdmi_ti_4xxx_ip.c,
  one thing  i could do is to add a patch to used this enumerator
  instead of int mode ?
 
  Oh, so the int mode in the header is actually enum hdmi_core_hdmi_dvi
  mode? Using int is obviously wrong, then.
 sure i shall fix it in a separate patch.
 
   - hdmi_pll_pwr seems to be very low level thing. It requires the user to
   know HW details about the PLL, I don't think it should be visible to the
   users. It is used in parameters for hdmi_ti_4xxx_set_pll_pwr(). When is
   it supposed to be used? Why is hdmi_ti_4xxx_wp_video_start() not enough?
  
  HDMI has core , PLL and PHY blocks, so user of the ip driver should
  configure them separately.
 
  Why? Is there some requirement/benefit to it?

 Yes , When the core block is replaced in future OMAP's same PLL and
 PHY blocks are used.

 That doesn't answer the question why the user should know about the
 details of the HW. As far as I see, the user wants basically to
 configure, enable and disable the HDMI HW. The user wants to do the same
 things, regardless of the HDMI HW blocks used in that particular OMAP.

 However, there is a reason why the HDMI PLL should be considered
 separately: the PLL can be used as a clock source for DISPC and VENC. So
 the API should offer functionality to configure and use the PLL
 separately from the HDMI core/phy. But even so, hdmi_pll_pwr feels a bit
 too low-level function for this API.

 As for HDMI core/phy, is there ever reason to use one but not another?
 Or is there a reason to configure them separately? If not, I don't see
 why they should be present separately in the API.

   - The same goes to hdmi_ti_4xxx_pll_program. Who does the PLL
   calculations, omapdss driver? Shouldn't the HDMI driver do it, the PLL
   is a HDMI HW thing, not DSS HW? Or if they are defined in the board
   file, and omapdss just gives them to the HDMI driver, I think the data
   should be somehow passed through omapdss without omapdss actively
   participating in the PLL programming. This would be easy if the HDMI
   would use a platform device/driver model, the data could be passed via
   the device data.
 
  Can you comment on this? Why the PLL calculation is not in the HDMI IP
  driver?
 The calculation of the PLL would depend on the sysclk freq right ? but
 only the TMDS is the concern of HDMI IP block , so i would configure
 only the TMDS portion in HDMI IP and calculation in DSS.

 The HDMI PLL is part of the HDMI HW, not DSS HW, right? What if the HDMI
 PLL used in next OMAP would contain one more divider? Using it would
 require writing new calculation code into DSS driver. The PLL dividers
 and calculations sound very much like HDMI PLL internal thing, not
 something that should be handled in the DSS level.

 Yes, PLL calculations depend on the input clock, in this case sys_clk.
 So it should be provided to the HDMI code.

   - If the PLL calculations have to be done by omapdss, couldn't the
   hdmi_pll_info be part of hdmi_config?
 
  hdmi_pll_info would be used only once while configuring (power on) ,
  Why would we have to store the pll_info then in hdmi_config?
 
  Whether you store the hdmi_pll_info or not is HDMI IP driver internal
  thing, not related to the API. We're talking about the API here.
 
  Look at the include/video/hdmi_ti_4xxx_ip.h file and think from the
  users view point: what is the most obvious and easiest way to use the
  API. Then make the HDMI IP driver work

Re: [PATCH 0/8] HDMI: Split hdmi.c to seperate HDMI IP dependant code from DSS.

2011-06-27 Thread K, Mythri P

Hi Tomi,

On Thu, Jun 23, 2011 at 6:01 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2011-06-23 at 17:35 +0530, K, Mythri P wrote:
 Hi Tomi,

 On Thu, Jun 23, 2011 at 3:28 PM, Tomi Valkeinen tomi.valkei...@ti.com 
 wrote:
  On Fri, 2011-06-17 at 13:47 +0530, Mythri P K wrote:
  HDMI IP block is common between TI OMAP4 Procerssor and Netra processor 
  although
  the Display subsytem is different. Thus to reuse the code between these 
  two
  processors , HDMI IP dependant code is seperated out from hdmi.c and 
  moved to
  new library file hdmi_ti_4xxx_ip.c which now resides in /drivers/video a 
  more
  generic location out of omap2/dss folder.
 
  This patch series does the split and also renames hdmi_omap4_panel.c to
  hdmi_panel.c as that file has nothing specific to OMAP4 and can be reused 
  for
  other OMAP family of processors as well.
 
  These comments are based on the branch you have, not to particular
  patches:
 
  include/video/hdmi_ti_4xxx_ip.h:
 
  - This file is in a way the most important one, because it's the public
  API of the component. So you should be extra careful with this, and see
  that everything in it makes sense, and it's clear how it is to be used.
 
  - hdmi_core_hdmi_dvi is defined but not used. Should it be in the
  private header?

  HDMI_HDMI  and HDMI_DVI are used everywhere in hdmi.c and hdmi_ti_4xxx_ip.c,
 one thing  i could do is to add a patch to used this enumerator
 instead of int mode ?

 Oh, so the int mode in the header is actually enum hdmi_core_hdmi_dvi
 mode? Using int is obviously wrong, then.
sure i shall fix it in a separate patch.

  - hdmi_pll_pwr seems to be very low level thing. It requires the user to
  know HW details about the PLL, I don't think it should be visible to the
  users. It is used in parameters for hdmi_ti_4xxx_set_pll_pwr(). When is
  it supposed to be used? Why is hdmi_ti_4xxx_wp_video_start() not enough?
 
 HDMI has core , PLL and PHY blocks, so user of the ip driver should
 configure them separately.

 Why? Is there some requirement/benefit to it?

Yes , When the core block is replaced in future OMAP's same PLL and
PHY blocks are used.

  - The same goes to hdmi_ti_4xxx_pll_program. Who does the PLL
  calculations, omapdss driver? Shouldn't the HDMI driver do it, the PLL
  is a HDMI HW thing, not DSS HW? Or if they are defined in the board
  file, and omapdss just gives them to the HDMI driver, I think the data
  should be somehow passed through omapdss without omapdss actively
  participating in the PLL programming. This would be easy if the HDMI
  would use a platform device/driver model, the data could be passed via
  the device data.

 Can you comment on this? Why the PLL calculation is not in the HDMI IP
 driver?
The calculation of the PLL would depend on the sysclk freq right ? but
only the TMDS is the concern of HDMI IP block , so i would configure
only the TMDS portion in HDMI IP and calculation in DSS.

  - If the PLL calculations have to be done by omapdss, couldn't the
  hdmi_pll_info be part of hdmi_config?

 hdmi_pll_info would be used only once while configuring (power on) ,
 Why would we have to store the pll_info then in hdmi_config?

 Whether you store the hdmi_pll_info or not is HDMI IP driver internal
 thing, not related to the API. We're talking about the API here.

 Look at the include/video/hdmi_ti_4xxx_ip.h file and think from the
 users view point: what is the most obvious and easiest way to use the
 API. Then make the HDMI IP driver work with the API.

As the pll_info in needed only for config of the PLL block it wouldnt
make sense to store it.[even from API point of view].

  - Is hdmi_ti_4xxx_phy_off the counterpart of hdmi_ti_4xxx_phy_init? If
  so, the naming could be better to clearly show this. Like init/uninit,
  enable/disable, etc.

 I shall rename. yes phy_off is counterpart of init , i shall renmane
 it to enable ,disable.
 
  - Is the phy relevant (from the user's point of view) in
  hdmi_ti_4xxx_phy_init and off? Or could they be just init and uninit?

 PHY is a seperate block in HDMI and even from the user point of view
 user should be aware of PHY, PLL and core blocks. so phy_init is
 relevant.

 Again, why? Why cannot the user just give the configurations to the HDMI
 IP driver, and start it, without knowing anything about PHY/PLL/core.

Please see the comment above. The PHY , PLL and core blocks are interchangeable.

  - Perhaps hdmi_ti_4xxx_basic_configure could be just configure, there's
  no advanced configure or similar.
 Well, makes sense but this is the init_configuration(default
 configuration) is what was intended in the patch.
 
  - is the wp important in hdmi_ti_4xxx_wp_video_start?
 
 yes wp == wrapper , so it would be clear to have a wp_video_Start.

 Why does the user need to know wp == wrapper? Isn't the function just
 about enabling the video output?
Well it is just to distinguish , yes the user doesn't have to know.

  - read_ti_4xxx_edid() is named differently

Re: [PATCH 3/8] OMAP4: DSS: HDMI: Use specific HDMI timings structure

Hi Tomi,

On Thu, Jun 23, 2011 at 2:00 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-06-17 at 13:47 +0530, Mythri P K wrote:
 Define new HDMI timings structure to replace the OMAP DSS timing strucutre 
 in hdmi.c
 to have the HDMI include defintion out of DSS.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/hdmi.c |   23 ---
  drivers/video/omap2/dss/hdmi.h |   15 ++-
  2 files changed, 34 insertions(+), 4 deletions(-)

 diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
 index 4ee879a..c24d573 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -593,6 +593,20 @@ static int read_edid(struct hdmi_ip_data *ip_data, u8 
 *pedid, u16 max_length)
       return 0;
  }

 +static void copy_hdmi_to_dss_timings(struct hdmi_video_timings hdmi_timings,
 +                     struct omap_video_timings *timings)

 There's no reason to pass hdmi_timings as a value, so it should be a
 pointer.
As the hdmi_timings is not changed in the function , i passed it by
value, Not a problem to change.

 +{
 +     timings-x_res = hdmi_timings.x_res;
 +     timings-y_res = hdmi_timings.y_res;
 +     timings-pixel_clock = hdmi_timings.pixel_clock;
 +     timings-hbp = hdmi_timings.hbp;
 +     timings-hfp = hdmi_timings.hfp;
 +     timings-hsw = hdmi_timings.hsw;
 +     timings-vbp = hdmi_timings.vbp;
 +     timings-vfp = hdmi_timings.vfp;
 +     timings-vsw = hdmi_timings.vsw;
 +}
 +
  static int get_timings_index(void)
  {
       int code;
 @@ -617,7 +631,7 @@ static struct hdmi_cm hdmi_get_code(struct 
 omap_video_timings *timing)
  {
       int i = 0, code = -1, temp_vsync = 0, temp_hsync = 0;
       int timing_vsync = 0, timing_hsync = 0;
 -     struct omap_video_timings temp;
 +     struct hdmi_video_timings temp;
       struct hdmi_cm cm = {-1};
       DSSDBG(hdmi_get_code\n);

 @@ -775,7 +789,8 @@ static void hdmi_read_edid(struct omap_video_timings *dp)

 dp feels a rather odd name for video timings variable. What does it
 stand for?

*display pointer, but then it is not a part of this patch.


  Tomi






-- 
Thanks and regards,
Mythri.
--
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 1/8] OMAP4: DSS: HDMI: HDMI clean up to pass base_address

Hi Sanjeev,

snip
 
      The changes in the patch aren't limited to DSS alone.
      See updated to hdmi_core_audio_config(),
 hdmi_wp_audio_config_format(),
      hdmi_wp_audio_config_dma(), etc.
 
 The changes are related to the HDMI driver which as of this patch is
 in DSS. You see any concerns ? It is only later that those functions
 are moved out .

 [sp] The subject of mail indicates that changes are meant for DSS.
     How are the functions I quoted related to DSS? Unless DSS on OMAP4
     supports audio...

Some of the Audio configuration for HDMI is part of DSS (as of 2.6.40)
, This patch series is precisely to separate the DSS portion from
non-DSS portion of HDMI.
It would be great if you have a look at the 2.6.40 kernel and this
gitorious tree with my patch set to get a fair idea.

 
  -     r = hdmi_read_reg(PLLCTRL_CFG1);
  +     r = hdmi_read_reg(hdmi_pll_base(ip_data), PLLCTRL_CFG1);
        r = FLD_MOD(r, fmt-regm, 20, 9); /* CFG1_PLL_REGM */
        r = FLD_MOD(r, fmt-regn, 8, 1);  /* CFG1_PLL_REGN */
 
  -     hdmi_write_reg(PLLCTRL_CFG1, r);
  +     hdmi_write_reg(hdmi_pll_base(ip_data), PLLCTRL_CFG1, r);
 
  [sp] hdmi_pll_base(ip_data) is used at many places withing this
      function. Have you checked if saving the result in a local
      variable generates any better code than repetitive ADD operation
      in the function.
 
      Same comment applies to similar use in other functions
      modified in this patch.
 
 I had this comment from Tomi as well , Yes in some places it makes
 sense to have a local variable to save , but then in certain functions
 where there are four writes with 3 different base address it doesn't
 make sense to add 3 variables.

 [sp] I don't see base address for the IP changing within the function.
     so a local variable still makes sense - instead of using CPU cycles
     to do reduntant calculations each time.

I shall make those changes where applicable.

  [snip]...[snip]
  +                                             ,
  HDMI_CORE_DDC_CMD, 0x9, 3, 0);
 
  [sp] Misplaced , at beginning of the line.
 
 Not a part of the patch i suppose?

 [sp] Actually the + indicates it is very much part of the patch.
     Ideally it should be at end of prev line.
     [quote]
        /* Clear FIFO */
     -  REG_FLD_MOD(HDMI_CORE_DDC_CMD, 0x9, 3, 0);
     +  REG_FLD_MOD(hdmi_core_sys_base(ip_data)
     +                          , HDMI_CORE_DDC_CMD, 0x9, 3, 0);
     [/quote]

 ~sanjeev




-- 
Thanks and regards,
Mythri.
--
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 1/8] OMAP4: DSS: HDMI: HDMI clean up to pass base_address

Hi Sanjeev,

On Thu, Jun 23, 2011 at 4:33 PM, Premi, Sanjeev pr...@ti.com wrote:
 -Original Message-
 From: K, Mythri P
 Sent: Thursday, June 23, 2011 4:30 PM
 To: Premi, Sanjeev
 Cc: linux-omap@vger.kernel.org; Valkeinen, Tomi
 Subject: Re: [PATCH 1/8] OMAP4: DSS: HDMI: HDMI clean up to
 pass base_address

 Hi Sanjeev,

 snip

       The changes in the patch aren't limited to DSS alone.
       See updated to hdmi_core_audio_config(),
  hdmi_wp_audio_config_format(),
       hdmi_wp_audio_config_dma(), etc.

  The changes are related to the HDMI driver which as of
 this patch is
  in DSS. You see any concerns ? It is only later that those
 functions
  are moved out .

  [sp] The subject of mail indicates that changes are meant for DSS.
      How are the functions I quoted related to DSS? Unless
 DSS on OMAP4
      supports audio...

 Some of the Audio configuration for HDMI is part of DSS (as of 2.6.40)
 , This patch series is precisely to separate the DSS portion from
 non-DSS portion of HDMI.
 It would be great if you have a look at the 2.6.40 kernel and this
 gitorious tree with my patch set to get a fair idea.

 [sp] Then I suggest dropping DSS from the subject.

The DSS is to indicate that the file that is changed in the current
patch  is a part of DSS directory.

 ~sanjeev

-- 
Thanks and regards,
Mythri.
--
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 0/8] HDMI: Split hdmi.c to seperate HDMI IP dependant code from DSS.

Hi Tomi,

On Thu, Jun 23, 2011 at 3:28 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-06-17 at 13:47 +0530, Mythri P K wrote:
 HDMI IP block is common between TI OMAP4 Procerssor and Netra processor 
 although
 the Display subsytem is different. Thus to reuse the code between these two
 processors , HDMI IP dependant code is seperated out from hdmi.c and moved to
 new library file hdmi_ti_4xxx_ip.c which now resides in /drivers/video a more
 generic location out of omap2/dss folder.

 This patch series does the split and also renames hdmi_omap4_panel.c to
 hdmi_panel.c as that file has nothing specific to OMAP4 and can be reused for
 other OMAP family of processors as well.

 These comments are based on the branch you have, not to particular
 patches:

 include/video/hdmi_ti_4xxx_ip.h:

 - This file is in a way the most important one, because it's the public
 API of the component. So you should be extra careful with this, and see
 that everything in it makes sense, and it's clear how it is to be used.

 - hdmi_core_hdmi_dvi is defined but not used. Should it be in the
 private header?

 HDMI_HDMI  and HDMI_DVI are used everywhere in hdmi.c and hdmi_ti_4xxx_ip.c,
one thing  i could do is to add a patch to used this enumerator
instead of int mode ?


 - hdmi_pll_pwr seems to be very low level thing. It requires the user to
 know HW details about the PLL, I don't think it should be visible to the
 users. It is used in parameters for hdmi_ti_4xxx_set_pll_pwr(). When is
 it supposed to be used? Why is hdmi_ti_4xxx_wp_video_start() not enough?

HDMI has core , PLL and PHY blocks, so user of the ip driver should
configure them separately.

 - The same goes to hdmi_ti_4xxx_pll_program. Who does the PLL
 calculations, omapdss driver? Shouldn't the HDMI driver do it, the PLL
 is a HDMI HW thing, not DSS HW? Or if they are defined in the board
 file, and omapdss just gives them to the HDMI driver, I think the data
 should be somehow passed through omapdss without omapdss actively
 participating in the PLL programming. This would be easy if the HDMI
 would use a platform device/driver model, the data could be passed via
 the device data.

 - If the PLL calculations have to be done by omapdss, couldn't the
 hdmi_pll_info be part of hdmi_config?

hdmi_pll_info would be used only once while configuring (power on) ,
Why would we have to store the pll_info then in hdmi_config?


 - Is hdmi_ti_4xxx_phy_off the counterpart of hdmi_ti_4xxx_phy_init? If
 so, the naming could be better to clearly show this. Like init/uninit,
 enable/disable, etc.

I shall rename. yes phy_off is counterpart of init , i shall renmane
it to enable ,disable.

 - Is the phy relevant (from the user's point of view) in
 hdmi_ti_4xxx_phy_init and off? Or could they be just init and uninit?

PHY is a seperate block in HDMI and even from the user point of view
user should be aware of PHY, PLL and core blocks. so phy_init is
relevant.

 - Perhaps hdmi_ti_4xxx_basic_configure could be just configure, there's
 no advanced configure or similar.
Well, makes sense but this is the init_configuration(default
configuration) is what was intended in the patch.

 - is the wp important in hdmi_ti_4xxx_wp_video_start?

yes wp == wrapper , so it would be clear to have a wp_video_Start.

 - read_ti_4xxx_edid() is named differently than the other functions.

I shall rename to hdmi_ti_4xxx_read_edid.

 drivers/video/hdmi_ti_4xxx_ip.c:

 - EXPORT_SYMBOL(hdmi_ti_4xxx_phy_init); is not below the function.

 drivers/video/omap2/dss/hdmi.c:

 - hdmi_ti_4xxx_set_pll_pwr() is only called in power_off(), but never
 enabled. I presume the HDMI driver does that internally. That doesn't
 sound like a valid use of the function.

It is enabled in hdmi_ti_4xxx_pll_program called from power on.


 - Looking at hdmi_power_on(), I feel that the API of the HDMI driver
 could be simpler. Would it be enough to have just one function to setup
 the HDMI, and another to turn the output on?
No , I suppose it is the responsibility of the user of IP driver to
configure PHY , PLL and CORE blocks seperately as we can reuse the PLL
or PHY block in different SOC's as well.


 - omapdss_hdmi_display_set_timing() seems to be broken. It just sets
 some variables and calls display_enable().
It sets a variable custom_set , appropriate action will be taken while
configuring timings based on that parameter.

  Tomi



-- 
Thanks and regards,
Mythri.
--
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 3/8] OMAP4: DSS: HDMI: Use specific HDMI timings structure

2011-06-22 Thread K, Mythri P

Hi Sanjeev,

On Mon, Jun 20, 2011 at 6:16 PM, Premi, Sanjeev pr...@ti.com wrote:
 -Original Message-
 From: linux-omap-ow...@vger.kernel.org
 [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of K, Mythri P
 Sent: Friday, June 17, 2011 1:47 PM
 To: linux-omap@vger.kernel.org; Valkeinen, Tomi
 Cc: K, Mythri P
 Subject: [PATCH 3/8] OMAP4: DSS: HDMI: Use specific HDMI
 timings structure

 Define new HDMI timings structure to replace the OMAP DSS
 timing strucutre in hdmi.c

 [sp] Fix spelling of structure.

Thanks will fix it.
 to have the HDMI include defintion out of DSS.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/hdmi.c |   23 ---
  drivers/video/omap2/dss/hdmi.h |   15 ++-
  2 files changed, 34 insertions(+), 4 deletions(-)

 diff --git a/drivers/video/omap2/dss/hdmi.c
 b/drivers/video/omap2/dss/hdmi.c
 index 4ee879a..c24d573 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -593,6 +593,20 @@ static int read_edid(struct hdmi_ip_data
 *ip_data, u8 *pedid, u16 max_length)
       return 0;
  }

 +static void copy_hdmi_to_dss_timings(struct
 hdmi_video_timings hdmi_timings,
 +                     struct omap_video_timings *timings)

 [sp] Assuming that we really need 2 separate structures, suggest
     renaming function to copy_hdmi_timings_to_dss or
     copy_timings_hdmi_to_dss for better readability.

I shall rename.
 +{
 +     timings-x_res = hdmi_timings.x_res;
 +     timings-y_res = hdmi_timings.y_res;
 +     timings-pixel_clock = hdmi_timings.pixel_clock;
 +     timings-hbp = hdmi_timings.hbp;
 +     timings-hfp = hdmi_timings.hfp;
 +     timings-hsw = hdmi_timings.hsw;
 +     timings-vbp = hdmi_timings.vbp;
 +     timings-vfp = hdmi_timings.vfp;
 +     timings-vsw = hdmi_timings.vsw;
 +}
 +
  static int get_timings_index(void)
  {
       int code;
 @@ -617,7 +631,7 @@ static struct hdmi_cm
 hdmi_get_code(struct omap_video_timings *timing)
  {
       int i = 0, code = -1, temp_vsync = 0, temp_hsync = 0;
       int timing_vsync = 0, timing_hsync = 0;
 -     struct omap_video_timings temp;
 +     struct hdmi_video_timings temp;
       struct hdmi_cm cm = {-1};
       DSSDBG(hdmi_get_code\n);

 @@ -775,7 +789,8 @@ static void hdmi_read_edid(struct
 omap_video_timings *dp)

       code = get_timings_index();

 -     *dp = cea_vesa_timings[code].timings;
 +     copy_hdmi_to_dss_timings(cea_vesa_timings[code].timings, dp);
 +
  }

  static void hdmi_core_init(struct hdmi_core_video_config *video_cfg,
 @@ -1234,7 +1249,9 @@ static int hdmi_power_on(struct
 omap_dss_device *dssdev)
               hdmi_read_edid(p);
       }
       code = get_timings_index();
 -     dssdev-panel.timings = cea_vesa_timings[code].timings;
 +     copy_hdmi_to_dss_timings(cea_vesa_timings[code].timings,
 +                     dssdev-panel.timings);
 +
       update_hdmi_timings(hdmi.cfg, p, code);

       phy = p-pixel_clock;
 diff --git a/drivers/video/omap2/dss/hdmi.h
 b/drivers/video/omap2/dss/hdmi.h
 index f57ef4a..dcc30b7 100644
 --- a/drivers/video/omap2/dss/hdmi.h
 +++ b/drivers/video/omap2/dss/hdmi.h
 @@ -188,9 +188,22 @@ struct hdmi_reg { u16 idx; };
  #define REG_GET(base, idx, start, end) \
       FLD_GET(hdmi_read_reg(base, idx), start, end)

 +struct hdmi_video_timings {
 +     u16 x_res;
 +     u16 y_res;
 +     /* Unit: KHz */
 +     u32 pixel_clock;

 [sp] Could be:
        u32 pixel_clock; /* unit: KHz */

 +     u16 hsw;
 +     u16 hfp;
 +     u16 hbp;
 +     u16 vsw;
 +     u16 vfp;
 +     u16 vbp;
 +};
 +
  /* HDMI timing structure */
  struct hdmi_timings {
 -     struct omap_video_timings timings;
 +     struct hdmi_video_timings timings;
       int vsync_pol;
       int hsync_pol;
  };
 --
 1.7.5.4

 --
 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




-- 
Thanks and regards,
Mythri.
--
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 1/8] OMAP4: DSS: HDMI: HDMI clean up to pass base_address

2011-06-22 Thread K, Mythri P

Hi Sanjeev,

On Mon, Jun 20, 2011 at 7:03 PM, Premi, Sanjeev pr...@ti.com wrote:
 -Original Message-
 From: linux-omap-ow...@vger.kernel.org
 [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of K, Mythri P
 Sent: Friday, June 17, 2011 1:47 PM
 To: linux-omap@vger.kernel.org; Valkeinen, Tomi
 Cc: K, Mythri P
 Subject: [PATCH 1/8] OMAP4: DSS: HDMI: HDMI clean up to pass
 base_address

 [sp] HDMI appears twice on the subject. One of these can be removed.
     Also, it isn't clear (though implied) whose base address is being
     passed.

     The changes in the patch aren't limited to DSS alone.
     See updated to hdmi_core_audio_config(), hdmi_wp_audio_config_format(),
     hdmi_wp_audio_config_dma(), etc.

The changes are related to the HDMI driver which as of this patch is
in DSS. You see any concerns ? It is only later that those functions
are moved out .


 As the base_address of the HDMI might differ across SoC's,

 [sp] Is 'base_address' a variable? OR did you mean 'base address'?

 offset of the HDMI
 logical blocks and base address got from the platform data are passed
 dynamically to the functions that modify HDMI IP registers.

 [sp] Overall, the sentence can be simplified.


 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/hdmi.c |  518
 
  drivers/video/omap2/dss/hdmi.h |  292 +++
  2 files changed, 452 insertions(+), 358 deletions(-)

 diff --git a/drivers/video/omap2/dss/hdmi.c
 b/drivers/video/omap2/dss/hdmi.c
 index 5e5b98b..1d16ee2 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -43,11 +43,17 @@
  #include hdmi.h
  #include dss_features.h

 +#define HDMI_WP                      0x0
 +#define HDMI_CORE_SYS                0x400
 +#define HDMI_CORE_AV         0x900
 +#define HDMI_PLLCTRL         0x200
 +#define HDMI_PHY             0x300
 +
  static struct {
       struct mutex lock;
       struct omap_display_platform_data *pdata;
       struct platform_device *pdev;
 -     void __iomem *base_wp;  /* HDMI wrapper */
 +     struct hdmi_ip_data hdmi_data;
       int code;
       int mode;
       u8 edid[HDMI_EDID_MAX_LENGTH];
 @@ -146,21 +152,51 @@ static const int code_vesa[85] = {

  static const u8 edid_header[8] = {0x0, 0xff, 0xff, 0xff,
 0xff, 0xff, 0xff, 0x0};

 -static inline void hdmi_write_reg(const struct hdmi_reg idx, u32 val)
 +static inline void hdmi_write_reg(void __iomem *base_addr,
 +                             const struct hdmi_reg idx, u32 val)
 +{
 +     __raw_writel(val, base_addr + idx.idx);
 +}
 +
 +static inline u32 hdmi_read_reg(void __iomem *base_addr,
 +                             const struct hdmi_reg idx)
 +{
 +     return __raw_readl(base_addr + idx.idx);
 +}
 +
 +static inline void __iomem *hdmi_wp_base(struct hdmi_ip_data
 *ip_data)
 +{
 +     return (void __iomem *) (ip_data-base_wp);
 +}
 +
 +static inline void __iomem *hdmi_phy_base(struct
 hdmi_ip_data *ip_data)
  {
 -     __raw_writel(val, hdmi.base_wp + idx.idx);
 +     return (void __iomem *) (ip_data-base_wp +
 ip_data-hdmi_phy_offset);
  }

 -static inline u32 hdmi_read_reg(const struct hdmi_reg idx)
 +static inline void __iomem *hdmi_pll_base(struct
 hdmi_ip_data *ip_data)
  {
 -     return __raw_readl(hdmi.base_wp + idx.idx);
 +     return (void __iomem *) (ip_data-base_wp +
 ip_data-hdmi_pll_offset);
  }

 -static inline int hdmi_wait_for_bit_change(const struct hdmi_reg idx,
 +static inline void __iomem *hdmi_av_base(struct hdmi_ip_data
 *ip_data)
 +{
 +     return (void __iomem *)
 +                     (ip_data-base_wp +
 ip_data-hdmi_core_av_offset);
 +}
 +
 +static inline void __iomem *hdmi_core_sys_base(struct
 hdmi_ip_data *ip_data)
 +{
 +     return (void __iomem *)
 +                     (ip_data-base_wp +
 ip_data-hdmi_core_sys_offset);
 +}
 +
 +static inline int hdmi_wait_for_bit_change(void __iomem *base_addr,
 +                             const struct hdmi_reg idx,
                               int b2, int b1, u32 val)
  {
       u32 t = 0;
 -     while (val != REG_GET(idx, b2, b1)) {
 +     while (val != REG_GET(base_addr, idx, b2, b1)) {
               udelay(1);
               if (t++  1)
                       return !val;
 @@ -225,21 +261,22 @@ int hdmi_init_display(struct
 omap_dss_device *dssdev)
       return 0;
  }

 -static int hdmi_pll_init(enum hdmi_clk_refsel refsel, int dcofreq,
 +static int hdmi_pll_init(struct hdmi_ip_data *ip_data,
 +             enum hdmi_clk_refsel refsel, int dcofreq,
               struct hdmi_pll_info *fmt, u16 sd)
  {
       u32 r;

       /* PLL start always use manual mode */
 -     REG_FLD_MOD(PLLCTRL_PLL_CONTROL, 0x0, 0, 0);
 +     REG_FLD_MOD(hdmi_pll_base(ip_data),
 PLLCTRL_PLL_CONTROL, 0x0, 0, 0);

 -     r = hdmi_read_reg(PLLCTRL_CFG1);
 +     r = hdmi_read_reg(hdmi_pll_base(ip_data), PLLCTRL_CFG1);
       r = FLD_MOD(r, fmt-regm, 20, 9); /* CFG1_PLL_REGM */
       r = FLD_MOD(r, fmt-regn, 8, 1

Re: [PATCH 5/8] OMAP4: DSS2: HDMI: Split the HDMI driver to DSS and IP

2011-06-22 Thread K, Mythri P

Hi,

On Mon, Jun 20, 2011 at 7:18 PM, Premi, Sanjeev pr...@ti.com wrote:
 -Original Message-
 From: linux-omap-ow...@vger.kernel.org
 [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of K, Mythri P
 Sent: Friday, June 17, 2011 1:47 PM
 To: linux-omap@vger.kernel.org; Valkeinen, Tomi
 Cc: K, Mythri P
 Subject: [PATCH 5/8] OMAP4: DSS2: HDMI: Split the HDMI driver
 to DSS and IP

 Splitting HDMI IP dependent IP configuring code from HDMI DSS
 dependent code and
 moving to a new IP file.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/Makefile                   |    1 +
  drivers/video/omap2/dss/hdmi.c                     |  751
 ---
  drivers/video/omap2/dss/hdmi_ti_4xxx_ip.c          |  781

  .../video/omap2/dss/{hdmi.h = hdmi_ti_4xxx_ip.h}  |    9 +-
  include/video/hdmi_ti_4xxx_ip.h                    |   10 +

 [sp] Is it okay to have 2 files by same name in two dirs?
     'include/video' and 'drivers/.../omap2/dss'.
     Compiler won't complain, but my comments is more from readability.
     There has to be something characteristically different in these
     headers - maybe this should reflect in the names as well.

I have seen some instances of such nomenclature , where one is generic
to be used by other drivers across and is located located in
/include/video and other in local folder restricted to that file. Do
anyone else see an issue ?

 ~sanjeev

-- 
Thanks and regards,
Mythri.
--
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: [RFC PATCH] HDMI:Support for EDID parsing in kernel.

2011-03-25 Thread K, Mythri P

Hi Gunnedi,

snip
  Dave's point is that we can't ditch the existing code without
  introducing a lot of risk; it would be better to start a library-ized
  EDID codebase from the most complete one we have already, i.e. the DRM
  EDID code.

 Does the DRM EDID-parser also process blocks beyond the first one and
 also parses SVD entries similar to what I've recently added to fbdev? Yes,
 we definitely need a common EDID parses, and maybe we'll have to collect
 various pieces from different implementations.

As far as I know , it does not parse SVD ,But it should not be that
difficult to add.
We could take the SVD parsing from your code . In the RFC i have
posted I parse for detailed timing and other VSDB blocks.
Also the parsing should be based on the extension type for  multiple
128 byte blocks.

Thanks and regards,
Mythri.

 Thanks
 Guennadi

--
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: [RFC PATCH] HDMI:Support for EDID parsing in kernel.

2011-03-25 Thread K, Mythri P

Hi Florian,

snip

 So why should this be a common library? Most kernel code doesn't need
 it. Or is there a serious need for video input to parse EDIDs?

 It's true that most kernel code does not need it as it is only useful for
 display output systems (and only the ones that can be connected to something
 sending EDID data) but it would be good anyway.
 Because sharing code that should fulfill the same purpose is always a good
 idea. But of course only if the scope is clearly limited as we don't want to
 end up with a mess that nobody dares touching again as it became to complex.
 So I totally agree that we should share the common stuff we all need and
 adding the extras one needs in the subsystem/driver.
 This is good because it looks like we'll have 3 display subsystems within
 the kernel for a long future and with a common library the same patch would
 not need to be done 3 times but only once. Or even more often if drivers
 have there private EDID implementation which I just throw out of mine to
 replace it later with a common one.


Precisely my point . Also if there are some bad TV models which
doesn't adhere to standard EDID, It would help to add quirks.
Anyone out there want to help me split the DRM code ? As i don't want
DRMer's to fret over changed code :).

Thanks and regards,
Mythri.
 Regards,

 Florian Tobias Schandinat

--
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: [RFC PATCH] HDMI:Support for EDID parsing in kernel.

2011-03-24 Thread K, Mythri P

Hi Jesse,

On Wed, Mar 23, 2011 at 8:48 PM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 On Wed, 23 Mar 2011 18:58:27 +0530
 K, Mythri P mythr...@ti.com wrote:

 Hi Dave,

 On Wed, Mar 23, 2011 at 6:16 AM, Dave Airlie airl...@gmail.com wrote:
  On Wed, Mar 23, 2011 at 3:32 AM, Mythri P K mythr...@ti.com wrote:
  Adding support for common EDID parsing in kernel.
 
  EDID - Extended display identification data is a data structure provided 
  by
  a digital display to describe its capabilities to a video source, This a
  standard supported by CEA and VESA.
 
  There are several custom implementations for parsing EDID in kernel, some
  of them are present in fbmon.c, drm_edid.c, sh_mobile_hdmi.c, Ideally
  parsing of EDID should be done in a library, which is agnostic of the
  framework (V4l2, DRM, FB)  which is using the functionality, just based on
  the raw EDID pointer with size/segment information.
 
  With other RFC's such as the one below, which tries to standardize HDMI 
  API's
  It would be better to have a common EDID code in one place.It also helps 
  to
  provide better interoperability with variety of TV/Monitor may be even by
  listing out quirks which might get missed with several custom 
  implementation
  of EDID.
  http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/30401
 
  This patch tries to add functions to parse some portion EDID (detailed 
  timing,
  monitor limits, AV delay information, deep color mode support, Audio and 
  VSDB)
  If we can align on this library approach i can enhance this library to 
  parse
  other blocks and probably we could also add quirks from other 
  implementation
  as well.
 
 
  If you want to take this approach, you need to start from the DRM EDID 
  parser,
  its the most well tested and I can guarantee its been plugged into more 
  monitors
  than any of the others. There is just no way we would move the DRM parser 
  to a
  library one that isn't derived from it + enhancements, as we'd throw away 
  the
  years of testing and the regression count would be way too high.
 
 I had a look at the DRM EDID code, but for quirks it looks pretty much the 
 same.
 yes i could take quirks and other DRM tested code and enhance, but
 still the code has to do away with struct drm_display_mode
 which is very much custom to DRM.

 If that's the only issue you have, we could easily rename that
 structure or add conversion funcs to a smaller structure if that's what
 you need.

 Dave's point is that we can't ditch the existing code without
 introducing a lot of risk; it would be better to start a library-ized
 EDID codebase from the most complete one we have already, i.e. the DRM
 EDID code.

This sounds good. If we can remove the DRM dependent portion to have a
library-ized EDID code,
That would be perfect. The main Intention to have a library is,
Instead of having several different Implementation in kernel, all
doing the same EDID parsing , if we could have one single
implementation , it would help in better testing and interoperability.

 Do you really think the differences between your code and the existing
 DRM code are irreconcilable?

On the contrary if there is a library-ized  EDID parsing using the
drm_edid, and there is any delta / fields( Parsing the video block in
CEA extension for Short Video Descriptor, Vendor block for AV delay
/Deep color information etc) that are parsed with the RFC i posted i
would be happy to add.

Thanks and regards,
Mythri.
 --
 Jesse Barnes, Intel Open Source Technology Center

--
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: [RFC PATCH] HDMI:Support for EDID parsing in kernel.

2011-03-23 Thread K, Mythri P

Hi Dave,

On Wed, Mar 23, 2011 at 6:16 AM, Dave Airlie airl...@gmail.com wrote:
 On Wed, Mar 23, 2011 at 3:32 AM, Mythri P K mythr...@ti.com wrote:
 Adding support for common EDID parsing in kernel.

 EDID - Extended display identification data is a data structure provided by
 a digital display to describe its capabilities to a video source, This a
 standard supported by CEA and VESA.

 There are several custom implementations for parsing EDID in kernel, some
 of them are present in fbmon.c, drm_edid.c, sh_mobile_hdmi.c, Ideally
 parsing of EDID should be done in a library, which is agnostic of the
 framework (V4l2, DRM, FB)  which is using the functionality, just based on
 the raw EDID pointer with size/segment information.

 With other RFC's such as the one below, which tries to standardize HDMI API's
 It would be better to have a common EDID code in one place.It also helps to
 provide better interoperability with variety of TV/Monitor may be even by
 listing out quirks which might get missed with several custom implementation
 of EDID.
 http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/30401

 This patch tries to add functions to parse some portion EDID (detailed 
 timing,
 monitor limits, AV delay information, deep color mode support, Audio and 
 VSDB)
 If we can align on this library approach i can enhance this library to parse
 other blocks and probably we could also add quirks from other implementation
 as well.


 If you want to take this approach, you need to start from the DRM EDID parser,
 its the most well tested and I can guarantee its been plugged into more 
 monitors
 than any of the others. There is just no way we would move the DRM parser to a
 library one that isn't derived from it + enhancements, as we'd throw away the
 years of testing and the regression count would be way too high.

I had a look at the DRM EDID code, but for quirks it looks pretty much the same.
yes i could take quirks and other DRM tested code and enhance, but
still the code has to do away with struct drm_display_mode
which is very much custom to DRM.

 Dave.


Thanks and regards,
Mythri.
--
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: [RFC PATCH] HDMI:Support for EDID parsing in kernel.

2011-03-23 Thread K, Mythri P

Hi Paul,

On Tue, Mar 22, 2011 at 11:28 PM, Paul Mundt let...@linux-sh.org wrote:
 On Tue, Mar 22, 2011 at 02:52:59PM -0300, Mauro Carvalho Chehab wrote:
 Em 22-03-2011 14:32, Mythri P K escreveu:
  Adding support for common EDID parsing in kernel.
 
  EDID - Extended display identification data is a data structure provided by
  a digital display to describe its capabilities to a video source, This a
  standard supported by CEA and VESA.
 
  There are several custom implementations for parsing EDID in kernel, some
  of them are present in fbmon.c, drm_edid.c, sh_mobile_hdmi.c, Ideally
  parsing of EDID should be done in a library, which is agnostic of the
  framework (V4l2, DRM, FB)  which is using the functionality, just based on
  the raw EDID pointer with size/segment information.
 
  With other RFC's such as the one below, which tries to standardize HDMI 
  API's
  It would be better to have a common EDID code in one place.It also helps to
  provide better interoperability with variety of TV/Monitor may be even by
  listing out quirks which might get missed with several custom 
  implementation
  of EDID.
  http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/30401
 
  This patch tries to add functions to parse some portion EDID (detailed 
  timing,
  monitor limits, AV delay information, deep color mode support, Audio and 
  VSDB)
  If we can align on this library approach i can enhance this library to 
  parse
  other blocks and probably we could also add quirks from other 
  implementation
  as well.
 
  Signed-off-by: Mythri P K mythr...@ti.com
  ---
   arch/arm/include/asm/edid.h |  243 ++
   drivers/video/edid.c        |  340 
  +++

 Hmm... if you want this to be agnostic, the header file should not be inside
 arch/arm, but on some other place, like include/video/.

 Ironically this adds a drivers/video/edid.c but completely ignores
 drivers/video/edid.h which already exists and already contains many of
 these definitions.

 I like the idea of a generalized library, but it would be nice to see the
 existing edid.h evolved and its users updated incrementally.

well yes , That could be enhanced and that would take care of Mauro's
comment too.

Thanks and regards,
Mythri.
--
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 v5 00/10] OMAP4 : DSS2 : HDMI support on OMAP4

2011-03-16 Thread K, Mythri P

Hi Tomi,

On Wed, Mar 16, 2011 at 12:21 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Sun, 2011-03-13 at 11:20 -0500, Stephan Raue wrote:
 Am 13.03.2011 15:32, schrieb Stephan Raue:

 snip

  i have tried this omap2plus defconfig now too, with some little
  changes needed to boot my system (config:
  http://paste.pocoo.org/show/352917/). Now i get this error (see also:
  http://paste.pocoo.org/show/352915/)
 
  [    3.255065] omapdss HDMI: fallback to VGA
  [    3.271636] omapdss HDMI error: Failed to set PHY_PWR_STATUS
  [    3.277587] omapdss HDMI error: failed to power on device
  [    3.283325] omapdss error: failed to power on
  [    3.287902] omapfb omapfb: Failed to enable display 'hdmi'
  [    3.293853] Console: switching to colour dummy device 80x30
  [    3.304595] omapfb omapfb: failed to setup omapfb
  [    3.309539] omapfb: probe of omapfb failed with error -5

 using omap2plus defconfig with my little changes i need to boot my
 system i get the error above, also if i remove CONFIG_ARCH_OMAP2. but if
 i remove CONFIG_ARCH_OMAP3 too i get the error i reported originally
 (the kernel begins to boot, but crashes) (diff between omap2plus
 defconfig without CONFIG_ARCH_OMAP2 and omap2plus defconfig without
 CONFIG_ARCH_OMAP2, CONFIG_ARCH_OMAP3: http://paste.pocoo.org/show/352960/)

 I'm currently traveling for this week so I have quite limited ability to
 test this. Mythri still haven't been able to reproduce any of these
 errors, right?

That's right i have not been able to reproduce the error.  I have
tried with 4 Panda Boards with atleast 4  - 5 Montiors connected (
Also without connecting any monitor).

 Anybody else there to test HDMI on Panda and/or Blaze? I pushed the
 latest DSS + HDMI patches to a test branch for easier testing (I'll push
 Panda DVI patches there also when I get the new version):

 git://gitorious.org/linux-omap-dss2/linux.git test

Stephan,
It would be great if you could try out with this tree with omap2plus_defconfig.
Note : Please disable VENC , there is a known issue of kernel crash
with VENC enabled.

Thanks and regards,
Mythri.
--
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 v5 00/10] OMAP4 : DSS2 : HDMI support on OMAP4

2011-03-14 Thread K, Mythri P

Hi Tomi,Sebastien,

On Tue, Mar 15, 2011 at 9:36 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
On Mon, 2011-03-14 at 05:37 -0500, Jan, Sebastien wrote:
Hi Tomi,

On Thu, Mar 10, 2011 at 2:44 PM, Tomi Valkeinen tomi.valkei...@ti.com
wrote:
I think this version is good. It works for me, at least =).

Any Panda or Blaze owners reading this want to give this a try?

Tested on my pandaboard, with these patches applied on top of your
dss2 tree: I get a working display on my DVI or my HDMI screen. = I
tested by running the Ubuntu UI.

I have a couple of these traces from times to times (seems linked to
screen saver enabling/disabling the screen):
omapdss DISPC error: timeout waiting for EVSYNC

Yes, I get those every time I enable or disable the HDMI output. That
shouldn't cause any problems, but it's something that needs to be fixed
at some point.

The best screen resolution is properly selected on both of my screens,
but the frame-buffer is sized to 640x480. So the display works but the
usable surface when going though /dev/fb0 is quite reduced.
Is there a way to get the FB sized according to the screen resolution
(in the init sequence, it appears that the FBs are allocated before
the init of the HDMI)?

Currently the resolution has to be configured by the user manually. I
haven't tried, but I think giving the resolution with boot parameters
should also work. Mythri, have you tried that?

You could configure the timings later on with the sysfs ( set_timings ) or
HDMI driver would pick the best timing (as suggested by standard) , but
yes framebuffer would have picked the timing on probe so it will not update.

I don't think it is possible for the omapdss, panel or omapfb drivers to
configure the resolution automatically, except at boot time. After we
have booted up, there may be users for the framebuffer and changing it
secretly would cause problems.

I think the best way here is that the HDMI/omapdss driver gives hotplug
event, which is conveyed through omapfb to an userspace application.
This application can then query the available resolutions via some
mechanism, and select one of those.

Fyi, I have also tested with the patches for DVI support and can get
both outputs working simultaneously (my tree is here:
http://dev.omapzoom.org/?p=sebjan/kernel.git;a=shortlog;h=refs/tags/topic-display-iv1-2.6.38-rc7).

Thanks. I'll also set up a test branch in DSS2 tree shortly, to which
I'll add DVI and HDMI patches for easier testing.

Tomi

Thanks and regards,
Mythri.
--
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 v5 00/10] OMAP4 : DSS2 : HDMI support on OMAP4

2011-03-11 Thread K, Mythri P

Hi,


On Sat, Mar 12, 2011 at 12:12 AM, Stephan Raue mailingli...@openelec.tv wrote:
 Am 11.03.2011 19:27, schrieb Tomi Valkeinen:

 On Fri, 2011-03-11 at 10:18 -0600, Stephan Raue wrote:

 Am 11.03.2011 14:22, schrieb Tomi Valkeinen:

 On Fri, 2011-03-11 at 02:43 -0600, Stephan Raue wrote:

 Am 11.03.2011 08:12, schrieb Tomi Valkeinen:

 On Thu, 2011-03-10 at 08:45 -0600, Stephan Raue wrote:

 Hi,

 if i try this patch series the boot stops after (seeing via serial
 console):

      Starting kernel ...

      Uncompressing Linux... done, booting the kernel.

 without this patch series i can see the boot to th OS. I am using
 this
 patches on top of

      http://gitorious.org/linux-omap-dss2/linux/commits/master

 Do i anything wrong or do i use a wrong git repo (the patches apply
 cleanly)? Her my kernel config: http://fpaste.org/9Esw/

 Hmm, I cannot even compile the kernel with that config:

     CC      arch/arm/mach-omap2/io.o
 In file included from arch/arm/mm/init.c:27:0:
 /home/tomba/work/linux/arch/arm/include/asm/tlb.h: In function
 'tlb_flush_mmu':
 /home/tomba/work/linux/arch/arm/include/asm/tlb.h:104:3: error:
 implicit declaration of function 'release_pages'
 In file included from arch/arm/mm/init.c:27:0:
 /home/tomba/work/linux/arch/arm/include/asm/tlb.h: In function
 'tlb_remove_page':
 /home/tomba/work/linux/arch/arm/include/asm/tlb.h:168:3: error:
 implicit declaration of function 'page_cache_release'
 make[1]: *** [arch/arm/mm/init.o] Error 1
 make: *** [arch/arm/mm] Error 2
 make: *** Waiting for unfinished jobs

    Tomi

 please enable swap support or try:

 http://ftp.arm.linux.org.uk/git/?p=linux-2.6-arm.git;a=commit;h=97594b0f35c0708cb9551c070b9693a52ec24ebf
 which is fixed in 2.6.38-rc8

 Ok. So you didn't have just my master branch and the HDMI patch set? I
 see that the config you gave is for rc8, and mine is rc7, so I guess you
 also merged rc8 into your tree?

   Tomi

 yes, you are right, i have tested this with different kernel configs and
 both rc7 and rc8 with the same results.

 Ok. I need to dig out my panda and try to get it running. Blaze board
 seems to work fine, at least for me.

 Does the crash happen every time?

  Tomi

 omap2plus_defconfig

 yes, with all configs i tried. will try a make omap2plus_defconfig too and
 let you know.

I have tried booting on Panda board and it works fine , I am using
OMAP2PLUS defconfig.
I have disabled VENC though( without that i see that kernel hangs , ie
because reading VENC register in hwmod probe is resulting in a crash.

 thanks much

 Stephan

Vaibhav,
You see a crash even without this patch set right ? so should we take
that in a different thread?

Thanks and regards,
Mythri.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 6/9] OMAP4 : DSS2 : HDMI: HDMI panel driver addition in the DSS

Hi Tomi,

On Thu, Mar 10, 2011 at 1:22 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Wed, 2011-03-09 at 05:45 -0600, K, Mythri P wrote:
 The panel driver(hdmi_omap4_panel.c) in omap2/dss acts as a controller
 to manage the enable and disable requests and synchronize audio and video.

 Signed-off-by: Mythri P K mythr...@ti.com

 snip

 +static int hdmi_panel_probe(struct omap_dss_device *dssdev)
 +{
 +     DSSDBG(ENTER hdmi_panel_probe\n);
 +
 +     dssdev-panel.config = OMAP_DSS_LCD_TFT |
 +                     OMAP_DSS_LCD_IVS | OMAP_DSS_LCD_IHS;
 +
 +     /*
 +      * Initialize the timings to 1920 * 1080
 +      * This is only for framebuffer update not for TV timing setting
 +      * Setting TV timing will be done only on enable
 +      */
 +     dssdev-panel.timings.x_res = 1920;
 +     dssdev-panel.timings.y_res = 1080;

 This will cause the framebuffer to be initialized to 1920x1080,
 regardless of the timings the hdmi driver will select.

I have set it to VGA , But anyways we need to fix FB to take the
feedback from the driver
for the timing , atleast when you disable the overlay set manager and
enable it again.


 I think you should either probe the display here to find what it
 supports, and initialize the size accordingly, or if the display is not
 connected, use some safe resolution most of the displays should support.
 VGA probably.

 And the timings selected here should also be used by the hdmi driver.
 What happens now with my monitor is that I get a fb of 1920x1028, but
 the hdmi driver doesn't like the modes my monitor gives via EDID, and
 falls back to VGA - messed up display.

 Also, I'm getting timeout waiting for EVSYNC when I load or unload the
 driver.

This is ok, I have not seen any issue because of this warning,
Also we need to understand the rationale as to why the wait_timeout was added
in the code dispc_enable_digit_out, It seems more like a hack, and
not necessarily needed?

  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 6/9] OMAP4 : DSS2 : HDMI: HDMI panel driver addition in the DSS

Hi Tomi,

On Thu, Mar 10, 2011 at 2:49 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2011-03-10 at 03:00 -0600, K, Mythri P wrote:
 Hi Tomi,

 On Thu, Mar 10, 2011 at 1:22 PM, Tomi Valkeinen tomi.valkei...@ti.com 
 wrote:
  On Wed, 2011-03-09 at 05:45 -0600, K, Mythri P wrote:
  The panel driver(hdmi_omap4_panel.c) in omap2/dss acts as a controller
  to manage the enable and disable requests and synchronize audio and video.
 
  Signed-off-by: Mythri P K mythr...@ti.com
 
  snip
 
  +static int hdmi_panel_probe(struct omap_dss_device *dssdev)
  +{
  +     DSSDBG(ENTER hdmi_panel_probe\n);
  +
  +     dssdev-panel.config = OMAP_DSS_LCD_TFT |
  +                     OMAP_DSS_LCD_IVS | OMAP_DSS_LCD_IHS;
  +
  +     /*
  +      * Initialize the timings to 1920 * 1080
  +      * This is only for framebuffer update not for TV timing setting
  +      * Setting TV timing will be done only on enable
  +      */
  +     dssdev-panel.timings.x_res = 1920;
  +     dssdev-panel.timings.y_res = 1080;
 
  This will cause the framebuffer to be initialized to 1920x1080,
  regardless of the timings the hdmi driver will select.

 I have set it to VGA , But anyways we need to fix FB to take the
 feedback from the driver
 for the timing , atleast when you disable the overlay set manager and
 enable it again.

 No, the FB cannot do anything with that information. FB driver can't
 change the resolution by itself. It has to happen on some upper level,
 in the user space.

 The only case when the FB can select the resolution is when the driver
 is loading. At that point nobody is using the framebuffer, as it didn't
 even exist earlier. That's why I suggested reading the EDID in the
 probe.

Reading the EDID during probe is not feasible ,
1. To read EDID we have to enable clocks and configure HDMI , which
may not be intended - as probe does not mean
that the user would want to enable HDMI.
2. If HDMI is  not connected at boot up , but the cable is plugged in
later on we will run into issues.
There are mechanisms in v4l2 to reconfigure the overlay size and its
own parameters , Even if a notification is sent there is no
mechanism today in FB to do it.
What happens if FB is switched from one manager to other with  resolution ?
There is not neat way to do it today, So i think a more reasonable way
is to find a way to atleast
let the user space change the virtual size of FB dynamically given
there is enough VRAM allocated.



  I think you should either probe the display here to find what it
  supports, and initialize the size accordingly, or if the display is not
  connected, use some safe resolution most of the displays should support.
  VGA probably.
 
  And the timings selected here should also be used by the hdmi driver.
  What happens now with my monitor is that I get a fb of 1920x1028, but
  the hdmi driver doesn't like the modes my monitor gives via EDID, and
  falls back to VGA - messed up display.
 
  Also, I'm getting timeout waiting for EVSYNC when I load or unload the
  driver.
 
 This is ok, I have not seen any issue because of this warning,

 It's an error, it's not ok =).

 Also we need to understand the rationale as to why the wait_timeout was added
 in the code dispc_enable_digit_out, It seems more like a hack, and
 not necessarily needed?

 When disabling, the point is to wait until the DISPC has really turned
 off the output. For VENC this needed waiting for EVSYNC_EVEN and ODD. I
 don't remember why the same code is ran when enabling.

 HDMI may work differently, so it is possible that the function needs to
 be fixed. But I wonder why it gives timeouts when enabling HDMI also...
 Shouldn't the EVSYNCs happen with HDMI too?

 We can look at that later, presuming the timeout doesn't cause any
 problems.

HDMI is not dependent on the EVSYNC_ODD , but on EVSYNC EVEN,
it has never needed to wait and has not caused any issue for last 8-10
months it is being used :).
Why the same function was used ? Because it is a generic digit_out
function to do a TVENABLE.

  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 6/9] OMAP4 : DSS2 : HDMI: HDMI panel driver addition in the DSS

Hi Vaibhav,

On Thu, Mar 10, 2011 at 3:09 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:

 -Original Message-
 From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
 ow...@vger.kernel.org] On Behalf Of K, Mythri P
 Sent: Thursday, March 10, 2011 3:03 PM
 To: Valkeinen, Tomi
 Cc: linux-omap@vger.kernel.org
 Subject: Re: [PATCH v4 6/9] OMAP4 : DSS2 : HDMI: HDMI panel driver
 addition in the DSS

 Hi Tomi,

 On Thu, Mar 10, 2011 at 2:49 PM, Tomi Valkeinen tomi.valkei...@ti.com
 wrote:
  On Thu, 2011-03-10 at 03:00 -0600, K, Mythri P wrote:
  Hi Tomi,

  On Thu, Mar 10, 2011 at 1:22 PM, Tomi Valkeinen tomi.valkei...@ti.com
 wrote:
   On Wed, 2011-03-09 at 05:45 -0600, K, Mythri P wrote:
   The panel driver(hdmi_omap4_panel.c) in omap2/dss acts as a
 controller
   to manage the enable and disable requests and synchronize audio and
 video.

   Signed-off-by: Mythri P K mythr...@ti.com

   snip

 snip

  I have set it to VGA , But anyways we need to fix FB to take the
  feedback from the driver
  for the timing , atleast when you disable the overlay set manager and
  enable it again.

  No, the FB cannot do anything with that information. FB driver can't
  change the resolution by itself. It has to happen on some upper level,
  in the user space.

  The only case when the FB can select the resolution is when the driver
  is loading. At that point nobody is using the framebuffer, as it didn't
  even exist earlier. That's why I suggested reading the EDID in the
  probe.

 Reading the EDID during probe is not feasible ,
 1. To read EDID we have to enable clocks and configure HDMI , which
 may not be intended - as probe does not mean
 that the user would want to enable HDMI.
 2. If HDMI is  not connected at boot up , but the cable is plugged in
 later on we will run into issues.
 There are mechanisms in v4l2 to reconfigure the overlay size and its
 own parameters , Even if a notification is sent there is no
 mechanism today in FB to do it.
 What happens if FB is switched from one manager to other with
 resolution ?
 [Hiremath, Vaibhav] Isn't we get interrupt on HDMI connection back to CPU?

 There is not neat way to do it today, So i think a more reasonable way
 is to find a way to atleast
 let the user space change the virtual size of FB dynamically given
 there is enough VRAM allocated.

 [Hiremath, Vaibhav] I think let default VRAM allocation be minimal and let 
 user configure omapfb.vram using bootargs (or IOCTL) based on his requirement.

yes  I agree , so as Tomi also suggested i'm now setting it to VGA FB
, But it would be nice to have a sysfs to do the same,
ie FB should be able to look for the resolution of the driver it is
connected to and realloc if enough memory is present else default to
original , if there is a change in the manager.
Just a nice to have intelligence i would think.

Thanks and regards,
Mythri.
 Thanks,
 Vaibhav

   I think you should either probe the display here to find what it
   supports, and initialize the size accordingly, or if the display is
 not
   connected, use some safe resolution most of the displays should
 support.
   VGA probably.

   And the timings selected here should also be used by the hdmi driver.
   What happens now with my monitor is that I get a fb of 1920x1028, but
   the hdmi driver doesn't like the modes my monitor gives via EDID, and
   falls back to VGA - messed up display.

   Also, I'm getting timeout waiting for EVSYNC when I load or unload
 the
   driver.

  This is ok, I have not seen any issue because of this warning,

  It's an error, it's not ok =).

  Also we need to understand the rationale as to why the wait_timeout was
 added
  in the code dispc_enable_digit_out, It seems more like a hack, and
  not necessarily needed?

  When disabling, the point is to wait until the DISPC has really turned
  off the output. For VENC this needed waiting for EVSYNC_EVEN and ODD. I
  don't remember why the same code is ran when enabling.

  HDMI may work differently, so it is possible that the function needs to
  be fixed. But I wonder why it gives timeouts when enabling HDMI also...
  Shouldn't the EVSYNCs happen with HDMI too?

  We can look at that later, presuming the timeout doesn't cause any
  problems.

 HDMI is not dependent on the EVSYNC_ODD , but on EVSYNC EVEN,
 it has never needed to wait and has not caused any issue for last 8-10
 months it is being used :).
 Why the same function was used ? Because it is a generic digit_out
 function to do a TVENABLE.

   Tomi

 --
 To unsubscribe from this list: send the line unsubscribe linux-omap in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
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 v5 00/10] OMAP4 : DSS2 : HDMI support on OMAP4

Hi Stephan,

On Fri, Mar 11, 2011 at 5:37 AM, Stephan Raue mailingli...@openelec.tv wrote:
 Hi Tomi,

 Am 10.03.2011 15:51, schrieb Tomi Valkeinen:

 On Thu, 2011-03-10 at 08:45 -0600, Stephan Raue wrote:

 Hi,

 if i try this patch series the boot stops after (seeing via serial
 console):

    Starting kernel ...

    Uncompressing Linux... done, booting the kernel.

 without this patch series i can see the boot to th OS. I am using this
 patches on top of

    http://gitorious.org/linux-omap-dss2/linux/commits/master

 Do i anything wrong or do i use a wrong git repo (the patches apply
 cleanly)? Her my kernel config: http://fpaste.org/9Esw/

 Ah, I guess you hit one yet unsolved problem on on omap4: VENC register
 access fails.

 Can you try switching this off: CONFIG_OMAP2_DSS_VENC.

 I need to make some quick hack to get VENC disabled on OMAP4 until we
 find the solution for this.

  Tomi

 thanks, this helps to boot the kernel. but now i get:


snip

 see also: http://paste.pocoo.org/show/351648/


I see that you kernel is not booting because of dss clk
 [3.335601] PC is at dss_clk_disable_no_ctx+0x0/0xa4
[3.335632] LR is at omap_dispc_register_isr+0xa4/0xcc
Tomi is this related to the clock issue you were mentioning , which
gets solved by adding a delay ?

Thanks and regards,
Mythri.

 if i remove this patchserie again it boots, see here:
 http://paste.pocoo.org/show/351650/

 thanks

 Stephan



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Hi Sanjeev,

On Wed, Mar 9, 2011 at 9:26 PM, Premi, Sanjeev pr...@ti.com wrote:
 -Original Message-
 From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
 ow...@vger.kernel.org] On Behalf Of K, Mythri P
 Sent: Wednesday, March 09, 2011 5:15 PM
 To: linux-omap@vger.kernel.org; Valkeinen, Tomi
 Cc: K, Mythri P
 Subject: [PATCH v4 4/9] OMAP4 : DSS2 : HDMI: HDMI driver header file
 addition

 Adding the hdmi interface driver header file (hdmi.h) to the dss driver.
 Register and structure declaration done here.

 [sp] Any specific reason to keep header file in separate patch?

When i had posted it earlier there were several compliants  that the
patch is huge,
if i include header file with the corresponding c file , and hence
difficult to review. So it just to make
reviewers job easy.

 [snip]...[snip]

Thanks and regards,
Mythri.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/9] OMAP4 : DSS2 : HDMI: HDMI driver addition in the DSS

Hi Sanjeev,

On Wed, Mar 9, 2011 at 9:20 PM, Premi, Sanjeev pr...@ti.com wrote:
 -Original Message-
 From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
 ow...@vger.kernel.org] On Behalf Of K, Mythri P
 Sent: Wednesday, March 09, 2011 5:15 PM
 To: linux-omap@vger.kernel.org; Valkeinen, Tomi
 Cc: K, Mythri P
 Subject: [PATCH v4 5/9] OMAP4 : DSS2 : HDMI: HDMI driver addition in the
 DSS

 Adding the hdmi interface driver(hdmi.c) to the dss driver.
 It configures the audio and video portion of HDMI based on
 functionality called by the  panel driver.

 Signed-off-by: Mythri P K mythr...@ti.com
               Yong Zhi y-...@ti.com
 ---
  drivers/video/omap2/dss/Kconfig   |    8 +
  drivers/video/omap2/dss/Makefile  |    1 +
  drivers/video/omap2/dss/display.c |    3 +
  drivers/video/omap2/dss/dss.h     |   43 ++
  drivers/video/omap2/dss/hdmi.c    | 1315
 +
  5 files changed, 1370 insertions(+), 0 deletions(-)
  create mode 100644 drivers/video/omap2/dss/hdmi.c

 diff --git a/drivers/video/omap2/dss/Kconfig
 b/drivers/video/omap2/dss/Kconfig
 index db01473..7749ddb 100644
 --- a/drivers/video/omap2/dss/Kconfig
 +++ b/drivers/video/omap2/dss/Kconfig
 @@ -60,6 +60,14 @@ config OMAP2_DSS_VENC
       help
         OMAP Video Encoder support for S-Video and composite TV-out.

 +config OMAP2_DSS_HDMI
 +     bool HDMI support
 +     depends on ARCH_OMAP4
 +        default y
 +     help
 +       HDMI Interface. This adds the High Definition Multimedia
 Interface.
 +       See http://www.hdmi.org/ for HDMI specification.
 +
 [sp] This may have been discussed earlier... but if the interface is
     supported only on OMAP4, wouldn't it be better to rename
     OMAP2_DSS_HDMI as OMAP4_DSS_HDMI?
     Or, the implementation can be used by OMAP3 as well. If so, you
     may want to update the depends.
I shall change it to OMAP4.

  config OMAP2_DSS_SDI
       bool SDI support
       depends on ARCH_OMAP3
 diff --git a/drivers/video/omap2/dss/Makefile
 b/drivers/video/omap2/dss/Makefile
 index 7db17b5..5998b69 100644
 --- a/drivers/video/omap2/dss/Makefile
 +++ b/drivers/video/omap2/dss/Makefile
 @@ -5,3 +5,4 @@ omapdss-$(CONFIG_OMAP2_DSS_RFBI) += rfbi.o
  omapdss-$(CONFIG_OMAP2_DSS_VENC) += venc.o
  omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
  omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o
 +omapdss-$(CONFIG_OMAP2_DSS_HDMI) += hdmi.o
 diff --git a/drivers/video/omap2/dss/display.c
 b/drivers/video/omap2/dss/display.c
 index c40bcbd..a85a6f3 100644
 --- a/drivers/video/omap2/dss/display.c
 +++ b/drivers/video/omap2/dss/display.c
 @@ -418,6 +418,9 @@ void dss_init_device(struct platform_device *pdev,
               r = dsi_init_display(dssdev);
               break;
  #endif
 +     case OMAP_DISPLAY_TYPE_HDMI:
 +             r = hdmi_init_display(dssdev);
 +             break;
       default:
               DSSERR(Support for display '%s' not compiled in.\n,
                               dssdev-name);
 diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
 index 008cbf4..3eb2661 100644
 --- a/drivers/video/omap2/dss/dss.h
 +++ b/drivers/video/omap2/dss/dss.h
 @@ -174,6 +174,16 @@ struct dsi_clock_info {
       bool use_sys_clk;
  };

 +/* HDMI PLL structure */
 +struct hdmi_pll_info {
 +     u16 regn;
 +     u16 regm;
 +     u32 regmf;
 +     u16 regm2;
 +     u16 regsd;
 +     u16 dcofreq;
 +};
 +
  struct seq_file;
  struct platform_device;

 @@ -437,6 +447,39 @@ static inline void venc_uninit_platform_driver(void)
  }
  #endif

 +/* HDMI */
 +#ifdef CONFIG_OMAP2_DSS_HDMI
 +int hdmi_init_platform_driver(void);
 +void hdmi_uninit_platform_driver(void);
 +int hdmi_init_display(struct omap_dss_device *dssdev);
 +#else
 +static inline int hdmi_init_display(struct omap_dss_device *dssdev)
 +{
 +     return 0;
 +}
 +static inline int hdmi_init_platform_driver(void)
 +{
 +     return 0;
 +}
 +static inline void hdmi_uninit_platform_driver(void)
 +{
 +}
 +#endif
 +int omapdss_hdmi_display_enable(struct omap_dss_device *dssdev);
 +void omapdss_hdmi_display_disable(struct omap_dss_device *dssdev);
 +#ifdef CONFIG_OMAP4_PANEL_HDMI
 +int hdmi_panel_init(void);
 +void hdmi_panel_exit(void);
 +#else
 +static inline int hdmi_panel_init(void)
 +{
 +     return 0;
 +}
 +static inline void hdmi_panel_exit(void)
 +{
 +}
 +#endif
 +
  /* RFBI */
  #ifdef CONFIG_OMAP2_DSS_RFBI
  int rfbi_init_platform_driver(void);
 diff --git a/drivers/video/omap2/dss/hdmi.c
 b/drivers/video/omap2/dss/hdmi.c
 new file mode 100644
 index 000..6852843
 --- /dev/null
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -0,0 +1,1315 @@
 +/*
 + * hdmi.c
 + *
 + * HDMI interface DSS driver setting for TI's OMAP4 family of processor.
 + * Copyright (C) 2010-2011 Texas Instruments Incorporated -
 http://www.ti.com/
 + * Authors: Yong Zhi
 + *   Mythri pk mythr...@ti.com
 + *
 + *

 [sp] Extra blank lines here

 + * This program is free software; you can redistribute it and/or modify
 it
 + * under the terms

Re: [PATCH v4 7/9] OMAP4 : DSS : HDMI: Call to HDMI module init to register driver.

Hi Sanjeev,

On Wed, Mar 9, 2011 at 9:24 PM, Premi, Sanjeev pr...@ti.com wrote:
 -Original Message-
 From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
 ow...@vger.kernel.org] On Behalf Of K, Mythri P
 Sent: Wednesday, March 09, 2011 5:15 PM
 To: linux-omap@vger.kernel.org; Valkeinen, Tomi
 Cc: K, Mythri P
 Subject: [PATCH v4 7/9] OMAP4 : DSS : HDMI: Call to HDMI module init to
 register driver.

 calling the platform registration of HDMI driver from core
 during initialization.

 [sp] Contrary to the subject, patch adds calls for both init and uninit
     functions.
Subject says calling of the registration function.

     Shouldn't this be included in same patch where init and uninit are
     being defined?
Again i would prefer to keep the hdmi.c separate , do you see any
issue in splitting ? I have made sure every
patch compiles.

 [snip]...[snip]

Thanks and regards,
Mythri.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 6/9] OMAP4 : DSS2 : HDMI: HDMI panel driver addition in the DSS

Hi Tomi,

On Wed, Mar 9, 2011 at 7:53 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Wed, 2011-03-09 at 05:45 -0600, K, Mythri P wrote:
 The panel driver(hdmi_omap4_panel.c) in omap2/dss acts as a controller
 to manage the enable and disable requests and synchronize audio and video.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/Kconfig            |    8 +
  drivers/video/omap2/dss/Makefile           |    1 +
  drivers/video/omap2/dss/hdmi_omap4_panel.c |  209 
 
  3 files changed, 218 insertions(+), 0 deletions(-)
  create mode 100644 drivers/video/omap2/dss/hdmi_omap4_panel.c

 diff --git a/drivers/video/omap2/dss/Kconfig 
 b/drivers/video/omap2/dss/Kconfig
 index 7749ddb..f66043f 100644
 --- a/drivers/video/omap2/dss/Kconfig
 +++ b/drivers/video/omap2/dss/Kconfig
 @@ -125,4 +125,12 @@ config OMAP2_DSS_MIN_FCK_PER_PCK
         Max FCK is 173MHz, so this doesn't work if your PCK
         is very high.

 +config OMAP4_PANEL_HDMI
 +     bool HDMI panel support
 +     depends on OMAP2_DSS_HDMI
 +        default n
 +     help
 +       HDMI panel. This adds the High Definition Multimedia panel.
 +       See http://www.hdmi.org/ for HDMI specification.
 +
  endif
 diff --git a/drivers/video/omap2/dss/Makefile 
 b/drivers/video/omap2/dss/Makefile
 index 5998b69..7ee4093 100644
 --- a/drivers/video/omap2/dss/Makefile
 +++ b/drivers/video/omap2/dss/Makefile
 @@ -6,3 +6,4 @@ omapdss-$(CONFIG_OMAP2_DSS_VENC) += venc.o
  omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
  omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o
  omapdss-$(CONFIG_OMAP2_DSS_HDMI) += hdmi.o
 +omapdss-$(CONFIG_OMAP4_PANEL_HDMI) += hdmi_omap4_panel.o

 You don't need CONFIG_OMAP4_PANEL_HDMI. In fact, you cannot have it:
 hdmi.c calls the panel driver, so they must be compiled in or out
 together.

 Also, as they depend on each other, adding them in patches like this
 breaks compilation. Do it so that first add hdmi.c in a patch. Then
 hdmi_omap4_panel.c. After those, add Kconfig and Makefile changes. This
 way the kernel stays compilable, and you can still have the additions
 divided into separate patches.
Sure i will do that.

  Tomi

Thanks and regards,
Mythri.
--
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 v3 0/9] OMAP4 : DSS2 : HDMI support on OMAP4

2011-03-08 Thread K, Mythri P

Hi Tomi,

On Mon, Mar 7, 2011 at 2:10 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-03-04 at 01:48 -0600, K, Mythri P wrote:
 Adding HDMI support on OMAP4.

 HDMI is a driver that is similar to the VENC or the DSI driver to support
 HDMI/DVI sink device.

 snip

 1. v10 of omap2,3 hwmod DSS adaptation:
 http://www.mail-archive.com/linux-omap@vger.kernel.org/msg42914.html
 2. v3 of OMAP2PLUS: DSS: Generalize clock names
 http://www.mail-archive.com/linux-omap@vger.kernel.org/msg43338.html
 3. v3 of OMAP4: hwmod DSS support:
 http://www.mail-archive.com/linux-omap@vger.kernel.org/msg43177.html
 4. OMAP: DSS2: Adding dss_features for independent core clk divider
 https://patchwork.kernel.org/patch/529561/
 5. OMAP2PLUS-DSS2-Make-members-of-dss_clk_source-generic

 I believe all these are now in DSS2 tree's master branch, so can you
 rebase the next version on top of the master branch?
I will rebase my patch. And list only dependency ie not present , if needed.

  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2011-03-08 Thread K, Mythri P

Hi Tomi,

On Mon, Mar 7, 2011 at 3:16 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 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.

 The subject and description are wrong. Always before sending patches do
 a quick sanity check, by check the subjects, looking at the stats in the
 intro letter, etc.
My bad ,will reread.

 Signed-off-by: Mythri P K mythr...@ti.com

snip

 +const u8 header[8] = {0x0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x0};

 Bad variable name, use something more descriptive.

will rename to edid_header.
 +

snip
 +static int hdmi_wait_phy_pwr(int val)

 These comments apply also to the function below.

 The parameter should be an enum.

 The function is misnamed. This doesn't wait for the power mode to
 change, as the name implies. This sets the power mode.


will make refsel an enum and add other option , also make all these
functions to take enum as a parameter.

snip

 +        * SW HACK : DDC needs time to stablize else it sometimes reads 0 
 values
 +        * or right shifted values.
 +        */
 +       usleep_range(800, 1000);

 This is still unclear. Is it an OMAP HW problem? OMAP HW feature? A
 feature in HDMI TVs? A HDMI spec delay? Or unknown reason?

there is an internal DDC (i2c bus) , without this delay I have
sometimes seen that while reading EDID
we sometimes (Not always and only with some TV's , even with a
particular TV it is not consistent)
read a shifted value or EDID value is 0. I can clarify with the h/w
team , but  can we add this as a s/w hack for time being?

snip

Thanks and regards,
Mythri.
--
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 v2 2/8] OMAP4 : DSS2 : HDMI: HDMI specific display controller and dss change.

2011-03-02 Thread K, Mythri P

Hi Tomi,

On Tue, Mar 1, 2011 at 10:31 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2011-03-01 at 08:16 -0600, K, Mythri P wrote:
 Adding changes to set gamma table bit for TV interface and function to select
 between VENC and HDMI.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/dispc.c |    5 +
  drivers/video/omap2/dss/dss.c   |    5 +
  drivers/video/omap2/dss/dss.h   |    7 +++
  3 files changed, 17 insertions(+), 0 deletions(-)

 diff --git a/drivers/video/omap2/dss/dispc.c 
 b/drivers/video/omap2/dss/dispc.c
 index 69e1e9d..cf385f8 100644
 --- a/drivers/video/omap2/dss/dispc.c
 +++ b/drivers/video/omap2/dss/dispc.c
 @@ -1224,6 +1224,11 @@ void dispc_enable_zorder(enum omap_plane plane, bool 
 enable)
       dispc_write_reg(dispc_reg_att[plane], val);
  }

 +void dispc_enable_gamma_table(bool enable)
 +{
 +     REG_FLD_MOD(DISPC_CONFIG, enable, 9, 9);
 +}

 I would separate the gamma function out from this patch. These two
 things are not related in any way.

 And I'm not quite happy with this partial gamma implementation, as only
 the enable function is implemented. At the minimum, put BUG_ON(enable);
 there, and a comment that this can currently only be used to disable
 gamma support.
I shall separate these out.

 +
  static void _dispc_set_vid_color_conv(enum omap_plane plane, bool enable)
  {
       u32 val;
 diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
 index 99de4e1..ca76e68 100644
 --- a/drivers/video/omap2/dss/dss.c
 +++ b/drivers/video/omap2/dss/dss.c
 @@ -559,6 +559,11 @@ void dss_set_dac_pwrdn_bgz(bool enable)
       REG_FLD_MOD(DSS_CONTROL, enable, 5, 5); /* DAC Power-Down Control */
  }

 +void dss_select_hdmi_venc(enum dss_hdmi_venc_select hdmi)
 +{
 +     REG_FLD_MOD(DSS_CONTROL, hdmi, 15, 15); /* 0x1 for HDMI, 0x0 VENC */

 That comment is not needed, but you could put VENC_HDMI_SWITCH there
 in the comments.

 +}
 +
  static int dss_init(bool skip_init)
  {
       int r;
 diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
 index b6f27fe..5b5b90c 100644
 --- a/drivers/video/omap2/dss/dss.h
 +++ b/drivers/video/omap2/dss/dss.h
 @@ -123,6 +123,11 @@ enum dss_clk_source {
       DSS_SRC_DSS1_ALWON_FCLK,
  };

 +enum dss_hdmi_venc_select {
 +     DSS_VENC_SELECT,        /* Select VENC */
 +     DSS_HDMI_SELECT,        /* Select HDMI */
 +};

 If the integer value of the enum is important, like in this case, you
 need to define the value: DSS_VENC_SELECT = 0.

 Also, the comments are not needed, they just say the same thing as the
 enum itself.

 And only now I realized that this is a clock source switch. The enum and
 the function should be named appropriately, for example enum
 dss_tv_clk_out_source. And the sources are TV_CLK and HDMI_M_PCLK, so
 the the enum values should use those as part of their name.

ok i shall name them appropriately.
  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 5/8] OMAP4 : DSS2 : HDMI: HDMI panel driver addition in the DSS

2011-03-02 Thread K, Mythri P

On Tue, Mar 1, 2011 at 10:41 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2011-03-01 at 08:16 -0600, K, Mythri P wrote:
 The panel driver(hdmi_omap4_panel.c) in dss file acts as a controller
 to manage the enable and disable requests and synchronize audio and video.
 Also the header file to export the hdmi registers is added in the
 plat-omap , so that it can be accessed by audio driver.

 Could you review the description. The panel driver is not in dss file
 and there's no header in this patch.

 The locking is broken in this patch. You cannot return from the function
 while holding the lock.
I shall fix the lock and describe appropriately.

  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/8] OMAP4 : DSS2 : HDMI: HDMI driver addition in the DSS drivers interface

2011-03-02 Thread K, Mythri P

Hi Tomi,
I have checked for static functions :) .
Ok i will revisit the global variables to either move it to hdmi
struct or make it const.

Thanks and regards,
Mythri.

On Tue, Mar 1, 2011 at 11:07 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2011-03-01 at 08:16 -0600, K, Mythri P wrote:
 Adding the hdmi interface driver(hdmi.c) to the dss driver.
 It configures the audio and video portion of HDMI in the
 display header file to be accessed by the panels.

 Check once more if the private functions and variables are marked as
 static.

 The edid variables should be in the hdmi struct. The constant data
 arrays should be const.

 hdmi_power_on() leaves clocks on in case of error.

  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] OMAP4 : DSS2 : Add display structure in the board file for OMAP4 sdp

Hi Tomi,


On Sun, Feb 27, 2011 at 2:43 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-02-25 at 08:21 -0600, K, Mythri P wrote:
 Adding  board file changes for display which adds the display structure
 with HDMI as the default driver when the display init is called.
 HDMI GPIO configurations are also done in this file.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  arch/arm/mach-omap2/board-4430sdp.c |   82 
 +++
  1 files changed, 82 insertions(+), 0 deletions(-)

 You could move this patch in the end of the set, together with the panda
 board patch.

 The prefix OMAP4: DSS2 is not quite right for this, as this is a board
 file change. And you should mention HDMI in the subject. Perhaps
 something like OMAP: 4430SDP: Add HDMI support

Sure i will move and change the patch name.

 diff --git a/arch/arm/mach-omap2/board-4430sdp.c 
 b/arch/arm/mach-omap2/board-4430sdp.c
 index 07d1b20..334b6fd 100644
 --- a/arch/arm/mach-omap2/board-4430sdp.c
 +++ b/arch/arm/mach-omap2/board-4430sdp.c
 @@ -35,6 +35,7 @@
  #include plat/common.h
  #include plat/usb.h
  #include plat/mmc.h
 +#include plat/display.h

  #include mux.h
  #include hsmmc.h
 @@ -47,6 +48,8 @@
  #define OMAP4SDP_MDM_PWR_EN_GPIO     157
  #define OMAP4_SFH7741_SENSOR_OUTPUT_GPIO     184
  #define OMAP4_SFH7741_ENABLE_GPIO            188
 +#define HDMI_GPIO_HPD 60 /* Hot plug pin for HDMI */
 +#define HDMI_GPIO_LS_OE 41 /* Level shifter for HDMI */

  static struct gpio_led sdp4430_gpio_leds[] = {
       {
 @@ -552,6 +555,84 @@ static void __init omap_sfh7741prox_init(void)
       }
  }

 +static void sdp4430_hdmi_mux_init(void)
 +{
 +     /* PAD0_HDMI_HPD_PAD1_HDMI_CEC */
 +     omap_mux_init_signal(hdmi_hpd,
 +                     OMAP_PIN_INPUT_PULLUP);
 +     omap_mux_init_signal(hdmi_cec,
 +                     OMAP_PIN_INPUT_PULLUP);
 +     /* PAD0_HDMI_DDC_SCL_PAD1_HDMI_DDC_SDA */
 +     omap_mux_init_signal(hdmi_ddc_scl,
 +                     OMAP_PIN_INPUT_PULLUP);
 +     omap_mux_init_signal(hdmi_ddc_sda,
 +                     OMAP_PIN_INPUT_PULLUP);
 +}
 +
 +static int sdp4430_panel_enable_hdmi(struct omap_dss_device *dssdev)
 +{
 +     int status;

 r is quite often used as a name for return values.

Variable status is consistent with other gpio request calls made in the file ,
so it would be good to have status instead of r ,
I have used r in all DSS files though.
 +
 +     status = gpio_request_one(HDMI_GPIO_HPD, GPIOF_DIR_OUT,
 +                                                     hdmi_gpio_hpd);
 +     if (status) {
 +             pr_err(Cannot request GPIO %d\n, HDMI_GPIO_HPD);
 +             return status;
 +     }
 +     status = gpio_request_one(HDMI_GPIO_LS_OE, GPIOF_DIR_OUT,
 +                                                     hdmi_gpio_ls_oe);
 +     if (status) {
 +             pr_err(Cannot request GPIO %d\n, HDMI_GPIO_LS_OE);
 +             goto error1;
 +     }
 +
 +     /* The value set a pulse */

 I still don't understand that comment. It's not even English.
I was seeing issues where I had to set the pulse [ 101] , But i no
longer see the problem in my tests.
So i shall just enable GPIO.

  Tomi



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] OMAP4 : DSS : HDMI: HDMI driver header file addition

Hi Tomi,

On Sun, Feb 27, 2011 at 2:58 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-02-25 at 08:21 -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 mythr...@ti.com
 ---
  drivers/video/omap2/dss/hdmi.h |  691 
 
  1 files changed, 691 insertions(+), 0 deletions(-)
  create mode 100644 drivers/video/omap2/dss/hdmi.h

 diff --git a/drivers/video/omap2/dss/hdmi.h b/drivers/video/omap2/dss/hdmi.h
 new file mode 100644
 index 000..7441835
 --- /dev/null
 +++ b/drivers/video/omap2/dss/hdmi.h
 @@ -0,0 +1,691 @@
 +/*
 + * hdmi.h
 + *
 + * HDMI driver definition for TI OMAP4 processors.
 + *
 + * Copyright (C) 2010-2011 Texas Instruments Incorporated - 
 http://www.ti.com/
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms of the GNU General Public License version 2 as published 
 by
 + * the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful, but 
 WITHOUT
 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 + * more details.
 + *
 + * You should have received a copy of the GNU General Public License along 
 with
 + * this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#ifndef _HDMI_H_
 +#define _HDMI_H_

 This should be more specific. For example, __OMAP2_DSS_HDMI_H would be
 in line with the other includes.

ok i shall change
 +
 +#include linux/string.h
 +#include plat/display.h
 +
 +#define HDMI_WP                        0x0
 +#define HDMI_CORE_SYS          0x400
 +#define HDMI_CORE_AV           0x900
 +#define HDMI_PLLCTRL           0x200
 +#define HDMI_PHY               0x300
 +
 +struct hdmi_reg { u16 idx; };
 +
 +#define HDMI_REG(idx)                  ((const struct hdmi_reg) { idx })
 +
 +/* HDMI Wrapper */
 +#define HDMI_WP_REG(idx)                       HDMI_REG(HDMI_WP + idx)
 +
 +#define HDMI_WP_REVISION                       HDMI_WP_REG(0x0)
 +#define HDMI_WP_SYSCONFIG                      HDMI_WP_REG(0x10)
 +#define HDMI_WP_IRQSTATUS_RAW                  HDMI_WP_REG(0x24)
 +#define HDMI_WP_IRQSTATUS                      HDMI_WP_REG(0x28)
 +#define HDMI_WP_PWR_CTRL                       HDMI_WP_REG(0x40)
 +#define HDMI_WP_IRQENABLE_SET                  HDMI_WP_REG(0x2C)
 +#define HDMI_WP_VIDEO_CFG                      HDMI_WP_REG(0x50)
 +#define HDMI_WP_VIDEO_SIZE                     HDMI_WP_REG(0x60)
 +#define HDMI_WP_VIDEO_TIMING_H                 HDMI_WP_REG(0x68)
 +#define HDMI_WP_VIDEO_TIMING_V                 HDMI_WP_REG(0x6C)
 +#define HDMI_WP_WP_CLK                         HDMI_WP_REG(0x70)
 +
 +/* HDMI IP Core System */
 +#define HDMI_CORE_SYS_REG(idx)                 HDMI_REG(HDMI_CORE_SYS + idx)
 +
 +#define HDMI_CORE_SYS_VND_IDL                  HDMI_CORE_SYS_REG(0x0)
 +#define HDMI_CORE_SYS_DEV_IDL                  HDMI_CORE_SYS_REG(0x8)
 +#define HDMI_CORE_SYS_DEV_IDH                  HDMI_CORE_SYS_REG(0xC)
 +#define HDMI_CORE_SYS_DEV_REV                  HDMI_CORE_SYS_REG(0x10)
 +#define HDMI_CORE_SYS_SRST                     HDMI_CORE_SYS_REG(0x14)
 +#define HDMI_CORE_CTRL1                                
 HDMI_CORE_SYS_REG(0x20)
 +#define HDMI_CORE_SYS_SYS_STAT                 HDMI_CORE_SYS_REG(0x24)
 +#define HDMI_CORE_SYS_VID_ACEN                 HDMI_CORE_SYS_REG(0x124)
 +#define HDMI_CORE_SYS_VID_MODE                 HDMI_CORE_SYS_REG(0x128)
 +#define HDMI_CORE_SYS_INTR_STATE               HDMI_CORE_SYS_REG(0x1C0)
 +#define HDMI_CORE_SYS_INTR1                    HDMI_CORE_SYS_REG(0x1C4)
 +#define HDMI_CORE_SYS_INTR2                    HDMI_CORE_SYS_REG(0x1C8)
 +#define HDMI_CORE_SYS_INTR3                    HDMI_CORE_SYS_REG(0x1CC)
 +#define HDMI_CORE_SYS_INTR4                    HDMI_CORE_SYS_REG(0x1D0)
 +#define HDMI_CORE_SYS_UMASK1                   HDMI_CORE_SYS_REG(0x1D4)
 +#define HDMI_CORE_SYS_TMDS_CTRL                        
 HDMI_CORE_SYS_REG(0x208)
 +#define HDMI_CORE_SYS_DE_DLY                   HDMI_CORE_SYS_REG(0xC8)
 +#define HDMI_CORE_SYS_DE_CTRL                  HDMI_CORE_SYS_REG(0xCC)
 +#define HDMI_CORE_SYS_DE_TOP                   HDMI_CORE_SYS_REG(0xD0)
 +#define HDMI_CORE_SYS_DE_CNTL                  HDMI_CORE_SYS_REG(0xD8)
 +#define HDMI_CORE_SYS_DE_CNTH                  HDMI_CORE_SYS_REG(0xDC)
 +#define HDMI_CORE_SYS_DE_LINL                  HDMI_CORE_SYS_REG(0xE0)
 +#define HDMI_CORE_SYS_DE_LINH_1                        
 HDMI_CORE_SYS_REG(0xE4)
 +#define HDMI_CORE_CTRL1_VEN_FOLLOWVSYNC                0x1
 +#define HDMI_CORE_CTRL1_HEN_FOLLOWHSYNC                0x1
 +#define HDMI_CORE_CTRL1_BSEL_24BITBUS          0x1
 +#define HDMI_CORE_CTRL1_EDGE_RISINGEDGE                0x1
 +
 +/* HDMI DDC E-DID

Re: [PATCH 5/8] OMAP4 : DSS2 : HDMI: HDMI driver addition in the DSS drivers interface

Hi Tomi,

On Sun, Feb 27, 2011 at 3:47 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-02-25 at 08:21 -0600, K, Mythri P wrote:
 Adding the hdmi interface driver(hdmi.c) to the dss driver.
 It configures the audio and video portion of HDMI in the
 display header file to be accessed by the panels.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/Kconfig   |    8 +
  drivers/video/omap2/dss/Makefile  |    1 +
  drivers/video/omap2/dss/display.c |    3 +
  drivers/video/omap2/dss/dss.h     |   33 +
  drivers/video/omap2/dss/hdmi.c    | 1276 
 +
  drivers/video/omap2/dss/hdmi.h    |    9 +
  6 files changed, 1330 insertions(+), 0 deletions(-)
  create mode 100644 drivers/video/omap2/dss/hdmi.c

 diff --git a/drivers/video/omap2/dss/Kconfig 
 b/drivers/video/omap2/dss/Kconfig
 index 0d031b2..fe1ab09 100644
 --- a/drivers/video/omap2/dss/Kconfig
 +++ b/drivers/video/omap2/dss/Kconfig
 @@ -60,6 +60,14 @@ config OMAP2_DSS_VENC
         help
           OMAP Video Encoder support for S-Video and composite TV-out.

 +config OMAP2_DSS_HDMI
 +       bool HDMI support
 +       depends on ARCH_OMAP4
 +        default n
 +       help
 +         HDMI Interface. This adds the High Definition Multimedia Interface.
 +         See http://www.hdmi.org/ for HDMI specification.
 +
  config OMAP2_DSS_SDI
         bool SDI support
         depends on ARCH_OMAP3
 diff --git a/drivers/video/omap2/dss/Makefile 
 b/drivers/video/omap2/dss/Makefile
 index 7db17b5..5998b69 100644
 --- a/drivers/video/omap2/dss/Makefile
 +++ b/drivers/video/omap2/dss/Makefile
 @@ -5,3 +5,4 @@ omapdss-$(CONFIG_OMAP2_DSS_RFBI) += rfbi.o
  omapdss-$(CONFIG_OMAP2_DSS_VENC) += venc.o
  omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
  omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o
 +omapdss-$(CONFIG_OMAP2_DSS_HDMI) += hdmi.o
 diff --git a/drivers/video/omap2/dss/display.c 
 b/drivers/video/omap2/dss/display.c
 index e10b303..cbab61a 100644
 --- a/drivers/video/omap2/dss/display.c
 +++ b/drivers/video/omap2/dss/display.c
 @@ -447,6 +447,9 @@ void dss_init_device(struct platform_device *pdev,
                 r = dsi_init_display(dssdev);
                 break;
  #endif
 +       case OMAP_DISPLAY_TYPE_HDMI:
 +               r = hdmi_init_display(dssdev);
 +               break;
         default:
                 BUG();
         }
 diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
 index d199ba7..171877e 100644
 --- a/drivers/video/omap2/dss/dss.h
 +++ b/drivers/video/omap2/dss/dss.h
 @@ -163,6 +163,16 @@ struct dsi_clock_info {
         bool use_dss2_fck;
  };

 +/* HDMI PLL structure */
 +struct hdmi_pll_info {
 +       u16 regn;
 +       u16 regm;
 +       u32 regmf;
 +       u16 regm2;
 +       u16 regsd;
 +       u16 dcofreq;
 +};
 +
  struct seq_file;
  struct platform_device;

 @@ -428,6 +438,29 @@ static inline void venc_uninit_platform_driver(void)
  }
  #endif

 +/* HDMI */
 +#ifdef CONFIG_OMAP2_DSS_HDMI
 +int hdmi_init_platform_driver(void);
 +void hdmi_uninit_platform_driver(void);
 +int hdmi_init_display(struct omap_dss_device *dssdev);
 +#else
 +static inline int hdmi_init_display(struct omap_dss_device *dssdev)
 +{
 +       return 0;
 +}
 +static inline int hdmi_init_platform_driver(void)
 +{
 +       return 0;
 +}
 +static inline void hdmi_uninit_platform_driver(void)
 +{
 +}
 +#endif
 +int omapdss_hdmi_display_enable(struct omap_dss_device *dssdev);
 +void omapdss_hdmi_display_disable(struct omap_dss_device *dssdev);
 +int omapdss_hdmi_display_suspend(struct omap_dss_device *dssdev);
 +int omapdss_hdmi_display_resume(struct omap_dss_device *dssdev);
 +
  /* RFBI */
  #ifdef CONFIG_OMAP2_DSS_RFBI
  int rfbi_init_platform_driver(void);
 diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
 new file mode 100644
 index 000..1cd861f
 --- /dev/null
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -0,0 +1,1276 @@
 +/*
 + * hdmi.c
 + *
 + * HDMI interface DSS driver setting for TI's OMAP4 family of processor.
 + * Copyright (C) 2010-2011 Texas Instruments Incorporated - 
 http://www.ti.com/
 + * Authors: Yong Zhi

 Who is Yong Zhi and where's his signed-off-by?
Yong zhi had written some of the functions in this file , which was in
a library.He no longer works on the driver though .

 + *     Mythri pk mythr...@ti.com
 + *
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms of the GNU General Public License version 2 as published 
 by
 + * the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful, but 
 WITHOUT
 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 + * more details.
 + *
 + * You should have received a copy of the GNU General Public License along 
 with
 + * this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#define

Re: [PATCH 6/8] OMAP4 : DSS2 : HDMI: HDMI panel driver addition in the DSS

Hi Tomi,
I shall add per device lock and make the lock consistent , as audio
can also call these functions.

Thanks and regards,
Mythri.

On Sun, Feb 27, 2011 at 3:13 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-02-25 at 08:21 -0600, K, Mythri P wrote:
 The panel driver(hdmi_omap4_panel.c) in dss file acts as a controller
 to manage the enable and disable requests and synchronize audio and video.
 Also the header file to export the hdmi registers is added in the
 plat-omap , so that it can be accessed by audio driver.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/Kconfig            |    8 ++
  drivers/video/omap2/dss/Makefile           |    1 +
  drivers/video/omap2/dss/hdmi_omap4_panel.c |  190 
 
  3 files changed, 199 insertions(+), 0 deletions(-)
  create mode 100644 drivers/video/omap2/dss/hdmi_omap4_panel.c

 diff --git a/drivers/video/omap2/dss/Kconfig 
 b/drivers/video/omap2/dss/Kconfig
 index fe1ab09..670a55d 100644
 --- a/drivers/video/omap2/dss/Kconfig
 +++ b/drivers/video/omap2/dss/Kconfig
 @@ -125,4 +125,12 @@ config OMAP2_DSS_MIN_FCK_PER_PCK
         Max FCK is 173MHz, so this doesn't work if your PCK
         is very high.

 +config OMAP4_PANEL_HDMI
 +     bool HDMI panel support
 +     depends on OMAP2_DSS_HDMI
 +        default n
 +     help
 +       HDMI panel. This adds the High Definition Multimedia panel.
 +       See http://www.hdmi.org/ for HDMI specification.
 +
  endif
 diff --git a/drivers/video/omap2/dss/Makefile 
 b/drivers/video/omap2/dss/Makefile
 index 5998b69..95234d4 100644
 --- a/drivers/video/omap2/dss/Makefile
 +++ b/drivers/video/omap2/dss/Makefile
 @@ -6,3 +6,4 @@ omapdss-$(CONFIG_OMAP2_DSS_VENC) += venc.o
  omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
  omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o
  omapdss-$(CONFIG_OMAP2_DSS_HDMI) += hdmi.o
 +obj-$(CONFIG_OMAP4_PANEL_HDMI) += hdmi_omap4_panel.o
 diff --git a/drivers/video/omap2/dss/hdmi_omap4_panel.c 
 b/drivers/video/omap2/dss/hdmi_omap4_panel.c
 new file mode 100644
 index 000..c28b1f6
 --- /dev/null
 +++ b/drivers/video/omap2/dss/hdmi_omap4_panel.c
 @@ -0,0 +1,190 @@
 +/*
 + * hdmi_omap4_panel.c
 + *
 + * HDMI library support functions for TI OMAP4 processors.
 + *
 + * Copyright (C) 2010-2011 Texas Instruments Incorporated - 
 http://www.ti.com/
 + * Authors:  MythriPk mythr...@ti.com
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms of the GNU General Public License version 2 as published 
 by
 + * the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful, but 
 WITHOUT
 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 + * more details.
 + *
 + * You should have received a copy of the GNU General Public License along 
 with
 + * this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#define DSS_SUBSYS_NAME HDMI

 This is not used.

 +
 +#include linux/kernel.h
 +#include linux/err.h
 +#include linux/io.h
 +#include linux/mutex.h
 +#include linux/string.h
 +#include linux/delay.h
 +#include linux/module.h
 +#include linux/seq_file.h
 +#include plat/display.h

 Most of these includes are not needed.

 +
 +#include dss.h
 +
 +static struct {
 +     struct mutex hdmi_lock;
 +} hdmi;

 Static variables in panel drivers are not good, as they won't work if
 there are two devices which use the same driver. In this case that
 cannot happen, as there's only one HDMI module in DSS. However, it's
 worth to mention here in a comment. Or, do it properly and create a per
 device data structure.

 +
 +
 +static int hdmi_panel_probe(struct omap_dss_device *dssdev)
 +{
 +     DSSDBG(ENTER hdmi_panel_probe\n);
 +
 +     dssdev-panel.config = OMAP_DSS_LCD_TFT |
 +                     OMAP_DSS_LCD_IVS | OMAP_DSS_LCD_IHS;
 +
 +     /*
 +      * Initialize the timings to 1920 * 1080
 +      * This is only for framebuffer update not for TV timing setting
 +      * Setting TV timing will be done only on enable
 +      */
 +     dssdev-panel.timings.x_res = 1920;
 +     dssdev-panel.timings.y_res = 1080;
 +
 +     DSSDBG(hdmi_panel_probe x_res= %d y_res = %d\n,
 +             dssdev-panel.timings.x_res,
 +             dssdev-panel.timings.y_res);
 +     return 0;
 +}
 +
 +static void hdmi_panel_remove(struct omap_dss_device *dssdev)
 +{
 +
 +}
 +
 +static int hdmi_panel_enable(struct omap_dss_device *dssdev)
 +{
 +     int r = 0;
 +     DSSDBG(ENTER hdmi_panel_enable\n);
 +
 +     if (dssdev-state != OMAP_DSS_DISPLAY_DISABLED)
 +             return -EINVAL;
 +
 +     mutex_lock(hdmi.hdmi_lock);

 I commented about this in earlier mails already. You need to use the
 locks consistently and have the dssdev-state reads also inside the
 lock.

 Think what happens if, say, two threads call hdmi_panel_enable at the
 same time.

  Tomi



--
To unsubscribe from

Re: [PATCH 3/8] OMAP4 : DSS : HDMI: HDMI specific display controller and dss change.

Hi Tomi,

On Sun, Feb 27, 2011 at 2:53 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Fri, 2011-02-25 at 08:21 -0600, K, Mythri P wrote:
 Adding changes to set gamma table bit for TV interface and function to select
 between VENC and HDMI.

 Signed-off-by: Mythri P K mythr...@ti.com
 ---
  drivers/video/omap2/dss/dispc.c |    5 +
  drivers/video/omap2/dss/dss.c   |    7 +++
  drivers/video/omap2/dss/dss.h   |    2 ++
  3 files changed, 14 insertions(+), 0 deletions(-)

 diff --git a/drivers/video/omap2/dss/dispc.c 
 b/drivers/video/omap2/dss/dispc.c
 index 6d9bb17..16f1106 100644
 --- a/drivers/video/omap2/dss/dispc.c
 +++ b/drivers/video/omap2/dss/dispc.c
 @@ -1213,6 +1213,11 @@ void dispc_enable_zorder(enum omap_plane plane, bool 
 enable)
       dispc_write_reg(dispc_reg_att[plane], val);
  }

 +void dispc_enable_gamma_table(bool enable)
 +{
 +     REG_FLD_MOD(DISPC_CONFIG, enable, 9, 9);
 +}
 +
  static void _dispc_set_vid_color_conv(enum omap_plane plane, bool enable)
  {
       u32 val;
 diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
 index 99de4e1..127d42e 100644
 --- a/drivers/video/omap2/dss/dss.c
 +++ b/drivers/video/omap2/dss/dss.c
 @@ -559,6 +559,13 @@ void dss_set_dac_pwrdn_bgz(bool enable)
       REG_FLD_MOD(DSS_CONTROL, enable, 5, 5); /* DAC Power-Down Control */
  }

 +void dss_select_hdmi_venc(bool hdmi)
 +{
 +     REG_FLD_MOD(DSS_CONTROL, hdmi, 15, 15); /* 0x1 for HDMI, 0x0 VENC */
 +     if (hdmi)
 +             REG_FLD_MOD(DSS_CONTROL, 0, 9, 8);
 +}

 bool argument for this function is very confusing. Which one is true?
 Perhaps this could use an enum.
I have added a comment , 1 is for HDMI and 0 for VENC.

 I've tried to keep the habit of having the name of the register field in
 comments to make the code easier to read. For example, /*
 VENC_HDMI_SWITCH */ for the bit 15.
I can add this.

 The bits 8 and 9 are FCK_CLK_SWITCH. Why do you set it here? Looks
 rather dangerous to change the clock source like that.

This is to select what would be the clock source to DISPC, HDMI would
need the DISPC
to run at the rate of pixel clock (TMDS if deep color is set ) required by HDMI.
Where as DSI does not have any such constraint( correct me if i am wrong).
I suppose we can have a patch to select the correct clock source to
DISPC dynamically
based on what panels are connected later on ?

  Tomi




--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/8] OMAP4 : DSS2 : HDMI: HDMI driver addition in the DSS drivers interface

Hi Tomi,

On Mon, Feb 28, 2011 at 11:57 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2011-02-28 at 00:11 -0600, K, Mythri P wrote:
 Hi Tomi,

 On Sun, Feb 27, 2011 at 3:47 PM, Tomi Valkeinen tomi.valkei...@ti.com 
 wrote:

  + * HDMI interface DSS driver setting for TI's OMAP4 family of processor.
  + * Copyright (C) 2010-2011 Texas Instruments Incorporated - 
  http://www.ti.com/
  + * Authors: Yong Zhi
 
  Who is Yong Zhi and where's his signed-off-by?
 Yong zhi had written some of the functions in this file , which was in
 a library.He no longer works on the driver though .

 Ok. Generally there should be a signed-off-by from everybody who has
 written the code. It's like I have written this code and I have
 permission to send it upstream etc.

 His signed-off would be good for this too, but if the code is just based
 on his work, and doesn't really resemble it anymore, perhaps you could
 then say Based on code by Yong Zhi, or similar.

sure i shall check with him to add his sign-off.
  +
  +static inline int hdmi_wait_for_bit_change(const struct hdmi_reg idx,
  +                               int b2, int b1, u32 val)
  +{
  +       u32 t = 0;
  +       while (val != FLD_GET(hdmi_read_reg(idx), b2, b1)) {
 
  There's REG_GET macro used in other DSS files to read bits from the
  registers.
 Hmm i see that in dsi.c REG_GET is defined to do the same,
 #define REG_GET(idx, start, end) \
         FLD_GET(dsi_read_reg(idx), start, end)
 So should this be fine of should i define a new REG_GET ?

 REG_GET() uses the private dsi_read_reg() function, so it has to be
 defined again in each interface file. So for HDMI, define REG_REG
 similarly with hdmi_read_reg().

But it is used in only one function here , so is it really needed :) ?

  +       /* HACK : DDC needs time to stablize */
  +       mdelay(10);
 
  So what is the reason for this? It's a sleep that for unknown reason
  make the code work? Or is there an idea why this is needed?
 This is the time it needs to stabilize DDC, else most of the time it
 reads a 0 or
 wrong shifted values , so i have prefixed with a HACK.

 Ok. I'm fine with the hack for now, but I'm just interested: Is the 10ms
 just something that happens to work? This sleep is not discussed in any
 documentation? Has this been discussed with HW people?

yes i have checked with the silicon validation.

  +static inline void print_omap_video_timings(struct omap_video_timings 
  *timings)
 
  Why is this inline?
 Its only a print function , called many times by the driver. Do you
 see any reason not to make it inline?

 Well, generally you shouldn't use inline. I think that's the basic rule.
 Compiler usually makes better choices than humans. I have used them for
 xxx_read_reg() and xxx_write_reg(), although I'm not quite sure if even
 that is needed.

 And in this case the function is quite long. I'm guessing here, but most
 likely the code runs faster if it's _not_ inlined, because not inlining
 reduces the size of the code. Also, print functions are on the slower
 side anyway, so the overhead of calling this function is probably
 negligible compared to the contents of the function.

ok i shall change.

  Tomi




Thanks and regards,
Mythri.
--
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 0/2]OMAP:DSS:RFC for HDMI in OMAP4

Hi Tomi,
Can you please comment on the below patch series?

Thanks and regards,
Mythri.

 -Original Message-
 From: K, Mythri P
 Sent: Monday, August 23, 2010 2:48 PM
 To: linux-omap@vger.kernel.org
 Cc: tomi.valkei...@nokia.com; K, Mythri P
 Subject: [PATCH 0/2]OMAP:DSS:RFC for HDMI in OMAP4

 From: Mythri P K mythr...@ti.com

 This patch is a outline of how the HDMI support is added to OMAP4.

 HDMI is a driver that is similar to the VENC or the DSI driver to
 support
 HDMI/DVI sink device.

 The current Design adheres to the DSS2 Architecture.

 It is split into the HDMI DSS driver and HDMI panel Driver.

 HDMI DSS driver  Located in drivers/video/omap2/dss/hdmi.c
 is responsible for OMAP related configuration such as listening to
 the
 DSS_HDMI irq which signals changes  such as Hot-plug detect ,
 Physical attach/detach.

 This driver is responsible to calculate the PLL values based on the
 TV resolution  that is selected.
 Yet another functionality is to call necessary configuration for the
 Mux/GPIO settings.

 HDMI Panel Driver is located in
 drivers/video/omap2/displays/hdmi_omap4_panel.c.
 This is a panel driver which acts as the HDMI source  and is
 responsible for all the configuration of the HDMI, based on the
 parameters read from the EDID of the sink device.
 It registers hdmi driver to the omap_dss bus and calls the
 functionality
 of the HDMI DSS driver.
 This driver is responsible for configuration of the HDMI IP, which
 are:
   1. Configuration of the PHY registers.
   2. Configuration of the PLL registers and setting of the TMDS
 clock.
   3. Configuration of the DDC to read the EDID data when
 available.
   4. Configuration of the core reigsters to set:
   a. set the video registers to the timing and format that
   is selected.
   b. set the audio reigsters based on the EDID value read
 and
   user selected parameters.
   c. Set the AVI info frame reigsters to configure the
 auxilary
   info frame which are repeated.
 It is also provides the interface for users
   1.To read the EDID contents and also confiure the timings
 based on EDID.
   2.To configure AVI Inforframe Based on the the EDID(sink
 capability).

 Mythri P K (2):
   OMAP:DSS:Patch to add support for HDMI as panel driver
   OMAP:DSS:Patch to add HDMI DSS driver support

  drivers/video/omap2/displays/hdmi_omap4_panel.c | 1443
 +++
  drivers/video/omap2/displays/hdmi_omap4_panel.h |  672 +++
  drivers/video/omap2/dss/hdmi.c  |  292 +
  3 files changed, 2407 insertions(+), 0 deletions(-)
  create mode 100644 drivers/video/omap2/displays/hdmi_omap4_panel.c
  create mode 100644 drivers/video/omap2/displays/hdmi_omap4_panel.h
  create mode 100644 drivers/video/omap2/dss/hdmi.c

--
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 0/2]OMAP:DSS:RFC for HDMI in OMAP4

Hi Tomi,
 -Original Message-
 From: Tomi Valkeinen [mailto:tomi.valkei...@nokia.com]
 Sent: Tuesday, September 07, 2010 4:23 PM
 To: K, Mythri P
 Cc: linux-omap@vger.kernel.org
 Subject: Re: [PATCH 0/2]OMAP:DSS:RFC for HDMI in OMAP4

 Hi,

 On Thu, 2010-08-26 at 07:16 +0200, ext K, Mythri P wrote:
  From: Mythri P K mythr...@ti.com

  This patch is a outline of how the HDMI support is added to OMAP4.

 Please run checkpatch.pl for the patches before you send them.
 Checkpatch reported over 300 errors.
I am not sure which of the 2 patches you are talking about , I have run 
checkpatch.pl for the patches and I see some over 80 lines warning which I 
have ignored but I see no errors. 

  HDMI is a driver that is similar to the VENC or the DSI driver to
 support
  HDMI/DVI sink device.

 When I made the VENC driver, I put both the core VENC and the venc
 panel
 into the same file, inside dss driver. The reason was that VENC is
 part
 of OMAP DSS, and not an external panel. I've been thinking about
 this a
 few times, but so far I haven't changed it.

 Did you have some reason to put the HDMI panel driver into
 displays/?
The reason why I separated out the DSS specific code and the panel driver with 
the HDMI core specific code is because, when the OMAP related code might 
change, the HDMI core related changes could be reusable and logically as it 
reads the EDID and configures the IP based on the TV i.e. connected to it. Also 
given that there are several user level configurations that are supported such 
as the YUV format conversion/AVI information based on the TV capability it 
would be cleaner to handle that in a panel driver.

  The current Design adheres to the DSS2 Architecture.

  It is split into the HDMI DSS driver and HDMI panel Driver.

  HDMI DSS driver  Located in drivers/video/omap2/dss/hdmi.c
  is responsible for OMAP related configuration such as listening to
 the
  DSS_HDMI irq which signals changes  such as Hot-plug detect ,
  Physical attach/detach.

  This driver is responsible to calculate the PLL values based on
 the
  TV resolution  that is selected.
  Yet another functionality is to call necessary configuration for
 the
  Mux/GPIO settings.

 Why does HDMI need mux/GPIO changes?
Some of the HDMI functionality like hot-plug-detect which works on the level 
shifters(0/5v) and the DDC line to read the EDID from the TV needs GPIO 
configurations.

  HDMI Panel Driver is located in
 drivers/video/omap2/displays/hdmi_omap4_panel.c.
  This is a panel driver which acts as the HDMI source  and is
 responsible for all the configuration of the HDMI, based on the
 parameters read from the EDID of the sink device.
  It registers hdmi driver to the omap_dss bus and calls the
 functionality
  of the HDMI DSS driver.
  This driver is responsible for configuration of the HDMI IP, which
 are:
  1. Configuration of the PHY registers.
  2. Configuration of the PLL registers and setting of the TMDS
 clock.
  3. Configuration of the DDC to read the EDID data when
 available.
  4. Configuration of the core reigsters to set:
  a. set the video registers to the timing and format that
  is selected.
  b. set the audio reigsters based on the EDID value read
 and
  user selected parameters.
  c. Set the AVI info frame reigsters to configure the
 auxilary
  info frame which are repeated.
  It is also provides the interface for users
  1.To read the EDID contents and also confiure the timings
 based on EDID.
  2.To configure AVI Inforframe Based on the the EDID(sink
 capability).

  Mythri P K (2):
OMAP:DSS:Patch to add support for HDMI as panel driver
OMAP:DSS:Patch to add HDMI DSS driver support

   drivers/video/omap2/displays/hdmi_omap4_panel.c | 1443
 +++
   drivers/video/omap2/displays/hdmi_omap4_panel.h |  672
 +++
   drivers/video/omap2/dss/hdmi.c  |  292 +
   3 files changed, 2407 insertions(+), 0 deletions(-)
   create mode 100644
 drivers/video/omap2/displays/hdmi_omap4_panel.c
   create mode 100644
 drivers/video/omap2/displays/hdmi_omap4_panel.h
   create mode 100644 drivers/video/omap2/dss/hdmi.c

--
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 2/2] OMAP:DSS: RFC Patch to add HDMI DSS driver support

Hi Tomi,

 -Original Message-
 From: Tomi Valkeinen [mailto:tomi.valkei...@nokia.com]
 Sent: Tuesday, September 07, 2010 6:47 PM
 To: K, Mythri P
 Cc: linux-omap@vger.kernel.org
 Subject: Re: [PATCH 2/2] OMAP:DSS: RFC Patch to add HDMI DSS driver
 support
 
 On Thu, 2010-08-26 at 07:17 +0200, ext K, Mythri P wrote:
  From: Mythri P K mythr...@ti.com
 
  This is an RFC patch to add support for HDMI DSS driver in OMAP.
 
  Signed-off-by: Mythri P K mythr...@ti.com
  ---
   drivers/video/omap2/dss/hdmi.c |  292
 
   1 files changed, 292 insertions(+), 0 deletions(-)
   create mode 100644 drivers/video/omap2/dss/hdmi.c
 
  diff --git a/drivers/video/omap2/dss/hdmi.c
 b/drivers/video/omap2/dss/hdmi.c
  new file mode 100644
  index 000..3d7acd2
  --- /dev/null
  +++ b/drivers/video/omap2/dss/hdmi.c
  @@ -0,0 +1,292 @@
  +/*
  + * linux/drivers/video/omap2/dss/hdmi.c
  + *
  + * Copyright (C) 2009 Texas Instruments
  + * Author: Yong Zhi
  + *
  + * HDMI settings from TI's DSS driver
  + *
  + * This program is free software; you can redistribute it and/or
 modify it
  + * under the terms of the GNU General Public License version 2 as
 published by
  + * the Free Software Foundation.
  + *
  + * This program is distributed in the hope that it will be useful,
 but WITHOUT
  + * ANY WARRANTY; without even the implied warranty of
 MERCHANTABILITY or
  + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
 License for
  + * more details.
  + *
  + * You should have received a copy of the GNU General Public
 License along with
  + * this program.  If not, see http://www.gnu.org/licenses/.
  + * History:
  + * Mythripk mythr...@ti.com  -Redesigned on the driver to
 adhere to DSS2 model.
  + * -GPIO calls for HDMI.
  + *
  + *
  + */
  +
  +#define DSS_SUBSYS_NAME HDMI
  +
  +#include linux/kernel.h
  +#include linux/module.h
  +#include linux/clk.h
  +#include linux/err.h
  +#include linux/io.h
  +#include linux/interrupt.h
  +#include linux/mutex.h
  +#include linux/completion.h
  +#include linux/delay.h
  +#include linux/string.h
  +#include linux/platform_device.h
  +#include linux/slab.h
  +#include plat/display.h
  +#include plat/cpu.h
  +#include plat/gpio.h
  +
  +#include dss.h
  +
  +static struct {
  +   struct mutex lock;
  +} hdmi;
  +
  +#define FLD_GET(val, start, end) (((val)  FLD_MASK(start, end))
  (end))
  +#define FLD_MOD(orig, val, start, end) \
  +   (((orig)  ~FLD_MASK(start, end)) | FLD_VAL(val, start, end))
 
 These are already defined in dss.h
 
  +
  +
  +#define CPF2
  +
  +int hdmi_init_display(struct omap_dss_device *dssdev)
  +{
  +   DSSDBG(init_display\n);
  +
  +   return 0;
  +}
  +
  +void compute_hdmi_pll(int clkin, int phy,
  +   int n, struct hdmi_pll_info *pi)
 
 This is a non-static function, and you don't introduce it in any
 header
 file.
 
 It is not exported, so it should be used only from inside DSS driver.
 I
 see you call it in the next patch from the display driver, which is
 not
 ok. Only exported functions should be used from the display drivers
 (you'll notice the problem if you build DSS as a module...).
 
 If the function is not static, it should have some meaningful prefix.
 
 The same comments apply to some other functions in this file also.
I have introduced these in the display.h , do you suggest adding prefix like 
omapdss_hdmi* for all these functions ?
 
  +{
  +   int refclk;
  +   u32 temp, mf;
  +
  +   if (clkin  3200) /* 32 mHz */
  +   refclk = clkin / (2 * (n + 1));
  +   else
  +   refclk = clkin / (n + 1);
  +
  +   temp = phy * 100/(CPF * refclk);
  +
  +   pi-regn = n;
  +   pi-regm = temp/100;
  +   pi-regm2 = 1;
  +
  +   mf = (phy - pi-regm * CPF * refclk) * 262144;
  +   pi-regmf = mf/(CPF * refclk);
  +
  +   if (phy  1000 * 100) {
  +   pi-regm4 = phy / 1;
  +   pi-dcofreq = 1;
  +   pi-regsd = ((pi-regm * 384)/((n + 1) * 250) + 5)/10;
  +   } else {
  +   pi-regm4 = 1;
  +   pi-dcofreq = 0;
  +   pi-regsd = 0;
  +   }
  +
  +   DSSDBG(M = %d Mf = %d, m4= %d\n, pi-regm, pi-regmf, pi-
 regm4);
  +   DSSDBG(range = %d sd = %d\n, pi-dcofreq, pi-regsd);
  +}
  +
  +
  +static void hdmi_enable_clocks(int enable)
  +{
  +   if (enable)
  +   dss_clk_enable(DSS_CLK_ICK | DSS_CLK_FCK1 | DSS_CLK_54M
 |
  +   DSS_CLK_96M);
  +   else
  +   dss_clk_disable(DSS_CLK_ICK | DSS_CLK_FCK1 | DSS_CLK_54M
 |
  +   DSS_CLK_96M);
  +}
  +
  +static irqreturn_t hdmi_irq_handler(int irq, void *arg)
  +{
  +   DSSDBG(Will be used in future to handle HPD);
  +   return IRQ_HANDLED;
  +}
  +
  +int hdmi_init(struct platform_device *pdev, int code, int mode)
  +{
  +   int r = 0;
  +   DSSDBG(Enter hdmi_init()\n);
  +
  +   mutex_init(hdmi.lock);
  +
  +   r = request_irq(OMAP44XX_IRQ_DSS_HDMI, hdmi_irq_handler

RE: [PATCH 2/2] OMAP:DSS: RFC Patch to add HDMI DSS driver support