linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/sun4i: dw-hdmi: Improve CEC support
@ 2019-03-24 21:21 Jernej Skrabec
  2019-03-24 21:21 ` [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver Jernej Skrabec
  2019-03-24 21:21 ` [PATCH 2/2] drm/sun4i: dw-hdmi: Bit bang CEC on some SoCs Jernej Skrabec
  0 siblings, 2 replies; 7+ messages in thread
From: Jernej Skrabec @ 2019-03-24 21:21 UTC (permalink / raw)
  To: maxime.ripard
  Cc: a.hajda, Laurent.pinchart, airlied, daniel, wens, dri-devel,
	linux-kernel, linux-arm-kernel

It turns out that additional logic between HDMI CEC controller and
pins on PHY on some Allwinner SoCs prevents proper communication.
It might be possible to fix it, but it's much easier and less error
prone to just directly drive pins using software implementation of
CEC protocol.

Let me know what do you think.

Best regards,
Jernej

Jernej Skrabec (2):
  drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC
    driver
  drm/sun4i: dw-hdmi: Bit bang CEC on some SoCs

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
 drivers/gpu/drm/sun4i/Kconfig             | 10 +++
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h     | 11 +++
 drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c    | 83 ++++++++++++++++++++++-
 include/drm/bridge/dw_hdmi.h              |  2 +
 5 files changed, 105 insertions(+), 3 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver
  2019-03-24 21:21 [PATCH 0/2] drm/sun4i: dw-hdmi: Improve CEC support Jernej Skrabec
