Re: [PATCH v2 1/3] OMAPDSS: HDMI: HPD support in boardfile

2012-01-09 Thread Tomi Valkeinen 
On Fri, 2012-01-06 at 18:14 +0530, mythr...@ti.com wrote:
 From: Mythri P K mythr...@ti.com
 
 Add support for HPD GPIO configuration in board file.
 Also remove the enabling  of GPIO's required for HDMI from
 hdmi driver file to display.c based on the GPIO #'s sent from
 board file.
 
 Signed-off-by: Mythri P K mythr...@ti.com
 Acked-by: Tony Lindgren t...@atomide.com
 ---

You are doing too much in this patch. This changes the HPD gpio to
CT_CP_HPD, changes how gpios are allocated, moves gpio allocation into
another file, changes the pulls of the gpios, introduces a new field
into dss device...

You should do just one logical thing in a patch.

 -static void omap4_hdmi_mux_pads(enum omap_hdmi_flags flags)
 +static void omap4_hdmi_mux_pads(enum omap_hdmi_flags flags, int hpd_gpio)
  {
   u32 reg;
   u16 control_i2c_1;
  
   /* PAD0_HDMI_HPD_PAD1_HDMI_CEC */
   omap_mux_init_signal(hdmi_hpd,
 - OMAP_PIN_INPUT_PULLUP);
 + OMAP_PIN_INPUT_PULLDOWN);

Is the hdmi_hpd always the same pin? If so, it can be handled in
display.c, no need to handle it in the board files.

   omap_mux_init_signal(hdmi_cec,
   OMAP_PIN_INPUT_PULLUP);
   /* PAD0_HDMI_DDC_SCL_PAD1_HDMI_DDC_SDA */
 @@ -112,6 +113,10 @@ static void omap4_hdmi_mux_pads(enum omap_hdmi_flags 
 flags)
   OMAP_PIN_INPUT_PULLUP);
   omap_mux_init_signal(hdmi_ddc_sda,
   OMAP_PIN_INPUT_PULLUP);
 + omap_mux_init_signal(gpmc_wait2.gpio_100,
 + OMAP_PIN_INPUT_PULLDOWN);

Hmm, what is gpmc_wait2.gpio_100?

 + /* HPD GPIO needs to be muxed*/
 + omap_mux_init_gpio(hpd_gpio, OMAP_PIN_INPUT | OMAP_PULL_ENA);

