Hi Tomi, > -----Original Message----- > From: Tomi Valkeinen [mailto:tomi.valkeinen@xxxxxxxxx] > Sent: Tuesday, September 07, 2010 6:47 PM > To: K, Mythri P > Cc: linux-omap@xxxxxxxxxxxxxxx > 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@xxxxxx> > > > > This is an RFC patch to add support for HDMI DSS driver in OMAP. > > > > Signed-off-by: Mythri P K <mythripk@xxxxxx> > > --- > > 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@xxxxxx> -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 > -- 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