@ 2019-03-24 21:21 ` Jernej Skrabec
  2019-03-25  1:35   ` Laurent Pinchart
  2019-03-24 21:21 ` [PATCH 2/2] drm/sun4i: dw-hdmi: Bit bang CEC on some SoCs Jernej Skrabec
  1 sibling, 1 reply; 7+ messages in thread
From: Jernej Skrabec @ 2019-03-24 21:21 UTC (permalink / raw)
  To: maxime.ripard
  Cc: a.hajda, Laurent.pinchart, airlied, daniel, wens, dri-devel,
	linux-kernel, linux-arm-kernel

DW HDMI controller on some Allwinner SoCs has support for CEC, but due
to additional logic put between CEC controller and pins, it doesn't work
correctly, at least not with a lot of instrusive changes. Fortunately,
it's still possible to bitbang protocol.

For such cases, add a platform option to suppress loading CEC driver. If
DW HDMI CEC driver would be loaded, it wouldn't work anyway and only
cause a confusion with multiple /dev entries.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
 include/drm/bridge/dw_hdmi.h              | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index a63e5f0dae56..fdda26f8b056 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2634,7 +2634,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		hdmi->audio = platform_device_register_full(&pdevinfo);
 	}
 
-	if (config0 & HDMI_CONFIG0_CEC) {
+	if (!plat_data->is_cec_unusable && (config0 & HDMI_CONFIG0_CEC)) {
 		cec.hdmi = hdmi;
 		cec.ops = &dw_hdmi_cec_ops;
 		cec.irq = irq;
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 66e70770cce5..764b8bcfa62c 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -144,6 +144,8 @@ struct dw_hdmi_plat_data {
 	int (*configure_phy)(struct dw_hdmi *hdmi,
 			     const struct dw_hdmi_plat_data *pdata,
 			     unsigned long mpixelclock);
+
+	unsigned int is_cec_unusable : 1;
 };
 
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] drm/sun4i: dw-hdmi: Bit bang CEC on some SoCs
  2019-03-24 21:21 [PATCH 0/2] drm/sun4i: dw-hdmi: Improve CEC support Jernej Skrabec
  2019-03-24 21:21 ` [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver Jernej Skrabec
@ 2019-03-24 21:21 ` Jernej Skrabec
  1 sibling, 0 replies; 7+ messages in thread
From: Jernej Skrabec @ 2019-03-24 21:21 UTC (permalink / raw)
  To: maxime.ripard
  Cc: a.hajda, Laurent.pinchart, airlied, daniel, wens, dri-devel,
	linux-kernel, linux-arm-kernel

All DW HDMI controllers used by Allwinner SoCs include CEC controller.
However, due to additional logic put between CEC controller and pins,
CEC communication doesn't work well on some of them.

Based on observations, it seems that only outgoing messages are
properly transmitted. It's possible that it would still work correctly
if pins are switched between input and output mode manually in right
moment. But that's very error prone. It's better and easier just to bit
bang protocol.

Enable bit banging just for controller and phy combination found in H3
and other 40nm SoCs. Other combinations work (H6) or the status is
unknown (A83T).

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/sun4i/Kconfig          | 10 ++++
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h  | 11 ++++
 drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 83 +++++++++++++++++++++++++-
 3 files changed, 102 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index 1dbbc3a1b763..7149c72e44c8 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -60,6 +60,16 @@ config DRM_SUN8I_DW_HDMI
 	  DesignWare HDMI controller with custom HDMI PHY. If M is
 	  selected the module will be called sun8i_dw_hdmi.
 
+config DRM_SUN8I_DW_HDMI_CEC
+       bool "Allwinner DesignWare HDMI CEC Support for 40nm SoCs"
+       depends on DRM_SUN8I_DW_HDMI
+       select CEC_CORE
+       select CEC_PIN
+       help
+	  Choose this option if you have an 40nm Allwinner SoC with
+	  the DesignWare HDMI controller with custom HDMI PHY and
+	  you want to use CEC.
+
 config DRM_SUN8I_MIXER
 	tristate "Support for Allwinner Display Engine 2.0 Mixer"
 	default MACH_SUN8I
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
index 720c5aa8adc1..49ca001923e3 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
@@ -12,6 +12,7 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
+#include <media/cec-pin.h>
 
 #define SUN8I_HDMI_PHY_DBG_CTRL_REG	0x0000
 #define SUN8I_HDMI_PHY_DBG_CTRL_PX_LOCK		BIT(0)
@@ -144,6 +145,13 @@
 #define SUN8I_HDMI_PHY_ANA_STS_RCAL_MASK	GENMASK(5, 0)
 
 #define SUN8I_HDMI_PHY_CEC_REG		0x003c
+#define SUN8I_HDMI_PHY_CEC_PIN_CTRL		BIT(7)
+/*
+ * Documentation says that this bit is output enable. However,
+ * it seems that this bit is actually output disable.
+ */
+#define SUN8I_HDMI_PHY_CEC_OUT_DIS		BIT(2)
+#define SUN8I_HDMI_PHY_CEC_IN_DATA		BIT(1)
 
 struct sun8i_hdmi_phy;
 
@@ -151,6 +159,7 @@ struct sun8i_hdmi_phy_variant {
 	bool has_phy_clk;
 	bool has_second_pll;
 	unsigned int is_custom_phy : 1;
+	unsigned int bit_bang_cec : 1;
 	const struct dw_hdmi_curr_ctrl *cur_ctr;
 	const struct dw_hdmi_mpll_config *mpll_cfg;
 	const struct dw_hdmi_phy_config *phy_cfg;
@@ -163,6 +172,8 @@ struct sun8i_hdmi_phy_variant {
 };
 
 struct sun8i_hdmi_phy {
+	struct cec_adapter		*cec_adapter;
+	struct cec_notifier		*cec_notifier;
 	struct clk			*clk_bus;
 	struct clk			*clk_mod;
 	struct clk			*clk_phy;
diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
index 66ea3a902e36..70e291353569 100644
--- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
+++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
@@ -503,8 +503,9 @@ static void sun8i_hdmi_phy_init_h3(struct sun8i_hdmi_phy *phy)
 	regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
 			   SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK, 0);
 
-	/* set HW control of CEC pins */
-	regmap_write(phy->regs, SUN8I_HDMI_PHY_CEC_REG, 0);
+	/* manual control of CEC pins */
+	regmap_write(phy->regs, SUN8I_HDMI_PHY_CEC_REG,
+		     SUN8I_HDMI_PHY_CEC_PIN_CTRL);
 
 	/* read calibration data */
 	regmap_read(phy->regs, SUN8I_HDMI_PHY_ANA_STS_REG, &val);
@@ -530,8 +531,49 @@ void sun8i_hdmi_phy_set_ops(struct sun8i_hdmi_phy *phy,
 		plat_data->cur_ctr = variant->cur_ctr;
 		plat_data->phy_config = variant->phy_cfg;
 	}
+	plat_data->is_cec_unusable = phy->variant->bit_bang_cec;
 }
 
+#ifdef CONFIG_DRM_SUN8I_DW_HDMI_CEC
+static bool sun8i_hdmi_phy_cec_pin_read(struct cec_adapter *adap)
+{
+	struct sun8i_hdmi_phy *phy = cec_get_drvdata(adap);
+	unsigned int val;
+
+	regmap_read(phy->regs, SUN8I_HDMI_PHY_CEC_REG, &val);
+
+	return val & SUN8I_HDMI_PHY_CEC_IN_DATA;
+}
+
+static void sun8i_hdmi_phy_cec_pin_low(struct cec_adapter *adap)
+{
+	struct sun8i_hdmi_phy *phy = cec_get_drvdata(adap);
+
+	/* Start driving the CEC pin low */
+	regmap_write(phy->regs, SUN8I_HDMI_PHY_CEC_REG,
+		     SUN8I_HDMI_PHY_CEC_PIN_CTRL);
+}
+
+static void sun8i_hdmi_phy_cec_pin_high(struct cec_adapter *adap)
+{
+	struct sun8i_hdmi_phy *phy = cec_get_drvdata(adap);
+
+	/*
+	 * Stop driving the CEC pin, the pull up will take over
+	 * unless another CEC device is driving the pin low.
+	 */
+	regmap_write(phy->regs, SUN8I_HDMI_PHY_CEC_REG,
+		     SUN8I_HDMI_PHY_CEC_PIN_CTRL |
+		     SUN8I_HDMI_PHY_CEC_OUT_DIS);
+}
+
+static const struct cec_pin_ops sun8i_hdmi_phy_cec_pin_ops = {
+	.read = sun8i_hdmi_phy_cec_pin_read,
+	.low = sun8i_hdmi_phy_cec_pin_low,
+	.high = sun8i_hdmi_phy_cec_pin_high,
+};
+#endif
+
 static struct regmap_config sun8i_hdmi_phy_regmap_config = {
 	.reg_bits	= 32,
 	.val_bits	= 32,
@@ -548,6 +590,7 @@ static const struct sun8i_hdmi_phy_variant sun8i_a83t_hdmi_phy = {
 };
 
 static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {
+	.bit_bang_cec = true,
 	.has_phy_clk = true,
 	.is_custom_phy = true,
 	.phy_init = &sun8i_hdmi_phy_init_h3,
@@ -556,6 +599,7 @@ static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {
 };
 
 static const struct sun8i_hdmi_phy_variant sun8i_r40_hdmi_phy = {
+	.bit_bang_cec = true,
 	.has_phy_clk = true,
 	.has_second_pll = true,
 	.is_custom_phy = true,
@@ -565,6 +609,7 @@ static const struct sun8i_hdmi_phy_variant sun8i_r40_hdmi_phy = {
 };
 
 static const struct sun8i_hdmi_phy_variant sun50i_a64_hdmi_phy = {
+	.bit_bang_cec = true,
 	.has_phy_clk = true,
 	.is_custom_phy = true,
 	.phy_init = &sun8i_hdmi_phy_init_h3,
@@ -708,10 +753,40 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi, struct device_node *node)
 		goto err_disable_clk_bus;
 	}
 
+#ifdef CONFIG_DRM_SUN8I_DW_HDMI_CEC
+	if (phy->variant->bit_bang_cec) {
+		phy->cec_notifier = cec_notifier_get(dev);
+		if (!phy->cec_notifier) {
+			ret = -ENOMEM;
+			goto err_disable_clk_mod;
+		}
+
+		phy->cec_adapter =
+			cec_pin_allocate_adapter(&sun8i_hdmi_phy_cec_pin_ops,
+						 phy, "sun8i-cec",
+						 CEC_CAP_DEFAULTS);
+		ret = PTR_ERR_OR_ZERO(phy->cec_adapter);
+		if (ret < 0)
+			goto err_put_cec_notifier;
+
+		ret = cec_register_adapter(phy->cec_adapter, dev);
+		if (ret < 0)
+			goto err_delete_cec_adapter;
+
+		cec_register_cec_notifier(phy->cec_adapter, phy->cec_notifier);
+	}
+#endif
+
 	hdmi->phy = phy;
 
 	return 0;
 
+err_delete_cec_adapter:
+	cec_delete_adapter(phy->cec_adapter);
+err_put_cec_notifier:
+	cec_notifier_put(phy->cec_notifier);
+err_disable_clk_mod:
+	clk_disable_unprepare(phy->clk_mod);
 err_disable_clk_bus:
 	clk_disable_unprepare(phy->clk_bus);
 err_deassert_rst_phy:
@@ -736,6 +811,10 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi *hdmi)
 {
 	struct sun8i_hdmi_phy *phy = hdmi->phy;
 
+	cec_unregister_adapter(phy->cec_adapter);
+	if (phy->cec_notifier)
+		cec_notifier_put(phy->cec_notifier);
+
 	clk_disable_unprepare(phy->clk_mod);
 	clk_disable_unprepare(phy->clk_bus);
 	clk_disable_unprepare(phy->clk_phy);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver
  2019-03-24 21:21 ` [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver Jernej Skrabec
@ 2019-03-25  1:35   ` Laurent Pinchart
  2019-03-25  6:18     ` Jernej Škrabec
  2019-03-25  8:33     ` Neil Armstrong
  0 siblings, 2 replies; 7+ messages in thread
From: Laurent Pinchart @ 2019-03-25  1:35 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: maxime.ripard, a.hajda, airlied, daniel, wens, dri-devel,
	linux-kernel, linux-arm-kernel

Hi Jernej,

Thank you for the patch.

On Sun, Mar 24, 2019 at 10:21:42PM +0100, Jernej Skrabec wrote:
> DW HDMI controller on some Allwinner SoCs has support for CEC, but due
> to additional logic put between CEC controller and pins, it doesn't work
> correctly, at least not with a lot of instrusive changes. Fortunately,
> it's still possible to bitbang protocol.
> 
> For such cases, add a platform option to suppress loading CEC driver. If
> DW HDMI CEC driver would be loaded, it wouldn't work anyway and only
> cause a confusion with multiple /dev entries.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>  include/drm/bridge/dw_hdmi.h              | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index a63e5f0dae56..fdda26f8b056 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2634,7 +2634,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		hdmi->audio = platform_device_register_full(&pdevinfo);
>  	}
>  
> -	if (config0 & HDMI_CONFIG0_CEC) {
> +	if (!plat_data->is_cec_unusable && (config0 & HDMI_CONFIG0_CEC)) {
>  		cec.hdmi = hdmi;
>  		cec.ops = &dw_hdmi_cec_ops;
>  		cec.irq = irq;
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 66e70770cce5..764b8bcfa62c 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -144,6 +144,8 @@ struct dw_hdmi_plat_data {
>  	int (*configure_phy)(struct dw_hdmi *hdmi,
>  			     const struct dw_hdmi_plat_data *pdata,
>  			     unsigned long mpixelclock);
> +
> +	unsigned int is_cec_unusable : 1;

Strictly speaking your CEC controller isn't unusable, it's just a bit
difficult to use it according to your commit message. Would disable_cec
be a more appropriate field name ? And how difficult would it be to
support the hardware CEC controller, would that result in changes that
could be useful to other vendors too ?

>  };
>  
>  struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver
  2019-03-25  1:35   ` Laurent Pinchart
@ 2019-03-25  6:18     ` Jernej Škrabec
  2019-03-25  8:33     ` Neil Armstrong
  1 sibling, 0 replies; 7+ messages in thread
From: Jernej Škrabec @ 2019-03-25  6:18 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: maxime.ripard, a.hajda, airlied, daniel, wens, dri-devel,
	linux-kernel, linux-arm-kernel

Dne ponedeljek, 25. marec 2019 ob 02:35:31 CET je Laurent Pinchart napisal(a):
> Hi Jernej,
> 
> Thank you for the patch.
> 
> On Sun, Mar 24, 2019 at 10:21:42PM +0100, Jernej Skrabec wrote:
> > DW HDMI controller on some Allwinner SoCs has support for CEC, but due
> > to additional logic put between CEC controller and pins, it doesn't work
> > correctly, at least not with a lot of instrusive changes. Fortunately,
> > it's still possible to bitbang protocol.
> > 
> > For such cases, add a platform option to suppress loading CEC driver. If
> > DW HDMI CEC driver would be loaded, it wouldn't work anyway and only
> > cause a confusion with multiple /dev entries.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> >  include/drm/bridge/dw_hdmi.h              | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > a63e5f0dae56..fdda26f8b056 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -2634,7 +2634,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
> > 
> >  		hdmi->audio = platform_device_register_full(&pdevinfo);
> >  	
> >  	}
> > 
> > -	if (config0 & HDMI_CONFIG0_CEC) {
> > +	if (!plat_data->is_cec_unusable && (config0 & HDMI_CONFIG0_CEC)) {
> > 
> >  		cec.hdmi = hdmi;
> >  		cec.ops = &dw_hdmi_cec_ops;
> >  		cec.irq = irq;
> > 
> > diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> > index 66e70770cce5..764b8bcfa62c 100644
> > --- a/include/drm/bridge/dw_hdmi.h
> > +++ b/include/drm/bridge/dw_hdmi.h
> > @@ -144,6 +144,8 @@ struct dw_hdmi_plat_data {
> > 
> >  	int (*configure_phy)(struct dw_hdmi *hdmi,
> >  	
> >  			     const struct dw_hdmi_plat_data *pdata,
> >  			     unsigned long mpixelclock);
> > 
> > +
> > +	unsigned int is_cec_unusable : 1;
> 
> Strictly speaking your CEC controller isn't unusable, it's just a bit
> difficult to use it according to your commit message. Would disable_cec
> be a more appropriate field name ? And how difficult would it be to
> support the hardware CEC controller, would that result in changes that
> could be useful to other vendors too ?

Ok, maybe naming of the variable is a bit harsh. Currently I don't know any 
other hardware with the same issue. While I don't know CEC protocol or DW HDMI 
CEC controller very well, I think it would be difficult to implement change of 
direction during ACK transmission. At that time, packet is not fully received 
or transmitted, so I doubt we could use any interrupt for that. In any case, I 
don't think it's worth the hassle and we should just use proven software 
implementation.

Early attempt on fixing this on kernel provided by Allwinner resulted in very 
ugly code and I'm not sure it works 100% (driver based on i.MX6 one):
https://github.com/jernejsk/OpenELEC-OPi2/blob/openelec-7.0/projects/H3/
patches/linux/linux-008-add-hw-hdmi-cec-support.patch#L1321

> 
> >  };
> >  
> >  struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver
  2019-03-25  1:35   ` Laurent Pinchart
  2019-03-25  6:18     ` Jernej Škrabec
@ 2019-03-25  8:33     ` Neil Armstrong
  1 sibling, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2019-03-25  8:33 UTC (permalink / raw)
  To: Laurent Pinchart, Jernej Skrabec
  Cc: maxime.ripard, airlied, linux-kernel, dri-devel, wens, linux-arm-kernel



Le 25/03/2019 02:35, Laurent Pinchart a écrit :
> Hi Jernej,
> 
> Thank you for the patch.
> 
> On Sun, Mar 24, 2019 at 10:21:42PM +0100, Jernej Skrabec wrote:
>> DW HDMI controller on some Allwinner SoCs has support for CEC, but due
>> to additional logic put between CEC controller and pins, it doesn't work
>> correctly, at least not with a lot of instrusive changes. Fortunately,
>> it's still possible to bitbang protocol.
>>
>> For such cases, add a platform option to suppress loading CEC driver. If
>> DW HDMI CEC driver would be loaded, it wouldn't work anyway and only
>> cause a confusion with multiple /dev entries.
>>
>> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>  include/drm/bridge/dw_hdmi.h              | 2 ++
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index a63e5f0dae56..fdda26f8b056 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -2634,7 +2634,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>  		hdmi->audio = platform_device_register_full(&pdevinfo);
>>  	}
>>  
>> -	if (config0 & HDMI_CONFIG0_CEC) {
>> +	if (!plat_data->is_cec_unusable && (config0 & HDMI_CONFIG0_CEC)) {
>>  		cec.hdmi = hdmi;
>>  		cec.ops = &dw_hdmi_cec_ops;
>>  		cec.irq = irq;
>> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
>> index 66e70770cce5..764b8bcfa62c 100644
>> --- a/include/drm/bridge/dw_hdmi.h
>> +++ b/include/drm/bridge/dw_hdmi.h
>> @@ -144,6 +144,8 @@ struct dw_hdmi_plat_data {
>>  	int (*configure_phy)(struct dw_hdmi *hdmi,
>>  			     const struct dw_hdmi_plat_data *pdata,
>>  			     unsigned long mpixelclock);
>> +
>> +	unsigned int is_cec_unusable : 1;
> 
> Strictly speaking your CEC controller isn't unusable, it's just a bit
> difficult to use it according to your commit message. Would disable_cec
> be a more appropriate field name ? 

Actually, it would be useful to have in for Amlogic SoCs also, the dw-hdmi
is configured with the CEC engine, but isn't connected to an external I/O.
And we have easy way to disable it unless disabling the DW-HDMI CEC driver...
With a more appropriate name,
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

> And how difficult would it be to
> support the hardware CEC controller, would that result in changes that
> could be useful to other vendors too ?
> 
>>  };
>>  
>>  struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver
  2021-04-16  9:27 [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver Neil Armstrong
@ 2021-04-16  9:27 ` Neil Armstrong
  0 siblings, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2021-04-16  9:27 UTC (permalink / raw)
  To: jonas, jernej.skrabec, robert.foss, Laurent.pinchart
  Cc: dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel, Neil Armstrong

From: Jernej Skrabec <jernej.skrabec@siol.net>

This adds DW-HDMI driver a glue option to disable loading of the CEC sub-driver.

On some SoCs, the CEC functionality is enabled in the IP config bits, but the
CEC bus is non-functional like on Amlogic SoCs, where the CEC config bit is set
but the DW-HDMI CEC signal is not connected to a physical pin, leading to some
confusion when the DW-HDMI CEC controller can't communicate on the bus.

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
 include/drm/bridge/dw_hdmi.h              | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index dda4fa9a1a08..ae97513ef886 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -3421,7 +3421,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 		hdmi->audio = platform_device_register_full(&pdevinfo);
 	}
 
-	if (config0 & HDMI_CONFIG0_CEC) {
+	if (!plat_data->disable_cec && (config0 & HDMI_CONFIG0_CEC)) {
 		cec.hdmi = hdmi;
 		cec.ops = &dw_hdmi_cec_ops;
 		cec.irq = irq;
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index ea34ca146b82..6a5716655619 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -153,6 +153,8 @@ struct dw_hdmi_plat_data {
 	const struct dw_hdmi_phy_config *phy_config;
 	int (*configure_phy)(struct dw_hdmi *hdmi, void *data,
 			     unsigned long mpixelclock);
+
+	unsigned int disable_cec : 1;
 };
 
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-04-16  9:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-24 21:21 [PATCH 0/2] drm/sun4i: dw-hdmi: Improve CEC support Jernej Skrabec
2019-03-24 21:21 ` [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver Jernej Skrabec
2019-03-25  1:35   ` Laurent Pinchart
2019-03-25  6:18     ` Jernej Škrabec
2019-03-25  8:33     ` Neil Armstrong
2019-03-24 21:21 ` [PATCH 2/2] drm/sun4i: dw-hdmi: Bit bang CEC on some SoCs Jernej Skrabec
2021-04-16  9:27 [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver Neil Armstrong
2021-04-16  9:27 ` [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver Neil Armstrong

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).