Re: [PATCH v2 00/15] drm/vc4: hdmi: Add CEC support for the BCM2711

[Hans Verkuil]

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 <hverkuil-cisco@xxxxxxxxx>
Tested-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>

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