[PATCHv2 0/3] dw-hdmi: add property to disable CEC

[PATCHv2 0/3] dw-hdmi: add property to disable CEC

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Some boards (amlogic) have two CEC controllers: the DesignWare controller
and their own CEC controller (meson ao-cec).

Since the CEC line is not hooked up to the DW controller we need a way
to disable that controller. This patch series adds the cec-disable
property for that purpose.

Regards,

	Hans

Changes since v1:

- Move the dts change to meson-gx.dtsi since according to Neil it is
  valid for all meson-gx boards.
- Fix bad subject line of patch 1.

Hans Verkuil (3):
  dt-bindings: display: dw_hdmi.txt: add cec-disable property
  drm: bridge: dw-hdmi: check the cec-disable property
  arm64: dts: meson-gx.dtsi: add cec-disable

 Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi                    | 1 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c                    | 3 ++-
 3 files changed, 6 insertions(+), 1 deletion(-)

-- 
2.15.1

[PATCHv2 1/3] dt-bindings: display: dw_hdmi.txt: add cec-disable property

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Some boards have both a DesignWare and their own CEC controller.
The CEC pin is only hooked up to their own CEC controller and not
to the DW controller.

Add the cec-disable property to disable the DW CEC controller.

This particular situation happens on Amlogic boards that have their
own meson CEC controller.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
---
 Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
index 33bf981fbe33..4a13f4858bc0 100644
--- a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
@@ -27,6 +27,9 @@ responsible for defining whether each property is required or optional.
   - "isfr" is the internal register configuration clock (mandatory).
   - "cec" is the HDMI CEC controller main clock (optional).
 
+- cec-disable: Do not use the DWC CEC controller since the CEC line is not
+  hooked up even though the CEC DWC IP is present.
+
 - ports: The connectivity of the DWC HDMI TX with the rest of the system is
   expressed in using ports as specified in the device graph bindings defined
   in Documentation/devicetree/bindings/graph.txt. The numbering of the ports
-- 
2.15.1

[PATCHv2 1/3] dt-bindings: display: dw_hdmi.txt: add cec-disable property

[Rob Herring]

On Fri, Mar 23, 2018 at 01:59:13PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Some boards have both a DesignWare and their own CEC controller.
> The CEC pin is only hooked up to their own CEC controller and not
> to the DW controller.
> 
> Add the cec-disable property to disable the DW CEC controller.
> 
> This particular situation happens on Amlogic boards that have their
> own meson CEC controller.

Seems like we could avoid this by describing how the CEC line is hooked 
up which could be needed for other reasons.

> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
>  1 file changed, 3 insertions(+)

[PATCHv2 1/3] dt-bindings: display: dw_hdmi.txt: add cec-disable property

[Hans Verkuil]

On 27/03/18 00:25, Rob Herring wrote:
> On Fri, Mar 23, 2018 at 01:59:13PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Some boards have both a DesignWare and their own CEC controller.
>> The CEC pin is only hooked up to their own CEC controller and not
>> to the DW controller.
>>
>> Add the cec-disable property to disable the DW CEC controller.
>>
>> This particular situation happens on Amlogic boards that have their
>> own meson CEC controller.
> 
> Seems like we could avoid this by describing how the CEC line is hooked 
> up which could be needed for other reasons.

So there are three situations:

1) The cec pin is connected to the DW HDMI TX. That's already supported.
2) The cec pin is not connected at all, but the CEC IP is instantiated.
   We need the cec-disable property for that. This simply states that the
   CEC pin is not connected.
