supplierdeeply

HDMI power and alternative remote inputs ( was Re: G7 X porting thread)

  • 29 Replies
  • 824 Views
*

Offline srsa_4c

  • ******
  • 3629
Re: Re: G7 X porting thread
« Reply #20 on: 10 / June / 2018, 16:45:21 »
Advertisements
- The HDMI power related function will likely need to be called after every play -> rec transition on a camera that disables HDMI power in rec mode.
yes, you are right. the code for that is already prepared in the patch. from shooting_set_playrec_mode() the camera specific function pf_notify_modeswitch() is called. Here the HDMI power can be enabled. (depending on mode, and if HDMI is used as remote input)
I did not explain fully what I meant. Doing this in shooting_set_playrec_mode() is only okay if you control the rec<->play transitions from a script. In any other cases (say, user operates camera by hand), it won't be executed. There is a function somewhere (mode_get() in core/shooting.c) that updates state variables in conf structure, placing the hdmi mode control there would be more appropriate, I believe.

Quote
- It might be beneficial to mask the HDMI hotplug bit (if it is even possible), so that the camera won't do unnecessary work when that bit is toggled.
yes, you are right. also this is already prepared in the camera specific function set_pf_remote_override(). it is called from kbd_common.c
Is that effective for the HDMI bit? Comparing camera log with/without masking that bit while toggling the pin would probably show whether masking is effective.

The conf variable I meant is conf.remote_input_channel (which will be camera dependent), but it's not a big deal at this stage.

Re: Re: G7 X porting thread
« Reply #21 on: 11 / June / 2018, 03:35:25 »
- The HDMI power related function will likely need to be called after every play -> rec transition on a camera that disables HDMI power in rec mode.
yes, you are right. the code for that is already prepared in the patch. from shooting_set_playrec_mode() the camera specific function pf_notify_modeswitch() is called. Here the HDMI power can be enabled. (depending on mode, and if HDMI is used as remote input)
I did not explain fully what I meant. Doing this in shooting_set_playrec_mode() is only okay if you control the rec<->play transitions from a script. In any other cases (say, user operates camera by hand), it won't be executed. There is a function somewhere (mode_get() in core/shooting.c) that updates state variables in conf structure, placing the hdmi mode control there would be more appropriate, I believe.
Thanks srsa for pointing that out, i will thake a look and most probaly update the patch

Quote
- It might be beneficial to mask the HDMI hotplug bit (if it is even possible), so that the camera won't do unnecessary work when that bit is toggled.
yes, you are right. also this is already prepared in the camera specific function set_pf_remote_override(). it is called from kbd_common.c
Is that effective for the HDMI bit? Comparing camera log with/without masking that bit while toggling the pin would probably show whether masking is effective.
i will do a log with/without maskin and compare it.

G7X - CHDK 100d

i have updated the patch based on srsa's notes, so the HDMI power is now enabled in the context of mode_get() if the mode changes to shooting and remote(using HDMI hotplug detect) is active. This is done just once because from earlier posts i know the HDMI switch is maybe done using I2C communication. I have also added the stubs definition for EnableHDMIPower() and DisableHDMIPower() to stubs_entry_2.S for all 3 G7X firmware versions. I have already checked that the addresses are the same on all firmware's.

I have also done some logs using ShowCameraLog with and without masking the HDMI hotplug detect bit.
At least on the G7X it makes no difference if the bit is masked or not, the camera shows no reaction in the log in both cases.
But to be on the safe side the bit is still masked in the code.

The is one additional change in generic/wrappers.c because i wanted to use dbg_printf() from there, but it was crashing. After replacing _ExecuteEventProcedure("Printf",s) with _LogCameraEvent(0x20,"%s", s) it worked. but the output was just in the camera log. Is there a way to use printf from code (to be used with UART redirection)?

G7X - CHDK 100d

*

Offline reyalp

  • ******
  • 11336
The is one additional change in generic/wrappers.c because i wanted to use dbg_printf() from there, but it was crashing. After replacing _ExecuteEventProcedure("Printf",s) with _LogCameraEvent(0x20,"%s", s) it worked.
I think this is a digic 6 issue build issue

