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

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

[K, Mythri P]

From: Mythri P K <mythripk@ti.com>

This is an RFC patch to add support for HDMI DSS driver in OMAP.

Signed-off-by: Mythri P K <mythripk@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 0000000..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 <mythripk@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))
+
+
+#define CPF			2
+
+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)
+{
+	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 / 10000;
+		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,
+			0, "OMAP HDMI", (void *)0);
+
+	return 0;
+
+}
+
+void hdmi_exit(void)
+{
+	free_irq(OMAP44XX_IRQ_DSS_HDMI, NULL);
+
+}
+
+static int hdmi_power_on(struct omap_dss_device *dssdev)
+{
+	hdmi_enable_clocks(1);
+
+	dispc_enable_digit_out(0);
+
+	printk("poweron returns");
+
+	return 0;
+}
+
+int hdmi_dispc_setting(struct omap_dss_device *dssdev)
+{
+
+
+	/* these settings are independent of overlays */
+	dss_switch_tv_hdmi(1);
+
+	/* bypass TV gamma table*/
+	dispc_enable_gamma_table(0);
+
+	/* tv size */
+	dispc_set_digit_size(dssdev->panel.timings.x_res,
+			dssdev->panel.timings.y_res);
+
+
+	dispc_enable_digit_out(1);
+
+	return 0;
+}
+
+static void hdmi_power_off(struct omap_dss_device *dssdev)
+{
+
+	dispc_enable_digit_out(0);
+
+	if (dssdev->platform_disable)
+		dssdev->platform_disable(dssdev);
+
+
+	hdmi_enable_clocks(0);
+
+}
+
+int hdmi_enable_display(struct omap_dss_device *dssdev)
+{
+	int r = 0;
+	DSSDBG("ENTER hdmi_enable_display()\n");
+
+	mutex_lock(&hdmi.lock);
+
+	r = omap_dss_start_device(dssdev);
+	if (r) {
+		DSSDBG("failed to start device\n");
+		return r;
+	}
+
+	free_irq(OMAP44XX_IRQ_DSS_HDMI, NULL);
+
+	/* PAD0_HDMI_HPD_PAD1_HDMI_CEC */
+	omap_writel(0x01180118, 0x4A100098);
+	/* PAD0_HDMI_DDC_SCL_PAD1_HDMI_DDC_SDA */
+	omap_writel(0x01180118 , 0x4A10009C);
+	/* CONTROL_HDMI_TX_PHY */
+	omap_writel(0x10000000, 0x4A100610);
+
+	if (dssdev->platform_enable)
+		dssdev->platform_enable(dssdev);
+
+	r = hdmi_power_on(dssdev);
+	if (r) {
+		DSSERR("failed to power on device\n");
+		goto err;
+	}
+	r = request_irq(OMAP44XX_IRQ_DSS_HDMI, hdmi_irq_handler,
+			0, "OMAP HDMI", (void *)0);
+
+
+err:
+	mutex_unlock(&hdmi.lock);
+	printk("enable display returns");
+	return r;
+
+}
+
+void hdmi_disable_display(struct omap_dss_device *dssdev)
+{
+	DSSDBG("Enter hdmi_disable_display()\n");
+
+	mutex_lock(&hdmi.lock);
+
+	omap_dss_stop_device(dssdev);
+
+	hdmi_power_off(dssdev);
+
+end:
+	mutex_unlock(&hdmi.lock);
+}
+
+int hdmi_display_suspend(struct omap_dss_device *dssdev)
+{
+	int r = 0;
+
+	DSSDBG("hdmi_display_suspend\n");
+		mutex_lock(&hdmi.lock);
+
+	omap_dss_stop_device(dssdev);
+
+	hdmi_power_off(dssdev);
+end:
+	mutex_unlock(&hdmi.lock);
+	return r;
+}
+
+int hdmi_display_resume(struct omap_dss_device *dssdev)
+{
+	int r = 0;
+
+	DSSDBG("hdmi_display_resume\n");
+	mutex_lock(&hdmi.lock);
+
+	/* the tv overlay manager is shared*/
+	r = omap_dss_start_device(dssdev);
+	if (r) {
+		DSSERR("failed to start device\n");
+		goto err;
+	}
+
+	/* PAD0_HDMI_HPD_PAD1_HDMI_CEC */
+	omap_writel(0x01180118, 0x4A100098);
+	/* PAD0_HDMI_DDC_SCL_PAD1_HDMI_DDC_SDA */
+	omap_writel(0x01180118 , 0x4A10009C);
+	/* CONTROL_HDMI_TX_PHY */
+	omap_writel(0x10000000, 0x4A100610);
+
+	if (dssdev->platform_enable)
+		dssdev->platform_enable(dssdev);
+
+	r = hdmi_power_on(dssdev);
+	if (r) {
+		DSSERR("failed to power on device\n");
+		goto err;
+	}
+
+err:
+	mutex_unlock(&hdmi.lock);
+
+	return r;
+}
+
+void hdmi_dump_regs(struct seq_file *s)
+{
+	DSSDBG("0x4a100060 x%x\n", omap_readl(0x4A100060));
+	DSSDBG("0x4A100088 x%x\n", omap_readl(0x4A100088));
+	DSSDBG("0x48055134 x%x\n", omap_readl(0x48055134));
+	DSSDBG("0x48055194 x%x\n", omap_readl(0x48055194));
+}
-- 
1.5.6.3

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