Isn't this the same as the hdmi_hpd case above? Why are both needed?

   /*
* CONTROL_I2C_1: HDMI_DDC_SDA_PULLUPRESX (bit 28) and
 @@ -160,10 +165,18 @@ static int omap4_dsi_mux_pads(int dsi_id, unsigned 
 lanes)
   return 0;
  }
  
 -int omap_hdmi_init(enum omap_hdmi_flags flags)
 +int omap_hdmi_init(enum omap_hdmi_flags flags, int hpd_gpio,
 + struct gpio hdmi_gpios[], int size)
  {
 + int status;
 +
 + status = gpio_request_array(hdmi_gpios, size);
 + if (status) {
 + pr_err(%s: Cannot request HDMI GPIOs\n, __func__);
 + return status;
 + }

I don't see much point in this. The GPIOs are still declared in the
board files, and this function doesn't use the GPIOs for anything. It's
better to request them in the board file.

 @@ -319,7 +320,8 @@ struct omap_dss_board_info {
  /* Init with the board info */
  extern int omap_display_init(struct omap_dss_board_info *board_data);
  /* HDMI mux init*/
 -extern int omap_hdmi_init(enum omap_hdmi_flags flags);
 +extern int omap_hdmi_init(enum omap_hdmi_flags flags, int hpd_gpio,
 + struct gpio hdmi_gpios[], int size);
  
  struct omap_display_platform_data {
   struct omap_dss_board_info *board_data;
 @@ -568,6 +570,8 @@ struct omap_dss_device {
  
   int reset_gpio;
  
 + int hpd_gpio;

The hpd_gpio is for the hdmi device, not for all dss devices. So it
shouldn't be in the omap_dss_device. Did you check the example patch I
made to fix the phy issue?

I still think we should try to fix the bug at hand, not try to implement
proper HPD support. Was there something wrong with the example patch I
gave? (other than it needs some splitting up).

 Tomi



signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 1/3] OMAPDSS: HDMI: HPD support in boardfile

2012-01-09 Thread Tomi Valkeinen 
On Tue, 2012-01-10 at 09:37 +0200, Tomi Valkeinen wrote:

 I still think we should try to fix the bug at hand, not try to implement
 proper HPD support. Was there something wrong with the example patch I
 gave? (other than it needs some splitting up).

And one reason I'm saying we shouldn't try to implement HPD properly is
that with device tree the handling of the GPIOs will change, and if you
now make the driver handle the HPD in the generic hdmi layer, it may
well be that with dev tree it will change (I'm not sure yet how the
GPIOs will be managed there, but most likely they will be passed to the
IP driver).

So I think we should make the simplest possible implementation to handle
the PHY bug, inside the IP driver.

 Tomi



signature.asc
Description: This is a digitally signed message part

[PATCH v2 1/3] OMAPDSS: HDMI: HPD support in boardfile

2012-01-06 Thread mythripk 
From: Mythri P K mythr...@ti.com

Add support for HPD GPIO configuration in board file.
Also remove the enabling  of GPIO's required for HDMI from
hdmi driver file to display.c based on the GPIO #'s sent from
board file.

Signed-off-by: Mythri P K mythr...@ti.com
Acked-by: Tony Lindgren t...@atomide.com
---
 arch/arm/mach-omap2/board-4430sdp.c|   37 ++-
 arch/arm/mach-omap2/board-omap4panda.c |   37 +--
 arch/arm/mach-omap2/display.c  |   21 ++---
 drivers/video/omap2/dss/hdmi.c |   16 +-
 include/video/omapdss.h|6 -
 5 files changed, 46 insertions(+), 71 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c 
b/arch/arm/mach-omap2/board-4430sdp.c
index 4af874a..d6b647d 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -52,8 +52,9 @@
 #define ETH_KS8851_QUART   138
 #define OMAP4_SFH7741_SENSOR_OUTPUT_GPIO   184
 #define OMAP4_SFH7741_ENABLE_GPIO  188
-#define HDMI_GPIO_HPD 60 /* Hot plug pin for HDMI */
+#define HDMI_GPIO_CT_CP_HPD 60 /* Hot plug pin for HDMI */
 #define HDMI_GPIO_LS_OE 41 /* Level shifter for HDMI */
+#define HDMI_GPIO_HPD 63
 #define DISPLAY_SEL_GPIO   59  /* LCD2/PicoDLP switch */
 #define DLP_POWER_ON_GPIO  40
 
@@ -596,27 +597,11 @@ static void __init omap_sfh7741prox_init(void)
 }
 
 static struct gpio sdp4430_hdmi_gpios[] = {
-   { HDMI_GPIO_HPD,GPIOF_OUT_INIT_HIGH,hdmi_gpio_hpd   },
-   { HDMI_GPIO_LS_OE,  GPIOF_OUT_INIT_HIGH,hdmi_gpio_ls_oe },
+   { HDMI_GPIO_CT_CP_HPD,  GPIOF_OUT_INIT_HIGH,hdmi_gpio_ct_cp_hpd },
+   { HDMI_GPIO_LS_OE,  GPIOF_OUT_INIT_HIGH,hdmi_gpio_ls_oe },
+   { HDMI_GPIO_HPD,  GPIOF_DIR_IN,hdmi_gpio_hpd },
 };
 
