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


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to