3) The cec pin is connected to an HDMI RX. We do not support this at the
   moment. If we want to support this, then we need a 'hdmi-rx' phandle
   that points to the HDMI receiver that the CEC pin is associated with.
   This will be similar to the already existing 'hdmi-phandle' property
   used to associate a CEC driver with an HDMI transmitter. In hindsight
   it would have been better if 'hdmi-phandle' was named 'hdmi-tx' :-(

I can make a binding proposal for 3, but I have no hardware to test it,
so I think it is better to add this only when someone has hardware. It
will require quite a few changes to the driver and likely also the CEC core.

Regards,

	Hans

> 
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
>>  1 file changed, 3 insertions(+)

[PATCHv2 1/3] dt-bindings: display: dw_hdmi.txt: add cec-disable property

[Neil Armstrong]

Hi Hans,

On 03/04/2018 10:27, Hans Verkuil wrote:
> On 27/03/18 00:25, Rob Herring wrote:
>> On Fri, Mar 23, 2018 at 01:59:13PM +0100, Hans Verkuil wrote:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> Some boards have both a DesignWare and their own CEC controller.
>>> The CEC pin is only hooked up to their own CEC controller and not
>>> to the DW controller.
>>>
>>> Add the cec-disable property to disable the DW CEC controller.
>>>
>>> This particular situation happens on Amlogic boards that have their
>>> own meson CEC controller.
>>
>> Seems like we could avoid this by describing how the CEC line is hooked 
>> up which could be needed for other reasons.
> 
> So there are three situations:
> 
> 1) The cec pin is connected to the DW HDMI TX. That's already supported.
> 2) The cec pin is not connected at all, but the CEC IP is instantiated.
>    We need the cec-disable property for that. This simply states that the
>    CEC pin is not connected.
> 3) The cec pin is connected to an HDMI RX. We do not support this at the
>    moment. If we want to support this, then we need a 'hdmi-rx' phandle
>    that points to the HDMI receiver that the CEC pin is associated with.
>    This will be similar to the already existing 'hdmi-phandle' property
>    used to associate a CEC driver with an HDMI transmitter. In hindsight
>    it would have been better if 'hdmi-phandle' was named 'hdmi-tx' :-(
> 
> I can make a binding proposal for 3, but I have no hardware to test it,
> so I think it is better to add this only when someone has hardware. It
> will require quite a few changes to the driver and likely also the CEC core.

Can't we simply add a property to override the HW config fields in this case ?
It will be then usable with any feature the is enabled by reading the config
bits like AHB Audio, I2c, CEC, ... and maybe many more in the future.

Neil

> 
> Regards,
> 
> 	Hans
> 
>>
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>>  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
>>>  1 file changed, 3 insertions(+)
> 

[PATCHv2 1/3] dt-bindings: display: dw_hdmi.txt: add cec-disable property

[Hans Verkuil]

On 29/06/18 09:17, Neil Armstrong wrote:
> Hi Hans,
> 
> On 03/04/2018 10:27, Hans Verkuil wrote:
>> On 27/03/18 00:25, Rob Herring wrote:
>>> On Fri, Mar 23, 2018 at 01:59:13PM +0100, Hans Verkuil wrote:
>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>
>>>> Some boards have both a DesignWare and their own CEC controller.
>>>> The CEC pin is only hooked up to their own CEC controller and not
>>>> to the DW controller.
>>>>
>>>> Add the cec-disable property to disable the DW CEC controller.
>>>>
>>>> This particular situation happens on Amlogic boards that have their
>>>> own meson CEC controller.
>>>
>>> Seems like we could avoid this by describing how the CEC line is hooked 
>>> up which could be needed for other reasons.
>>
>> So there are three situations:
>>
>> 1) The cec pin is connected to the DW HDMI TX. That's already supported.
>> 2) The cec pin is not connected at all, but the CEC IP is instantiated.
>>    We need the cec-disable property for that. This simply states that the
>>    CEC pin is not connected.
>> 3) The cec pin is connected to an HDMI RX. We do not support this at the
>>    moment. If we want to support this, then we need a 'hdmi-rx' phandle
>>    that points to the HDMI receiver that the CEC pin is associated with.
>>    This will be similar to the already existing 'hdmi-phandle' property
>>    used to associate a CEC driver with an HDMI transmitter. In hindsight
>>    it would have been better if 'hdmi-phandle' was named 'hdmi-tx' :-(
>>
>> I can make a binding proposal for 3, but I have no hardware to test it,
>> so I think it is better to add this only when someone has hardware. It
>> will require quite a few changes to the driver and likely also the CEC core.
> 
> Can't we simply add a property to override the HW config fields in this case ?
> It will be then usable with any feature the is enabled by reading the config
> bits like AHB Audio, I2c, CEC, ... and maybe many more in the future.

Yes, we can do that as well. But is that a proper description of the hardware?
The HW *does* support CEC, but in this case the CEC line is just not hooked
up to anything. So overriding the HW config field doesn't really sound right
to me.

I still believe that the cec-disable property in order to describe situation
2 in the list above is the right approach.

Perhaps giving it a different name (cec-not-connected?) helps overcome Rob's
objections? Rob?

Sorry for not following up on this earlier.

Regards,

	Hans

> 
> Neil
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>>>
>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

[PATCHv2 2/3] drm: bridge: dw-hdmi: check the cec-disable property

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

If the cec-disable property was set, then disable the DW CEC
controller. This is needed for boards that have their own
CEC controller.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index a38db40ce990..597220e40541 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2508,7 +2508,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		hdmi->audio = platform_device_register_full(&pdevinfo);
 	}
 
