tvservice: Fix freeze on old firmware.

[andryblack]

This fixes infinity loop at tvservice_wait_for_reply on initialize with older firmare. [E] No multi display support in firmware! Checked with tvservice --list

[popcornmix]

Looks plausible. @JamesH65 ?

[JamesH65]

I'll take a look tomorrow.

@andryblack How to I make it fail prior to patching?

[andryblack]

tvservice --list just hang instead show error. Any software uses bcm_host_init hang too. Firmware provided with openwrt-19.07 (use raspberrypi/firmware from rev b428bdd819df8d0ad3009b64492a4b3d1f9453e4)

[andryblack]

I used raspberry pi zero w with dsi display attached, if it matters.

[JamesH65]

I didn't think the Zero had a DSI connector....

[pelwell]

It doesn't - the DSI pins are tied off/disconnected.

[andryblack]

DPI with 24bpp, sorry

[6by9]

The patch looks reasonable to me.

[JamesH65]

I don't really have the facilities set up to test this right now, but I agree that it looks sensible. Probably ok to merge without too much investigation. Surprise though it wasn't spotted during dev work, thought I had checked that particular use case. Maybe the firmware in OpenWRT is very VERY old. Should really be updated.

[6by9]

https://github.com/raspberrypi/firmware/commit/b428bdd819df8d0ad3009b64492a4b3d1f9453e4 29 Nov 2018 "kernel: Bump to 4.14.84". Sounds pretty old to me. Why update userland if not updating firmware?

[andryblack]

raspberrypi/firmware@b428bdd 29 Nov 2018 "kernel: Bump to 4.14.84". Sounds pretty old to me. Why update userland if not updating firmware?

Openwrt dnt contains userland at official repo, builded from actual sources. https://github.com/andryblack/openwrt-build/blob/master/packages/rpi-userland/Makefile

[andryblack]

Maybe the firmware in OpenWRT is very VERY old. Should really be updated.

Yes, it is old. But openwrt releases at all looks pretty stable. It’s very difficult to make changes to their releases.

[JamesH65]

No Buts. They need to use a more recent version of firmware. 18 months old is a lot of bug fixes and code missing for the later Pi models. There should be no problems with using the latest release version as they should be backwards compatible.

[andryblack]

Anyway infinity loop at this point look like a bug.

[Ruffio]

Has this been resolved or should it be dropped?

[andryblack]

Has this been resolved or should it be dropped?

It should be merged.

[popcornmix]

This breaks tvservice:

$ tvservice -m CEA
vchi_msg_dequeue -> -1(90)
tvservice-client: Failed to query supported modes for group CEA in [vc_tv_hdmi_get_supported_modes_new_id]
Group CEA has 0 modes:

So sequence we get is loop iterates twice:

   do {
      //TODO : we need to deal with messages coming through on more than one connections properly
      //At the moment it will always try to read the first connection if there is something there
      //Check if there is something in the queue, if so return immediately
      //otherwise wait for the semaphore and read again
      success = vchi_msg_dequeue( tvservice_client.client_handle[0], response, max_length, &length_read, VCHI_FLAGS_NONE );
   } while( length_read == 0 && vcos_event_wait(&tvservice_message_available_event) == VCOS_SUCCESS);

with first iteration there is no message available yet, (success = -1, length_read = 0), so we do the vcos_event_wait and next iteration has the message available and success = 0, length_read > 0.

But the "fixed" code does:

   } while( (success == VCOS_SUCCESS) &&
            (length_read == 0) &&
            (vcos_event_wait(&tvservice_message_available_event) == VCOS_SUCCESS));

which means if message is ever not available (success = -1) on first iteration we bail out with an error. I've reverted this.