[Tomi Valkeinen]

On Thu, 2010-08-26 at 07:17 +0200, ext K, Mythri P wrote:
> From: Mythri P K <mythripk@ti.com>
> 
> This is an RFC patch to add support for HDMI DSS driver in OMAP.
> 
> Signed-off-by: Mythri P K <mythripk@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 0000000..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 <mythripk@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 CPF			2
> +
> +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.

> +{
> +	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 / 10000;
> +		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,
> +			0, "OMAP HDMI", (void *)0);
> +
> +	return 0;
> +
> +}
> +
> +void hdmi_exit(void)
> +{
> +	free_irq(OMAP44XX_IRQ_DSS_HDMI, NULL);
> +
> +}
> +
> +static int hdmi_power_on(struct omap_dss_device *dssdev)
> +{
> +	hdmi_enable_clocks(1);
> +
> +	dispc_enable_digit_out(0);
> +
> +	printk("poweron returns");
> +
> +	return 0;
> +}
> +
> +int hdmi_dispc_setting(struct omap_dss_device *dssdev)
> +{
> +
> +
> +	/* these settings are independent of overlays */
> +	dss_switch_tv_hdmi(1);
> +
> +	/* bypass TV gamma table*/
> +	dispc_enable_gamma_table(0);
> +
> +	/* tv size */
> +	dispc_set_digit_size(dssdev->panel.timings.x_res,
> +			dssdev->panel.timings.y_res);
> +
> +
> +	dispc_enable_digit_out(1);
> +
> +	return 0;
> +}
> +
> +static void hdmi_power_off(struct omap_dss_device *dssdev)
> +{
> +
> +	dispc_enable_digit_out(0);
> +
> +	if (dssdev->platform_disable)
> +		dssdev->platform_disable(dssdev);
> +
> +
> +	hdmi_enable_clocks(0);
> +
> +}
> +
> +int hdmi_enable_display(struct omap_dss_device *dssdev)
> +{
> +	int r = 0;
> +	DSSDBG("ENTER hdmi_enable_display()\n");
> +
> +	mutex_lock(&hdmi.lock);
> +
> +	r = omap_dss_start_device(dssdev);
> +	if (r) {
> +		DSSDBG("failed to start device\n");
> +		return r;
> +	}
> +
> +	free_irq(OMAP44XX_IRQ_DSS_HDMI, NULL);
> +
> +	/* PAD0_HDMI_HPD_PAD1_HDMI_CEC */
> +	omap_writel(0x01180118, 0x4A100098);
> +	/* PAD0_HDMI_DDC_SCL_PAD1_HDMI_DDC_SDA */
> +	omap_writel(0x01180118 , 0x4A10009C);

I don't quite understand these (and in resume()). You write the same
values here and in resume(), but never anything else. Shouldn't
pinmuxing be done in bootloader/board-file, and not changed by the
driver?

 Tomi

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

[K, Mythri P]

Hi Tomi,

> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@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 <mythripk@ti.com>
> >
> > This is an RFC patch to add support for HDMI DSS driver in OMAP.
> >
> > Signed-off-by: Mythri P K <mythripk@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 0000000..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 <mythripk@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 CPF			2
> > +
> > +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 / 10000;
> > +		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,
> > +			0, "OMAP HDMI", (void *)0);
> > +
> > +	return 0;
> > +
> > +}
> > +
> > +void hdmi_exit(void)
> > +{
> > +	free_irq(OMAP44XX_IRQ_DSS_HDMI, NULL);
> > +
> > +}
> > +
> > +static int hdmi_power_on(struct omap_dss_device *dssdev)
> > +{
> > +	hdmi_enable_clocks(1);
> > +
> > +	dispc_enable_digit_out(0);
> > +
> > +	printk("poweron returns");
> > +
> > +	return 0;
> > +}
> > +
> > +int hdmi_dispc_setting(struct omap_dss_device *dssdev)
> > +{
> > +
> > +
> > +	/* these settings are independent of overlays */
> > +	dss_switch_tv_hdmi(1);
> > +
> > +	/* bypass TV gamma table*/
> > +	dispc_enable_gamma_table(0);
> > +
> > +	/* tv size */
> > +	dispc_set_digit_size(dssdev->panel.timings.x_res,
> > +			dssdev->panel.timings.y_res);
> > +
> > +
> > +	dispc_enable_digit_out(1);
> > +
> > +	return 0;
> > +}
> > +
> > +static void hdmi_power_off(struct omap_dss_device *dssdev)
> > +{
> > +
> > +	dispc_enable_digit_out(0);
> > +
> > +	if (dssdev->platform_disable)
> > +		dssdev->platform_disable(dssdev);
> > +
> > +
> > +	hdmi_enable_clocks(0);
> > +
> > +}
> > +
> > +int hdmi_enable_display(struct omap_dss_device *dssdev)
> > +{
> > +	int r = 0;
> > +	DSSDBG("ENTER hdmi_enable_display()\n");
> > +
> > +	mutex_lock(&hdmi.lock);
> > +
> > +	r = omap_dss_start_device(dssdev);
> > +	if (r) {
> > +		DSSDBG("failed to start device\n");
> > +		return r;
> > +	}
> > +
> > +	free_irq(OMAP44XX_IRQ_DSS_HDMI, NULL);
> > +
> > +	/* PAD0_HDMI_HPD_PAD1_HDMI_CEC */
> > +	omap_writel(0x01180118, 0x4A100098);
> > +	/* PAD0_HDMI_DDC_SCL_PAD1_HDMI_DDC_SDA */
> > +	omap_writel(0x01180118 , 0x4A10009C);
> 
> I don't quite understand these (and in resume()). You write the same
> values here and in resume(), but never anything else. Shouldn't
> pinmuxing be done in bootloader/board-file, and not changed by the
> driver?
> 
Ya I shall I shall move this to with other GPIO setting in the board-omap4430 file :)
>  Tomi
> 

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

