Skip to content

BCM270X_DT: Flip the polarity of the CM3's HDMI HPD. #1869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 8, 2017

Conversation

anholt
Copy link
Contributor

@anholt anholt commented Mar 2, 2017

Testing on my CM3 board, xrandr showed HDMI connected when the cable
was unplugged, and disconnected when it was plugged in.

Signed-off-by: Eric Anholt [email protected]

@anholt
Copy link
Contributor Author

anholt commented Mar 2, 2017

I haven't confirmed the fix -- I don't have a dev environment set up for booting new kernels on the CM3 yet. I'm hoping @ghollingworth might have a chance to test it.

@6by9
Copy link
Contributor

6by9 commented Mar 3, 2017

Sounds highly plausible as the VPU code reading the GPIO expander also has a notion of active high/low, and will invert the value read back based on that.

I'd have expected the same to apply to bcm2710-rpi-3-b.dts too. Is that not the case?

@anholt
Copy link
Contributor Author

anholt commented Mar 4, 2017

I've had no trouble with pi3, though, and we've had polarity of the HPD flipped both ways across other boards.

@popcornmix
Copy link
Collaborator

@6by9 perhaps this needs inverting in the firmware dtb?
I suspect if it is wrong here it may also be wrong with firmware driver.
(note: firmware driver still tries to read edid when HPD is missing, and if that succeeds we assume HPD is faulty and treat it as asserted, so a mistake may not be immediately apparent).

@6by9
Copy link
Contributor

6by9 commented Mar 6, 2017

With both a CM3 and Pi3 vcdbg log msg is showing the correct sense of hotplug attached / deassert messages as I connect and disconnect HDMI. The firmware therefore looks to have the correct sense on both boards. Inverting in the firmware would compromise hotplug when not using vc4-kms-v3d.

On both CM3 and Pi3 I've needed to set GPIO_ACTIVE_HIGH to get HDMI working ACTIVE_LOW I get the monitor going to sleep.
@anholt Could you double check your behaviour for Pi3? It'd be nice to correct both platforms at the same time, but you seem to have contradictory evidence to mine.

2 minor comments as this is the first time I've booted into the vc4 driver:

  • I don't see gpio 132 (Pi3) or 128 (CM3) claimed by any driver. Is that expected?
  • on the CM3 I get a NULL pointer dereference during DSI screen probe:
