* [PATCH 0/3] OMAPDSS: HDMI: Interrupt and PHY state handling support @ 2012-03-20 13:15 mythripk 2012-03-20 13:15 ` [PATCH 1/3] OMAPDSS: HDMI: support for interrupt enabling mythripk 2012-03-27 10:29 ` [PATCH 0/3] OMAPDSS: HDMI: Interrupt and PHY state handling support Tomi Valkeinen 0 siblings, 2 replies; 8+ messages in thread From: mythripk @ 2012-03-20 13:15 UTC (permalink / raw) To: linux-omap; +Cc: tomi.valkeinen, Mythri P K From: Mythri P K <mythripk@ti.com> Add support for handling the DSS_HDMI interrupt in HDMI, A line that serves to notify HDMI of status change in PHY, PLL and CORE based on the registration. Also logic to support enabling of the PHY in TX_ON state only when a PHY_CONNECT which would make sure that TMDS lines are high before putting it in TX_ON state. Mythri P K (3): OMAPDSS: HDMI: support for interrupt enabling OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt OMAPDSS: HDMI: wait for TMDS to be high before putting phy in TX_ON drivers/video/omap2/dss/dss_features.c | 1 + drivers/video/omap2/dss/hdmi.c | 21 +++++ drivers/video/omap2/dss/ti_hdmi.h | 20 +++++ drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 117 +++++++++++++++++++++++++++- drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h | 3 + 5 files changed, 157 insertions(+), 5 deletions(-) -- 1.7.5.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] OMAPDSS: HDMI: support for interrupt enabling 2012-03-20 13:15 [PATCH 0/3] OMAPDSS: HDMI: Interrupt and PHY state handling support mythripk @ 2012-03-20 13:15 ` mythripk 2012-03-20 13:15 ` [PATCH 2/3] OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt mythripk 2012-03-27 10:29 ` [PATCH 1/3] OMAPDSS: HDMI: support for interrupt enabling Tomi Valkeinen 2012-03-27 10:29 ` [PATCH 0/3] OMAPDSS: HDMI: Interrupt and PHY state handling support Tomi Valkeinen 1 sibling, 2 replies; 8+ messages in thread From: mythripk @ 2012-03-20 13:15 UTC (permalink / raw) To: linux-omap; +Cc: tomi.valkeinen, Mythri P K From: Mythri P K <mythripk@ti.com> Add function to enable or clear interrupts in the HDMI wrapper. Signed-off-by: Mythri P K <mythripk@ti.com> --- drivers/video/omap2/dss/ti_hdmi.h | 16 ++++++++++ drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 45 +++++++++++++++++++++++++++++ drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h | 1 + 3 files changed, 62 insertions(+), 0 deletions(-) diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h index 1f58b84..6d65b3b 100644 --- a/drivers/video/omap2/dss/ti_hdmi.h +++ b/drivers/video/omap2/dss/ti_hdmi.h @@ -80,6 +80,22 @@ struct hdmi_pll_info { enum hdmi_clk_refsel refsel; }; +struct hdmi_irq_vector { + u8 pll_recal; + u8 pll_unlock; + u8 pll_lock; + u8 phy_disconnect; + u8 phy_connect; + u8 phy_short_5v; + u8 video_end_fr; + u8 video_vsync; + u8 fifo_sample_req; + u8 fifo_overflow; + u8 fifo_underflow; + u8 ocp_timeout; + u8 core; +}; + struct ti_hdmi_ip_ops { void (*video_configure)(struct hdmi_ip_data *ip_data); diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c index bfe6fe6..5272f49 100644 --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c @@ -224,6 +224,51 @@ void ti_hdmi_4xxx_pll_disable(struct hdmi_ip_data *ip_data) hdmi_set_pll_pwr(ip_data, HDMI_PLLPWRCMD_ALLOFF); } +static void hdmi_wp_irq_init(struct hdmi_irq_vector *irq_enable) +{ + irq_enable->pll_recal = 0; + irq_enable->pll_unlock = 0; + irq_enable->pll_lock = 0; + irq_enable->phy_disconnect = 0; + irq_enable->phy_connect = 0; + irq_enable->phy_short_5v = 0; + irq_enable->video_end_fr = 0; + irq_enable->video_vsync = 0; + irq_enable->fifo_sample_req = 0; + irq_enable->fifo_overflow = 0; + irq_enable->fifo_underflow = 0; + irq_enable->ocp_timeout = 0; + irq_enable->core = 0; +} + +static void hdmi_wp_clr_irq(struct hdmi_ip_data *ip_data) +{ + hdmi_write_reg(hdmi_wp_base(ip_data), + HDMI_WP_IRQENABLE_CLR, 0XFFFFFFFF); +} + +static void hdmi_wp_irq_enable(struct hdmi_ip_data *ip_data, + struct hdmi_irq_vector *irq_enable) +{ + u32 r = 0; + + r = ((irq_enable->pll_recal << 31) | + (irq_enable->pll_unlock << 30) | + (irq_enable->pll_lock << 29) | + (irq_enable->phy_disconnect << 26) | + (irq_enable->phy_connect << 25) | + (irq_enable->phy_short_5v << 24) | + (irq_enable->video_end_fr << 17) | + (irq_enable->video_vsync << 16) | + (irq_enable->fifo_sample_req << 10) | + (irq_enable->fifo_overflow << 9) | + (irq_enable->fifo_underflow << 8) | + (irq_enable->ocp_timeout << 4) | + (irq_enable->core << 0)); + + hdmi_write_reg(hdmi_wp_base(ip_data), HDMI_WP_IRQENABLE_SET, r); +} + static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data) { unsigned long flags; diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h index a14d1a0..3090e81 100644 --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h @@ -38,6 +38,7 @@ #define HDMI_WP_IRQSTATUS 0x28 #define HDMI_WP_PWR_CTRL 0x40 #define HDMI_WP_IRQENABLE_SET 0x2C +#define HDMI_WP_IRQENABLE_CLR 0x30 #define HDMI_WP_VIDEO_CFG 0x50 #define HDMI_WP_VIDEO_SIZE 0x60 #define HDMI_WP_VIDEO_TIMING_H 0x68 -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt 2012-03-20 13:15 ` [PATCH 1/3] OMAPDSS: HDMI: support for interrupt enabling mythripk @ 2012-03-20 13:15 ` mythripk 2012-03-20 13:15 ` [PATCH 3/3] OMAPDSS: HDMI: wait for TMDS to be high before putting mythripk 2012-03-27 10:30 ` [PATCH 2/3] OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt Tomi Valkeinen 2012-03-27 10:29 ` [PATCH 1/3] OMAPDSS: HDMI: support for interrupt enabling Tomi Valkeinen 1 sibling, 2 replies; 8+ messages in thread From: mythripk @ 2012-03-20 13:15 UTC (permalink / raw) To: linux-omap; +Cc: tomi.valkeinen, Mythri P K From: Mythri P K <mythripk@ti.com> Add support for DSS_HDMI interrupt handling in HDMI driver by registering for the same. This is the path for many necessary HDMI interrupts such as PLL lock/unlock, PHY connect/disconnet, video frame done etc. Signed-off-by: Mythri P K <mythripk@ti.com> --- drivers/video/omap2/dss/dss_features.c | 1 + drivers/video/omap2/dss/hdmi.c | 21 +++++++++++++++++++++ drivers/video/omap2/dss/ti_hdmi.h | 3 +++ drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 13 +++++++++++++ 4 files changed, 38 insertions(+), 0 deletions(-) diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c index ce14aa6..29c5548 100644 --- a/drivers/video/omap2/dss/dss_features.c +++ b/drivers/video/omap2/dss/dss_features.c @@ -566,6 +566,7 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions = { .dump_core = ti_hdmi_4xxx_core_dump, .dump_pll = ti_hdmi_4xxx_pll_dump, .dump_phy = ti_hdmi_4xxx_phy_dump, + .irq_handle = ti_hdmi_4xxx_intr_handler, #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \ defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE) .audio_enable = ti_hdmi_4xxx_wp_audio_enable, diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c index c4b4f69..14e90b5 100644 --- a/drivers/video/omap2/dss/hdmi.c +++ b/drivers/video/omap2/dss/hdmi.c @@ -67,6 +67,8 @@ static struct { struct platform_device *pdev; struct hdmi_ip_data ip_data; + int irq; + spinlock_t irq_lock; struct clk *sys_clk; } hdmi; @@ -790,6 +792,19 @@ static void hdmi_put_clocks(void) clk_put(hdmi.sys_clk); } +static irqreturn_t hdmi_irq_handler(int irq, void *arg) +{ + unsigned long flags; + + spin_lock_irqsave(&hdmi.irq_lock, flags); + + hdmi.ip_data.ops->irq_handle(&hdmi.ip_data); + + spin_unlock_irqrestore(&hdmi.irq_lock, flags); + + return IRQ_HANDLED; +} + /* HDMI HW IP initialisation */ static int omapdss_hdmihw_probe(struct platform_device *pdev) { @@ -823,6 +838,10 @@ static int omapdss_hdmihw_probe(struct platform_device *pdev) pm_runtime_enable(&pdev->dev); + hdmi.irq = platform_get_irq(pdev, 0); + r = request_threaded_irq(hdmi.irq, NULL, hdmi_irq_handler, 0, + "OMAP HDMI", (void *)0); + hdmi.ip_data.core_sys_offset = HDMI_CORE_SYS; hdmi.ip_data.core_av_offset = HDMI_CORE_AV; hdmi.ip_data.pll_offset = HDMI_PLLCTRL; @@ -853,6 +872,8 @@ static int omapdss_hdmihw_remove(struct platform_device *pdev) snd_soc_unregister_codec(&pdev->dev); #endif + free_irq(hdmi.irq, NULL); + pm_runtime_disable(&pdev->dev); hdmi_put_clocks(); diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h index 6d65b3b..5e7e0da 100644 --- a/drivers/video/omap2/dss/ti_hdmi.h +++ b/drivers/video/omap2/dss/ti_hdmi.h @@ -122,6 +122,8 @@ struct ti_hdmi_ip_ops { void (*dump_phy)(struct hdmi_ip_data *ip_data, struct seq_file *s); + void (*irq_handle)(struct hdmi_ip_data *ip_data); + #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \ defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE) void (*audio_enable)(struct hdmi_ip_data *ip_data, bool start); @@ -197,6 +199,7 @@ void ti_hdmi_4xxx_wp_dump(struct hdmi_ip_data *ip_data, struct seq_file *s); void ti_hdmi_4xxx_pll_dump(struct hdmi_ip_data *ip_data, struct seq_file *s); void ti_hdmi_4xxx_core_dump(struct hdmi_ip_data *ip_data, struct seq_file *s); void ti_hdmi_4xxx_phy_dump(struct hdmi_ip_data *ip_data, struct seq_file *s); +void ti_hdmi_4xxx_intr_handler(struct hdmi_ip_data *ip_data); #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \ defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE) void ti_hdmi_4xxx_wp_audio_enable(struct hdmi_ip_data *ip_data, bool enable); diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c index 5272f49..31d9927 100644 --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c @@ -312,6 +312,19 @@ static irqreturn_t hpd_irq_handler(int irq, void *data) return IRQ_HANDLED; } +/* Interrupt handler */ +void ti_hdmi_4xxx_intr_handler(struct hdmi_ip_data *ip_data) +{ + u32 val; + + val = hdmi_read_reg(hdmi_wp_base(ip_data), HDMI_WP_IRQSTATUS); + /* Ack other interrupts if any */ + hdmi_write_reg(hdmi_wp_base(ip_data), HDMI_WP_IRQSTATUS, val); + /* flush posted write */ + hdmi_read_reg(hdmi_wp_base(ip_data), HDMI_WP_IRQSTATUS); + +} + int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data) { u16 r = 0; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] OMAPDSS: HDMI: wait for TMDS to be high before putting 2012-03-20 13:15 ` [PATCH 2/3] OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt mythripk @ 2012-03-20 13:15 ` mythripk 2012-03-27 10:30 ` Tomi Valkeinen 2012-03-27 10:30 ` [PATCH 2/3] OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt Tomi Valkeinen 1 sibling, 1 reply; 8+ messages in thread From: mythripk @ 2012-03-20 13:15 UTC (permalink / raw) To: linux-omap; +Cc: tomi.valkeinen, Mythri P K From: Mythri P K <mythripk@ti.com> Currently TX_PHY is put to TX_ON(transmission state) on receiving HPD. It just ensures that the TV is connected but does not guarantee that TMDS data lines and clock lines are up and ready for transmission. Which although is very rare scenario has a potential to damage the HDMI port. Thus this patch adds the support based on PHY interrupts. On getting a HPD it registers for PHY connect/disconnect interrupt, On recieving those it is made sure TMDS lines are UP before changing the PHY state from LDO_ON to TX_ON. Signed-off-by: Mythri P K <mythripk@ti.com> --- drivers/video/omap2/dss/ti_hdmi.h | 1 + drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 61 ++++++++++++++++++++++++++--- drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h | 2 + 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h index 5e7e0da..5051df6 100644 --- a/drivers/video/omap2/dss/ti_hdmi.h +++ b/drivers/video/omap2/dss/ti_hdmi.h @@ -186,6 +186,7 @@ struct hdmi_ip_data { /* ti_hdmi_4xxx_ip private data. These should be in a separate struct */ int hpd_gpio; bool phy_tx_enabled; + bool phy_enabled; }; int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data); void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data); diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c index 31d9927..a54c811 100644 --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c @@ -273,7 +273,8 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data) { unsigned long flags; bool hpd; - int r; + int r = 0; + struct hdmi_irq_vector irq_enable; /* this should be in ti_hdmi_4xxx_ip private data */ static DEFINE_SPINLOCK(phy_tx_lock); @@ -286,11 +287,21 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data) return 0; } - if (hpd) - r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON); - else + hdmi_wp_clr_irq(ip_data); + hdmi_wp_irq_init(&irq_enable); + if (hpd) { + if (ip_data->phy_enabled) { + r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON); + } else { + irq_enable.phy_connect = 1; + irq_enable.phy_disconnect = 0; + } + } else { r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON); - + irq_enable.phy_connect = 0; + irq_enable.phy_disconnect = 0; + ip_data->phy_enabled = false; + } if (r) { DSSERR("Failed to %s PHY TX power\n", hpd ? "enable" : "disable"); @@ -300,6 +311,7 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data) ip_data->phy_tx_enabled = hpd; err: spin_unlock_irqrestore(&phy_tx_lock, flags); + hdmi_wp_irq_enable(ip_data, &irq_enable); return r; } @@ -312,17 +324,54 @@ static irqreturn_t hpd_irq_handler(int irq, void *data) return IRQ_HANDLED; } +int hdmi_ti_4xxx_rxdet(struct hdmi_ip_data *ip_data) +{ + int tmds_lines =0; + + tmds_lines = hdmi_read_reg(hdmi_phy_base(ip_data), + HDMI_TXPHY_PAD_CFG_CTRL); + + return (tmds_lines && 0x7F80); +} /* Interrupt handler */ void ti_hdmi_4xxx_intr_handler(struct hdmi_ip_data *ip_data) { - u32 val; + u32 val, r = 0; + struct hdmi_irq_vector irq_enable; val = hdmi_read_reg(hdmi_wp_base(ip_data), HDMI_WP_IRQSTATUS); + + hdmi_wp_clr_irq(ip_data); + hdmi_wp_irq_init(&irq_enable); + + if (val & HDMI_WP_IRQSTATUS_PHYCONNECT) { + if (ip_data->phy_tx_enabled && hdmi_ti_4xxx_rxdet(ip_data)) { + r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON); + irq_enable.phy_connect = 0; + irq_enable.phy_disconnect = 1; + ip_data->phy_enabled = true; + } + } + + /* We can get connect / disconnect simultaneously due to glitch */ + if (val & HDMI_WP_IRQSTATUS_PHYDISCONNECT) { + if (ip_data->phy_tx_enabled && !hdmi_ti_4xxx_rxdet(ip_data)) { + r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON); + irq_enable.phy_connect = 1; + irq_enable.phy_disconnect = 0; + ip_data->phy_enabled = false; + } + } + + if (r) + DSSERR("Failed to set PHY TX power\n"); + /* Ack other interrupts if any */ hdmi_write_reg(hdmi_wp_base(ip_data), HDMI_WP_IRQSTATUS, val); /* flush posted write */ hdmi_read_reg(hdmi_wp_base(ip_data), HDMI_WP_IRQSTATUS); + hdmi_wp_irq_enable(ip_data, &irq_enable); } int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data) diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h index 3090e81..38af675 100644 --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h @@ -48,6 +48,8 @@ #define HDMI_WP_AUDIO_CFG2 0x84 #define HDMI_WP_AUDIO_CTRL 0x88 #define HDMI_WP_AUDIO_DATA 0x8C +#define HDMI_WP_IRQSTATUS_PHYCONNECT 0x02000000 +#define HDMI_WP_IRQSTATUS_PHYDISCONNECT 0x04000000 /* HDMI IP Core System */ -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: wait for TMDS to be high before putting 2012-03-20 13:15 ` [PATCH 3/3] OMAPDSS: HDMI: wait for TMDS to be high before putting mythripk @ 2012-03-27 10:30 ` Tomi Valkeinen 0 siblings, 0 replies; 8+ messages in thread From: Tomi Valkeinen @ 2012-03-27 10:30 UTC (permalink / raw) To: mythripk; +Cc: linux-omap [-- Attachment #1: Type: text/plain, Size: 6044 bytes --] On Tue, 2012-03-20 at 18:45 +0530, mythripk@ti.com wrote: > From: Mythri P K <mythripk@ti.com> > > Currently TX_PHY is put to TX_ON(transmission state) on receiving HPD. > It just ensures that the TV is connected but does not guarantee > that TMDS data lines and clock lines are up and ready for transmission. > Which although is very rare scenario has a potential to damage the HDMI > port. > Thus this patch adds the support based on PHY interrupts. > On getting a HPD it registers for PHY connect/disconnect interrupt, > On recieving those it is made sure TMDS lines are UP before changing > the PHY state from LDO_ON to TX_ON. > > Signed-off-by: Mythri P K <mythripk@ti.com> > --- > drivers/video/omap2/dss/ti_hdmi.h | 1 + > drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 61 ++++++++++++++++++++++++++--- > drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h | 2 + > 3 files changed, 58 insertions(+), 6 deletions(-) > > diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h > index 5e7e0da..5051df6 100644 > --- a/drivers/video/omap2/dss/ti_hdmi.h > +++ b/drivers/video/omap2/dss/ti_hdmi.h > @@ -186,6 +186,7 @@ struct hdmi_ip_data { > /* ti_hdmi_4xxx_ip private data. These should be in a separate struct */ > int hpd_gpio; > bool phy_tx_enabled; > + bool phy_enabled; > }; > int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data); > void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data); > diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c > index 31d9927..a54c811 100644 > --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c > +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c > @@ -273,7 +273,8 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data) > { > unsigned long flags; > bool hpd; > - int r; > + int r = 0; > + struct hdmi_irq_vector irq_enable; > /* this should be in ti_hdmi_4xxx_ip private data */ > static DEFINE_SPINLOCK(phy_tx_lock); > > @@ -286,11 +287,21 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data) > return 0; > } > > - if (hpd) > - r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON); > - else > + hdmi_wp_clr_irq(ip_data); > + hdmi_wp_irq_init(&irq_enable); > + if (hpd) { > + if (ip_data->phy_enabled) { > + r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON); > + } else { > + irq_enable.phy_connect = 1; > + irq_enable.phy_disconnect = 0; > + } > + } else { > r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON); > - > + irq_enable.phy_connect = 0; > + irq_enable.phy_disconnect = 0; > + ip_data->phy_enabled = false; > + } Why do you need this elaborate mechanism where you turn on/off the PHY CONNECT/DISCONNECT interrupts? Why don't you just enable them when the HDMI is enabled? > if (r) { > DSSERR("Failed to %s PHY TX power\n", > hpd ? "enable" : "disable"); > @@ -300,6 +311,7 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data) > ip_data->phy_tx_enabled = hpd; > err: > spin_unlock_irqrestore(&phy_tx_lock, flags); > + hdmi_wp_irq_enable(ip_data, &irq_enable); What happens if the connect irq happened before you enable the interrupt? > return r; > } > > @@ -312,17 +324,54 @@ static irqreturn_t hpd_irq_handler(int irq, void *data) > return IRQ_HANDLED; > } > > +int hdmi_ti_4xxx_rxdet(struct hdmi_ip_data *ip_data) > +{ > + int tmds_lines =0; > + > + tmds_lines = hdmi_read_reg(hdmi_phy_base(ip_data), > + HDMI_TXPHY_PAD_CFG_CTRL); > + > + return (tmds_lines && 0x7F80); I'm quite sure the line above is not correct, and thus the whole rxdet system doesn't work properly. > /* Interrupt handler */ > void ti_hdmi_4xxx_intr_handler(struct hdmi_ip_data *ip_data) > { > - u32 val; > + u32 val, r = 0; > + struct hdmi_irq_vector irq_enable; > > val = hdmi_read_reg(hdmi_wp_base(ip_data), HDMI_WP_IRQSTATUS); > + > + hdmi_wp_clr_irq(ip_data); > + hdmi_wp_irq_init(&irq_enable); > + > + if (val & HDMI_WP_IRQSTATUS_PHYCONNECT) { > + if (ip_data->phy_tx_enabled && hdmi_ti_4xxx_rxdet(ip_data)) { > + r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON); > + irq_enable.phy_connect = 0; > + irq_enable.phy_disconnect = 1; > + ip_data->phy_enabled = true; > + } > + } > + > + /* We can get connect / disconnect simultaneously due to glitch */ > + if (val & HDMI_WP_IRQSTATUS_PHYDISCONNECT) { > + if (ip_data->phy_tx_enabled && !hdmi_ti_4xxx_rxdet(ip_data)) { > + r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON); > + irq_enable.phy_connect = 1; > + irq_enable.phy_disconnect = 0; > + ip_data->phy_enabled = false; > + } > + } Instead of spreading this code into two functions, why not handle everything in one function. You have four states, connected/disconnected for both HPD and PHY. Just track the states of those, and have one function decide what to do depending on those states. > + > + if (r) > + DSSERR("Failed to set PHY TX power\n"); > + > /* Ack other interrupts if any */ > hdmi_write_reg(hdmi_wp_base(ip_data), HDMI_WP_IRQSTATUS, val); > /* flush posted write */ > hdmi_read_reg(hdmi_wp_base(ip_data), HDMI_WP_IRQSTATUS); > > + hdmi_wp_irq_enable(ip_data, &irq_enable); > } > > int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data) > diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h > index 3090e81..38af675 100644 > --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h > +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h > @@ -48,6 +48,8 @@ > #define HDMI_WP_AUDIO_CFG2 0x84 > #define HDMI_WP_AUDIO_CTRL 0x88 > #define HDMI_WP_AUDIO_DATA 0x8C > +#define HDMI_WP_IRQSTATUS_PHYCONNECT 0x02000000 > +#define HDMI_WP_IRQSTATUS_PHYDISCONNECT 0x04000000 Use (1 << xx) style there. And you could name them the same way as other components: HDMI_WP_IRQ_PHYCONNECT. And if you add few, just add them all. Tomi [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt 2012-03-20 13:15 ` [PATCH 2/3] OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt mythripk 2012-03-20 13:15 ` [PATCH 3/3] OMAPDSS: HDMI: wait for TMDS to be high before putting mythripk @ 2012-03-27 10:30 ` Tomi Valkeinen 1 sibling, 0 replies; 8+ messages in thread From: Tomi Valkeinen @ 2012-03-27 10:30 UTC (permalink / raw) To: mythripk; +Cc: linux-omap [-- Attachment #1: Type: text/plain, Size: 2319 bytes --] On Tue, 2012-03-20 at 18:45 +0530, mythripk@ti.com wrote: > From: Mythri P K <mythripk@ti.com> > > Add support for DSS_HDMI interrupt handling in HDMI driver > by registering for the same. This is the path for many necessary HDMI > interrupts such as PLL lock/unlock, PHY connect/disconnet, video frame > done etc. > > Signed-off-by: Mythri P K <mythripk@ti.com> > --- > drivers/video/omap2/dss/dss_features.c | 1 + > drivers/video/omap2/dss/hdmi.c | 21 +++++++++++++++++++++ > drivers/video/omap2/dss/ti_hdmi.h | 3 +++ > drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 13 +++++++++++++ > 4 files changed, 38 insertions(+), 0 deletions(-) > > diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c > index ce14aa6..29c5548 100644 > --- a/drivers/video/omap2/dss/dss_features.c > +++ b/drivers/video/omap2/dss/dss_features.c > @@ -566,6 +566,7 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions = { > .dump_core = ti_hdmi_4xxx_core_dump, > .dump_pll = ti_hdmi_4xxx_pll_dump, > .dump_phy = ti_hdmi_4xxx_phy_dump, > + .irq_handle = ti_hdmi_4xxx_intr_handler, > #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \ > defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE) > .audio_enable = ti_hdmi_4xxx_wp_audio_enable, > diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c > index c4b4f69..14e90b5 100644 > --- a/drivers/video/omap2/dss/hdmi.c > +++ b/drivers/video/omap2/dss/hdmi.c > @@ -67,6 +67,8 @@ static struct { > struct platform_device *pdev; > struct hdmi_ip_data ip_data; > > + int irq; > + spinlock_t irq_lock; > struct clk *sys_clk; > } hdmi; > > @@ -790,6 +792,19 @@ static void hdmi_put_clocks(void) > clk_put(hdmi.sys_clk); > } > > +static irqreturn_t hdmi_irq_handler(int irq, void *arg) > +{ > + unsigned long flags; > + > + spin_lock_irqsave(&hdmi.irq_lock, flags); > + > + hdmi.ip_data.ops->irq_handle(&hdmi.ip_data); > + > + spin_unlock_irqrestore(&hdmi.irq_lock, flags); So what is this spinlock supposed to protect? This is the only place you use it. > + > + return IRQ_HANDLED; > +} I think all this code should be in the ti_hdmi_4xxx_ip.c, as it's dealing with on omap4 hdmi interrupt. Tomi [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] OMAPDSS: HDMI: support for interrupt enabling 2012-03-20 13:15 ` [PATCH 1/3] OMAPDSS: HDMI: support for interrupt enabling mythripk 2012-03-20 13:15 ` [PATCH 2/3] OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt mythripk @ 2012-03-27 10:29 ` Tomi Valkeinen 1 sibling, 0 replies; 8+ messages in thread From: Tomi Valkeinen @ 2012-03-27 10:29 UTC (permalink / raw) To: mythripk; +Cc: linux-omap [-- Attachment #1: Type: text/plain, Size: 1496 bytes --] On Tue, 2012-03-20 at 18:45 +0530, mythripk@ti.com wrote: > From: Mythri P K <mythripk@ti.com> > > Add function to enable or clear interrupts in the HDMI wrapper. > > Signed-off-by: Mythri P K <mythripk@ti.com> > --- > drivers/video/omap2/dss/ti_hdmi.h | 16 ++++++++++ > drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 45 +++++++++++++++++++++++++++++ > drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h | 1 + > 3 files changed, 62 insertions(+), 0 deletions(-) > > diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h > index 1f58b84..6d65b3b 100644 > --- a/drivers/video/omap2/dss/ti_hdmi.h > +++ b/drivers/video/omap2/dss/ti_hdmi.h > @@ -80,6 +80,22 @@ struct hdmi_pll_info { > enum hdmi_clk_refsel refsel; > }; > > +struct hdmi_irq_vector { > + u8 pll_recal; > + u8 pll_unlock; > + u8 pll_lock; > + u8 phy_disconnect; > + u8 phy_connect; > + u8 phy_short_5v; > + u8 video_end_fr; > + u8 video_vsync; > + u8 fifo_sample_req; > + u8 fifo_overflow; > + u8 fifo_underflow; > + u8 ocp_timeout; > + u8 core; > +}; This feels like a very complex way to do a simple thing... Why is this in ti_hdmi.h anyway? These are omap4 HDMI interrupts, right? Why don't you just define the interrupts the same way dispc/dsi do: #define HDMI_IRQ_RECAL (1 << 31) Then you could have simple functions like hdmi_enable_irq(u32 irqmask) and call it like: hdmi_enable_irq(HDMI_IRQ_RECAL | HDMI_IRQ_SOMETHING); Tomi [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] OMAPDSS: HDMI: Interrupt and PHY state handling support 2012-03-20 13:15 [PATCH 0/3] OMAPDSS: HDMI: Interrupt and PHY state handling support mythripk 2012-03-20 13:15 ` [PATCH 1/3] OMAPDSS: HDMI: support for interrupt enabling mythripk @ 2012-03-27 10:29 ` Tomi Valkeinen 1 sibling, 0 replies; 8+ messages in thread From: Tomi Valkeinen @ 2012-03-27 10:29 UTC (permalink / raw) To: mythripk; +Cc: linux-omap [-- Attachment #1: Type: text/plain, Size: 1374 bytes --] On Tue, 2012-03-20 at 18:45 +0530, mythripk@ti.com wrote: > From: Mythri P K <mythripk@ti.com> > > Add support for handling the DSS_HDMI interrupt in HDMI, A line that serves to > notify HDMI of status change in PHY, PLL and CORE based on the registration. > Also logic to support enabling of the PHY in TX_ON state only when a PHY_CONNECT > which would make sure that TMDS lines are high before putting it in TX_ON state. > > Mythri P K (3): > OMAPDSS: HDMI: support for interrupt enabling > OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt > OMAPDSS: HDMI: wait for TMDS to be high before putting phy in TX_ON Sorry, but this patch set is quite bad. First of all, the subject on your third patch is again cut short. How do you even do that? The subject seems to be fine above. I've never seen other people do that, but you somehow manage to do it all the time. So, please, read the git manual, fix your environment, your way of working, or whatever is causing this so that we'll never see it again. Then, the intro text above is rather confusing. Are there words missing around the part with "PHY_CONNECT"? Please spend more time on the descriptions, and possibly ask someone to proof-read them so that at least there aren't words missing and they are somehow understandable. I'll comment on other things in later mails. Tomi [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-03-27 10:31 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-20 13:15 [PATCH 0/3] OMAPDSS: HDMI: Interrupt and PHY state handling support mythripk 2012-03-20 13:15 ` [PATCH 1/3] OMAPDSS: HDMI: support for interrupt enabling mythripk 2012-03-20 13:15 ` [PATCH 2/3] OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt mythripk 2012-03-20 13:15 ` [PATCH 3/3] OMAPDSS: HDMI: wait for TMDS to be high before putting mythripk 2012-03-27 10:30 ` Tomi Valkeinen 2012-03-27 10:30 ` [PATCH 2/3] OMAPDSS: HDMI: Add support for DSS_HDMI Interrupt Tomi Valkeinen 2012-03-27 10:29 ` [PATCH 1/3] OMAPDSS: HDMI: support for interrupt enabling Tomi Valkeinen 2012-03-27 10:29 ` [PATCH 0/3] OMAPDSS: HDMI: Interrupt and PHY state handling support Tomi Valkeinen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).