-	if (config0 & HDMI_CONFIG0_CEC) {
+	if ((config0 & HDMI_CONFIG0_CEC) &&
+	    !of_property_read_bool(np, "cec-disable")) {
 		cec.hdmi = hdmi;
 		cec.ops = &dw_hdmi_cec_ops;
 		cec.irq = irq;
-- 
2.15.1

[PATCHv2 3/3] arm64: dts: meson-gx.dtsi: add cec-disable

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The meson-gx boards have two CEC controllers: the DesignWare controller
and a meson-specific controller. Disable the DW controller since the
CEC line is hooked up to the meson controller.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 4ee2e7951482..4e2567012335 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -544,6 +544,7 @@
 			interrupts = <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			cec-disable;
 			status = "disabled";
 
 			/* VPU VENC Input */
-- 
2.15.1

[PATCHv2 0/3] dw-hdmi: add property to disable CEC

[Martin Blumenstingl]

Hello Hans, Hi Neil,

(apologies in advance if any of this is wrong, I don't have any CEC
capable TV so I can't test it)

On Fri, Mar 23, 2018 at 1:59 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Some boards (amlogic) have two CEC controllers: the DesignWare controller
> and their own CEC controller (meson ao-cec).
as far as I understand the Amlogic Meson SoCs have two domains:
- AO (always-on, powered even in suspend mode) where meson-ao-cec can
wake up the system from suspend
- EE (everything else, not powered during suspend) where dw-hdmi-cec lives

this far everything is OK

> Since the CEC line is not hooked up to the DW controller we need a way
> to disable that controller. This patch series adds the cec-disable
> property for that purpose.
drivers/pinctrl/meson/pinctrl-meson-gxbb.c has ao_cec_pins and
ee_cec_pins, both use GPIOAO_12
drivers/pinctrl/meson/pinctrl-meson-gxl.c has ao_cec_pins and
ee_cec_pins, both use GPIOAO_8

@Neil: do you know if the CEC signal routing is:
ao_cec_pins -> meson-ao-cec
ee_cec_pins -> dw-hdmi-cec

I'm curious because if both CEC controllers can be used then it might
be worth mentioning this in the cover-letter and patch description


Regards
Martin

[PATCHv2 0/3] dw-hdmi: add property to disable CEC

[Neil Armstrong]

Hi Martin,

> Le 24 mars 2018 ? 12:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> a ?crit :
> 
> Hello Hans, Hi Neil,
> 
> (apologies in advance if any of this is wrong, I don't have any CEC
> capable TV so I can't test it)
> 
> On Fri, Mar 23, 2018 at 1:59 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>> 
>> Some boards (amlogic) have two CEC controllers: the DesignWare controller
>> and their own CEC controller (meson ao-cec).
> as far as I understand the Amlogic Meson SoCs have two domains:
> - AO (always-on, powered even in suspend mode) where meson-ao-cec can
> wake up the system from suspend
> - EE (everything else, not powered during suspend) where dw-hdmi-cec lives
> 

Exact, except ? the EE CEC is not hooked to the DW-HDMI TX but the RX, and thus cannot be used on GXBB/GXL/GXM.

> this far everything is OK
> 
>> Since the CEC line is not hooked up to the DW controller we need a way
>> to disable that controller. This patch series adds the cec-disable
>> property for that purpose.
> drivers/pinctrl/meson/pinctrl-meson-gxbb.c has ao_cec_pins and
> ee_cec_pins, both use GPIOAO_12
> drivers/pinctrl/meson/pinctrl-meson-gxl.c has ao_cec_pins and
> ee_cec_pins, both use GPIOAO_8
> 
> @Neil: do you know if the CEC signal routing is:
> ao_cec_pins -> meson-ao-cec
> ee_cec_pins -> dw-hdmi-cec

It?s hooked to the DW-HDMI RX IP used in the TV SoCs.

> 
> I'm curious because if both CEC controllers can be used then it might
> be worth mentioning this in the cover-letter and patch description
> 

Initially I thought it was hooked to the DW-HDMI TX, but no, I guess I should remove the ee_cec pinmux?

Neil

> 
> Regards
> Martin

[PATCHv2 0/3] dw-hdmi: add property to disable CEC

[Martin Blumenstingl]

Hi Neil,

On Sat, Mar 24, 2018 at 2:41 PM, Neil Armstrong <narmstrong@baylibre.com> wrote:
> Hi Martin,
>
>> Le 24 mars 2018 ? 12:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> a ?crit :
>>
>> Hello Hans, Hi Neil,
>>
>> (apologies in advance if any of this is wrong, I don't have any CEC
>> capable TV so I can't test it)
>>
>> On Fri, Mar 23, 2018 at 1:59 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> Some boards (amlogic) have two CEC controllers: the DesignWare controller
>>> and their own CEC controller (meson ao-cec).
>> as far as I understand the Amlogic Meson SoCs have two domains:
>> - AO (always-on, powered even in suspend mode) where meson-ao-cec can
>> wake up the system from suspend
>> - EE (everything else, not powered during suspend) where dw-hdmi-cec lives
>>
>
> Exact, except ? the EE CEC is not hooked to the DW-HDMI TX but the RX, and thus cannot be used on GXBB/GXL/GXM.
I see, thank you for the explanation

>> this far everything is OK
>>
>>> Since the CEC line is not hooked up to the DW controller we need a way
>>> to disable that controller. This patch series adds the cec-disable
>>> property for that purpose.
>> drivers/pinctrl/meson/pinctrl-meson-gxbb.c has ao_cec_pins and
>> ee_cec_pins, both use GPIOAO_12
>> drivers/pinctrl/meson/pinctrl-meson-gxl.c has ao_cec_pins and
>> ee_cec_pins, both use GPIOAO_8
>>
>> @Neil: do you know if the CEC signal routing is:
>> ao_cec_pins -> meson-ao-cec
>> ee_cec_pins -> dw-hdmi-cec
>
> It?s hooked to the DW-HDMI RX IP used in the TV SoCs.
>
>>
>> I'm curious because if both CEC controllers can be used then it might
>> be worth mentioning this in the cover-letter and patch description
>>
>
> Initially I thought it was hooked to the DW-HDMI TX, but no, I guess I should remove the ee_cec pinmux?
right, or rename it to ee_cec_rx (or something similar)


Regards
Martin