vc4_hdmi: Add CEC support for 2711 #3601

popcornmix · 2020-05-05T18:15:10Z

No description provided.

popcornmix · 2020-05-05T18:20:15Z

Ping @6by9
A few open questions:

is adding variant functions for each difference in behaviour best, or would conditional code based on an is_vc5 flag be more concise?
Shall we strip out cec_available now it's always true?
Hardware retry is mentioned in the docs. I can't be sure it's doing useful stuff but it doesn't appear to break things. It could be dropped with no loss of functionality (but potentially less efficient).

6by9 · 2020-05-07T13:57:16Z

drivers/gpu/drm/vc4/vc4_hdmi.c

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


Is this correct for Pi3 as well? It appears to be unconditional on revision.

Yes it is a bug on Pi3. Any long messages would trample other registers (and we are getting longer messages in log).

6by9 · 2020-05-07T14:00:39Z

drivers/gpu/drm/vc4/vc4_hdmi.c

@@ -1751,6 +1803,7 @@ static int vc4_hdmi_dev_remove(struct platform_device *pdev)

 static const struct vc4_hdmi_variant bcm2835_variant = {
 	.max_pixel_clock	= 162000000,
+	.cec_input_clock	= 163682864,


To nit pick, it'd be nice to have this as a define along the lines of VC4_HDMI_HSM_CLOCK_RATE, as the same value is used in vc4_hdmi_calc_hsm_clock.

6by9 · 2020-05-07T14:01:16Z

drivers/gpu/drm/vc4/vc4_hdmi.c

 	.channel_map		= vc5_hdmi_channel_map,
+
+	.cec_mask = VC5_HDMI0_CPU_CEC_RX | VC5_HDMI0_CPU_CEC_TX | VC5_HDMI0_CPU_HOTPLUG_CONN | VC5_HDMI0_CPU_HOTPLUG_REM,


Hotplug bits as well? Why?

6by9 · 2020-05-07T14:09:57Z

is adding variant functions for each difference in behaviour best, or would conditional code based on an is_vc5 flag be more concise?

I'll leave that as one for @mripard to resolve when he rolls this into the upstream patchset. If we can get it working, then he can worry about the preferred way of expressing it.

Shall we strip out cec_available now it's always true?

Yes

Hardware retry is mentioned in the docs. I can't be sure it's doing useful stuff but it doesn't appear to break things. It could be dropped with no loss of functionality (but potentially less efficient).

Seeing as the CEC framework also appears to have a retry mechanism that we can plumb it into, then why not.

Maxime had mentioned that there is already a driver for the irq-brcmstb-l2 controller at https://github.com/raspberrypi/linux/blob/rpi-5.4.y/drivers/irqchip/irq-brcmstb-l2.c, but he hadn't got it to work when trying to make the HDMI I2C blocks work (they still currently poll).
It'd be nice to use that for the CEC interrupt controller instead of reimplementing the same controller code, but that can be a cleanup. The Pi3 version appears to be the same interrupt controller arrangement too, so it should be common to both platforms.

popcornmix · 2020-05-07T16:21:12Z

Updated based on comments and removed WIP.

6by9 · 2020-05-07T16:39:45Z

LGTM

pelwell

Mainly niggles, apart from the arm64 support.

drivers/gpu/drm/vc4/vc4_hdmi.c

drivers/gpu/drm/vc4/vc4_hdmi_regs.h

drivers/gpu/drm/vc4/vc4_hdmi.c

pelwell · 2020-05-07T16:42:19Z

arch/arm/configs/bcm2711_defconfig

@@ -890,6 +890,7 @@ CONFIG_DRM_PANEL_SIMPLE=m
 CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN=m
 CONFIG_DRM_V3D=m
 CONFIG_DRM_VC4=m
+CONFIG_DRM_VC4_HDMI_CEC=y


Is this not also applicable to 64-bit builds?

Untested but no reason I can think of why 64-bit build would fail.

Throw it in there, then.

6by9 · 2020-05-07T16:47:47Z

As Phil's being picky (rightly so), there's also the matter of the author and Signed-off-by being a pseudonym.

pelwell · 2020-05-07T16:51:28Z

I was going to say that we shouldn't unmask @popcornmix, but there is prior evidence in the logs.

Signed-off-by: Dom Cobley <popcornmix@gmail.com>

Fix an incorrect register address, add a missing one and reorder into address order Signed-off-by: Dom Cobley <popcornmix@gmail.com>

Signed-off-by: Dom Cobley <popcornmix@gmail.com>

2711 uses a fixed 27MHz input, earlier models use the HSM clock Signed-off-by: Dom Cobley <popcornmix@gmail.com>

Signed-off-by: Dom Cobley <popcornmix@gmail.com>

popcornmix · 2020-05-07T17:22:44Z

Updated.

kernel: vc4_hdmi: Add CEC support for 2711 See: raspberrypi/linux#3601 kernel: overlays: Move fixed-clock nodes to the root See: raspberrypi/linux#3607 kernel: bcm2835_isp fixes for constness See: raspberrypi/linux#3592 kernel: video: bcm2708_fb: Disable FB if no displays found See: raspberrypi/linux#3598 kernel: vc4_hdmi_phy: Fix typo in phy_get_cp_current See: raspberrypi/linux#3594 kernel: configs: Add missing TOUCHSCREEN_RASPBERRYPI_FW=m See: Hexxeh/rpi-firmware#223 kernel: configs: Add missing PPS configs See: raspberrypi/linux#3593 kernel: overlays: gpio-keys: Avoid open-drain warnings See: #1381 kernel: overlays: Make the i2c-gpio overlay safe again kernel: ARM: dts: bcm2711: Allow 40-bit DMA for SPI See: raspberrypi/linux#3570 firmware: In KMS mode, prevent use of the firmware rotations firmware: power: Adjust ARM:AXI divider ratio if ARM freq > 1500 MHz firmware: imx477: Remove STILLS flag from 720p mode userland: tvservice: Fix freeze on old firmware

kernel: vc4_hdmi: Add CEC support for 2711 See: raspberrypi/linux#3601 kernel: overlays: Move fixed-clock nodes to the root See: raspberrypi/linux#3607 kernel: bcm2835_isp fixes for constness See: raspberrypi/linux#3592 kernel: video: bcm2708_fb: Disable FB if no displays found See: raspberrypi/linux#3598 kernel: vc4_hdmi_phy: Fix typo in phy_get_cp_current See: raspberrypi/linux#3594 kernel: configs: Add missing TOUCHSCREEN_RASPBERRYPI_FW=m See: #223 kernel: configs: Add missing PPS configs See: raspberrypi/linux#3593 kernel: overlays: gpio-keys: Avoid open-drain warnings See: raspberrypi/firmware#1381 kernel: overlays: Make the i2c-gpio overlay safe again kernel: ARM: dts: bcm2711: Allow 40-bit DMA for SPI See: raspberrypi/linux#3570 firmware: In KMS mode, prevent use of the firmware rotations firmware: power: Adjust ARM:AXI divider ratio if ARM freq > 1500 MHz firmware: imx477: Remove STILLS flag from 720p mode userland: tvservice: Fix freeze on old firmware

popcornmix force-pushed the cec_fix branch from 5988614 to 1d9eab1 Compare May 5, 2020 18:23

6by9 reviewed May 7, 2020

View reviewed changes

popcornmix force-pushed the cec_fix branch from 1d9eab1 to 875abea Compare May 7, 2020 16:20

popcornmix changed the title ~~WIP: vc4_hdmi: Add CEC support for 2711~~ vc4_hdmi: Add CEC support for 2711 May 7, 2020

pelwell reviewed May 7, 2020

View reviewed changes

popcornmix added 8 commits May 7, 2020 18:20

vc4_hdmi: Fix register offset when sending longer CEC messages

c91e9a2

Signed-off-by: Dom Cobley <popcornmix@gmail.com>

vc4_hdmi: Fix up CEC registers

6073eb2

Fix an incorrect register address, add a missing one and reorder into address order Signed-off-by: Dom Cobley <popcornmix@gmail.com>

vc4_hdmi_regs: Add Intr2 register block

3a51340

Signed-off-by: Dom Cobley <popcornmix@gmail.com>

vc4_hdmi_regs: Make interrupt mask variant specific

5a28d49

Signed-off-by: Dom Cobley <popcornmix@gmail.com>

vc4_hdmi: Make irq shared

c3d9165

Signed-off-by: Dom Cobley <popcornmix@gmail.com>

vc4_hdmi: Adjust CEC ref clock based on its input clock

47948ef

2711 uses a fixed 27MHz input, earlier models use the HSM clock Signed-off-by: Dom Cobley <popcornmix@gmail.com>

vc4_hdmi: Remove cec_available flag as always supported

e5735e2

Signed-off-by: Dom Cobley <popcornmix@gmail.com>

config: Enable CONFIG_DRM_VC4_HDMI_CEC

80c1ffd

Signed-off-by: Dom Cobley <popcornmix@gmail.com>

popcornmix force-pushed the cec_fix branch from 875abea to 80c1ffd Compare May 7, 2020 17:21

pelwell merged commit ef04c31 into raspberrypi:rpi-5.4.y May 7, 2020

popcornmix deleted the cec_fix branch May 12, 2020 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vc4_hdmi: Add CEC support for 2711 #3601

vc4_hdmi: Add CEC support for 2711 #3601

popcornmix commented May 5, 2020

popcornmix commented May 5, 2020

6by9 May 7, 2020

popcornmix May 7, 2020

6by9 May 7, 2020

6by9 May 7, 2020

6by9 commented May 7, 2020

popcornmix commented May 7, 2020

6by9 commented May 7, 2020

pelwell left a comment

pelwell May 7, 2020

popcornmix May 7, 2020

pelwell May 7, 2020

6by9 commented May 7, 2020

pelwell commented May 7, 2020

popcornmix commented May 7, 2020

		.channel_map = vc5_hdmi_channel_map,

		.cec_mask = VC5_HDMI0_CPU_CEC_RX \| VC5_HDMI0_CPU_CEC_TX \| VC5_HDMI0_CPU_HOTPLUG_CONN \| VC5_HDMI0_CPU_HOTPLUG_REM,

vc4_hdmi: Add CEC support for 2711 #3601

vc4_hdmi: Add CEC support for 2711 #3601

Conversation

popcornmix commented May 5, 2020

popcornmix commented May 5, 2020

6by9 May 7, 2020

Choose a reason for hiding this comment

popcornmix May 7, 2020

Choose a reason for hiding this comment

6by9 May 7, 2020

Choose a reason for hiding this comment

6by9 May 7, 2020

Choose a reason for hiding this comment

6by9 commented May 7, 2020

popcornmix commented May 7, 2020

6by9 commented May 7, 2020

pelwell left a comment

Choose a reason for hiding this comment

pelwell May 7, 2020

Choose a reason for hiding this comment

popcornmix May 7, 2020

Choose a reason for hiding this comment

pelwell May 7, 2020

Choose a reason for hiding this comment

6by9 commented May 7, 2020

pelwell commented May 7, 2020

popcornmix commented May 7, 2020