Re: [PATCH 2/2] OMAP:DSS: RFC Patch to add HDMI DSS driver support
K, Mythri P mythr...@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 -- 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
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 -- 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
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 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 / 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, + 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 */ +
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
On Tue, 2010-09-07 at 15:37 +0200, ext K, Mythri P wrote: 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 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 -- 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 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.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 ? 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. -- 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