drm/vc4: hdmi: Add a workqueue to set scrambling

[mripard]

It looks like some displays (like the LG 27UL850-W) don't enable the scrambling when the HDMI driver enables it. However, if we set later the scrambler enable bit, the display will work as expected.

Let's create delayed work queue to periodically look at the display scrambling status, and if it's not set yet try to enable it again.

This should fix the issue that has been reported by some users where 4k @ 60Hz would leave the display completely blank, without any other symptom.

[6by9]

cancel_delayed_work_sync(&vc4_hdmi->scrambling_work); in vc4_hdmi_unbind?

Otherwise looks good.

[mripard]

I'm not sure we do? I think that you would have the encoder disable hook called prior to the unbind.

[6by9]

I'll trust you on that one.

[mripard]

I checked, and vc4_drv_unbind calls drm_atomic_helper_disable_all which indeed disables the whole pipeline.

[HiassofT]

I tested latest 5.10.y branch (commit 7d9d969) plus PR #4311 (which should be unrelated) and I still have the issue I reported on LibreELEC slack last week:

If the video mode is set to 3840x2160p50-60 and I put my LG 55C8 TV into standby and back on it reports no signal. This doesn't happen with 4k modes and refresh rates <=30Hz. Switching to a different mode or refresh rate (eg from 3840x2160p60 to p50) brings back the signal.

Here's dmesg http://ix.io/3m7I - I put the TV to standby and back on 2 times and each time got 2 "hotplug" log entries on power-on (with drm.debug=0x04)

[  169.578070] vc4-drm gpu: [drm:drm_client_dev_hotplug] fbdev: ret=0
[  172.713277] vc4-drm gpu: [drm:drm_client_dev_hotplug] fbdev: ret=0
[  359.794199] vc4-drm gpu: [drm:drm_client_dev_hotplug] fbdev: ret=0
[  362.993903] vc4-drm gpu: [drm:drm_client_dev_hotplug] fbdev: ret=0

[mripard]

Do you have i2cset on that system so that we can check if this is indeed the scrambler or something else?

[HiassofT]

@mripard yes, I have i2c-tools available here, just ping me what info you need / which commands I should run

[mripard]

If you could run i2cset -y 11 0x54 0x20 0x3 (or 12 if it's connected on HDMI1), and let me know if it works it would be great.

I just tested on my LG 48CX, but unfortunately it appears that it doesn't support any mode where the scrambler would be on. Your test seems to work though, so it looks like it might be a similar issue to the one we fixed here.

[mripard]

I think I found what the issue is. If the C8 is anything like the CX in that regard, the TV will still allow the EDID to be read while it pulls HPD low, while our current detection code is (stripped down of the irrelevant stuff for that particular issue)

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);
	enum drm_connector_status ret = connector_status_disconnected;
	bool connected = false;

	if (vc4_hdmi->hpd_gpio) {
		if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^
		    vc4_hdmi->hpd_active_low)
			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 (connected) {
		ret = connector_status_connected;
		goto out;
	}

	cec_phys_addr_invalidate(vc4_hdmi->cec_adap);

out:
	return ret;
}

The RPi4 doesn't have an HPD GPIO, so we first read the EDID to determine if the display is connected or not, and if it succeeds we report the display as connected. This means that the Pi4 will detect the screen as on all the time, therefore there's no need to change anything. Meanwhile, the TV is likely to have cleared any setup it has done with the hotplug pulse and therefore relies on the configuration to be done again.

This was working before because we weren't using the modes using the scrambler at all, so the display was completely out of the equation. Now, we need to signal to the display we want the scrambler to be enabled and we fail to do so since we aren't aware anything changed, really.

I couldn't find anything related to whether we could read or not the EDID while HPD was low, so I'm not sure whether it's something we can consider, but removing the call to drm_probe_ddc in the snippet above makes it behave normally here.

[popcornmix]

I think displays that don't have working hotplug but do allow reading the edid do exist.
We have similar logic in the firmware code, but it's a little different.

If on boot hotplug is low, we still try to read the edid. If that works we consider hotplug to be faulty and assume it is always asserted from then on.

That means a dodgy display that lacks hotplug but has a valid edid can still be used - just hotplug is ignored.
If display had hotplug asserted when we started, then we will respect it going low (regardless of ability to read edid).

[HiassofT]

I did a few tests and the i2cset command you mentioned worked - TV reported signal again.

I also checked again with fkms and that worked just fine - putting TV out of standby reliably showed the video signal

[mripard]

It looks like the reason to rely on DDC in the first place was something simpler: judging from 9d44abb it was needed because back then the Pi3 didn't have a driver for the GPIO expander that the HPD GPIO is connected to. However, b1b8f45 then changed that and it shouldn't be needed aside from what @popcornmix described.

I'd be inclined to just drop it for now, and if the broken-hpd case ever arises, we can add the check you were describing?

[popcornmix]

Okay. I guess that keeps the majority happy.