-static int sdp4430_panel_enable_hdmi(struct omap_dss_device *dssdev)
-{
-   int status;
-
-   status = gpio_request_array(sdp4430_hdmi_gpios,
-   ARRAY_SIZE(sdp4430_hdmi_gpios));
-   if (status)
-   pr_err(%s: Cannot request HDMI GPIOs\n, __func__);
-
-   return status;
-}
-
-static void sdp4430_panel_disable_hdmi(struct omap_dss_device *dssdev)
-{
-   gpio_free(HDMI_GPIO_LS_OE);
-   gpio_free(HDMI_GPIO_HPD);
-}
 
 static struct nokia_dsi_panel_data dsi1_panel = {
.name   = taal,
@@ -735,9 +720,8 @@ static struct omap_dss_device sdp4430_hdmi_device = {
.name = hdmi,
.driver_name = hdmi_panel,
.type = OMAP_DISPLAY_TYPE_HDMI,
-   .platform_enable = sdp4430_panel_enable_hdmi,
-   .platform_disable = sdp4430_panel_disable_hdmi,
.channel = OMAP_DSS_CHANNEL_DIGIT,
+   .hpd_gpio = 63,
 };
 
 static struct picodlp_panel_data sdp4430_picodlp_pdata = {
@@ -830,10 +814,13 @@ static void omap_4430sdp_display_init(void)
 * OMAP4460SDP/Blaze and OMAP4430 ES2.3 SDP/Blaze boards and
 * later have external pull up on the HDMI I2C lines
 */
-   if (cpu_is_omap446x() || omap_rev()  OMAP4430_REV_ES2_2)
-   omap_hdmi_init(OMAP_HDMI_SDA_SCL_EXTERNAL_PULLUP);
+   if (cpu_is_omap446x() || (omap_rev()  OMAP4430_REV_ES2_2))
+   omap_hdmi_init(OMAP_HDMI_SDA_SCL_EXTERNAL_PULLUP, HDMI_GPIO_HPD,
+   sdp4430_hdmi_gpios,
+   ARRAY_SIZE(sdp4430_hdmi_gpios));
else
-   omap_hdmi_init(0);
+   omap_hdmi_init(0, HDMI_GPIO_HPD, sdp4430_hdmi_gpios,
+   ARRAY_SIZE(sdp4430_hdmi_gpios));
 }
 
 #ifdef CONFIG_OMAP_MUX
diff --git a/arch/arm/mach-omap2/board-omap4panda.c 
b/arch/arm/mach-omap2/board-omap4panda.c
index 00103e3..d60c674 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -51,8 +51,9 @@
 #define GPIO_HUB_NRESET62
 #define GPIO_WIFI_PMENA43
 #define GPIO_WIFI_IRQ  53
-#define HDMI_GPIO_HPD 60 /* Hot plug pin for HDMI */
+#define HDMI_GPIO_CT_CP_HPD 60 /* Hot plug pin for HDMI */
 #define HDMI_GPIO_LS_OE 41 /* Level shifter for HDMI */
+#define HDMI_GPIO_HPD 63
 
 /* wl127x BT, FM, GPS connectivity chip */
 static int wl1271_gpios[] = {46, -1, -1};
@@ -479,35 +480,17 @@ int __init omap4_panda_dvi_init(void)
 }
 
 static struct gpio panda_hdmi_gpios[] = {
-   { HDMI_GPIO_HPD,GPIOF_OUT_INIT_HIGH, hdmi_gpio_hpd   },
-   { HDMI_GPIO_LS_OE,  GPIOF_OUT_INIT_HIGH, hdmi_gpio_ls_oe },
+   { HDMI_GPIO_CT_CP_HPD,  GPIOF_OUT_INIT_HIGH,hdmi_gpio_ct_cp_hpd },
+   { HDMI_GPIO_LS_OE,  GPIOF_OUT_INIT_HIGH,hdmi_gpio_ls_oe },
+   { HDMI_GPIO_HPD,  GPIOF_DIR_IN,hdmi_gpio_hpd },
 };
 
-static int omap4_panda_panel_enable_hdmi(struct omap_dss_device *dssdev)
-{
-   int status;
-
-   status = gpio_request_array(panda_hdmi_gpios,
-   ARRAY_SIZE(panda_hdmi_gpios));
-