Hi,
Here's a series introducing the CEC support for the BCM2711 found on the
RaspberryPi4.
The BCM2711 HDMI controller uses a similar layout for the CEC registers, the
main difference being that the interrupt handling part is now shared between
both HDMI controllers.
This series is mainly about fixing a couple of bugs, reworking the driver to
support having two different interrupts, one for each direction, provided by an
external irqchip, and enables the irqchip driver for the controller we have.
This has been tested on an RPi3 and RPi4, but requires the latest firmware.
It's is based on the 10 and 12 bpc series.
Here is the cec-compliance output:
pi@raspberrypi:~$ cec-ctl --tuner -p 1.0.0.0
The CEC adapter doesn't allow setting the physical address manually, ignore this option.
Driver Info:
Driver Name : vc4_hdmi
Adapter Name : vc4
Capabilities : 0x0000010e
Logical Addresses
Transmit
Passthrough
Driver version : 5.10.0
Available Logical Addresses: 1
Physical Address : 1.0.0.0
Logical Address Mask : 0x0008
CEC Version : 2.0
Vendor ID : 0x000c03 (HDMI)
OSD Name : Tuner
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : 3 (Tuner 1)
Primary Device Type : Tuner
Logical Address Type : Tuner
All Device Types : Tuner
RC TV Profile : None
Device Features :
None
pi@raspberrypi:~$ cec-compliance
cec-compliance SHA : not available
Driver Info:
Driver Name : vc4_hdmi
Adapter Name : vc4
Capabilities : 0x0000010e
Logical Addresses
Transmit
Passthrough
Driver version : 5.10.0
Available Logical Addresses: 1
Physical Address : 1.0.0.0
Logical Address Mask : 0x0008
CEC Version : 2.0
Vendor ID : 0x000c03 (HDMI)
OSD Name : Tuner
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : 3 (Tuner 1)
Primary Device Type : Tuner
Logical Address Type : Tuner
All Device Types : Tuner
RC TV Profile : None
Device Features :
None
Compliance test for device /dev/cec0:
The test results mean the following:
OK Supported correctly by the device.
OK (Not Supported) Not supported and not mandatory for the device.
OK (Presumed) Presumably supported. Manually check to confirm.
OK (Unexpected) Supported correctly but is not expected to be supported for this device.
OK (Refused) Supported by the device, but was refused.
FAIL Failed and was expected to be supported by this device.
Find remote devices:
Polling: OK
Network topology:
System Information for device 0 (TV) from device 3 (Tuner 1):
CEC Version : 2.0
Physical Address : 0.0.0.0
Primary Device Type : TV
Vendor ID : 0x000c03
OSD Name : 'TV '
Power Status : Tx, OK, Rx, OK, Feature Abort
Total: 1, Succeeded: 1, Failed: 0, Warnings: 0
pi@raspberrypi:~$ cec-ctl -d1 --tuner -p 1.0.0.0
The CEC adapter doesn't allow setting the physical address manually, ignore this option.
Driver Info:
Driver Name : vc4_hdmi
Adapter Name : vc4
Capabilities : 0x0000010e
Logical Addresses
Transmit
Passthrough
Driver version : 5.10.0
Available Logical Addresses: 1
Physical Address : 1.0.0.0
Logical Address Mask : 0x0008
CEC Version : 2.0
Vendor ID : 0x000c03 (HDMI)
OSD Name : Tuner
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : 3 (Tuner 1)
Primary Device Type : Tuner
Logical Address Type : Tuner
All Device Types : Tuner
RC TV Profile : None
Device Features :
None
pi@raspberrypi:~$ cec-compliance -d1
cec-compliance SHA : not available
Driver Info:
Driver Name : vc4_hdmi
Adapter Name : vc4
Capabilities : 0x0000010e
Logical Addresses
Transmit
Passthrough
Driver version : 5.10.0
Available Logical Addresses: 1
Physical Address : 1.0.0.0
Logical Address Mask : 0x0008
CEC Version : 2.0
Vendor ID : 0x000c03 (HDMI)
OSD Name : Tuner
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : 3 (Tuner 1)
Primary Device Type : Tuner
Logical Address Type : Tuner
All Device Types : Tuner
RC TV Profile : None
Device Features :
None
Compliance test for device /dev/cec1:
The test results mean the following:
OK Supported correctly by the device.
OK (Not Supported) Not supported and not mandatory for the device.
OK (Presumed) Presumably supported. Manually check to confirm.
OK (Unexpected) Supported correctly but is not expected to be supported for this device.
OK (Refused) Supported by the device, but was refused.
FAIL Failed and was expected to be supported by this device.
Find remote devices:
Polling: OK
Network topology:
System Information for device 0 (TV) from device 3 (Tuner 1):
CEC Version : 2.0
Physical Address : 0.0.0.0
Primary Device Type : TV
Vendor ID : 0x000c03
OSD Name : 'TV '
Power Status : Tx, OK, Rx, OK, Feature Abort
Total: 1, Succeeded: 1, Failed: 0, Warnings: 0
And for the hotplug detect test:
pi@raspberrypi:~$ cec-ctl --playback
Driver Info:
Driver Name : vc4_hdmi
Adapter Name : vc4
Capabilities : 0x0000010e
Logical Addresses
Transmit
Passthrough
Driver version : 5.10.0
Available Logical Addresses: 1
Physical Address : f.f.f.f
Logical Address Mask : 0x0000
CEC Version : 2.0
Vendor ID : 0x000c03 (HDMI)
OSD Name : Playback
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : Not Allocated
Primary Device Type : Playback
Logical Address Type : Playback
All Device Types : Playback
RC TV Profile : None
Device Features :
None
pi@raspberrypi:~$ cec-ctl -t0 --image-view-on
Driver Info:
Driver Name : vc4_hdmi
Adapter Name : vc4
Capabilities : 0x0000010e
Logical Addresses
Transmit
Passthrough
Driver version : 5.10.0
Available Logical Addresses: 1
Physical Address : f.f.f.f
Logical Address Mask : 0x0000
CEC Version : 2.0
Vendor ID : 0x000c03 (HDMI)
OSD Name : Playback
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : Not Allocated
Primary Device Type : Playback
Logical Address Type : Playback
All Device Types : Playback
RC TV Profile : None
Device Features :
None
Transmit from Unregistered to TV (15 to 0):
CEC_MSG_IMAGE_VIEW_ON (0x04)
Sequence: 1 Tx Timestamp: 9182.611s
pi@raspberrypi:~$ cec-ctl -d1 --playback
Driver Info:
Driver Name : vc4_hdmi
Adapter Name : vc4
Capabilities : 0x0000010e
Logical Addresses
Transmit
Passthrough
Driver version : 5.10.0
Available Logical Addresses: 1
Physical Address : f.f.f.f
Logical Address Mask : 0x0000
CEC Version : 2.0
Vendor ID : 0x000c03 (HDMI)
OSD Name : Playback
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : Not Allocated
Primary Device Type : Playback
Logical Address Type : Playback
All Device Types : Playback
RC TV Profile : None
Device Features :
None
pi@raspberrypi:~$ cec-ctl -d1 -t0 --image-view-on
Driver Info:
Driver Name : vc4_hdmi
Adapter Name : vc4
Capabilities : 0x0000010e
Logical Addresses
Transmit
Passthrough
Driver version : 5.10.0
Available Logical Addresses: 1
Physical Address : f.f.f.f
Logical Address Mask : 0x0000
CEC Version : 2.0
Vendor ID : 0x000c03 (HDMI)
OSD Name : Playback
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : Not Allocated
Primary Device Type : Playback
Logical Address Type : Playback
All Device Types : Playback
RC TV Profile : None
Device Features :
None
Transmit from Unregistered to TV (15 to 0):
CEC_MSG_IMAGE_VIEW_ON (0x04)
Sequence: 1 Tx Timestamp: 9207.191s
With the pulse-eight side reporting:
$ sudo cec-ctl -M
Driver Info:
Driver Name : pulse8-cec
Adapter Name : serio0
Capabilities : 0x0000003f
Physical Address
Logical Addresses
Transmit
Passthrough
Remote Control Support
Monitor All
Driver version : 5.9.16
Available Logical Addresses: 1
Connector Info : None
Physical Address : 0.0.0.0
Logical Address Mask : 0x0001
CEC Version : 2.0
Vendor ID : 0x000c03 (HDMI)
OSD Name : 'TV '
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : 0 (TV)
Primary Device Type : TV
Logical Address Type : TV
All Device Types : TV
RC TV Profile : None
Device Features :
None
Initial Event: State Change: PA: 0.0.0.0, LA mask: 0x0001, Conn Info: no
Received from Unregistered to TV (15 to 0): IMAGE_VIEW_ON (0x04)
Received from Unregistered to TV (15 to 0): IMAGE_VIEW_ON (0x04)
Let me know what you think,
Maxime
Changes from v1:
- Removed the irqchip patch in favor of a select in mach-bcm
- Fixed HDMI1 interrupt numbers
- Removed redundant call to drm_connector_update_edid_property
- Fixed the condition in vc4_hdmi_connector_detect
- Added the tags
- Rebased on top of drm-misc-next-2021-01-06
Dom Cobley (5):
drm/vc4: hdmi: Move hdmi reset to bind
drm/vc4: hdmi: Fix register offset with longer CEC messages
drm/vc4: hdmi: Fix up CEC registers
drm/vc4: hdmi: Restore cec physical address on reconnect
drm/vc4: hdmi: Remove cec_available flag
Maxime Ripard (10):
ARM: bcm: Select BRCMSTB_L2_IRQ for bcm2835
drm/vc4: hdmi: Compute the CEC clock divider from the clock rate
drm/vc4: hdmi: Update the CEC clock divider on HSM rate change
drm/vc4: hdmi: Introduce a CEC clock
drm/vc4: hdmi: Split the interrupt handlers
drm/vc4: hdmi: Support BCM2711 CEC interrupt setup
drm/vc4: hdmi: Don't register the CEC adapter if there's no interrupts
dt-binding: display: bcm2711-hdmi: Add CEC and hotplug interrupts
ARM: dts: bcm2711: Add the BSC interrupt controller
ARM: dts: bcm2711: Add the CEC interrupt controller
.../bindings/display/brcm,bcm2711-hdmi.yaml | 20 +-
arch/arm/boot/dts/bcm2711.dtsi | 30 +++
arch/arm/mach-bcm/Kconfig | 1 +
arch/arm64/Kconfig.platforms | 1 +
drivers/gpu/drm/vc4/vc4_hdmi.c | 223 ++++++++++++++----
drivers/gpu/drm/vc4/vc4_hdmi.h | 11 +-
drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 4 +-
7 files changed, 234 insertions(+), 56 deletions(-)
--
2.29.2
From: Dom Cobley <[email protected]>
The commit 311e305fdb4e ("drm/vc4: hdmi: Implement a register layout
abstraction") forgot one CEC register, and made a copy and paste mistake
for another one. Fix those mistakes.
Fixes: 311e305fdb4e ("drm/vc4: hdmi: Implement a register layout abstraction")
Reviewed-by: Dave Stevenson <[email protected]>
Signed-off-by: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
index 401863cb8c98..e1b58eac766f 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
@@ -29,6 +29,7 @@ enum vc4_hdmi_field {
HDMI_CEC_CPU_MASK_SET,
HDMI_CEC_CPU_MASK_STATUS,
HDMI_CEC_CPU_STATUS,
+ HDMI_CEC_CPU_SET,
/*
* Transmit data, first byte is low byte of the 32-bit reg.
@@ -199,9 +200,10 @@ static const struct vc4_hdmi_register __maybe_unused vc4_hdmi_fields[] = {
VC4_HDMI_REG(HDMI_TX_PHY_RESET_CTL, 0x02c0),
VC4_HDMI_REG(HDMI_TX_PHY_CTL_0, 0x02c4),
VC4_HDMI_REG(HDMI_CEC_CPU_STATUS, 0x0340),
+ VC4_HDMI_REG(HDMI_CEC_CPU_SET, 0x0344),
VC4_HDMI_REG(HDMI_CEC_CPU_CLEAR, 0x0348),
VC4_HDMI_REG(HDMI_CEC_CPU_MASK_STATUS, 0x034c),
- VC4_HDMI_REG(HDMI_CEC_CPU_MASK_SET, 0x034c),
+ VC4_HDMI_REG(HDMI_CEC_CPU_MASK_SET, 0x0350),
VC4_HDMI_REG(HDMI_CEC_CPU_MASK_CLEAR, 0x0354),
VC4_HDMI_REG(HDMI_RAM_PACKET_START, 0x0400),
};
--
2.29.2
From: Dom Cobley <[email protected]>
The code prior to 311e305fdb4e ("drm/vc4: hdmi: Implement a register
layout abstraction") was relying on the fact that the register offset
was incremented by 4 for each readl call. That worked since the register
width is 4 bytes.
However, since that commit the HDMI_READ macro is now taking an enum,
and the offset doesn't increment by 4 but 1 now. Divide the index by 4
to fix this.
Fixes: 311e305fdb4e ("drm/vc4: hdmi: Implement a register layout abstraction")
Reviewed-by: Dave Stevenson <[email protected]>
Signed-off-by: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index cb4594e7df6f..7945dbcee78c 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1443,13 +1443,20 @@ static irqreturn_t vc4_cec_irq_handler_thread(int irq, void *priv)
static void vc4_cec_read_msg(struct vc4_hdmi *vc4_hdmi, u32 cntrl1)
{
+ struct drm_device *dev = vc4_hdmi->connector.dev;
struct cec_msg *msg = &vc4_hdmi->cec_rx_msg;
unsigned int i;
msg->len = 1 + ((cntrl1 & VC4_HDMI_CEC_REC_WRD_CNT_MASK) >>
VC4_HDMI_CEC_REC_WRD_CNT_SHIFT);
+
+ if (msg->len > 16) {
+ drm_err(dev, "Attempting to read too much data (%d)\n", msg->len);
+ return;
+ }
+
for (i = 0; i < msg->len; i += 4) {
- u32 val = HDMI_READ(HDMI_CEC_RX_DATA_1 + i);
+ u32 val = HDMI_READ(HDMI_CEC_RX_DATA_1 + (i >> 2));
msg->msg[i] = val & 0xff;
msg->msg[i + 1] = (val >> 8) & 0xff;
@@ -1542,11 +1549,17 @@ static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
u32 signal_free_time, struct cec_msg *msg)
{
struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+ struct drm_device *dev = vc4_hdmi->connector.dev;
u32 val;
unsigned int i;
+ if (msg->len > 16) {
+ drm_err(dev, "Attempting to transmit too much data (%d)\n", msg->len);
+ return -ENOMEM;
+ }
+
for (i = 0; i < msg->len; i += 4)
- HDMI_WRITE(HDMI_CEC_TX_DATA_1 + i,
+ HDMI_WRITE(HDMI_CEC_TX_DATA_1 + (i >> 2),
(msg->msg[i]) |
(msg->msg[i + 1] << 8) |
(msg->msg[i + 2] << 16) |
--
2.29.2
The BCM2711 has two different interrupt sources to transmit and receive
CEC messages, provided through an external interrupt chip shared between
the two HDMI interrupt controllers.
The rest of the CEC controller is identical though so we need to change
a bit the code organisation to share the code as much as possible, yet
still allowing to register independant handlers.
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 86 +++++++++++++++++++++++++---------
1 file changed, 65 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 7b5c92df8f1b..12ca5f3084af 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1454,15 +1454,22 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
}
#ifdef CONFIG_DRM_VC4_HDMI_CEC
-static irqreturn_t vc4_cec_irq_handler_thread(int irq, void *priv)
+static irqreturn_t vc4_cec_irq_handler_rx_thread(int irq, void *priv)
{
struct vc4_hdmi *vc4_hdmi = priv;
- if (vc4_hdmi->cec_irq_was_rx) {
- if (vc4_hdmi->cec_rx_msg.len)
- cec_received_msg(vc4_hdmi->cec_adap,
- &vc4_hdmi->cec_rx_msg);
- } else if (vc4_hdmi->cec_tx_ok) {
+ if (vc4_hdmi->cec_rx_msg.len)
+ cec_received_msg(vc4_hdmi->cec_adap,
+ &vc4_hdmi->cec_rx_msg);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t vc4_cec_irq_handler_tx_thread(int irq, void *priv)
+{
+ struct vc4_hdmi *vc4_hdmi = priv;
+
+ if (vc4_hdmi->cec_tx_ok) {
cec_transmit_done(vc4_hdmi->cec_adap, CEC_TX_STATUS_OK,
0, 0, 0, 0);
} else {
@@ -1476,6 +1483,19 @@ static irqreturn_t vc4_cec_irq_handler_thread(int irq, void *priv)
return IRQ_HANDLED;
}
+static irqreturn_t vc4_cec_irq_handler_thread(int irq, void *priv)
+{
+ struct vc4_hdmi *vc4_hdmi = priv;
+ irqreturn_t ret;
+
+ if (vc4_hdmi->cec_irq_was_rx)
+ ret = vc4_cec_irq_handler_rx_thread(irq, priv);
+ else
+ ret = vc4_cec_irq_handler_tx_thread(irq, priv);
+
+ return ret;
+}
+
static void vc4_cec_read_msg(struct vc4_hdmi *vc4_hdmi, u32 cntrl1)
{
struct drm_device *dev = vc4_hdmi->connector.dev;
@@ -1500,31 +1520,55 @@ static void vc4_cec_read_msg(struct vc4_hdmi *vc4_hdmi, u32 cntrl1)
}
}
+static irqreturn_t vc4_cec_irq_handler_tx_bare(int irq, void *priv)
+{
+ struct vc4_hdmi *vc4_hdmi = priv;
+ u32 cntrl1;
+
+ cntrl1 = HDMI_READ(HDMI_CEC_CNTRL_1);
+ vc4_hdmi->cec_tx_ok = cntrl1 & VC4_HDMI_CEC_TX_STATUS_GOOD;
+ cntrl1 &= ~VC4_HDMI_CEC_START_XMIT_BEGIN;
+ HDMI_WRITE(HDMI_CEC_CNTRL_1, cntrl1);
+
+ return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t vc4_cec_irq_handler_rx_bare(int irq, void *priv)
+{
+ struct vc4_hdmi *vc4_hdmi = priv;
+ u32 cntrl1;
+
+ vc4_hdmi->cec_rx_msg.len = 0;
+ cntrl1 = HDMI_READ(HDMI_CEC_CNTRL_1);
+ vc4_cec_read_msg(vc4_hdmi, cntrl1);
+ cntrl1 |= VC4_HDMI_CEC_CLEAR_RECEIVE_OFF;
+ HDMI_WRITE(HDMI_CEC_CNTRL_1, cntrl1);
+ cntrl1 &= ~VC4_HDMI_CEC_CLEAR_RECEIVE_OFF;
+
+ HDMI_WRITE(HDMI_CEC_CNTRL_1, cntrl1);
+
+ return IRQ_WAKE_THREAD;
+}
+
static irqreturn_t vc4_cec_irq_handler(int irq, void *priv)
{
struct vc4_hdmi *vc4_hdmi = priv;
u32 stat = HDMI_READ(HDMI_CEC_CPU_STATUS);
- u32 cntrl1, cntrl5;
+ irqreturn_t ret;
+ u32 cntrl5;
if (!(stat & VC4_HDMI_CPU_CEC))
return IRQ_NONE;
- vc4_hdmi->cec_rx_msg.len = 0;
- cntrl1 = HDMI_READ(HDMI_CEC_CNTRL_1);
+
cntrl5 = HDMI_READ(HDMI_CEC_CNTRL_5);
vc4_hdmi->cec_irq_was_rx = cntrl5 & VC4_HDMI_CEC_RX_CEC_INT;
- if (vc4_hdmi->cec_irq_was_rx) {
- vc4_cec_read_msg(vc4_hdmi, cntrl1);
- cntrl1 |= VC4_HDMI_CEC_CLEAR_RECEIVE_OFF;
- HDMI_WRITE(HDMI_CEC_CNTRL_1, cntrl1);
- cntrl1 &= ~VC4_HDMI_CEC_CLEAR_RECEIVE_OFF;
- } else {
- vc4_hdmi->cec_tx_ok = cntrl1 & VC4_HDMI_CEC_TX_STATUS_GOOD;
- cntrl1 &= ~VC4_HDMI_CEC_START_XMIT_BEGIN;
- }
- HDMI_WRITE(HDMI_CEC_CNTRL_1, cntrl1);
+ if (vc4_hdmi->cec_irq_was_rx)
+ ret = vc4_cec_irq_handler_rx_bare(irq, priv);
+ else
+ ret = vc4_cec_irq_handler_tx_bare(irq, priv);
+
HDMI_WRITE(HDMI_CEC_CPU_CLEAR, VC4_HDMI_CPU_CEC);
-
- return IRQ_WAKE_THREAD;
+ return ret;
}
static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
--
2.29.2
The CEC and hotplug interrupts were missing when that binding was
introduced, let's add them in now that we've figured out how it works.
Signed-off-by: Maxime Ripard <[email protected]>
---
.../bindings/display/brcm,bcm2711-hdmi.yaml | 20 ++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
index 7ce06f9f9f8e..6e8ac910bdd8 100644
--- a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
+++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
@@ -53,6 +53,24 @@ properties:
- const: audio
- const: cec
+ interrupts:
+ items:
+ - description: CEC TX interrupt
+ - description: CEC RX interrupt
+ - description: CEC stuck at low interrupt
+ - description: Wake-up interrupt
+ - description: Hotplug connected interrupt
+ - description: Hotplug removed interrupt
+
+ interrupt-names:
+ items:
+ - const: cec-tx
+ - const: cec-rx
+ - const: cec-low
+ - const: wakeup
+ - const: hpd-connected
+ - const: hpd-removed
+
ddc:
allOf:
- $ref: /schemas/types.yaml#/definitions/phandle
@@ -90,7 +108,7 @@ required:
- resets
- ddc
-additionalProperties: false
+unevaluatedProperties: false
examples:
- |
--
2.29.2
The BSC controllers used for the HDMI DDC have an interrupt controller
shared between both instances. Let's add it to avoid polling.
Reviewed-by: Florian Fainelli <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
arch/arm/boot/dts/bcm2711.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index 4847dd305317..8bb46ae76a92 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -308,6 +308,14 @@ dvp: clock@7ef00000 {
#reset-cells = <1>;
};
+ bsc_intr: interrupt-controller@7ef00040 {
+ compatible = "brcm,bcm2711-l2-intc", "brcm,l2-intc";
+ reg = <0x7ef00040 0x30>;
+ interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
+
hdmi0: hdmi@7ef00700 {
compatible = "brcm,bcm2711-hdmi0";
reg = <0x7ef00700 0x300>,
@@ -341,6 +349,8 @@ ddc0: i2c@7ef04500 {
reg = <0x7ef04500 0x100>, <0x7ef00b00 0x300>;
reg-names = "bsc", "auto-i2c";
clock-frequency = <97500>;
+ interrupt-parent = <&bsc_intr>;
+ interrupts = <0>;
status = "disabled";
};
@@ -377,6 +387,8 @@ ddc1: i2c@7ef09500 {
reg = <0x7ef09500 0x100>, <0x7ef05b00 0x300>;
reg-names = "bsc", "auto-i2c";
clock-frequency = <97500>;
+ interrupt-parent = <&bsc_intr>;
+ interrupts = <1>;
status = "disabled";
};
};
--
2.29.2
We introduced the BCM2711 support to the vc4 HDMI controller with 5.10,
but this was lacking any of the interrupts of the CEC controller so we
have to deal with the backward compatibility.
Do so by simply ignoring the CEC setup if the DT doesn't have the
interrupts property.
Reviewed-by: Dave Stevenson <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 5a606b6f2917..eee9751009c2 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1667,9 +1667,15 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
{
struct cec_connector_info conn_info;
struct platform_device *pdev = vc4_hdmi->pdev;
+ struct device *dev = &pdev->dev;
u32 value;
int ret;
+ if (!of_find_property(dev->of_node, "interrupts", NULL)) {
+ dev_warn(dev, "'interrupts' DT property is missing, no CEC\n");
+ return 0;
+ }
+
vc4_hdmi->cec_adap = cec_allocate_adapter(&vc4_hdmi_cec_adap_ops,
vc4_hdmi, "vc4",
CEC_CAP_DEFAULTS |
--
2.29.2
While the BCM2835 had the CEC clock derived from the HSM clock, the
BCM2711 has a dedicated parent clock for it.
Let's introduce a separate clock for it so that we can handle both
cases.
Reviewed-by: Dave Stevenson <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 9 ++++++++-
drivers/gpu/drm/vc4/vc4_hdmi.h | 1 +
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 0627b8db32f6..7b5c92df8f1b 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -145,7 +145,7 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi)
* Set the clock divider: the hsm_clock rate and this divider
* setting will give a 40 kHz CEC clock.
*/
- clk_cnt = clk_get_rate(vc4_hdmi->hsm_clock) / CEC_CLOCK_FREQ;
+ clk_cnt = clk_get_rate(vc4_hdmi->cec_clock) / CEC_CLOCK_FREQ;
value |= clk_cnt << VC4_HDMI_CEC_DIV_CLK_CNT_SHIFT;
HDMI_WRITE(HDMI_CEC_CNTRL_1, value);
}
@@ -1752,6 +1752,7 @@ static int vc4_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi)
return PTR_ERR(vc4_hdmi->hsm_clock);
}
vc4_hdmi->audio_clock = vc4_hdmi->hsm_clock;
+ vc4_hdmi->cec_clock = vc4_hdmi->hsm_clock;
return 0;
}
@@ -1845,6 +1846,12 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi)
return PTR_ERR(vc4_hdmi->audio_clock);
}
+ vc4_hdmi->cec_clock = devm_clk_get(dev, "cec");
+ if (IS_ERR(vc4_hdmi->cec_clock)) {
+ DRM_ERROR("Failed to get CEC clock\n");
+ return PTR_ERR(vc4_hdmi->cec_clock);
+ }
+
vc4_hdmi->reset = devm_reset_control_get(dev, NULL);
if (IS_ERR(vc4_hdmi->reset)) {
DRM_ERROR("Failed to get HDMI reset line\n");
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 4c8994cfd932..6966db1a0957 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -155,6 +155,7 @@ struct vc4_hdmi {
bool cec_tx_ok;
bool cec_irq_was_rx;
+ struct clk *cec_clock;
struct clk *pixel_clock;
struct clk *hsm_clock;
struct clk *audio_clock;
--
2.29.2
From: Dom Cobley <[email protected]>
Now that our HDMI controller supports CEC for the BCM2711, let's remove
that flag.
Reviewed-by: Dave Stevenson <[email protected]>
Signed-off-by: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 4 ----
drivers/gpu/drm/vc4/vc4_hdmi.h | 3 ---
2 files changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index d116ecfd8cf7..5a606b6f2917 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1670,9 +1670,6 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
u32 value;
int ret;
- if (!vc4_hdmi->variant->cec_available)
- return 0;
-
vc4_hdmi->cec_adap = cec_allocate_adapter(&vc4_hdmi_cec_adap_ops,
vc4_hdmi, "vc4",
CEC_CAP_DEFAULTS |
@@ -2086,7 +2083,6 @@ static const struct vc4_hdmi_variant bcm2835_variant = {
.debugfs_name = "hdmi_regs",
.card_name = "vc4-hdmi",
.max_pixel_clock = 162000000,
- .cec_available = true,
.registers = vc4_hdmi_fields,
.num_registers = ARRAY_SIZE(vc4_hdmi_fields),
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index d71f6ed321bf..3cebd1fd00fc 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -42,9 +42,6 @@ struct vc4_hdmi_variant {
/* Filename to expose the registers in debugfs */
const char *debugfs_name;
- /* Set to true when the CEC support is available */
- bool cec_available;
-
/* Maximum pixel clock supported by the controller (in Hz) */
unsigned long long max_pixel_clock;
--
2.29.2
The CEC clock divider needs to output a frequency of 40kHz from the HSM
rate on the BCM2835. The driver used to have a fixed frequency for it,
but that changed for the BCM2711 and we now need to compute it
dynamically to maintain the proper rate.
Fixes: cd4cb49dc5bb ("drm/vc4: hdmi: Adjust HSM clock rate depending on pixel rate")
Reviewed-by: Dave Stevenson <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index c3a301396aad..50008513edfc 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1598,6 +1598,7 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
{
struct cec_connector_info conn_info;
struct platform_device *pdev = vc4_hdmi->pdev;
+ u16 clk_cnt;
u32 value;
int ret;
@@ -1623,8 +1624,9 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
* divider: the hsm_clock rate and this divider setting will
* give a 40 kHz CEC clock.
*/
+ clk_cnt = clk_get_rate(vc4_hdmi->hsm_clock) / CEC_CLOCK_FREQ;
value |= VC4_HDMI_CEC_ADDR_MASK |
- (4091 << VC4_HDMI_CEC_DIV_CLK_CNT_SHIFT);
+ (clk_cnt << VC4_HDMI_CEC_DIV_CLK_CNT_SHIFT);
HDMI_WRITE(HDMI_CEC_CNTRL_1, value);
ret = devm_request_threaded_irq(&pdev->dev, platform_get_irq(pdev, 0),
vc4_cec_irq_handler,
--
2.29.2
From: Dom Cobley <[email protected]>
Currently we call cec_phys_addr_invalidate on a hotplug deassert.
That may be due to a TV power cycling, or an AVR being switched
on (and switching edid).
This makes CEC unusable since our controller wouldn't have a physical
address anymore.
Set it back up again on the hotplug assert.
Fixes: 15b4511a4af6 ("drm/vc4: add HDMI CEC support")
Signed-off-by: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 7945dbcee78c..c3a301396aad 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -136,20 +136,32 @@ static enum drm_connector_status
vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
+ bool connected = false;
if (vc4_hdmi->hpd_gpio) {
if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^
vc4_hdmi->hpd_active_low)
- return connector_status_connected;
- cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
- return connector_status_disconnected;
+ connected = true;
+ } else if (drm_probe_ddc(vc4_hdmi->ddc)) {
+ connected = true;
+ } else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) {
+ connected = true;
}
- if (drm_probe_ddc(vc4_hdmi->ddc))
- return connector_status_connected;
+ if (connected) {
+ if (connector->status != connector_status_connected) {
+ struct edid *edid = drm_get_edid(connector, vc4_hdmi->ddc);
+
+ if (edid) {
+ cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
+ vc4_hdmi->encoder.hdmi_monitor = drm_detect_hdmi_monitor(edid);
+ kfree(edid);
+ }
+ }
- if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED)
return connector_status_connected;
+ }
+
cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
return connector_status_disconnected;
}
--
2.29.2
The HDMI controller found in the BCM2711 has an external interrupt
controller for the CEC and hotplug interrupt shared between the two
instances.
Let's add a variant flag to register a single interrupt handler and
deals with the interrupt handler setup, or two interrupt handlers
relying on an external irqchip.
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 42 ++++++++++++++++++++++++++--------
drivers/gpu/drm/vc4/vc4_hdmi.h | 7 ++++++
2 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 12ca5f3084af..d116ecfd8cf7 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1605,9 +1605,11 @@ static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
((3600 / usecs) << VC4_HDMI_CEC_CNT_TO_3600_US_SHIFT) |
((3500 / usecs) << VC4_HDMI_CEC_CNT_TO_3500_US_SHIFT));
- HDMI_WRITE(HDMI_CEC_CPU_MASK_CLEAR, VC4_HDMI_CPU_CEC);
+ if (!vc4_hdmi->variant->external_irq_controller)
+ HDMI_WRITE(HDMI_CEC_CPU_MASK_CLEAR, VC4_HDMI_CPU_CEC);
} else {
- HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, VC4_HDMI_CPU_CEC);
+ if (!vc4_hdmi->variant->external_irq_controller)
+ HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, VC4_HDMI_CPU_CEC);
HDMI_WRITE(HDMI_CEC_CNTRL_5, val |
VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET);
}
@@ -1682,8 +1684,6 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
cec_fill_conn_info_from_drm(&conn_info, &vc4_hdmi->connector);
cec_s_conn_info(vc4_hdmi->cec_adap, &conn_info);
- HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, 0xffffffff);
-
value = HDMI_READ(HDMI_CEC_CNTRL_1);
/* Set the logical address to Unregistered */
value |= VC4_HDMI_CEC_ADDR_MASK;
@@ -1691,12 +1691,32 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
vc4_hdmi_cec_update_clk_div(vc4_hdmi);
- ret = devm_request_threaded_irq(&pdev->dev, platform_get_irq(pdev, 0),
- vc4_cec_irq_handler,
- vc4_cec_irq_handler_thread, 0,
- "vc4 hdmi cec", vc4_hdmi);
- if (ret)
- goto err_delete_cec_adap;
+ if (vc4_hdmi->variant->external_irq_controller) {
+ ret = devm_request_threaded_irq(&pdev->dev,
+ platform_get_irq_byname(pdev, "cec-rx"),
+ vc4_cec_irq_handler_rx_bare,
+ vc4_cec_irq_handler_rx_thread, 0,
+ "vc4 hdmi cec rx", vc4_hdmi);
+ if (ret)
+ goto err_delete_cec_adap;
+
+ ret = devm_request_threaded_irq(&pdev->dev,
+ platform_get_irq_byname(pdev, "cec-tx"),
+ vc4_cec_irq_handler_tx_bare,
+ vc4_cec_irq_handler_tx_thread, 0,
+ "vc4 hdmi cec tx", vc4_hdmi);
+ if (ret)
+ goto err_delete_cec_adap;
+ } else {
+ HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, 0xffffffff);
+
+ ret = devm_request_threaded_irq(&pdev->dev, platform_get_irq(pdev, 0),
+ vc4_cec_irq_handler,
+ vc4_cec_irq_handler_thread, 0,
+ "vc4 hdmi cec", vc4_hdmi);
+ if (ret)
+ goto err_delete_cec_adap;
+ }
ret = cec_register_adapter(vc4_hdmi->cec_adap, &pdev->dev);
if (ret < 0)
@@ -2095,6 +2115,7 @@ static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = {
PHY_LANE_CK,
},
.unsupported_odd_h_timings = true,
+ .external_irq_controller = true,
.init_resources = vc5_hdmi_init_resources,
.csc_setup = vc5_hdmi_csc_setup,
@@ -2121,6 +2142,7 @@ static const struct vc4_hdmi_variant bcm2711_hdmi1_variant = {
PHY_LANE_2,
},
.unsupported_odd_h_timings = true,
+ .external_irq_controller = true,
.init_resources = vc5_hdmi_init_resources,
.csc_setup = vc5_hdmi_csc_setup,
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 6966db1a0957..d71f6ed321bf 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -64,6 +64,13 @@ struct vc4_hdmi_variant {
/* The BCM2711 cannot deal with odd horizontal pixel timings */
bool unsupported_odd_h_timings;
+ /*
+ * The BCM2711 CEC/hotplug IRQ controller is shared between the
+ * two HDMI controllers, and we have a proper irqchip driver for
+ * it.
+ */
+ bool external_irq_controller;
+
/* Callback to get the resources (memory region, interrupts,
* clocks, etc) for that variant.
*/
--
2.29.2
The CEC and hotplug interrupts go through an interrupt controller shared
between the two HDMI controllers.
Let's add that interrupt controller and the interrupts for both HDMI
controllers
Reviewed-by: Florian Fainelli <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
arch/arm/boot/dts/bcm2711.dtsi | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index 8bb46ae76a92..06b15797ec11 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -316,6 +316,14 @@ bsc_intr: interrupt-controller@7ef00040 {
#interrupt-cells = <1>;
};
+ aon_intr: interrupt-controller@7ef00100 {
+ compatible = "brcm,bcm2711-l2-intc", "brcm,l2-intc";
+ reg = <0x7ef00100 0x30>;
+ interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
+
hdmi0: hdmi@7ef00700 {
compatible = "brcm,bcm2711-hdmi0";
reg = <0x7ef00700 0x300>,
@@ -338,6 +346,11 @@ hdmi0: hdmi@7ef00700 {
"hd";
clock-names = "hdmi", "bvb", "audio", "cec";
resets = <&dvp 0>;
+ interrupt-parent = <&aon_intr>;
+ interrupts = <0>, <1>, <2>,
+ <3>, <4>, <5>;
+ interrupt-names = "cec-tx", "cec-rx", "cec-low",
+ "wakeup", "hpd-connected", "hpd-removed";
ddc = <&ddc0>;
dmas = <&dma 10>;
dma-names = "audio-rx";
@@ -377,6 +390,11 @@ hdmi1: hdmi@7ef05700 {
ddc = <&ddc1>;
clock-names = "hdmi", "bvb", "audio", "cec";
resets = <&dvp 1>;
+ interrupt-parent = <&aon_intr>;
+ interrupts = <8>, <7>, <6>,
+ <9>, <10>, <11>;
+ interrupt-names = "cec-tx", "cec-rx", "cec-low",
+ "wakeup", "hpd-connected", "hpd-removed";
dmas = <&dma 17>;
dma-names = "audio-rx";
status = "disabled";
--
2.29.2
As part of the enable sequence we might change the HSM clock rate if the
pixel rate is different than the one we were already dealing with.
On the BCM2835 however, the CEC clock derives from the HSM clock so any
rate change will need to be reflected in the CEC clock divider to output
40kHz.
Fixes: cd4cb49dc5bb ("drm/vc4: hdmi: Adjust HSM clock rate depending on pixel rate")
Reviewed-by: Dave Stevenson <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 39 +++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 50008513edfc..0627b8db32f6 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -132,6 +132,27 @@ static void vc5_hdmi_reset(struct vc4_hdmi *vc4_hdmi)
HDMI_READ(HDMI_CLOCK_STOP) | VC4_DVP_HT_CLOCK_STOP_PIXEL);
}
+#ifdef CONFIG_DRM_VC4_HDMI_CEC
+static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi)
+{
+ u16 clk_cnt;
+ u32 value;
+
+ value = HDMI_READ(HDMI_CEC_CNTRL_1);
+ value &= ~VC4_HDMI_CEC_DIV_CLK_CNT_MASK;
+
+ /*
+ * Set the clock divider: the hsm_clock rate and this divider
+ * setting will give a 40 kHz CEC clock.
+ */
+ clk_cnt = clk_get_rate(vc4_hdmi->hsm_clock) / CEC_CLOCK_FREQ;
+ value |= clk_cnt << VC4_HDMI_CEC_DIV_CLK_CNT_SHIFT;
+ HDMI_WRITE(HDMI_CEC_CNTRL_1, value);
+}
+#else
+static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
+#endif
+
static enum drm_connector_status
vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
{
@@ -770,6 +791,8 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder,
return;
}
+ vc4_hdmi_cec_update_clk_div(vc4_hdmi);
+
/*
* FIXME: When the pixel freq is 594MHz (4k60), this needs to be setup
* at 300MHz.
@@ -1598,7 +1621,6 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
{
struct cec_connector_info conn_info;
struct platform_device *pdev = vc4_hdmi->pdev;
- u16 clk_cnt;
u32 value;
int ret;
@@ -1617,17 +1639,14 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
cec_s_conn_info(vc4_hdmi->cec_adap, &conn_info);
HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, 0xffffffff);
+
value = HDMI_READ(HDMI_CEC_CNTRL_1);
- value &= ~VC4_HDMI_CEC_DIV_CLK_CNT_MASK;
- /*
- * Set the logical address to Unregistered and set the clock
- * divider: the hsm_clock rate and this divider setting will
- * give a 40 kHz CEC clock.
- */
- clk_cnt = clk_get_rate(vc4_hdmi->hsm_clock) / CEC_CLOCK_FREQ;
- value |= VC4_HDMI_CEC_ADDR_MASK |
- (clk_cnt << VC4_HDMI_CEC_DIV_CLK_CNT_SHIFT);
+ /* Set the logical address to Unregistered */
+ value |= VC4_HDMI_CEC_ADDR_MASK;
HDMI_WRITE(HDMI_CEC_CNTRL_1, value);
+
+ vc4_hdmi_cec_update_clk_div(vc4_hdmi);
+
ret = devm_request_threaded_irq(&pdev->dev, platform_get_irq(pdev, 0),
vc4_cec_irq_handler,
vc4_cec_irq_handler_thread, 0,
--
2.29.2
Hi Maxime,
On 11/01/2021 15:22, Maxime Ripard wrote:
> Hi,
>
> Here's a series introducing the CEC support for the BCM2711 found on the
> RaspberryPi4.
>
> The BCM2711 HDMI controller uses a similar layout for the CEC registers, the
> main difference being that the interrupt handling part is now shared between
> both HDMI controllers.
>
> This series is mainly about fixing a couple of bugs, reworking the driver to
> support having two different interrupts, one for each direction, provided by an
> external irqchip, and enables the irqchip driver for the controller we have.
>
> This has been tested on an RPi3 and RPi4, but requires the latest firmware.
> It's is based on the 10 and 12 bpc series.
Thank you for this series, I plan to test this later this week.
Regards,
Hans
>
> Here is the cec-compliance output:
>
> pi@raspberrypi:~$ cec-ctl --tuner -p 1.0.0.0
> The CEC adapter doesn't allow setting the physical address manually, ignore this option.
>
> Driver Info:
> Driver Name : vc4_hdmi
> Adapter Name : vc4
> Capabilities : 0x0000010e
> Logical Addresses
> Transmit
> Passthrough
> Driver version : 5.10.0
> Available Logical Addresses: 1
> Physical Address : 1.0.0.0
> Logical Address Mask : 0x0008
> CEC Version : 2.0
> Vendor ID : 0x000c03 (HDMI)
> OSD Name : Tuner
> Logical Addresses : 1 (Allow RC Passthrough)
>
> Logical Address : 3 (Tuner 1)
> Primary Device Type : Tuner
> Logical Address Type : Tuner
> All Device Types : Tuner
> RC TV Profile : None
> Device Features :
> None
>
> pi@raspberrypi:~$ cec-compliance
> cec-compliance SHA : not available
> Driver Info:
> Driver Name : vc4_hdmi
> Adapter Name : vc4
> Capabilities : 0x0000010e
> Logical Addresses
> Transmit
> Passthrough
> Driver version : 5.10.0
> Available Logical Addresses: 1
> Physical Address : 1.0.0.0
> Logical Address Mask : 0x0008
> CEC Version : 2.0
> Vendor ID : 0x000c03 (HDMI)
> OSD Name : Tuner
> Logical Addresses : 1 (Allow RC Passthrough)
>
> Logical Address : 3 (Tuner 1)
> Primary Device Type : Tuner
> Logical Address Type : Tuner
> All Device Types : Tuner
> RC TV Profile : None
> Device Features :
> None
>
> Compliance test for device /dev/cec0:
>
> The test results mean the following:
> OK Supported correctly by the device.
> OK (Not Supported) Not supported and not mandatory for the device.
> OK (Presumed) Presumably supported. Manually check to confirm.
> OK (Unexpected) Supported correctly but is not expected to be supported for this device.
> OK (Refused) Supported by the device, but was refused.
> FAIL Failed and was expected to be supported by this device.
>
> Find remote devices:
> Polling: OK
>
> Network topology:
> System Information for device 0 (TV) from device 3 (Tuner 1):
> CEC Version : 2.0
> Physical Address : 0.0.0.0
> Primary Device Type : TV
> Vendor ID : 0x000c03
> OSD Name : 'TV '
> Power Status : Tx, OK, Rx, OK, Feature Abort
>
> Total: 1, Succeeded: 1, Failed: 0, Warnings: 0
>
> pi@raspberrypi:~$ cec-ctl -d1 --tuner -p 1.0.0.0
> The CEC adapter doesn't allow setting the physical address manually, ignore this option.
>
> Driver Info:
> Driver Name : vc4_hdmi
> Adapter Name : vc4
> Capabilities : 0x0000010e
> Logical Addresses
> Transmit
> Passthrough
> Driver version : 5.10.0
> Available Logical Addresses: 1
> Physical Address : 1.0.0.0
> Logical Address Mask : 0x0008
> CEC Version : 2.0
> Vendor ID : 0x000c03 (HDMI)
> OSD Name : Tuner
> Logical Addresses : 1 (Allow RC Passthrough)
>
> Logical Address : 3 (Tuner 1)
> Primary Device Type : Tuner
> Logical Address Type : Tuner
> All Device Types : Tuner
> RC TV Profile : None
> Device Features :
> None
>
> pi@raspberrypi:~$ cec-compliance -d1
> cec-compliance SHA : not available
> Driver Info:
> Driver Name : vc4_hdmi
> Adapter Name : vc4
> Capabilities : 0x0000010e
> Logical Addresses
> Transmit
> Passthrough
> Driver version : 5.10.0
> Available Logical Addresses: 1
> Physical Address : 1.0.0.0
> Logical Address Mask : 0x0008
> CEC Version : 2.0
> Vendor ID : 0x000c03 (HDMI)
> OSD Name : Tuner
> Logical Addresses : 1 (Allow RC Passthrough)
>
> Logical Address : 3 (Tuner 1)
> Primary Device Type : Tuner
> Logical Address Type : Tuner
> All Device Types : Tuner
> RC TV Profile : None
> Device Features :
> None
>
> Compliance test for device /dev/cec1:
>
> The test results mean the following:
> OK Supported correctly by the device.
> OK (Not Supported) Not supported and not mandatory for the device.
> OK (Presumed) Presumably supported. Manually check to confirm.
> OK (Unexpected) Supported correctly but is not expected to be supported for this device.
> OK (Refused) Supported by the device, but was refused.
> FAIL Failed and was expected to be supported by this device.
>
> Find remote devices:
> Polling: OK
>
> Network topology:
> System Information for device 0 (TV) from device 3 (Tuner 1):
> CEC Version : 2.0
> Physical Address : 0.0.0.0
> Primary Device Type : TV
> Vendor ID : 0x000c03
> OSD Name : 'TV '
> Power Status : Tx, OK, Rx, OK, Feature Abort
>
> Total: 1, Succeeded: 1, Failed: 0, Warnings: 0
>
> And for the hotplug detect test:
>
> pi@raspberrypi:~$ cec-ctl --playback
> Driver Info:
> Driver Name : vc4_hdmi
> Adapter Name : vc4
> Capabilities : 0x0000010e
> Logical Addresses
> Transmit
> Passthrough
> Driver version : 5.10.0
> Available Logical Addresses: 1
> Physical Address : f.f.f.f
> Logical Address Mask : 0x0000
> CEC Version : 2.0
> Vendor ID : 0x000c03 (HDMI)
> OSD Name : Playback
> Logical Addresses : 1 (Allow RC Passthrough)
>
> Logical Address : Not Allocated
> Primary Device Type : Playback
> Logical Address Type : Playback
> All Device Types : Playback
> RC TV Profile : None
> Device Features :
> None
>
> pi@raspberrypi:~$ cec-ctl -t0 --image-view-on
> Driver Info:
> Driver Name : vc4_hdmi
> Adapter Name : vc4
> Capabilities : 0x0000010e
> Logical Addresses
> Transmit
> Passthrough
> Driver version : 5.10.0
> Available Logical Addresses: 1
> Physical Address : f.f.f.f
> Logical Address Mask : 0x0000
> CEC Version : 2.0
> Vendor ID : 0x000c03 (HDMI)
> OSD Name : Playback
> Logical Addresses : 1 (Allow RC Passthrough)
>
> Logical Address : Not Allocated
> Primary Device Type : Playback
> Logical Address Type : Playback
> All Device Types : Playback
> RC TV Profile : None
> Device Features :
> None
>
> Transmit from Unregistered to TV (15 to 0):
> CEC_MSG_IMAGE_VIEW_ON (0x04)
> Sequence: 1 Tx Timestamp: 9182.611s
> pi@raspberrypi:~$ cec-ctl -d1 --playback
> Driver Info:
> Driver Name : vc4_hdmi
> Adapter Name : vc4
> Capabilities : 0x0000010e
> Logical Addresses
> Transmit
> Passthrough
> Driver version : 5.10.0
> Available Logical Addresses: 1
> Physical Address : f.f.f.f
> Logical Address Mask : 0x0000
> CEC Version : 2.0
> Vendor ID : 0x000c03 (HDMI)
> OSD Name : Playback
> Logical Addresses : 1 (Allow RC Passthrough)
>
> Logical Address : Not Allocated
> Primary Device Type : Playback
> Logical Address Type : Playback
> All Device Types : Playback
> RC TV Profile : None
> Device Features :
> None
>
> pi@raspberrypi:~$ cec-ctl -d1 -t0 --image-view-on
> Driver Info:
> Driver Name : vc4_hdmi
> Adapter Name : vc4
> Capabilities : 0x0000010e
> Logical Addresses
> Transmit
> Passthrough
> Driver version : 5.10.0
> Available Logical Addresses: 1
> Physical Address : f.f.f.f
> Logical Address Mask : 0x0000
> CEC Version : 2.0
> Vendor ID : 0x000c03 (HDMI)
> OSD Name : Playback
> Logical Addresses : 1 (Allow RC Passthrough)
>
> Logical Address : Not Allocated
> Primary Device Type : Playback
> Logical Address Type : Playback
> All Device Types : Playback
> RC TV Profile : None
> Device Features :
> None
>
> Transmit from Unregistered to TV (15 to 0):
> CEC_MSG_IMAGE_VIEW_ON (0x04)
> Sequence: 1 Tx Timestamp: 9207.191s
>
> With the pulse-eight side reporting:
>
> $ sudo cec-ctl -M
> Driver Info:
> Driver Name : pulse8-cec
> Adapter Name : serio0
> Capabilities : 0x0000003f
> Physical Address
> Logical Addresses
> Transmit
> Passthrough
> Remote Control Support
> Monitor All
> Driver version : 5.9.16
> Available Logical Addresses: 1
> Connector Info : None
> Physical Address : 0.0.0.0
> Logical Address Mask : 0x0001
> CEC Version : 2.0
> Vendor ID : 0x000c03 (HDMI)
> OSD Name : 'TV '
> Logical Addresses : 1 (Allow RC Passthrough)
>
> Logical Address : 0 (TV)
> Primary Device Type : TV
> Logical Address Type : TV
> All Device Types : TV
> RC TV Profile : None
> Device Features :
> None
>
> Initial Event: State Change: PA: 0.0.0.0, LA mask: 0x0001, Conn Info: no
>
> Received from Unregistered to TV (15 to 0): IMAGE_VIEW_ON (0x04)
> Received from Unregistered to TV (15 to 0): IMAGE_VIEW_ON (0x04)
>
> Let me know what you think,
> Maxime
>
> Changes from v1:
> - Removed the irqchip patch in favor of a select in mach-bcm
> - Fixed HDMI1 interrupt numbers
> - Removed redundant call to drm_connector_update_edid_property
> - Fixed the condition in vc4_hdmi_connector_detect
> - Added the tags
> - Rebased on top of drm-misc-next-2021-01-06
>
> Dom Cobley (5):
> drm/vc4: hdmi: Move hdmi reset to bind
> drm/vc4: hdmi: Fix register offset with longer CEC messages
> drm/vc4: hdmi: Fix up CEC registers
> drm/vc4: hdmi: Restore cec physical address on reconnect
> drm/vc4: hdmi: Remove cec_available flag
>
> Maxime Ripard (10):
> ARM: bcm: Select BRCMSTB_L2_IRQ for bcm2835
> drm/vc4: hdmi: Compute the CEC clock divider from the clock rate
> drm/vc4: hdmi: Update the CEC clock divider on HSM rate change
> drm/vc4: hdmi: Introduce a CEC clock
> drm/vc4: hdmi: Split the interrupt handlers
> drm/vc4: hdmi: Support BCM2711 CEC interrupt setup
> drm/vc4: hdmi: Don't register the CEC adapter if there's no interrupts
> dt-binding: display: bcm2711-hdmi: Add CEC and hotplug interrupts
> ARM: dts: bcm2711: Add the BSC interrupt controller
> ARM: dts: bcm2711: Add the CEC interrupt controller
>
> .../bindings/display/brcm,bcm2711-hdmi.yaml | 20 +-
> arch/arm/boot/dts/bcm2711.dtsi | 30 +++
> arch/arm/mach-bcm/Kconfig | 1 +
> arch/arm64/Kconfig.platforms | 1 +
> drivers/gpu/drm/vc4/vc4_hdmi.c | 223 ++++++++++++++----
> drivers/gpu/drm/vc4/vc4_hdmi.h | 11 +-
> drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 4 +-
> 7 files changed, 234 insertions(+), 56 deletions(-)
>
On 12/01/2021 16:24, Hans Verkuil wrote:
> Hi Maxime,
>
> On 11/01/2021 15:22, Maxime Ripard wrote:
>> Hi,
>>
>> Here's a series introducing the CEC support for the BCM2711 found on the
>> RaspberryPi4.
>>
>> The BCM2711 HDMI controller uses a similar layout for the CEC registers, the
>> main difference being that the interrupt handling part is now shared between
>> both HDMI controllers.
>>
>> This series is mainly about fixing a couple of bugs, reworking the driver to
>> support having two different interrupts, one for each direction, provided by an
>> external irqchip, and enables the irqchip driver for the controller we have.
>>
>> This has been tested on an RPi3 and RPi4, but requires the latest firmware.
>> It's is based on the 10 and 12 bpc series.
>
> Thank you for this series, I plan to test this later this week.
Testing is delayed: my microHDMI to HDMI adapter has problems with the CEC pin
(possibly not connected at all). I've ordered adapter cables (hopefully of better
quality), but those haven't arrived yet. I expect them later this week.
Regards,
Hans
>
> Regards,
>
> Hans
>
>>
>> Here is the cec-compliance output:
>>
>> pi@raspberrypi:~$ cec-ctl --tuner -p 1.0.0.0
>> The CEC adapter doesn't allow setting the physical address manually, ignore this option.
>>
>> Driver Info:
>> Driver Name : vc4_hdmi
>> Adapter Name : vc4
>> Capabilities : 0x0000010e
>> Logical Addresses
>> Transmit
>> Passthrough
>> Driver version : 5.10.0
>> Available Logical Addresses: 1
>> Physical Address : 1.0.0.0
>> Logical Address Mask : 0x0008
>> CEC Version : 2.0
>> Vendor ID : 0x000c03 (HDMI)
>> OSD Name : Tuner
>> Logical Addresses : 1 (Allow RC Passthrough)
>>
>> Logical Address : 3 (Tuner 1)
>> Primary Device Type : Tuner
>> Logical Address Type : Tuner
>> All Device Types : Tuner
>> RC TV Profile : None
>> Device Features :
>> None
>>
>> pi@raspberrypi:~$ cec-compliance
>> cec-compliance SHA : not available
>> Driver Info:
>> Driver Name : vc4_hdmi
>> Adapter Name : vc4
>> Capabilities : 0x0000010e
>> Logical Addresses
>> Transmit
>> Passthrough
>> Driver version : 5.10.0
>> Available Logical Addresses: 1
>> Physical Address : 1.0.0.0
>> Logical Address Mask : 0x0008
>> CEC Version : 2.0
>> Vendor ID : 0x000c03 (HDMI)
>> OSD Name : Tuner
>> Logical Addresses : 1 (Allow RC Passthrough)
>>
>> Logical Address : 3 (Tuner 1)
>> Primary Device Type : Tuner
>> Logical Address Type : Tuner
>> All Device Types : Tuner
>> RC TV Profile : None
>> Device Features :
>> None
>>
>> Compliance test for device /dev/cec0:
>>
>> The test results mean the following:
>> OK Supported correctly by the device.
>> OK (Not Supported) Not supported and not mandatory for the device.
>> OK (Presumed) Presumably supported. Manually check to confirm.
>> OK (Unexpected) Supported correctly but is not expected to be supported for this device.
>> OK (Refused) Supported by the device, but was refused.
>> FAIL Failed and was expected to be supported by this device.
>>
>> Find remote devices:
>> Polling: OK
>>
>> Network topology:
>> System Information for device 0 (TV) from device 3 (Tuner 1):
>> CEC Version : 2.0
>> Physical Address : 0.0.0.0
>> Primary Device Type : TV
>> Vendor ID : 0x000c03
>> OSD Name : 'TV '
>> Power Status : Tx, OK, Rx, OK, Feature Abort
>>
>> Total: 1, Succeeded: 1, Failed: 0, Warnings: 0
>>
>> pi@raspberrypi:~$ cec-ctl -d1 --tuner -p 1.0.0.0
>> The CEC adapter doesn't allow setting the physical address manually, ignore this option.
>>
>> Driver Info:
>> Driver Name : vc4_hdmi
>> Adapter Name : vc4
>> Capabilities : 0x0000010e
>> Logical Addresses
>> Transmit
>> Passthrough
>> Driver version : 5.10.0
>> Available Logical Addresses: 1
>> Physical Address : 1.0.0.0
>> Logical Address Mask : 0x0008
>> CEC Version : 2.0
>> Vendor ID : 0x000c03 (HDMI)
>> OSD Name : Tuner
>> Logical Addresses : 1 (Allow RC Passthrough)
>>
>> Logical Address : 3 (Tuner 1)
>> Primary Device Type : Tuner
>> Logical Address Type : Tuner
>> All Device Types : Tuner
>> RC TV Profile : None
>> Device Features :
>> None
>>
>> pi@raspberrypi:~$ cec-compliance -d1
>> cec-compliance SHA : not available
>> Driver Info:
>> Driver Name : vc4_hdmi
>> Adapter Name : vc4
>> Capabilities : 0x0000010e
>> Logical Addresses
>> Transmit
>> Passthrough
>> Driver version : 5.10.0
>> Available Logical Addresses: 1
>> Physical Address : 1.0.0.0
>> Logical Address Mask : 0x0008
>> CEC Version : 2.0
>> Vendor ID : 0x000c03 (HDMI)
>> OSD Name : Tuner
>> Logical Addresses : 1 (Allow RC Passthrough)
>>
>> Logical Address : 3 (Tuner 1)
>> Primary Device Type : Tuner
>> Logical Address Type : Tuner
>> All Device Types : Tuner
>> RC TV Profile : None
>> Device Features :
>> None
>>
>> Compliance test for device /dev/cec1:
>>
>> The test results mean the following:
>> OK Supported correctly by the device.
>> OK (Not Supported) Not supported and not mandatory for the device.
>> OK (Presumed) Presumably supported. Manually check to confirm.
>> OK (Unexpected) Supported correctly but is not expected to be supported for this device.
>> OK (Refused) Supported by the device, but was refused.
>> FAIL Failed and was expected to be supported by this device.
>>
>> Find remote devices:
>> Polling: OK
>>
>> Network topology:
>> System Information for device 0 (TV) from device 3 (Tuner 1):
>> CEC Version : 2.0
>> Physical Address : 0.0.0.0
>> Primary Device Type : TV
>> Vendor ID : 0x000c03
>> OSD Name : 'TV '
>> Power Status : Tx, OK, Rx, OK, Feature Abort
>>
>> Total: 1, Succeeded: 1, Failed: 0, Warnings: 0
>>
>> And for the hotplug detect test:
>>
>> pi@raspberrypi:~$ cec-ctl --playback
>> Driver Info:
>> Driver Name : vc4_hdmi
>> Adapter Name : vc4
>> Capabilities : 0x0000010e
>> Logical Addresses
>> Transmit
>> Passthrough
>> Driver version : 5.10.0
>> Available Logical Addresses: 1
>> Physical Address : f.f.f.f
>> Logical Address Mask : 0x0000
>> CEC Version : 2.0
>> Vendor ID : 0x000c03 (HDMI)
>> OSD Name : Playback
>> Logical Addresses : 1 (Allow RC Passthrough)
>>
>> Logical Address : Not Allocated
>> Primary Device Type : Playback
>> Logical Address Type : Playback
>> All Device Types : Playback
>> RC TV Profile : None
>> Device Features :
>> None
>>
>> pi@raspberrypi:~$ cec-ctl -t0 --image-view-on
>> Driver Info:
>> Driver Name : vc4_hdmi
>> Adapter Name : vc4
>> Capabilities : 0x0000010e
>> Logical Addresses
>> Transmit
>> Passthrough
>> Driver version : 5.10.0
>> Available Logical Addresses: 1
>> Physical Address : f.f.f.f
>> Logical Address Mask : 0x0000
>> CEC Version : 2.0
>> Vendor ID : 0x000c03 (HDMI)
>> OSD Name : Playback
>> Logical Addresses : 1 (Allow RC Passthrough)
>>
>> Logical Address : Not Allocated
>> Primary Device Type : Playback
>> Logical Address Type : Playback
>> All Device Types : Playback
>> RC TV Profile : None
>> Device Features :
>> None
>>
>> Transmit from Unregistered to TV (15 to 0):
>> CEC_MSG_IMAGE_VIEW_ON (0x04)
>> Sequence: 1 Tx Timestamp: 9182.611s
>> pi@raspberrypi:~$ cec-ctl -d1 --playback
>> Driver Info:
>> Driver Name : vc4_hdmi
>> Adapter Name : vc4
>> Capabilities : 0x0000010e
>> Logical Addresses
>> Transmit
>> Passthrough
>> Driver version : 5.10.0
>> Available Logical Addresses: 1
>> Physical Address : f.f.f.f
>> Logical Address Mask : 0x0000
>> CEC Version : 2.0
>> Vendor ID : 0x000c03 (HDMI)
>> OSD Name : Playback
>> Logical Addresses : 1 (Allow RC Passthrough)
>>
>> Logical Address : Not Allocated
>> Primary Device Type : Playback
>> Logical Address Type : Playback
>> All Device Types : Playback
>> RC TV Profile : None
>> Device Features :
>> None
>>
>> pi@raspberrypi:~$ cec-ctl -d1 -t0 --image-view-on
>> Driver Info:
>> Driver Name : vc4_hdmi
>> Adapter Name : vc4
>> Capabilities : 0x0000010e
>> Logical Addresses
>> Transmit
>> Passthrough
>> Driver version : 5.10.0
>> Available Logical Addresses: 1
>> Physical Address : f.f.f.f
>> Logical Address Mask : 0x0000
>> CEC Version : 2.0
>> Vendor ID : 0x000c03 (HDMI)
>> OSD Name : Playback
>> Logical Addresses : 1 (Allow RC Passthrough)
>>
>> Logical Address : Not Allocated
>> Primary Device Type : Playback
>> Logical Address Type : Playback
>> All Device Types : Playback
>> RC TV Profile : None
>> Device Features :
>> None
>>
>> Transmit from Unregistered to TV (15 to 0):
>> CEC_MSG_IMAGE_VIEW_ON (0x04)
>> Sequence: 1 Tx Timestamp: 9207.191s
>>
>> With the pulse-eight side reporting:
>>
>> $ sudo cec-ctl -M
>> Driver Info:
>> Driver Name : pulse8-cec
>> Adapter Name : serio0
>> Capabilities : 0x0000003f
>> Physical Address
>> Logical Addresses
>> Transmit
>> Passthrough
>> Remote Control Support
>> Monitor All
>> Driver version : 5.9.16
>> Available Logical Addresses: 1
>> Connector Info : None
>> Physical Address : 0.0.0.0
>> Logical Address Mask : 0x0001
>> CEC Version : 2.0
>> Vendor ID : 0x000c03 (HDMI)
>> OSD Name : 'TV '
>> Logical Addresses : 1 (Allow RC Passthrough)
>>
>> Logical Address : 0 (TV)
>> Primary Device Type : TV
>> Logical Address Type : TV
>> All Device Types : TV
>> RC TV Profile : None
>> Device Features :
>> None
>>
>> Initial Event: State Change: PA: 0.0.0.0, LA mask: 0x0001, Conn Info: no
>>
>> Received from Unregistered to TV (15 to 0): IMAGE_VIEW_ON (0x04)
>> Received from Unregistered to TV (15 to 0): IMAGE_VIEW_ON (0x04)
>>
>> Let me know what you think,
>> Maxime
>>
>> Changes from v1:
>> - Removed the irqchip patch in favor of a select in mach-bcm
>> - Fixed HDMI1 interrupt numbers
>> - Removed redundant call to drm_connector_update_edid_property
>> - Fixed the condition in vc4_hdmi_connector_detect
>> - Added the tags
>> - Rebased on top of drm-misc-next-2021-01-06
>>
>> Dom Cobley (5):
>> drm/vc4: hdmi: Move hdmi reset to bind
>> drm/vc4: hdmi: Fix register offset with longer CEC messages
>> drm/vc4: hdmi: Fix up CEC registers
>> drm/vc4: hdmi: Restore cec physical address on reconnect
>> drm/vc4: hdmi: Remove cec_available flag
>>
>> Maxime Ripard (10):
>> ARM: bcm: Select BRCMSTB_L2_IRQ for bcm2835
>> drm/vc4: hdmi: Compute the CEC clock divider from the clock rate
>> drm/vc4: hdmi: Update the CEC clock divider on HSM rate change
>> drm/vc4: hdmi: Introduce a CEC clock
>> drm/vc4: hdmi: Split the interrupt handlers
>> drm/vc4: hdmi: Support BCM2711 CEC interrupt setup
>> drm/vc4: hdmi: Don't register the CEC adapter if there's no interrupts
>> dt-binding: display: bcm2711-hdmi: Add CEC and hotplug interrupts
>> ARM: dts: bcm2711: Add the BSC interrupt controller
>> ARM: dts: bcm2711: Add the CEC interrupt controller
>>
>> .../bindings/display/brcm,bcm2711-hdmi.yaml | 20 +-
>> arch/arm/boot/dts/bcm2711.dtsi | 30 +++
>> arch/arm/mach-bcm/Kconfig | 1 +
>> arch/arm64/Kconfig.platforms | 1 +
>> drivers/gpu/drm/vc4/vc4_hdmi.c | 223 ++++++++++++++----
>> drivers/gpu/drm/vc4/vc4_hdmi.h | 11 +-
>> drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 4 +-
>> 7 files changed, 234 insertions(+), 56 deletions(-)
>>
>
Hi Maxime
Sorry for the slow reply on these patches.
On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
>
> From: Dom Cobley <[email protected]>
>
> Currently we call cec_phys_addr_invalidate on a hotplug deassert.
> That may be due to a TV power cycling, or an AVR being switched
> on (and switching edid).
>
> This makes CEC unusable since our controller wouldn't have a physical
> address anymore.
>
> Set it back up again on the hotplug assert.
>
> Fixes: 15b4511a4af6 ("drm/vc4: add HDMI CEC support")
> Signed-off-by: Dom Cobley <[email protected]>
> Signed-off-by: Maxime Ripard <[email protected]>
I follow the logic, and trust Dom that it works, but I don't know if
that is the correct thing within CEC.
Ideally Hans will comment as the original author of the CEC code - I
believe he's testing the series anyway.
Acked-by: Dave Stevenson <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 7945dbcee78c..c3a301396aad 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -136,20 +136,32 @@ static enum drm_connector_status
> vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
> {
> struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
> + bool connected = false;
>
> if (vc4_hdmi->hpd_gpio) {
> if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^
> vc4_hdmi->hpd_active_low)
> - return connector_status_connected;
> - cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
> - return connector_status_disconnected;
> + connected = true;
> + } else if (drm_probe_ddc(vc4_hdmi->ddc)) {
> + connected = true;
> + } else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) {
> + connected = true;
> }
>
> - if (drm_probe_ddc(vc4_hdmi->ddc))
> - return connector_status_connected;
> + if (connected) {
> + if (connector->status != connector_status_connected) {
> + struct edid *edid = drm_get_edid(connector, vc4_hdmi->ddc);
> +
> + if (edid) {
> + cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
> + vc4_hdmi->encoder.hdmi_monitor = drm_detect_hdmi_monitor(edid);
> + kfree(edid);
> + }
> + }
>
> - if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED)
> return connector_status_connected;
> + }
> +
> cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
> return connector_status_disconnected;
> }
> --
> 2.29.2
>
Hi Maxime
On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
>
> The BCM2711 has two different interrupt sources to transmit and receive
> CEC messages, provided through an external interrupt chip shared between
> the two HDMI interrupt controllers.
>
> The rest of the CEC controller is identical though so we need to change
> a bit the code organisation to share the code as much as possible, yet
> still allowing to register independant handlers.
s/independant/independent
>
> Signed-off-by: Maxime Ripard <[email protected]>
With that
Reviewed-by: Dave Stevenson <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 86 +++++++++++++++++++++++++---------
> 1 file changed, 65 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 7b5c92df8f1b..12ca5f3084af 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -1454,15 +1454,22 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
> }
>
> #ifdef CONFIG_DRM_VC4_HDMI_CEC
> -static irqreturn_t vc4_cec_irq_handler_thread(int irq, void *priv)
> +static irqreturn_t vc4_cec_irq_handler_rx_thread(int irq, void *priv)
> {
> struct vc4_hdmi *vc4_hdmi = priv;
>
> - if (vc4_hdmi->cec_irq_was_rx) {
> - if (vc4_hdmi->cec_rx_msg.len)
> - cec_received_msg(vc4_hdmi->cec_adap,
> - &vc4_hdmi->cec_rx_msg);
> - } else if (vc4_hdmi->cec_tx_ok) {
> + if (vc4_hdmi->cec_rx_msg.len)
> + cec_received_msg(vc4_hdmi->cec_adap,
> + &vc4_hdmi->cec_rx_msg);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t vc4_cec_irq_handler_tx_thread(int irq, void *priv)
> +{
> + struct vc4_hdmi *vc4_hdmi = priv;
> +
> + if (vc4_hdmi->cec_tx_ok) {
> cec_transmit_done(vc4_hdmi->cec_adap, CEC_TX_STATUS_OK,
> 0, 0, 0, 0);
> } else {
> @@ -1476,6 +1483,19 @@ static irqreturn_t vc4_cec_irq_handler_thread(int irq, void *priv)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t vc4_cec_irq_handler_thread(int irq, void *priv)
> +{
> + struct vc4_hdmi *vc4_hdmi = priv;
> + irqreturn_t ret;
> +
> + if (vc4_hdmi->cec_irq_was_rx)
> + ret = vc4_cec_irq_handler_rx_thread(irq, priv);
> + else
> + ret = vc4_cec_irq_handler_tx_thread(irq, priv);
> +
> + return ret;
> +}
> +
> static void vc4_cec_read_msg(struct vc4_hdmi *vc4_hdmi, u32 cntrl1)
> {
> struct drm_device *dev = vc4_hdmi->connector.dev;
> @@ -1500,31 +1520,55 @@ static void vc4_cec_read_msg(struct vc4_hdmi *vc4_hdmi, u32 cntrl1)
> }
> }
>
> +static irqreturn_t vc4_cec_irq_handler_tx_bare(int irq, void *priv)
> +{
> + struct vc4_hdmi *vc4_hdmi = priv;
> + u32 cntrl1;
> +
> + cntrl1 = HDMI_READ(HDMI_CEC_CNTRL_1);
> + vc4_hdmi->cec_tx_ok = cntrl1 & VC4_HDMI_CEC_TX_STATUS_GOOD;
> + cntrl1 &= ~VC4_HDMI_CEC_START_XMIT_BEGIN;
> + HDMI_WRITE(HDMI_CEC_CNTRL_1, cntrl1);
> +
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t vc4_cec_irq_handler_rx_bare(int irq, void *priv)
> +{
> + struct vc4_hdmi *vc4_hdmi = priv;
> + u32 cntrl1;
> +
> + vc4_hdmi->cec_rx_msg.len = 0;
> + cntrl1 = HDMI_READ(HDMI_CEC_CNTRL_1);
> + vc4_cec_read_msg(vc4_hdmi, cntrl1);
> + cntrl1 |= VC4_HDMI_CEC_CLEAR_RECEIVE_OFF;
> + HDMI_WRITE(HDMI_CEC_CNTRL_1, cntrl1);
> + cntrl1 &= ~VC4_HDMI_CEC_CLEAR_RECEIVE_OFF;
> +
> + HDMI_WRITE(HDMI_CEC_CNTRL_1, cntrl1);
> +
> + return IRQ_WAKE_THREAD;
> +}
> +
> static irqreturn_t vc4_cec_irq_handler(int irq, void *priv)
> {
> struct vc4_hdmi *vc4_hdmi = priv;
> u32 stat = HDMI_READ(HDMI_CEC_CPU_STATUS);
> - u32 cntrl1, cntrl5;
> + irqreturn_t ret;
> + u32 cntrl5;
>
> if (!(stat & VC4_HDMI_CPU_CEC))
> return IRQ_NONE;
> - vc4_hdmi->cec_rx_msg.len = 0;
> - cntrl1 = HDMI_READ(HDMI_CEC_CNTRL_1);
> +
> cntrl5 = HDMI_READ(HDMI_CEC_CNTRL_5);
> vc4_hdmi->cec_irq_was_rx = cntrl5 & VC4_HDMI_CEC_RX_CEC_INT;
> - if (vc4_hdmi->cec_irq_was_rx) {
> - vc4_cec_read_msg(vc4_hdmi, cntrl1);
> - cntrl1 |= VC4_HDMI_CEC_CLEAR_RECEIVE_OFF;
> - HDMI_WRITE(HDMI_CEC_CNTRL_1, cntrl1);
> - cntrl1 &= ~VC4_HDMI_CEC_CLEAR_RECEIVE_OFF;
> - } else {
> - vc4_hdmi->cec_tx_ok = cntrl1 & VC4_HDMI_CEC_TX_STATUS_GOOD;
> - cntrl1 &= ~VC4_HDMI_CEC_START_XMIT_BEGIN;
> - }
> - HDMI_WRITE(HDMI_CEC_CNTRL_1, cntrl1);
> + if (vc4_hdmi->cec_irq_was_rx)
> + ret = vc4_cec_irq_handler_rx_bare(irq, priv);
> + else
> + ret = vc4_cec_irq_handler_tx_bare(irq, priv);
> +
> HDMI_WRITE(HDMI_CEC_CPU_CLEAR, VC4_HDMI_CPU_CEC);
> -
> - return IRQ_WAKE_THREAD;
> + return ret;
> }
>
> static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
> --
> 2.29.2
>
Hi Maxime
On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
>
> The HDMI controller found in the BCM2711 has an external interrupt
> controller for the CEC and hotplug interrupt shared between the two
> instances.
>
> Let's add a variant flag to register a single interrupt handler and
> deals with the interrupt handler setup, or two interrupt handlers
> relying on an external irqchip.
>
> Signed-off-by: Maxime Ripard <[email protected]>
Looks good
Reviewed-by: Dave Stevenson <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 42 ++++++++++++++++++++++++++--------
> drivers/gpu/drm/vc4/vc4_hdmi.h | 7 ++++++
> 2 files changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 12ca5f3084af..d116ecfd8cf7 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -1605,9 +1605,11 @@ static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
> ((3600 / usecs) << VC4_HDMI_CEC_CNT_TO_3600_US_SHIFT) |
> ((3500 / usecs) << VC4_HDMI_CEC_CNT_TO_3500_US_SHIFT));
>
> - HDMI_WRITE(HDMI_CEC_CPU_MASK_CLEAR, VC4_HDMI_CPU_CEC);
> + if (!vc4_hdmi->variant->external_irq_controller)
> + HDMI_WRITE(HDMI_CEC_CPU_MASK_CLEAR, VC4_HDMI_CPU_CEC);
> } else {
> - HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, VC4_HDMI_CPU_CEC);
> + if (!vc4_hdmi->variant->external_irq_controller)
> + HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, VC4_HDMI_CPU_CEC);
> HDMI_WRITE(HDMI_CEC_CNTRL_5, val |
> VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET);
> }
> @@ -1682,8 +1684,6 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
> cec_fill_conn_info_from_drm(&conn_info, &vc4_hdmi->connector);
> cec_s_conn_info(vc4_hdmi->cec_adap, &conn_info);
>
> - HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, 0xffffffff);
> -
> value = HDMI_READ(HDMI_CEC_CNTRL_1);
> /* Set the logical address to Unregistered */
> value |= VC4_HDMI_CEC_ADDR_MASK;
> @@ -1691,12 +1691,32 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
>
> vc4_hdmi_cec_update_clk_div(vc4_hdmi);
>
> - ret = devm_request_threaded_irq(&pdev->dev, platform_get_irq(pdev, 0),
> - vc4_cec_irq_handler,
> - vc4_cec_irq_handler_thread, 0,
> - "vc4 hdmi cec", vc4_hdmi);
> - if (ret)
> - goto err_delete_cec_adap;
> + if (vc4_hdmi->variant->external_irq_controller) {
> + ret = devm_request_threaded_irq(&pdev->dev,
> + platform_get_irq_byname(pdev, "cec-rx"),
> + vc4_cec_irq_handler_rx_bare,
> + vc4_cec_irq_handler_rx_thread, 0,
> + "vc4 hdmi cec rx", vc4_hdmi);
> + if (ret)
> + goto err_delete_cec_adap;
> +
> + ret = devm_request_threaded_irq(&pdev->dev,
> + platform_get_irq_byname(pdev, "cec-tx"),
> + vc4_cec_irq_handler_tx_bare,
> + vc4_cec_irq_handler_tx_thread, 0,
> + "vc4 hdmi cec tx", vc4_hdmi);
> + if (ret)
> + goto err_delete_cec_adap;
> + } else {
> + HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, 0xffffffff);
> +
> + ret = devm_request_threaded_irq(&pdev->dev, platform_get_irq(pdev, 0),
> + vc4_cec_irq_handler,
> + vc4_cec_irq_handler_thread, 0,
> + "vc4 hdmi cec", vc4_hdmi);
> + if (ret)
> + goto err_delete_cec_adap;
> + }
>
> ret = cec_register_adapter(vc4_hdmi->cec_adap, &pdev->dev);
> if (ret < 0)
> @@ -2095,6 +2115,7 @@ static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = {
> PHY_LANE_CK,
> },
> .unsupported_odd_h_timings = true,
> + .external_irq_controller = true,
>
> .init_resources = vc5_hdmi_init_resources,
> .csc_setup = vc5_hdmi_csc_setup,
> @@ -2121,6 +2142,7 @@ static const struct vc4_hdmi_variant bcm2711_hdmi1_variant = {
> PHY_LANE_2,
> },
> .unsupported_odd_h_timings = true,
> + .external_irq_controller = true,
>
> .init_resources = vc5_hdmi_init_resources,
> .csc_setup = vc5_hdmi_csc_setup,
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> index 6966db1a0957..d71f6ed321bf 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> @@ -64,6 +64,13 @@ struct vc4_hdmi_variant {
> /* The BCM2711 cannot deal with odd horizontal pixel timings */
> bool unsupported_odd_h_timings;
>
> + /*
> + * The BCM2711 CEC/hotplug IRQ controller is shared between the
> + * two HDMI controllers, and we have a proper irqchip driver for
> + * it.
> + */
> + bool external_irq_controller;
> +
> /* Callback to get the resources (memory region, interrupts,
> * clocks, etc) for that variant.
> */
> --
> 2.29.2
>
Hi Maxime
On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
>
> The CEC and hotplug interrupts were missing when that binding was
> introduced, let's add them in now that we've figured out how it works.
>
> Signed-off-by: Maxime Ripard <[email protected]>
Looks reasonable to me, but I'm not a DT bindings expert
Acked-by: Dave Stevenson <[email protected]>
> ---
> .../bindings/display/brcm,bcm2711-hdmi.yaml | 20 ++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> index 7ce06f9f9f8e..6e8ac910bdd8 100644
> --- a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> @@ -53,6 +53,24 @@ properties:
> - const: audio
> - const: cec
>
> + interrupts:
> + items:
> + - description: CEC TX interrupt
> + - description: CEC RX interrupt
> + - description: CEC stuck at low interrupt
> + - description: Wake-up interrupt
> + - description: Hotplug connected interrupt
> + - description: Hotplug removed interrupt
> +
> + interrupt-names:
> + items:
> + - const: cec-tx
> + - const: cec-rx
> + - const: cec-low
> + - const: wakeup
> + - const: hpd-connected
> + - const: hpd-removed
> +
> ddc:
> allOf:
> - $ref: /schemas/types.yaml#/definitions/phandle
> @@ -90,7 +108,7 @@ required:
> - resets
> - ddc
>
> -additionalProperties: false
> +unevaluatedProperties: false
>
> examples:
> - |
> --
> 2.29.2
>
Hi,
On Mon, 2021-01-11 at 15:22 +0100, Maxime Ripard wrote:
> Hi,
>
> Here's a series introducing the CEC support for the BCM2711 found on the
> RaspberryPi4.
>
> The BCM2711 HDMI controller uses a similar layout for the CEC registers, the
> main difference being that the interrupt handling part is now shared between
> both HDMI controllers.
>
> This series is mainly about fixing a couple of bugs, reworking the driver to
> support having two different interrupts, one for each direction, provided by an
> external irqchip, and enables the irqchip driver for the controller we have.
>
> This has been tested on an RPi3 and RPi4, but requires the latest firmware.
> It's is based on the 10 and 12 bpc series.
I applied patches #1 and #14 for-next. I'm waiting on Hans' testing for #15.
Regards,
Nicolas
On 18/01/2021 14:55, Hans Verkuil wrote:
> On 12/01/2021 16:24, Hans Verkuil wrote:
>> Hi Maxime,
>>
>> On 11/01/2021 15:22, Maxime Ripard wrote:
>>> Hi,
>>>
>>> Here's a series introducing the CEC support for the BCM2711 found on the
>>> RaspberryPi4.
>>>
>>> The BCM2711 HDMI controller uses a similar layout for the CEC registers, the
>>> main difference being that the interrupt handling part is now shared between
>>> both HDMI controllers.
>>>
>>> This series is mainly about fixing a couple of bugs, reworking the driver to
>>> support having two different interrupts, one for each direction, provided by an
>>> external irqchip, and enables the irqchip driver for the controller we have.
>>>
>>> This has been tested on an RPi3 and RPi4, but requires the latest firmware.
>>> It's is based on the 10 and 12 bpc series.
>>
>> Thank you for this series, I plan to test this later this week.
>
> Testing is delayed: my microHDMI to HDMI adapter has problems with the CEC pin
> (possibly not connected at all). I've ordered adapter cables (hopefully of better
> quality), but those haven't arrived yet. I expect them later this week.
After testing this you can add my:
Acked-by: Hans Verkuil <[email protected]>
Tested-by: Hans Verkuil <[email protected]>
for the patches in this series.
Thank you for working on this!
Regards,
Hans
>
> Regards,
>
> Hans
>
>>
>> Regards,
>>
>> Hans
>>
>>>
>>> Here is the cec-compliance output:
>>>
>>> pi@raspberrypi:~$ cec-ctl --tuner -p 1.0.0.0
>>> The CEC adapter doesn't allow setting the physical address manually, ignore this option.
>>>
>>> Driver Info:
>>> Driver Name : vc4_hdmi
>>> Adapter Name : vc4
>>> Capabilities : 0x0000010e
>>> Logical Addresses
>>> Transmit
>>> Passthrough
>>> Driver version : 5.10.0
>>> Available Logical Addresses: 1
>>> Physical Address : 1.0.0.0
>>> Logical Address Mask : 0x0008
>>> CEC Version : 2.0
>>> Vendor ID : 0x000c03 (HDMI)
>>> OSD Name : Tuner
>>> Logical Addresses : 1 (Allow RC Passthrough)
>>>
>>> Logical Address : 3 (Tuner 1)
>>> Primary Device Type : Tuner
>>> Logical Address Type : Tuner
>>> All Device Types : Tuner
>>> RC TV Profile : None
>>> Device Features :
>>> None
>>>
>>> pi@raspberrypi:~$ cec-compliance
>>> cec-compliance SHA : not available
>>> Driver Info:
>>> Driver Name : vc4_hdmi
>>> Adapter Name : vc4
>>> Capabilities : 0x0000010e
>>> Logical Addresses
>>> Transmit
>>> Passthrough
>>> Driver version : 5.10.0
>>> Available Logical Addresses: 1
>>> Physical Address : 1.0.0.0
>>> Logical Address Mask : 0x0008
>>> CEC Version : 2.0
>>> Vendor ID : 0x000c03 (HDMI)
>>> OSD Name : Tuner
>>> Logical Addresses : 1 (Allow RC Passthrough)
>>>
>>> Logical Address : 3 (Tuner 1)
>>> Primary Device Type : Tuner
>>> Logical Address Type : Tuner
>>> All Device Types : Tuner
>>> RC TV Profile : None
>>> Device Features :
>>> None
>>>
>>> Compliance test for device /dev/cec0:
>>>
>>> The test results mean the following:
>>> OK Supported correctly by the device.
>>> OK (Not Supported) Not supported and not mandatory for the device.
>>> OK (Presumed) Presumably supported. Manually check to confirm.
>>> OK (Unexpected) Supported correctly but is not expected to be supported for this device.
>>> OK (Refused) Supported by the device, but was refused.
>>> FAIL Failed and was expected to be supported by this device.
>>>
>>> Find remote devices:
>>> Polling: OK
>>>
>>> Network topology:
>>> System Information for device 0 (TV) from device 3 (Tuner 1):
>>> CEC Version : 2.0
>>> Physical Address : 0.0.0.0
>>> Primary Device Type : TV
>>> Vendor ID : 0x000c03
>>> OSD Name : 'TV '
>>> Power Status : Tx, OK, Rx, OK, Feature Abort
>>>
>>> Total: 1, Succeeded: 1, Failed: 0, Warnings: 0
>>>
>>> pi@raspberrypi:~$ cec-ctl -d1 --tuner -p 1.0.0.0
>>> The CEC adapter doesn't allow setting the physical address manually, ignore this option.
>>>
>>> Driver Info:
>>> Driver Name : vc4_hdmi
>>> Adapter Name : vc4
>>> Capabilities : 0x0000010e
>>> Logical Addresses
>>> Transmit
>>> Passthrough
>>> Driver version : 5.10.0
>>> Available Logical Addresses: 1
>>> Physical Address : 1.0.0.0
>>> Logical Address Mask : 0x0008
>>> CEC Version : 2.0
>>> Vendor ID : 0x000c03 (HDMI)
>>> OSD Name : Tuner
>>> Logical Addresses : 1 (Allow RC Passthrough)
>>>
>>> Logical Address : 3 (Tuner 1)
>>> Primary Device Type : Tuner
>>> Logical Address Type : Tuner
>>> All Device Types : Tuner
>>> RC TV Profile : None
>>> Device Features :
>>> None
>>>
>>> pi@raspberrypi:~$ cec-compliance -d1
>>> cec-compliance SHA : not available
>>> Driver Info:
>>> Driver Name : vc4_hdmi
>>> Adapter Name : vc4
>>> Capabilities : 0x0000010e
>>> Logical Addresses
>>> Transmit
>>> Passthrough
>>> Driver version : 5.10.0
>>> Available Logical Addresses: 1
>>> Physical Address : 1.0.0.0
>>> Logical Address Mask : 0x0008
>>> CEC Version : 2.0
>>> Vendor ID : 0x000c03 (HDMI)
>>> OSD Name : Tuner
>>> Logical Addresses : 1 (Allow RC Passthrough)
>>>
>>> Logical Address : 3 (Tuner 1)
>>> Primary Device Type : Tuner
>>> Logical Address Type : Tuner
>>> All Device Types : Tuner
>>> RC TV Profile : None
>>> Device Features :
>>> None
>>>
>>> Compliance test for device /dev/cec1:
>>>
>>> The test results mean the following:
>>> OK Supported correctly by the device.
>>> OK (Not Supported) Not supported and not mandatory for the device.
>>> OK (Presumed) Presumably supported. Manually check to confirm.
>>> OK (Unexpected) Supported correctly but is not expected to be supported for this device.
>>> OK (Refused) Supported by the device, but was refused.
>>> FAIL Failed and was expected to be supported by this device.
>>>
>>> Find remote devices:
>>> Polling: OK
>>>
>>> Network topology:
>>> System Information for device 0 (TV) from device 3 (Tuner 1):
>>> CEC Version : 2.0
>>> Physical Address : 0.0.0.0
>>> Primary Device Type : TV
>>> Vendor ID : 0x000c03
>>> OSD Name : 'TV '
>>> Power Status : Tx, OK, Rx, OK, Feature Abort
>>>
>>> Total: 1, Succeeded: 1, Failed: 0, Warnings: 0
>>>
>>> And for the hotplug detect test:
>>>
>>> pi@raspberrypi:~$ cec-ctl --playback
>>> Driver Info:
>>> Driver Name : vc4_hdmi
>>> Adapter Name : vc4
>>> Capabilities : 0x0000010e
>>> Logical Addresses
>>> Transmit
>>> Passthrough
>>> Driver version : 5.10.0
>>> Available Logical Addresses: 1
>>> Physical Address : f.f.f.f
>>> Logical Address Mask : 0x0000
>>> CEC Version : 2.0
>>> Vendor ID : 0x000c03 (HDMI)
>>> OSD Name : Playback
>>> Logical Addresses : 1 (Allow RC Passthrough)
>>>
>>> Logical Address : Not Allocated
>>> Primary Device Type : Playback
>>> Logical Address Type : Playback
>>> All Device Types : Playback
>>> RC TV Profile : None
>>> Device Features :
>>> None
>>>
>>> pi@raspberrypi:~$ cec-ctl -t0 --image-view-on
>>> Driver Info:
>>> Driver Name : vc4_hdmi
>>> Adapter Name : vc4
>>> Capabilities : 0x0000010e
>>> Logical Addresses
>>> Transmit
>>> Passthrough
>>> Driver version : 5.10.0
>>> Available Logical Addresses: 1
>>> Physical Address : f.f.f.f
>>> Logical Address Mask : 0x0000
>>> CEC Version : 2.0
>>> Vendor ID : 0x000c03 (HDMI)
>>> OSD Name : Playback
>>> Logical Addresses : 1 (Allow RC Passthrough)
>>>
>>> Logical Address : Not Allocated
>>> Primary Device Type : Playback
>>> Logical Address Type : Playback
>>> All Device Types : Playback
>>> RC TV Profile : None
>>> Device Features :
>>> None
>>>
>>> Transmit from Unregistered to TV (15 to 0):
>>> CEC_MSG_IMAGE_VIEW_ON (0x04)
>>> Sequence: 1 Tx Timestamp: 9182.611s
>>> pi@raspberrypi:~$ cec-ctl -d1 --playback
>>> Driver Info:
>>> Driver Name : vc4_hdmi
>>> Adapter Name : vc4
>>> Capabilities : 0x0000010e
>>> Logical Addresses
>>> Transmit
>>> Passthrough
>>> Driver version : 5.10.0
>>> Available Logical Addresses: 1
>>> Physical Address : f.f.f.f
>>> Logical Address Mask : 0x0000
>>> CEC Version : 2.0
>>> Vendor ID : 0x000c03 (HDMI)
>>> OSD Name : Playback
>>> Logical Addresses : 1 (Allow RC Passthrough)
>>>
>>> Logical Address : Not Allocated
>>> Primary Device Type : Playback
>>> Logical Address Type : Playback
>>> All Device Types : Playback
>>> RC TV Profile : None
>>> Device Features :
>>> None
>>>
>>> pi@raspberrypi:~$ cec-ctl -d1 -t0 --image-view-on
>>> Driver Info:
>>> Driver Name : vc4_hdmi
>>> Adapter Name : vc4
>>> Capabilities : 0x0000010e
>>> Logical Addresses
>>> Transmit
>>> Passthrough
>>> Driver version : 5.10.0
>>> Available Logical Addresses: 1
>>> Physical Address : f.f.f.f
>>> Logical Address Mask : 0x0000
>>> CEC Version : 2.0
>>> Vendor ID : 0x000c03 (HDMI)
>>> OSD Name : Playback
>>> Logical Addresses : 1 (Allow RC Passthrough)
>>>
>>> Logical Address : Not Allocated
>>> Primary Device Type : Playback
>>> Logical Address Type : Playback
>>> All Device Types : Playback
>>> RC TV Profile : None
>>> Device Features :
>>> None
>>>
>>> Transmit from Unregistered to TV (15 to 0):
>>> CEC_MSG_IMAGE_VIEW_ON (0x04)
>>> Sequence: 1 Tx Timestamp: 9207.191s
>>>
>>> With the pulse-eight side reporting:
>>>
>>> $ sudo cec-ctl -M
>>> Driver Info:
>>> Driver Name : pulse8-cec
>>> Adapter Name : serio0
>>> Capabilities : 0x0000003f
>>> Physical Address
>>> Logical Addresses
>>> Transmit
>>> Passthrough
>>> Remote Control Support
>>> Monitor All
>>> Driver version : 5.9.16
>>> Available Logical Addresses: 1
>>> Connector Info : None
>>> Physical Address : 0.0.0.0
>>> Logical Address Mask : 0x0001
>>> CEC Version : 2.0
>>> Vendor ID : 0x000c03 (HDMI)
>>> OSD Name : 'TV '
>>> Logical Addresses : 1 (Allow RC Passthrough)
>>>
>>> Logical Address : 0 (TV)
>>> Primary Device Type : TV
>>> Logical Address Type : TV
>>> All Device Types : TV
>>> RC TV Profile : None
>>> Device Features :
>>> None
>>>
>>> Initial Event: State Change: PA: 0.0.0.0, LA mask: 0x0001, Conn Info: no
>>>
>>> Received from Unregistered to TV (15 to 0): IMAGE_VIEW_ON (0x04)
>>> Received from Unregistered to TV (15 to 0): IMAGE_VIEW_ON (0x04)
>>>
>>> Let me know what you think,
>>> Maxime
>>>
>>> Changes from v1:
>>> - Removed the irqchip patch in favor of a select in mach-bcm
>>> - Fixed HDMI1 interrupt numbers
>>> - Removed redundant call to drm_connector_update_edid_property
>>> - Fixed the condition in vc4_hdmi_connector_detect
>>> - Added the tags
>>> - Rebased on top of drm-misc-next-2021-01-06
>>>
>>> Dom Cobley (5):
>>> drm/vc4: hdmi: Move hdmi reset to bind
>>> drm/vc4: hdmi: Fix register offset with longer CEC messages
>>> drm/vc4: hdmi: Fix up CEC registers
>>> drm/vc4: hdmi: Restore cec physical address on reconnect
>>> drm/vc4: hdmi: Remove cec_available flag
>>>
>>> Maxime Ripard (10):
>>> ARM: bcm: Select BRCMSTB_L2_IRQ for bcm2835
>>> drm/vc4: hdmi: Compute the CEC clock divider from the clock rate
>>> drm/vc4: hdmi: Update the CEC clock divider on HSM rate change
>>> drm/vc4: hdmi: Introduce a CEC clock
>>> drm/vc4: hdmi: Split the interrupt handlers
>>> drm/vc4: hdmi: Support BCM2711 CEC interrupt setup
>>> drm/vc4: hdmi: Don't register the CEC adapter if there's no interrupts
>>> dt-binding: display: bcm2711-hdmi: Add CEC and hotplug interrupts
>>> ARM: dts: bcm2711: Add the BSC interrupt controller
>>> ARM: dts: bcm2711: Add the CEC interrupt controller
>>>
>>> .../bindings/display/brcm,bcm2711-hdmi.yaml | 20 +-
>>> arch/arm/boot/dts/bcm2711.dtsi | 30 +++
>>> arch/arm/mach-bcm/Kconfig | 1 +
>>> arch/arm64/Kconfig.platforms | 1 +
>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 223 ++++++++++++++----
>>> drivers/gpu/drm/vc4/vc4_hdmi.h | 11 +-
>>> drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 4 +-
>>> 7 files changed, 234 insertions(+), 56 deletions(-)
>>>
>>
>
Hi!
On Mon, Jan 25, 2021 at 10:03:44PM +0100, Nicolas Saenz Julienne wrote:
> Hi,
>
> On Mon, 2021-01-11 at 15:22 +0100, Maxime Ripard wrote:
> > Hi,
> >
> > Here's a series introducing the CEC support for the BCM2711 found on the
> > RaspberryPi4.
> >
> > The BCM2711 HDMI controller uses a similar layout for the CEC registers, the
> > main difference being that the interrupt handling part is now shared between
> > both HDMI controllers.
> >
> > This series is mainly about fixing a couple of bugs, reworking the driver to
> > support having two different interrupts, one for each direction, provided by an
> > external irqchip, and enables the irqchip driver for the controller we have.
> >
> > This has been tested on an RPi3 and RPi4, but requires the latest firmware.
> > It's is based on the 10 and 12 bpc series.
>
> I applied patches #1 and #14 for-next. I'm waiting on Hans' testing for #15.
I've applied to drm-misc-next the patches 2 to 13
Maxime
On Mon, 2021-01-11 at 15:23 +0100, Maxime Ripard wrote:
> The CEC and hotplug interrupts go through an interrupt controller shared
> between the two HDMI controllers.
>
> Let's add that interrupt controller and the interrupts for both HDMI
> controllers
>
> Reviewed-by: Florian Fainelli <[email protected]>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
Applied for-next.
Thanks,
Nicolas
On Mon, Jan 11, 2021 at 8:27 AM Maxime Ripard <[email protected]> wrote:
>
> The CEC and hotplug interrupts were missing when that binding was
> introduced, let's add them in now that we've figured out how it works.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
> .../bindings/display/brcm,bcm2711-hdmi.yaml | 20 ++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> index 7ce06f9f9f8e..6e8ac910bdd8 100644
> --- a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> @@ -53,6 +53,24 @@ properties:
> - const: audio
> - const: cec
>
> + interrupts:
> + items:
> + - description: CEC TX interrupt
> + - description: CEC RX interrupt
> + - description: CEC stuck at low interrupt
> + - description: Wake-up interrupt
> + - description: Hotplug connected interrupt
> + - description: Hotplug removed interrupt
> +
> + interrupt-names:
> + items:
> + - const: cec-tx
> + - const: cec-rx
> + - const: cec-low
> + - const: wakeup
> + - const: hpd-connected
> + - const: hpd-removed
> +
> ddc:
> allOf:
> - $ref: /schemas/types.yaml#/definitions/phandle
> @@ -90,7 +108,7 @@ required:
> - resets
> - ddc
>
> -additionalProperties: false
> +unevaluatedProperties: false
/builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml:
'additionalProperties' is a required property
And you missed the DT list, so no checks ran.
Rob
Hi Maxime
On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
>
> The BSC controllers used for the HDMI DDC have an interrupt controller
> shared between both instances. Let's add it to avoid polling.
This seems to have unintended side effects.
GIC interrupt 117 is shared between the standard I2C controllers
(i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts.
Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that
doesn't appear to be an option for l2-intc registering as an interrupt
controller. i2c-bcm2835 therefore loses out and fails to register for
the interrupt.
Is there an equivalent flag that an interrupt controller can add to
say that the parent interrupt is shared? Is that even supported?
Thanks
Dave
> Reviewed-by: Florian Fainelli <[email protected]>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
> arch/arm/boot/dts/bcm2711.dtsi | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
> index 4847dd305317..8bb46ae76a92 100644
> --- a/arch/arm/boot/dts/bcm2711.dtsi
> +++ b/arch/arm/boot/dts/bcm2711.dtsi
> @@ -308,6 +308,14 @@ dvp: clock@7ef00000 {
> #reset-cells = <1>;
> };
>
> + bsc_intr: interrupt-controller@7ef00040 {
> + compatible = "brcm,bcm2711-l2-intc", "brcm,l2-intc";
> + reg = <0x7ef00040 0x30>;
> + interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + };
> +
> hdmi0: hdmi@7ef00700 {
> compatible = "brcm,bcm2711-hdmi0";
> reg = <0x7ef00700 0x300>,
> @@ -341,6 +349,8 @@ ddc0: i2c@7ef04500 {
> reg = <0x7ef04500 0x100>, <0x7ef00b00 0x300>;
> reg-names = "bsc", "auto-i2c";
> clock-frequency = <97500>;
> + interrupt-parent = <&bsc_intr>;
> + interrupts = <0>;
> status = "disabled";
> };
>
> @@ -377,6 +387,8 @@ ddc1: i2c@7ef09500 {
> reg = <0x7ef09500 0x100>, <0x7ef05b00 0x300>;
> reg-names = "bsc", "auto-i2c";
> clock-frequency = <97500>;
> + interrupt-parent = <&bsc_intr>;
> + interrupts = <1>;
> status = "disabled";
> };
> };
> --
> 2.29.2
>
Hi Dave,
On Tue, Feb 09, 2021 at 09:49:05AM +0000, Dave Stevenson wrote:
> On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
> >
> > The BSC controllers used for the HDMI DDC have an interrupt controller
> > shared between both instances. Let's add it to avoid polling.
>
> This seems to have unintended side effects.
> GIC interrupt 117 is shared between the standard I2C controllers
> (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts.
>
> Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that
> doesn't appear to be an option for l2-intc registering as an interrupt
> controller. i2c-bcm2835 therefore loses out and fails to register for
> the interrupt.
>
> Is there an equivalent flag that an interrupt controller can add to
> say that the parent interrupt is shared? Is that even supported?
Indeed, it looks like setting an equivalent to IRQF_SHARED would be the
solution, but I couldn't find anything that would allow us to in the
irqchip code.
Marc, Thomas, is it something that is allowed?
Thanks!
Maxime
Hi Maxime,
On 2021-02-10 14:40, Maxime Ripard wrote:
> Hi Dave,
>
> On Tue, Feb 09, 2021 at 09:49:05AM +0000, Dave Stevenson wrote:
>> On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
>> >
>> > The BSC controllers used for the HDMI DDC have an interrupt controller
>> > shared between both instances. Let's add it to avoid polling.
>>
>> This seems to have unintended side effects.
>> GIC interrupt 117 is shared between the standard I2C controllers
>> (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts.
>>
>> Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that
>> doesn't appear to be an option for l2-intc registering as an interrupt
>> controller. i2c-bcm2835 therefore loses out and fails to register for
>> the interrupt.
>>
>> Is there an equivalent flag that an interrupt controller can add to
>> say that the parent interrupt is shared? Is that even supported?
>
> Indeed, it looks like setting an equivalent to IRQF_SHARED would be the
> solution, but I couldn't find anything that would allow us to in the
> irqchip code.
>
> Marc, Thomas, is it something that is allowed?
No, not really. That's because the chained handler is actually an
interrupt flow, and not a normal handler. IRQF_SHARED acts at the wrong
level for that.
I can see two possibilities:
- the l2-intc gets turned into a normal handler, and does the demux
from there. Horrible stuff.
- the i2c controller gets parented to the l2c-int as a fake interrupt,
and gets called from there. Horrible stuff.
Pick your poison... :-/
M.
--
Jazz is not dead. It just smells funny...
Hi Marc.
On Wed, 10 Feb 2021 at 15:30, Marc Zyngier <[email protected]> wrote:
>
> Hi Maxime,
>
> On 2021-02-10 14:40, Maxime Ripard wrote:
> > Hi Dave,
> >
> > On Tue, Feb 09, 2021 at 09:49:05AM +0000, Dave Stevenson wrote:
> >> On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
> >> >
> >> > The BSC controllers used for the HDMI DDC have an interrupt controller
> >> > shared between both instances. Let's add it to avoid polling.
> >>
> >> This seems to have unintended side effects.
> >> GIC interrupt 117 is shared between the standard I2C controllers
> >> (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts.
> >>
> >> Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that
> >> doesn't appear to be an option for l2-intc registering as an interrupt
> >> controller. i2c-bcm2835 therefore loses out and fails to register for
> >> the interrupt.
> >>
> >> Is there an equivalent flag that an interrupt controller can add to
> >> say that the parent interrupt is shared? Is that even supported?
> >
> > Indeed, it looks like setting an equivalent to IRQF_SHARED would be the
> > solution, but I couldn't find anything that would allow us to in the
> > irqchip code.
> >
> > Marc, Thomas, is it something that is allowed?
>
> No, not really. That's because the chained handler is actually an
> interrupt flow, and not a normal handler. IRQF_SHARED acts at the wrong
> level for that.
>
> I can see two possibilities:
>
> - the l2-intc gets turned into a normal handler, and does the demux
> from there. Horrible stuff.
>
> - the i2c controller gets parented to the l2c-int as a fake interrupt,
> and gets called from there. Horrible stuff.
>
> Pick your poison... :-/
Thanks for the info.
Option 3 - remove l2-intc and drop back to polling the i2c-brcmstb
blocks (which the driver supports anyway).
HDMI I2C generally isn't heavily used once displays are connected, so
I'd be OK with that.
(We can keep the l2-intc that handles CEC and HPD as that is on a
unique GIC interrupt).
Dave
On 2/10/21 7:49 AM, Dave Stevenson wrote:
> Hi Marc.
>
> On Wed, 10 Feb 2021 at 15:30, Marc Zyngier <[email protected]> wrote:
>>
>> Hi Maxime,
>>
>> On 2021-02-10 14:40, Maxime Ripard wrote:
>>> Hi Dave,
>>>
>>> On Tue, Feb 09, 2021 at 09:49:05AM +0000, Dave Stevenson wrote:
>>>> On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
>>>>>
>>>>> The BSC controllers used for the HDMI DDC have an interrupt controller
>>>>> shared between both instances. Let's add it to avoid polling.
>>>>
>>>> This seems to have unintended side effects.
>>>> GIC interrupt 117 is shared between the standard I2C controllers
>>>> (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts.
>>>>
>>>> Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that
>>>> doesn't appear to be an option for l2-intc registering as an interrupt
>>>> controller. i2c-bcm2835 therefore loses out and fails to register for
>>>> the interrupt.
>>>>
>>>> Is there an equivalent flag that an interrupt controller can add to
>>>> say that the parent interrupt is shared? Is that even supported?
>>>
>>> Indeed, it looks like setting an equivalent to IRQF_SHARED would be the
>>> solution, but I couldn't find anything that would allow us to in the
>>> irqchip code.
>>>
>>> Marc, Thomas, is it something that is allowed?
>>
>> No, not really. That's because the chained handler is actually an
>> interrupt flow, and not a normal handler. IRQF_SHARED acts at the wrong
>> level for that.
>>
>> I can see two possibilities:
>>
>> - the l2-intc gets turned into a normal handler, and does the demux
>> from there. Horrible stuff.
>>
>> - the i2c controller gets parented to the l2c-int as a fake interrupt,
>> and gets called from there. Horrible stuff.
>>
>> Pick your poison... :-/
>
> Thanks for the info.
>
> Option 3 - remove l2-intc and drop back to polling the i2c-brcmstb
> blocks (which the driver supports anyway).
> HDMI I2C generally isn't heavily used once displays are connected, so
> I'd be OK with that.
>
> (We can keep the l2-intc that handles CEC and HPD as that is on a
> unique GIC interrupt).
Agreed, Maxime or Nicolas do you want me to send a revert of this patch?
--
Florian
On Wed, 2021-02-10 at 10:49 -0800, Florian Fainelli wrote:
> On 2/10/21 7:49 AM, Dave Stevenson wrote:
> > Hi Marc.
> >
> > On Wed, 10 Feb 2021 at 15:30, Marc Zyngier <[email protected]> wrote:
> > >
> > > Hi Maxime,
> > >
> > > On 2021-02-10 14:40, Maxime Ripard wrote:
> > > > Hi Dave,
> > > >
> > > > On Tue, Feb 09, 2021 at 09:49:05AM +0000, Dave Stevenson wrote:
> > > > > On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
> > > > > >
> > > > > > The BSC controllers used for the HDMI DDC have an interrupt controller
> > > > > > shared between both instances. Let's add it to avoid polling.
> > > > >
> > > > > This seems to have unintended side effects.
> > > > > GIC interrupt 117 is shared between the standard I2C controllers
> > > > > (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts.
> > > > >
> > > > > Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that
> > > > > doesn't appear to be an option for l2-intc registering as an interrupt
> > > > > controller. i2c-bcm2835 therefore loses out and fails to register for
> > > > > the interrupt.
> > > > >
> > > > > Is there an equivalent flag that an interrupt controller can add to
> > > > > say that the parent interrupt is shared? Is that even supported?
> > > >
> > > > Indeed, it looks like setting an equivalent to IRQF_SHARED would be the
> > > > solution, but I couldn't find anything that would allow us to in the
> > > > irqchip code.
> > > >
> > > > Marc, Thomas, is it something that is allowed?
> > >
> > > No, not really. That's because the chained handler is actually an
> > > interrupt flow, and not a normal handler. IRQF_SHARED acts at the wrong
> > > level for that.
> > >
> > > I can see two possibilities:
> > >
> > > - the l2-intc gets turned into a normal handler, and does the demux
> > > from there. Horrible stuff.
> > >
> > > - the i2c controller gets parented to the l2c-int as a fake interrupt,
> > > and gets called from there. Horrible stuff.
> > >
> > > Pick your poison... :-/
> >
> > Thanks for the info.
> >
> > Option 3 - remove l2-intc and drop back to polling the i2c-brcmstb
> > blocks (which the driver supports anyway).
> > HDMI I2C generally isn't heavily used once displays are connected, so
> > I'd be OK with that.
> >
> > (We can keep the l2-intc that handles CEC and HPD as that is on a
> > unique GIC interrupt).
>
> Agreed, Maxime or Nicolas do you want me to send a revert of this patch?
Reverting seems the safe move, but I'll defer to whatever Maxime says.
Regards,
Nicolas
On Fri, Feb 12, 2021 at 12:20:14PM +0100, Nicolas Saenz Julienne wrote:
> On Wed, 2021-02-10 at 10:49 -0800, Florian Fainelli wrote:
> > On 2/10/21 7:49 AM, Dave Stevenson wrote:
> > > Hi Marc.
> > >
> > > On Wed, 10 Feb 2021 at 15:30, Marc Zyngier <[email protected]> wrote:
> > > >
> > > > Hi Maxime,
> > > >
> > > > On 2021-02-10 14:40, Maxime Ripard wrote:
> > > > > Hi Dave,
> > > > >
> > > > > On Tue, Feb 09, 2021 at 09:49:05AM +0000, Dave Stevenson wrote:
> > > > > > On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <[email protected]> wrote:
> > > > > > >
> > > > > > > The BSC controllers used for the HDMI DDC have an interrupt controller
> > > > > > > shared between both instances. Let's add it to avoid polling.
> > > > > >
> > > > > > This seems to have unintended side effects.
> > > > > > GIC interrupt 117 is shared between the standard I2C controllers
> > > > > > (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts.
> > > > > >
> > > > > > Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that
> > > > > > doesn't appear to be an option for l2-intc registering as an interrupt
> > > > > > controller. i2c-bcm2835 therefore loses out and fails to register for
> > > > > > the interrupt.
> > > > > >
> > > > > > Is there an equivalent flag that an interrupt controller can add to
> > > > > > say that the parent interrupt is shared? Is that even supported?
> > > > >
> > > > > Indeed, it looks like setting an equivalent to IRQF_SHARED would be the
> > > > > solution, but I couldn't find anything that would allow us to in the
> > > > > irqchip code.
> > > > >
> > > > > Marc, Thomas, is it something that is allowed?
> > > >
> > > > No, not really. That's because the chained handler is actually an
> > > > interrupt flow, and not a normal handler. IRQF_SHARED acts at the wrong
> > > > level for that.
> > > >
> > > > I can see two possibilities:
> > > >
> > > > - the l2-intc gets turned into a normal handler, and does the demux
> > > > ???from there. Horrible stuff.
> > > >
> > > > - the i2c controller gets parented to the l2c-int as a fake interrupt,
> > > > ???and gets called from there. Horrible stuff.
> > > >
> > > > Pick your poison... :-/
> > >
> > > Thanks for the info.
> > >
> > > Option 3 - remove l2-intc and drop back to polling the i2c-brcmstb
> > > blocks (which the driver supports anyway).
> > > HDMI I2C generally isn't heavily used once displays are connected, so
> > > I'd be OK with that.
> > >
> > > (We can keep the l2-intc that handles CEC and HPD as that is on a
> > > unique GIC interrupt).
> >
> > Agreed, Maxime or Nicolas do you want me to send a revert of this patch?
>
> Reverting seems the safe move, but I'll defer to whatever Maxime says.
Yes, reverting it seems like the easiest way forward. If you can send it
Florian that would be great :)
Maxime
On Mon, Feb 1, 2021 at 12:56 PM Rob Herring <[email protected]> wrote:
>
> On Mon, Jan 11, 2021 at 8:27 AM Maxime Ripard <[email protected]> wrote:
> >
> > The CEC and hotplug interrupts were missing when that binding was
> > introduced, let's add them in now that we've figured out how it works.
> >
> > Signed-off-by: Maxime Ripard <[email protected]>
> > ---
> > .../bindings/display/brcm,bcm2711-hdmi.yaml | 20 ++++++++++++++++++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > index 7ce06f9f9f8e..6e8ac910bdd8 100644
> > --- a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > @@ -53,6 +53,24 @@ properties:
> > - const: audio
> > - const: cec
> >
> > + interrupts:
> > + items:
> > + - description: CEC TX interrupt
> > + - description: CEC RX interrupt
> > + - description: CEC stuck at low interrupt
> > + - description: Wake-up interrupt
> > + - description: Hotplug connected interrupt
> > + - description: Hotplug removed interrupt
> > +
> > + interrupt-names:
> > + items:
> > + - const: cec-tx
> > + - const: cec-rx
> > + - const: cec-low
> > + - const: wakeup
> > + - const: hpd-connected
> > + - const: hpd-removed
> > +
> > ddc:
> > allOf:
> > - $ref: /schemas/types.yaml#/definitions/phandle
> > @@ -90,7 +108,7 @@ required:
> > - resets
> > - ddc
> >
> > -additionalProperties: false
> > +unevaluatedProperties: false
>
> /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml:
> 'additionalProperties' is a required property
>
> And you missed the DT list, so no checks ran.
This is still failing in linux-next.
Rob