[Tomi Valkeinen]

On Tue, 2010-09-07 at 15:37 +0200, ext K, Mythri P wrote:
> Hi Tomi,
> 
> > -----Original Message-----
> > From: Tomi Valkeinen [mailto:tomi.valkeinen@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 <mythripk@ti.com>
> > >
> > > This is an RFC patch to add support for HDMI DSS driver in OMAP.
> > >
> > > Signed-off-by: Mythri P K <mythripk@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 0000000..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 <mythripk@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 CPF			2
> > > +
> > > +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 ?

Neither of the two HDMI patches you sent modify display.h...

Yes, if you export functions from DSS they should be prefixed, as they
are global functions.

 Tomi

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

[K, Mythri P]

Hi Tomi,

> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com]
> Sent: Tuesday, September 07, 2010 7:15 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 Tue, 2010-09-07 at 15:37 +0200, ext K, Mythri P wrote:
> > Hi Tomi,
> >
> > > -----Original Message-----
> > > From: Tomi Valkeinen [mailto:tomi.valkeinen@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 <mythripk@ti.com>
> > > >
> > > > This is an RFC patch to add support for HDMI DSS driver in
> OMAP.
> > > >
> > > > Signed-off-by: Mythri P K <mythripk@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 0000000..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 <mythripk@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 CPF			2
> > > > +
> > > > +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 ?
> 
> Neither of the two HDMI patches you sent modify display.h...
> 
> Yes, if you export functions from DSS they should be prefixed, as
> they
> are global functions.
> 
I shall correct the function names.
Yes this is just a RFC patch to introduce the HDMI driver and panel as such, if you have no other comments on these 2 patch set I shall incorporate these comments and send out the complete patch series with all the relevant changes in display.h and some overlay.c and manager.c changes.

>  Tomi
> 
Thanks and regards,
Mythri.

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

[Tomi Valkeinen]

On Wed, 2010-09-08 at 06:46 +0200, ext K, Mythri P wrote:
> Hi Tomi,

> > > I have introduced these in the display.h , do you suggest adding
> > prefix like omapdss_hdmi* for all these functions ?
> > 
> > Neither of the two HDMI patches you sent modify display.h...
> > 
> > Yes, if you export functions from DSS they should be prefixed, as
> > they
> > are global functions.
> > 
> I shall correct the function names.
> Yes this is just a RFC patch to introduce the HDMI driver and panel as such, if you have no other comments on these 2 patch set I shall incorporate these comments and send out the complete patch series with all the relevant changes in display.h and some overlay.c and manager.c changes.

Please don't send incomplete patch sets. It's a waste of reviewer's time
to try to review patches that are missing components, or are based on
custom kernel. Also make sure checkpatch.pl doesn't give any (or just a
few) warnings/errors.