The _ExecuteEventProcedure stub is ARM (NHSTUB2) to be compatible with the callfunc.S code. On pre-digic 6 cams, wrappers is compiled to arm, so it works. On digic6, everything is thumb2, but it doesn't know to interwork.

You could add a stub for Printf and call it directly, or call _ExecuteEventProcedure via call_func_ptr like the normally thumb parts of the code (e.g. gui.c save_romlog)

Thanks for working on the remote stuff, I haven't had time to look at it closely but will try to soon.
Don't forget what the H stands for.


*

Offline srsa_4c

  • ******
  • 3629
I took the v2 patch and simplified the menu related parts, using an additional platform_camera.h macro. Did not touch the rest.

*

Offline reyalp

  • ******
  • 11336
Some general comments. No need to address any of this in the initial patch.

A script controllable output is a feature that has been requested a few times, for UAV and micro-controller applications. Controlling HDMI power from script independent of the remote stuff would be potentially valuable. This this might interact with the stuff pf_notify_modeswitch, but not using HDMI as a remote input while using it as general output seems a reasonable restriction.

Script sees the remote through "key_remote" (modules/script_key_funcs.c) and get_usb_power. It looks like both will reflect the currently selected channel (this should be tested). This makes get_usb_power poorly named.

It would also be potentially useful to allow script to access all the channels independent of which is selected as the remote. The battery temp channel can already be accessed using get_temperature.

I experimented with sx160 a little bit:
HDMI +5v is only present in playback.
EnableHDMIPower/DisableHDMIPower similar to the g7x functions exist. On this camera, they boil down to
HdmiHpd5VOutput(state)
ChangePowerState(state)
These functions are identifiable (on both sx160 and g7x, so probably a usefully large number of cameras) by a reference to "HDMIConnectCnt", which should be quite easy to add to the sig finder. This string doesn't seem to be present on cameras without HDMI out (elph130, elph180) I don't see a HdmiHpd5VOutput eventproc on g7x


A HDMI to DVI adapter makes it easy to probe HDMI pins with a multimeter. Pinout on wikipedia: https://en.wikipedia.org/wiki/Digital_Visual_Interface#/media/File:DVI_pinout.svg
« Last Edit: 18 / June / 2018, 00:21:31 by reyalp »
Don't forget what the H stands for.

*

Offline reyalp

  • ******
  • 11336
I added EnableHDMIPower and DisableHDMIPower to finsig_thumb2. Unfortunately, the string match didn't work out: Cameras after r57 don't have that log message, and some had literals in the function that made the Disable match fail. I used a more complicated match on "HecHdmiCecPhysicalCheckForScript", which seems valid for all known digic 6 cameras.

edit:
Slightly updated version of srsa's patch, it just fixes the conditional on LANG_MENU_REMOTE_INPUT_CHANNEL and removes the stubs_entry_2.s changes that are no longer needed.

Some other stuff I had in mind but didn't get to:
Updating the remote setting should update the hdmi power state if needed. Currently if you are in rec mode and switch to hmdi remote, the power won't get turned on until you switch to play and back. This could maybe be a conf_info_func function like conf_change_dng

I'm not sure putting the implementation in platform pf_... functions is the right approach. I'd expect the HDMI stuff (and the battery terminal remote) to work the same on all cameras that have it (+ some constants and maybe a low level read function), so I think most of it should be under feature specific ifdefs, maybe in kbd_common.c. In the g7x kbd.c get_remote_bit and the pf functions would all be generic for any platform that had those combination of features. This might seem like a cosmetic choice, but with > 160 platforms I really like to keep the copy/paste in check where possible.

I'll look into this more later. One thing that might help is identifying the channels by type (e.g. REMOTE_CHANNEL_USB, REMOTE_CHANNEL_BATT_TEMP...) rather menu index.
« Last Edit: 18 / June / 2018, 03:13:26 by reyalp »
Don't forget what the H stands for.

Thank you for adding the HDMI Functions to signature finder.