[    5.427023] Unable to handle kernel NULL pointer dereference at virtual address 00000002
[    5.435656] pgd = b95fc000
[    5.435710] [00000002] *pgd=00000000
[    5.435725] Internal error: Oops: 5 [#1] SMP ARM
[    5.435797] Modules linked in: panel_raspberrypi_touchscreen(+) evdev vc4 drm_kms_helper drm syscopyarea sysfillrect sysimgblt fb_sys_fops i2c_gpio i2c_algo_bit i2c_bcm2835 bcm2835_gpiomem uio_pdrv_genirq fixed uio i2c_dev fuse ipv6
[    5.435818] CPU: 0 PID: 175 Comm: systemd-udevd Not tainted 4.9.13-v7+ #974
[    5.435825] Hardware name: BCM2835
[    5.435833] task: b9596740 task.stack: b95f0000
[    5.435861] PC is at i2c_smbus_read_byte_data+0x24/0x54
[    5.435871] LR is at 0x2
[    5.435886] pc : [<805a3100>]    lr : [<00000002>]    psr: 00000013
sp : b95f1c60  ip : b95f1ca8  fp : b95f1ca4
[    5.435891] r10: 00000000  r9 : ba7297cc  r8 : 00000001
[    5.435897] r7 : b9a9e408  r6 : 00000000  r5 : b9793350  r4 : b9a9e400
[    5.435902] r3 : 00000000  r2 : 00000000  r1 : 00000080  r0 : 00000000
[    5.435910] Flags: nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[    5.435916] Control: 10c5383d  Table: 395fc06a  DAC: 00000055
...
[    5.436434] [<805a3100>] (i2c_smbus_read_byte_data) from [<7f205424>] (rpi_touchscreen_dsi_probe+0x9c/0x14c [panel_raspberrypi_touchscreen])
[    5.436490] [<7f205424>] (rpi_touchscreen_dsi_probe [panel_raspberrypi_touchscreen]) from [<804f310c>] (mipi_dsi_drv_probe+0x28/0x2c)
[    5.436517] [<804f310c>] (mipi_dsi_drv_probe) from [<804f9840>] (driver_probe_device+0x220/0x2cc)
[    5.436538] [<804f9840>] (driver_probe_device) from [<804f99a8>] (__driver_attach+0xbc/0xc0)

I guess that is something else missing from the CM3 DT (i2c_dsi by the looks of it), but possibly intentionally so as the display isn't enabled by default on CMs.

@anholt
Copy link
Contributor Author

anholt commented Mar 6, 2017

Updated the branch now to fix pi3b as well (I hadn't tested it since expgpio landed), along with backporting @Electron752's cursor update patch and fixing a regression in DSI panel support in full kms mode.

@6by9
Copy link
Contributor

6by9 commented Mar 7, 2017

Fine by me - glad I wasn't going mad over Pi3 needing to be corrected too.
@pelwell any comments?

@Electron752
Copy link
Contributor

Since I got e-mail about my cursor patch being added, do people want a similar fix for fkms? I can understand though if people think it's better to just leave fkms alone.(Probably should be a separate pull request).

I got e-mail from someone giving feedback about the cursor fix that it's actually the version of the xserver that causing the update flood and not the desktop environment. So this may become an issue if Raspbian moves to a newer version of Debian.

@pelwell
Copy link
Contributor

pelwell commented Mar 7, 2017

Please confirm that the HPD polarity of the various Pi models really does differ. After the early models (A & B) I would expect them all to be active low, suggesting that the discrepancy may lie with the common element - the GPIO expander.

@6by9
Copy link
Contributor

6by9 commented Mar 7, 2017

I tested it yesterday.
All prior models Linux will have gone direct to the GPIO block and read the state. Linux therefore had to handle the inversion, hence ACTIVE_LOW.
With expgpio the firmware has already handled the inversion based on dt-blob.bin (needed so the firmware HPD logic works correctly), so Linux now sees it as ACTIVE_HIGH.

Without this patch neither CM3 nor Pi3 bring up the HDMI display. With it they do. That makes me agree with Eric that this is the correct solution.

@pelwell
Copy link
Contributor

pelwell commented Mar 7, 2017

I can imagine a time when the ARM takes over managing the GPIO expander, at which point the declarations become wrong. DT is supposed to document the hardware, not the hardware after it has been munged by the intervening layers of software.

The problem is that the mailbox interface for controlling the GPIOs is wired in at the wrong level. Either that wiring needs to be refactored or the mailbox interface needs to renormalise at the interface to the ARM.

@6by9
Copy link
Contributor

6by9 commented Mar 7, 2017

The problem is that the mailbox interface for controlling the GPIOs is wired in at the wrong level. Either that wiring needs to be refactored or the mailbox interface needs to renormalise at the interface to the ARM.

OK, I'll refactor the firmware code so the mailbox gets the raw values.

Electron752 and others added 2 commits March 7, 2017 14:23
Commonly used desktop environments such as xfce4 and gnome
on debian sid can flood the graphics drivers with cursor
updates.  Because the current implementation is waiting
for a vblank between cursor updates, this will cause the
display to hang for a long time since a typical refresh
rate is only 60Hz.

This is unnecessary and unexpected by user mode software,
so simply swap out the cursor frame buffer without waiting.

Signed-off-by: Michael Zoran <[email protected]>
Reviewed-by: Eric Anholt <[email protected]>
Commit 488f9bc slightly increased the
reported rate of PLLD, so the clk driver decided that PLLD/3/8 was now
higher than our requested pixel clock rate and rejected it in favor of
PLLD/4/8, which then ran the pixel clock way out of spec.

By bumping the requested clock rate just slightly, we get back to
PLLD/3/8 like we wanted and the panel displays content again.

Signed-off-by: Eric Anholt <[email protected]>
@anholt
Copy link
Contributor Author

anholt commented Mar 7, 2017

I've rebased out the HPD change on my branch, since the mbox interface is going to change.

@Electron752
Copy link
Contributor

Since the expander is no longer on the list, is their any reason this request can't be accepted? Both of the addressed issues are relatively severe.

@pelwell pelwell merged commit 073d94f into raspberrypi:rpi-4.9.y Mar 8, 2017
@pelwell
Copy link
Contributor

pelwell commented Mar 8, 2017

Eric, I notice that your touchscreen driver hasn't made it into rpi-4.10.y - is there any reason for me not to cherry-pick it across?

@anholt
Copy link
Contributor Author

anholt commented Mar 8, 2017

Please feel free!

I'm trying to maintain backports to whichever branches I think you might switch over to by default, but I don't think I have time to maintain every rpi-* branch. If switching to a newer branch is being considered, please let me know and I'll try to fix it up.

popcornmix added a commit to raspberrypi/firmware that referenced this pull request Mar 13, 2017
kernel: BCM270X_DT: Invert Pi3 power LED to match fw change
See: raspberrypi/linux#1879

kernel: Add support for the AudioInjector Octo sound card
See: raspberrypi/linux#1884

kernel: Fix gadget mode for bcm2835
See: raspberrypi/linux#1887

kernel: bcm2835-v4l2: Fix buffer overflow problem
See: raspberrypi/linux#1890

kernel: Add support for Fe-Pi audio sound card
See: raspberrypi/linux#1867

kernel: BCM270X_DT: Flip the polarity of the CM3's HDMI HPD
See: raspberrypi/linux#1869

kernel: Add overlay for ads1115 ADCs
See: raspberrypi/linux#1864

kernel: config: Add CONFIG_CRYPTO_LZ4
See: raspberrypi/linux#1875

kernel: clk-bcm2835: Read max core clock from firmware

firmware: GPIO expander: rework so the mailbox service reads raw values
firmware: arm_loader: Check GPIO direction for low_voltage
See: raspberrypi/linux#1879

firmware: warnings: Fix some mostly spurious warnings

firmware: clock: Calculate PLL multipliers with more precision
firmware: Set up HDMI VCO same for VEC as for HDMI

firmeare: gpu_server: Move detailed logging to LOGGING_VMCS_VERBOSE category
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Mar 13, 2017
kernel: BCM270X_DT: Invert Pi3 power LED to match fw change
See: raspberrypi/linux#1879

kernel: Add support for the AudioInjector Octo sound card
See: raspberrypi/linux#1884

kernel: Fix gadget mode for bcm2835
See: raspberrypi/linux#1887

kernel: bcm2835-v4l2: Fix buffer overflow problem
See: raspberrypi/linux#1890

kernel: Add support for Fe-Pi audio sound card
See: raspberrypi/linux#1867

kernel: BCM270X_DT: Flip the polarity of the CM3's HDMI HPD
See: raspberrypi/linux#1869

kernel: Add overlay for ads1115 ADCs
See: raspberrypi/linux#1864

kernel: config: Add CONFIG_CRYPTO_LZ4
See: raspberrypi/linux#1875

kernel: clk-bcm2835: Read max core clock from firmware

firmware: GPIO expander: rework so the mailbox service reads raw values
firmware: arm_loader: Check GPIO direction for low_voltage
See: raspberrypi/linux#1879

firmware: warnings: Fix some mostly spurious warnings

firmware: clock: Calculate PLL multipliers with more precision
firmware: Set up HDMI VCO same for VEC as for HDMI

firmeare: gpu_server: Move detailed logging to LOGGING_VMCS_VERBOSE category
@6by9
Copy link
Contributor

6by9 commented Mar 14, 2017

FYI the firmware change for expgpio was released last night. HDMI hotplug seems to work for me using vc4-kms-v3d.
Sorry for the slight delay in getting there.

@pelwell
Copy link
Contributor

pelwell commented Mar 14, 2017

You can blame me for the delay.

@schnitzeltony
Copy link
Contributor

FWIW: NULL pointer dereference during DSI screen probe mentioned by @6by9 is still there on Pi2 latest 4.9 kernel/firmware. Open another issue for this?

@anholt
Copy link
Contributor Author

anholt commented Mar 20, 2017

Yes, please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants