| Message ID | 1412844147-12061-1-git-send-email-benjamin.gaignard@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 9, 2014 at 4:42 AM, Benjamin Gaignard <benjamin.gaignard@linaro.org> wrote: > gpio used for HDMI hot plug detection is useless, > HDMI_STI register contains an hot plug detection status bit. > Fix binding documentation. Random thought, but depending on how much you trust your hw designers you may want to at least leave the DT entry as optional? I've started out before completely trusting the hdmi block's debounced hpd signal, but eventually giving up and using raw gpio, or combination of gpio + debounced hdmi signal for reliability.. BR, -R > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > --- > Documentation/devicetree/bindings/gpu/st,stih4xx.txt | 2 -- > drivers/gpu/drm/sti/sti_hdmi.c | 11 ++--------- > drivers/gpu/drm/sti/sti_hdmi.h | 5 +++-- > 3 files changed, 5 insertions(+), 13 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpu/st,stih4xx.txt b/Documentation/devicetree/bindings/gpu/st,stih4xx.txt > index 8885d9e..32cfc7b 100644 > --- a/Documentation/devicetree/bindings/gpu/st,stih4xx.txt > +++ b/Documentation/devicetree/bindings/gpu/st,stih4xx.txt > @@ -68,7 +68,6 @@ STMicroelectronics stih4xx platforms > number of clocks may depend of the SoC type. > - clock-names: names of the clocks listed in clocks property in the same > order. > - - hdmi,hpd-gpio: gpio id to detect if an hdmi cable is plugged or not. > - ddc: phandle of an I2C controller used for DDC EDID probing > > sti-hda: > @@ -174,7 +173,6 @@ Example: > interrupt-names = "irq"; > clock-names = "pix", "tmds", "phy", "audio"; > clocks = <&clockgen_c_vcc CLK_S_PIX_HDMI>, <&clockgen_c_vcc CLK_S_TMDS_HDMI>, <&clockgen_c_vcc CLK_S_HDMI_REJECT_PLL>, <&clockgen_b1 CLK_S_PCM_0>; > - hdmi,hpd-gpio = <&PIO2 5>; > }; > > sti-hda@fe85a000 { > diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c > index 7673189..09f39c5 100644 > --- a/drivers/gpu/drm/sti/sti_hdmi.c > +++ b/drivers/gpu/drm/sti/sti_hdmi.c > @@ -130,8 +130,7 @@ static irqreturn_t hdmi_irq_thread(int irq, void *arg) > > /* Hot plug/unplug IRQ */ > if (hdmi->irq_status & HDMI_INT_HOT_PLUG) { > - /* read gpio to get the status */ > - hdmi->hpd = gpio_get_value(hdmi->hpd_gpio); > + hdmi->hpd = readl(hdmi->regs + HDMI_STA) & HDMI_STA_HOT_PLUG; > if (hdmi->drm_dev) > drm_helper_hpd_irq_event(hdmi->drm_dev); > } > @@ -767,13 +766,7 @@ static int sti_hdmi_probe(struct platform_device *pdev) > return PTR_ERR(hdmi->clk_audio); > } > > - hdmi->hpd_gpio = of_get_named_gpio(np, "hdmi,hpd-gpio", 0); > - if (hdmi->hpd_gpio < 0) { > - DRM_ERROR("Failed to get hdmi hpd-gpio\n"); > - return -EIO; > - } > - > - hdmi->hpd = gpio_get_value(hdmi->hpd_gpio); > + hdmi->hpd = readl(hdmi->regs + HDMI_STA) & HDMI_STA_HOT_PLUG; > > init_waitqueue_head(&hdmi->wait_event); > > diff --git a/drivers/gpu/drm/sti/sti_hdmi.h b/drivers/gpu/drm/sti/sti_hdmi.h > index d00a3e0..3d22390 100644 > --- a/drivers/gpu/drm/sti/sti_hdmi.h > +++ b/drivers/gpu/drm/sti/sti_hdmi.h > @@ -14,6 +14,9 @@ > #define HDMI_STA 0x0010 > #define HDMI_STA_DLL_LCK BIT(5) > > +#define HDMI_STA_HOT_PLUG_SHIFT 4 > +#define HDMI_STA_HOT_PLUG (1 << HDMI_STA_HOT_PLUG_SHIFT) > + > struct sti_hdmi; > > struct hdmi_phy_ops { > @@ -37,7 +40,6 @@ struct hdmi_phy_ops { > * @irq_status: interrupt status register > * @phy_ops: phy start/stop operations > * @enabled: true if hdmi is enabled else false > - * @hpd_gpio: hdmi hot plug detect gpio number > * @hpd: hot plug detect status > * @wait_event: wait event > * @event_received: wait event status > @@ -57,7 +59,6 @@ struct sti_hdmi { > u32 irq_status; > struct hdmi_phy_ops *phy_ops; > bool enabled; > - int hpd_gpio; > bool hpd; > wait_queue_head_t wait_event; > bool event_received; > -- > 1.9.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
We have test with and without gpio and we haven't seen any difference. To be honest I prefer simplify the bindings which is already complex and so remove gpio. But I will in mind your advice if one day I have debounce issues. Regards, Benjamin 2014-10-09 14:10 GMT+02:00 Rob Clark <robdclark@gmail.com>: > On Thu, Oct 9, 2014 at 4:42 AM, Benjamin Gaignard > <benjamin.gaignard@linaro.org> wrote: >> gpio used for HDMI hot plug detection is useless, >> HDMI_STI register contains an hot plug detection status bit. >> Fix binding documentation. > > Random thought, but depending on how much you trust your hw designers > you may want to at least leave the DT entry as optional? I've started > out before completely trusting the hdmi block's debounced hpd signal, > but eventually giving up and using raw gpio, or combination of gpio + > debounced hdmi signal for reliability.. > > BR, > -R > >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> >> --- >> Documentation/devicetree/bindings/gpu/st,stih4xx.txt | 2 -- >> drivers/gpu/drm/sti/sti_hdmi.c | 11 ++--------- >> drivers/gpu/drm/sti/sti_hdmi.h | 5 +++-- >> 3 files changed, 5 insertions(+), 13 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/gpu/st,stih4xx.txt b/Documentation/devicetree/bindings/gpu/st,stih4xx.txt >> index 8885d9e..32cfc7b 100644 >> --- a/Documentation/devicetree/bindings/gpu/st,stih4xx.txt >> +++ b/Documentation/devicetree/bindings/gpu/st,stih4xx.txt >> @@ -68,7 +68,6 @@ STMicroelectronics stih4xx platforms >> number of clocks may depend of the SoC type. >> - clock-names: names of the clocks listed in clocks property in the same >> order. >> - - hdmi,hpd-gpio: gpio id to detect if an hdmi cable is plugged or not. >> - ddc: phandle of an I2C controller used for DDC EDID probing >> >> sti-hda: >> @@ -174,7 +173,6 @@ Example: >> interrupt-names = "irq"; >> clock-names = "pix", "tmds", "phy", "audio"; >> clocks = <&clockgen_c_vcc CLK_S_PIX_HDMI>, <&clockgen_c_vcc CLK_S_TMDS_HDMI>, <&clockgen_c_vcc CLK_S_HDMI_REJECT_PLL>, <&clockgen_b1 CLK_S_PCM_0>; >> - hdmi,hpd-gpio = <&PIO2 5>; >> }; >> >> sti-hda@fe85a000 { >> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c >> index 7673189..09f39c5 100644 >> --- a/drivers/gpu/drm/sti/sti_hdmi.c >> +++ b/drivers/gpu/drm/sti/sti_hdmi.c >> @@ -130,8 +130,7 @@ static irqreturn_t hdmi_irq_thread(int irq, void *arg) >> >> /* Hot plug/unplug IRQ */ >> if (hdmi->irq_status & HDMI_INT_HOT_PLUG) { >> - /* read gpio to get the status */ >> - hdmi->hpd = gpio_get_value(hdmi->hpd_gpio); >> + hdmi->hpd = readl(hdmi->regs + HDMI_STA) & HDMI_STA_HOT_PLUG; >> if (hdmi->drm_dev) >> drm_helper_hpd_irq_event(hdmi->drm_dev); >> } >> @@ -767,13 +766,7 @@ static int sti_hdmi_probe(struct platform_device *pdev) >> return PTR_ERR(hdmi->clk_audio); >> } >> >> - hdmi->hpd_gpio = of_get_named_gpio(np, "hdmi,hpd-gpio", 0); >> - if (hdmi->hpd_gpio < 0) { >> - DRM_ERROR("Failed to get hdmi hpd-gpio\n"); >> - return -EIO; >> - } >> - >> - hdmi->hpd = gpio_get_value(hdmi->hpd_gpio); >> + hdmi->hpd = readl(hdmi->regs + HDMI_STA) & HDMI_STA_HOT_PLUG; >> >> init_waitqueue_head(&hdmi->wait_event); >> >> diff --git a/drivers/gpu/drm/sti/sti_hdmi.h b/drivers/gpu/drm/sti/sti_hdmi.h >> index d00a3e0..3d22390 100644 >> --- a/drivers/gpu/drm/sti/sti_hdmi.h >> +++ b/drivers/gpu/drm/sti/sti_hdmi.h >> @@ -14,6 +14,9 @@ >> #define HDMI_STA 0x0010 >> #define HDMI_STA_DLL_LCK BIT(5) >> >> +#define HDMI_STA_HOT_PLUG_SHIFT 4 >> +#define HDMI_STA_HOT_PLUG (1 << HDMI_STA_HOT_PLUG_SHIFT) >> + >> struct sti_hdmi; >> >> struct hdmi_phy_ops { >> @@ -37,7 +40,6 @@ struct hdmi_phy_ops { >> * @irq_status: interrupt status register >> * @phy_ops: phy start/stop operations >> * @enabled: true if hdmi is enabled else false >> - * @hpd_gpio: hdmi hot plug detect gpio number >> * @hpd: hot plug detect status >> * @wait_event: wait event >> * @event_received: wait event status >> @@ -57,7 +59,6 @@ struct sti_hdmi { >> u32 irq_status; >> struct hdmi_phy_ops *phy_ops; >> bool enabled; >> - int hpd_gpio; >> bool hpd; >> wait_queue_head_t wait_event; >> bool event_received; >> -- >> 1.9.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/Documentation/devicetree/bindings/gpu/st,stih4xx.txt b/Documentation/devicetree/bindings/gpu/st,stih4xx.txt index 8885d9e..32cfc7b 100644 --- a/Documentation/devicetree/bindings/gpu/st,stih4xx.txt +++ b/Documentation/devicetree/bindings/gpu/st,stih4xx.txt @@ -68,7 +68,6 @@ STMicroelectronics stih4xx platforms number of clocks may depend of the SoC type. - clock-names: names of the clocks listed in clocks property in the same order. - - hdmi,hpd-gpio: gpio id to detect if an hdmi cable is plugged or not. - ddc: phandle of an I2C controller used for DDC EDID probing sti-hda: @@ -174,7 +173,6 @@ Example: interrupt-names = "irq"; clock-names = "pix", "tmds", "phy", "audio"; clocks = <&clockgen_c_vcc CLK_S_PIX_HDMI>, <&clockgen_c_vcc CLK_S_TMDS_HDMI>, <&clockgen_c_vcc CLK_S_HDMI_REJECT_PLL>, <&clockgen_b1 CLK_S_PCM_0>; - hdmi,hpd-gpio = <&PIO2 5>; }; sti-hda@fe85a000 { diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index 7673189..09f39c5 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -130,8 +130,7 @@ static irqreturn_t hdmi_irq_thread(int irq, void *arg) /* Hot plug/unplug IRQ */ if (hdmi->irq_status & HDMI_INT_HOT_PLUG) { - /* read gpio to get the status */ - hdmi->hpd = gpio_get_value(hdmi->hpd_gpio); + hdmi->hpd = readl(hdmi->regs + HDMI_STA) & HDMI_STA_HOT_PLUG; if (hdmi->drm_dev) drm_helper_hpd_irq_event(hdmi->drm_dev); } @@ -767,13 +766,7 @@ static int sti_hdmi_probe(struct platform_device *pdev) return PTR_ERR(hdmi->clk_audio); } - hdmi->hpd_gpio = of_get_named_gpio(np, "hdmi,hpd-gpio", 0); - if (hdmi->hpd_gpio < 0) { - DRM_ERROR("Failed to get hdmi hpd-gpio\n"); - return -EIO; - } - - hdmi->hpd = gpio_get_value(hdmi->hpd_gpio); + hdmi->hpd = readl(hdmi->regs + HDMI_STA) & HDMI_STA_HOT_PLUG; init_waitqueue_head(&hdmi->wait_event); diff --git a/drivers/gpu/drm/sti/sti_hdmi.h b/drivers/gpu/drm/sti/sti_hdmi.h index d00a3e0..3d22390 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.h +++ b/drivers/gpu/drm/sti/sti_hdmi.h @@ -14,6 +14,9 @@ #define HDMI_STA 0x0010 #define HDMI_STA_DLL_LCK BIT(5) +#define HDMI_STA_HOT_PLUG_SHIFT 4 +#define HDMI_STA_HOT_PLUG (1 << HDMI_STA_HOT_PLUG_SHIFT) + struct sti_hdmi; struct hdmi_phy_ops { @@ -37,7 +40,6 @@ struct hdmi_phy_ops { * @irq_status: interrupt status register * @phy_ops: phy start/stop operations * @enabled: true if hdmi is enabled else false - * @hpd_gpio: hdmi hot plug detect gpio number * @hpd: hot plug detect status * @wait_event: wait event * @event_received: wait event status @@ -57,7 +59,6 @@ struct sti_hdmi { u32 irq_status; struct hdmi_phy_ops *phy_ops; bool enabled; - int hpd_gpio; bool hpd; wait_queue_head_t wait_event; bool event_received;
gpio used for HDMI hot plug detection is useless, HDMI_STI register contains an hot plug detection status bit. Fix binding documentation. Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> --- Documentation/devicetree/bindings/gpu/st,stih4xx.txt | 2 -- drivers/gpu/drm/sti/sti_hdmi.c | 11 ++--------- drivers/gpu/drm/sti/sti_hdmi.h | 5 +++-- 3 files changed, 5 insertions(+), 13 deletions(-)