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) >> { >> dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 0); >> >> - ti_hdmi_4xxx_wp_video_start(&hdmi.hdmi_data, 0); >> - ti_hdmi_4xxx_phy_disable(&hdmi.hdmi_data); >> - ti_hdmi_4xxx_pll_disable(&hdmi.hdmi_data); >> + hdmi.hdmi_data.ops->video_enable(&hdmi.hdmi_data, 0); >> + hdmi.hdmi_data.ops->phy_disable(&hdmi.hdmi_data); >> + hdmi.hdmi_data.ops->pll_disable(&hdmi.hdmi_data); >> hdmi_runtime_put(); >> >> hdmi.edid_set = 0; >> 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 ?
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