Some other stuff I had in mind but didn't get to:
Updating the remote setting should update the hdmi power state if needed. Currently if you are in rec mode and switch to hmdi remote, the power won't get turned on until you switch to play and back. This could maybe be a conf_info_func function like conf_change_dng
This should be already working, it is handled in pf_notify_modeswitch(), but to be sure i will test it again.

I'm not sure putting the implementation in platform pf_... functions is the right approach. I'd expect the HDMI stuff (and the battery terminal remote) to work the same on all cameras that have it (+ some constants and maybe a low level read function), so I think most of it should be under feature specific ifdefs, maybe in kbd_common.c. In the g7x kbd.c get_remote_bit and the pf functions would all be generic for any platform that had those combination of features. This might seem like a cosmetic choice, but with > 160 platforms I really like to keep the copy/paste in check where possible.
The background for choosing this aproach was that theoretical you can have every combination of supported remote inputs.
And then you also have to take care in common code to be able to handle every combination of USB,ADC,HDMI and in future maybe AV input. So the idea was that in common code just the feature itself (MULTICHANNEL_REMOTE) is known, but the specifics and how to do the physical access is done in platform specific code. i.e on my S100 the HDMI 5V was always on, so there is no need to turn it on, maybe there is even no such function. Such things must then be handled in common code too.
   
G7X - CHDK 100d


*

Offline reyalp

  • ******
  • 11336
This should be already working, it is handled in pf_notify_modeswitch(), but to be sure i will test it again.
Your right, I missed that would be triggered by a setting change as well as mode change. It will not turn off if you change turn the remote off or switch it back to USB. I'm not sure what the most desirable behavior is, but I would lean toward turning it off if the camera normally keeps it off in rec mode.

Quote
The background for choosing this aproach was that theoretical you can have every combination of supported remote inputs.
Yes, it flexible. However, IMO there will be a handful of different remote types, while there are > 160 platforms. Anyway, you don't have do anything about this, I definitely appreciate your work and will post an updated patch when I have some time.
Don't forget what the H stands for.

*

Offline srsa_4c

  • ******
  • 3629
Following patch is for newer D4 and D5 models. Older ones don't have the string and the functions may not exist at all.
Did not commit this because I'd like to see whether the hits are reasonable first.
Code: [Select]
Index: tools/finsig_dryos.c
===================================================================
--- tools/finsig_dryos.c (revision 5052)
+++ tools/finsig_dryos.c (working copy)
@@ -507,6 +507,8 @@
     { "SS_MFOff", OPTIONAL },
 
     { "GetAdChValue", OPTIONAL },
+    { "EnableHDMIPower", OPTIONAL },
+    { "DisableHDMIPower", OPTIONAL },
 
     { 0, 0, 0 }
 };
@@ -2088,6 +2090,8 @@
     //                                                                           R20     R23     R31     R39     R43     R45     R47     R49     R50     R51     R52     R54     R55     R57     R58     R59
     { 23, "UnregisterInterruptHandler", "HeadInterrupt1", 76,                    1,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1 },
     { 23, "get_string_by_id", "NoError", 16,                                    99,     99,     99,     99,     99,     99,     99,     99,     99,     99,     99,     99,     99,     99,     99,     -2 },
+    { 23, "EnableHDMIPower", "HDMIConnectCnt", 9,                               99,     99,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1,      1,},
+    { 23, "DisableHDMIPower", "HDMIConnectCnt", 9,                              99,     99,      3,      3,      3,      3,      3,      3,      3,      3,      3,      3,      3,      3,      3,      3,},
 
     //                                                                           R20     R23     R31     R39     R43     R45     R47     R49     R50     R51     R52     R54     R55     R57     R58     R59
     { 24, "get_string_by_id", "StringID[%d] is not installed!!\n", 64,           0xf000, 0xf000, 0xf000, 0xf000, 0xf000, 0xf000, 0xf000, 0xf000, 0xf000, 0xf000, 0xf000, 0xf000, 0x0000, 0x0000, 0x0000, 0xf000 },
edit:
I checked this in, after reviewing the results. The functions are quite varying, some are simple, some are complicated.
« Last Edit: Today at 16:58:37 by srsa_4c »