Or, if you really want to send an incomplete RFC patch set with errors,
make it very clear in the introduction.

 Tomi

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

[Petr Štetiar]

K, Mythri P <mythripk@ti.com> [2010-09-08 10:16:29]:

Hi,

[snip]

> Yes this is just a RFC patch to introduce the HDMI driver and panel as such,
> if you have no other comments on these 2 patch set I shall incorporate these
> comments and send out the complete patch series with all the relevant
> changes in display.h and some overlay.c and manager.c changes.

there's EDID parser[1] already in kernel, so you might better reuse it and/or
extend it for your needs.  The next thing is, that there's and there will be
the demand for EDID on OMAP3 also, so it might be wise to rethink it from the
beginning and split or write the code in a way, that it can be reused on OMAP3
also. I think, that at least the code for calculation of ideal video
resolution and timings could be made generic and could be used on OMAP3 also.
Or am I mistaken?

I'm adding Ricardo Salveti to the CC, he showed me his first draft of EDID
parsing[2] for OMAP3 yesterday. Yes, it's just the draft, but it's at least
reusing the code already available in kernel. I think, that the same could or
should apply for the OMAP4 code.

What I'm trying to say is, that I'm not uber kernel hacker, but I would like
to have EDID working on my Beagle also and I'll definitely help with this.

Thanks.

1. drivers/video/fbmon.c and drivers/video/edid.h
2. http://kernel.ubuntu.com/git?p=rsalveti/ubuntu-maverick.git;a=commitdiff;h=a0ea712b64988ccd7984069b99864a1b4ac43881

-- ynezz