Hi,
On Mon, Sep 5, 2011 at 4:31 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> On Fri, 2011-09-02 at 16:17 +0530, mythripk@xxxxxx wrote:
>> From: Mythri P K <mythripk@xxxxxx>
>>
>> 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 <mythripk@xxxxxx>
>> ---
>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
