CHDK PTP interface

srsa_4c

Thanks for the recv_ptp_data details.

I found something in disassemblies that might be related to the r31 problem.
There is a cpu cache library of some sort in the firmware. It has approx. 4 routines, the last one is unused in many cameras. Routine #3 (FF810750 in d10) is used in usb-related functions. The first routine is called from the MemoryChecker eventproc (that's one way to find them).
From a not too large sample, I found that an older version of #3 is used in
a470 (r23), g10 (r31). A newer version is used in ixus110 (r31 with problem), d10 (r31 with problem), a3200 (r47).
Now, I did not check whether the old and new #3 are doing exactly the same, but if they differ in operation that might be the reason behind the problem.
In case they do operate the same, this could still be used to identify the possible problem models, I think.

reyalp

srsa_4c
Now, I did not check whether the old and new #3 are doing exactly the same, but if they differ in operation that might be the reason behind the problem.
In case they do operate the same, this could still be used to identify the possible problem models, I think.

Nice catch. I had sort of assumed it was a hardware difference, but could easily be a difference in the code that handles the transfer. I'll take a closer look at those cache functions later.

reyalp

Function 3 is something like
cache_control(cachetype,address,length)

If cachetype is 1, it jumps into the middle of the preceding function and flushes the instruction cache

If length is -1 or > 0x2000 (== 8k == size of the caches)
it disables interrupts, cleans and flushes the data caches by iterating index/segment, and then drains the write buffer.

Otherwise, it flushes and cleans the specified range (without disabling interrupts) and drains the write buffer.

In the < 0x2000 case, D10 uses separate instructions for clean and flush:

MCR     p15, 0, R1,c7,c10, 1
MCR     p15, 0, R1,c7,c6, 1

while g10 uses the combined instruction:

MCR     p15, 0, R1,c7,c14, 1

It seems like this difference should be cosmetic, but maybe there's a subtle difference with interrupts or something.

In the whole cache (length >0x2000 or -1) case, d10 the drains the write buffer for every iteration, before calling the flush and clean function 😐:

MCR     p15, 0, R4,c7,c10, 4
MCR     p15, 0, R2,c7,c14, 2

G10 just does flush and clean. Both drain write buffer at the end. Draining for every iteration seems weird to me.

ELPH130 seems identical to the D10 code, while ELPH180 is quite different. It looks like they got rid of the cache type argument.

reyalp

It occurred to me that the workaround in https://chdk.setepontos.com/d/4338/1178 is only unsafe at the ends, where it could potentially not be aligned on a cache line boundary.

To avoid this, we could allocate a small uncached buffer just for the ends.
1) Read the first little bit into the temp buffer
2) clean dcache
3) read main buffer + however much was read, up to the last full cache line, uncached,
4) clean and flush dcache
5) memcpy the first bit into the start of the buffer
6) read the last bit, memcpy that in.

This is annoyingly complicated to work around a corner case on a few cameras, but should be safe, negligible impact on performance or available memory, and completely contained in recv_ptp_data without requiring changes in the callers. For small transfers, it could just use the temp buffer and memcpy into whatever was passed to recv_ptp_data.

edit: or I guess transfers smaller than 0x23f4 could not do any cache workarounds, but but I'm somewhat hesitant to trust it.

reyalp

I implemented the idea in the previous post. It works great on d10... and fails miserably on a540.

Having platform specific versions would be OK, but it's hard to guess which they should be applied to.

On further investigation, a540 seems to have problems with multiple recv_data calls at certain sizes, and appears to have more problems that I haven't fully figured out when the memory is uncached, even when using umalloc and not doing any of the cache manipulation stuff.

Reading in blocks of 512, connection fails at sizes
1013-1015
1525-1527
etc
It only occurs on the second or later read, so with a 1024 block size, the first error happens at 1525

I tried changing the first read to 500 rather than 512, and the error occurred at the same points.

I haven't verified yet, but I expect the trunk code suffers from this, it just doesn't get hit often because it only affects multiple reads and certain magic sizes. The only time the trunk code does multiple reads is for file uploads that are > 1/2 max free block. On a540, diskboot.bin comes in less than that.

srsa_4c

reyalp
The only time the trunk code does multiple reads is for file uploads that are > 1/2 max free block.

Perhaps it's time to see how the 100d handler is doing it? I tried uploading files that are larger than the fw's usb buffer, and it worked. No idea whether it suffers from similar problems though.
I think it's possible to remove that handler at runtime and add our own.

Side note: r54 and newer cams use a dedicated fw usb buffer instead of exmem.

reyalp

srsa_4c
Perhaps it's time to see how the 100d handler is doing it? I tried uploading files that are larger than the fw's usb buffer, and it worked

Yeah, that's definitely something to look at. I didn't try SendObject on A540 when I was playing with it earlier, because it doesn't list FIR files as objects, so it's not obvious what the ObjectInfo should contain. But I didn't dig into it much.

A540 (and likely other vxworks cameras) appear to have another, unrelated problem, present in the current trunk. Rapid fire messages like

!m=require'extras/msgtest'
!m.test{size=1,sizeinc=1,count=10000,gc='step'}

occasionally crash (often after 3000-5000 iterations, but variable, sometimes it goes 10k without problems)

I got a couple romlogs for this in memPartInfoGet, (LR = 0xffee1444, ultimately from core_get_free_memory in handle_ptp) which looks related detecting memory corruption. I wondered if memPartInfoGet / malloc / free need to be protected with a semaphore or something, but they don't appear to be in the Canon code, and the VxWorks documentation doesn't obviously suggest that they should be.

Trying to reproduce this today, I got failures

send 7445 size:7445 0x1d15...Protocol error: data expected

After one of those, the camera crashed with an "Assert Memory.c line 67" which appears to be from malloc returning 0.

It's unclear whether these are related to PTP, or two tasks rapidly malloc/freeing, or some other memory corruption.

I think it's possible to remove that handler at runtime and add our own.

Another thing I've been thinking about is hooking the ptp_data functions, either by hooking TrnsCtrlTask which sets them up, ~~or just replacing them on the first ptp call~~ (this does not work). My impression is that the ptp_data structure is set up once per session at most. This would allow logging what the stock operations do.

reyalp

Despite not listing FIR files, uploading format BF01 works on A540. Like newer dryos cameras, it asserts if the path includes "A/".

The following successfully creates a 4 MB TEST.DAT in the SD root.

!newf={ Filename="TEST.DAT", ObjectFormat=0xBF01, ObjectCompressedSize=1024*1024*4 } return con:ptp_send_object_info(newf)
!return con:ptp_send_object(('x'):rep(newf.ObjectCompressedSize))

reyalp

srsa_4c
I think it's possible to remove that handler at runtime and add our own.

This works. Attached patch hooks opcode 0x100d (SendObject) for A540.

The stock handlers get installed when when the USB cable is detected (PTPRspnd_StartUpPTPFrameworkClient) and removed when it is disconnected (PTPRspnd_ShutDownPTPFrameworkClient). So you have to replace it later after boot. It this case, I modified the rmem handler so rmem 1 4 installs the hook.

PTP session connect/disconnect and reset don't reset the handlers.

calling ptp_add_handler without removing first returns 0x13, but seems to still replace the handler.

On a540, the SendObject handler appears to have 256K buffer in uncached memory. Every call to recv_data gets the same address (0x11f80000). It does not appear to have the n*512 - 11 problem our code has.

Unrelated to the current problems:
Hooking the handlers (specifically GetObjectHandles) might provide us a way to avoid the black screen / no rec issue that happens when non-CHDK software access the cameras.

On a540 at least, GetObjectInfo and GetObject still work if GetOjectHandles hasn't been called, even in rec mode (the handles appear to just be sequential numbers starting at 2), which suggests the black screen mode isn't deeply tied to the rest of the PTP code.

a540-hook-ptp-sendobj-1.patch

4.38KB

srsa_4c

reyalp
On a540, the SendObject handler appears to have 256K buffer in uncached memory. Every call to recv_data gets the same address (0x11f80000).

I experimented a bit on the a3200, and found that there are code paths using cached and uncached memory, so both are "legal" there. The uncached buffer is also 256k. The return value of recv_data is checked (bit0).

It does not appear to have the n*512 - 11 problem our code has.

How can I test this?

reyalp

srsa_4c

It does not appear to have the n*512 - 11 problem our code has.
How can I test this?

To test if it happens with CHDK code, you can use the attached patch, which buffers reads in TBUF_SIZE chunks. You can switch between malloc/free and umalloc/ufree.

On a540, running msgtest with

m.test{size=1,sizeinc=1,count=10000,gc='step'}

will fail at 1013 with a stalled connection and IO error on the chdkptp side. If you change the size to 1024, it will then fail at 1525, and so on for multiples of 512.

You could of course just test around that size (start with size=1000) but given the different magic values on different cameras, covering a larger range seems like a good idea.

To reproduce this in unmodified, stock code, you'd have to arrange a file upload that was n*512-11 (including filename and size int) and larger than half the largest free block. I haven't done this for a540.

To see if the Canon code was affected, I tried SendObject with sizes that would result in multiple reads and fall on a 512-11 boundary, like

!newf={ Filename="TEST.DAT", ObjectFormat=0xBF01, ObjectCompressedSize=256*3*1024-11 } con:ptp_send_object_info(newf) con:ptp_send_object(('x'):rep(newf.ObjectCompressedSize))

which did not fail.

On d10, the patch works fine with with 512 buffer size, albeit slowly with either malloc or umalloc. I expected this to fail at 0x23f4 or 0x43f4 but ...

With a buffer size of 0x3000 in cached memory, it fails at 0x23f4. Uncached works fine.

With a 0x23f0 buffer, both cached and uncached crash with an ASSERT!! BulkTrns.c Line 1549 at 0x23f5

Sorry for the random notes on d10, I went to confirm it behaved as I expected from previous tests and it didn't 😳

ptp-buffered-test-1.patch

1.14KB

reyalp

I looked at the PTP buffer functions some on D10.

As srsa_4c mentioned earlier, the first ComMemMan function (d10 sub_FF8C9114) allocates a 1 MB uncached exmem buffer.
This appears to be done in PTP startup, it's already allocated once a chdkptp connection is established.

On d10 (without CHDK exmem enabled) the buffer address is ~~0x43E42E00~~ 0x43E02E00
Comments in makefile.inc indicate the video buffer starts at 0x43F02E00, exactly 1MB after the start of the PTP buffer. This raises the question of whether some exmem types automatically stay clear of the video buffer.

The second ComMemMan (d10 sub_FF8C91A4) function frees the exmem buffer.

The third (d10 sub_FF8C922😎 is called by individual PTP functions that need a buffer. My impression is the big exmem buffer is split up into 7 "slots" used for different things. The main buffer for SendObject is slot 4 (edit: or 5, for dryos 43 through 52).

sub_FF8C9374 appears to return a size for each slot

con 1> =return call_func_ptr(0xFF8C9374,0)
2:return:8192
con 2> =return call_func_ptr(0xFF8C9374,1)
3:return:2048
con 3> =return call_func_ptr(0xFF8C9374,2)
4:return:16384
con 4> =return call_func_ptr(0xFF8C9374,3)
5:return:235520
con 5> =return call_func_ptr(0xFF8C9374,4)
6:return:524288
con 6> =return call_func_ptr(0xFF8C9374,5)
7:return:0
con 7> =return call_func_ptr(0xFF8C9374,6)

sub_FF9F6B28 is used by the SendObject code. It allocates slot 4 if needed (using the size from sub_FF8C9374 above), and returns the address. In my testing it was always ~~the start of the exmem buffer~~ 0x43e42e00.

This suggests that it would be fairly easy to use the stock Canon buffer, at least where it doesn't conflict with video stuff. Only one PTP operation can be in progress at a given time, so I think there's little risk of conflicting with Canon firmware code for slot 4. (other slots are used by other parts of the PTP code, e.g. something in BulkTrns.c uses slot 1)

Test patch for D10 attached. Unsurprisingly, the various msgtest tests work fine with this, since the buffer is uncached and far larger than usable message sizes.

It's probably not practical switch all ports to this approach. My current thought platform specific functions that fall back to malloc in the default case, but for cameras without a dedicated buffer, it's desirable to transfer directly to the destination address for script messages.

edit:
The same approach works for A540 using

NHSTUB(get_ptp_buf_size,            0xFFE38264)
NHSTUB(get_ptp_file_buf,            0xFFE5749😎

d10-canon-ptp-buf-work-1.patch

2.39KB

srsa_4c

reyalp
On d10 (without CHDK exmem enabled) the buffer address is 0x43E42E00
Comments in makefile.inc indicate the video buffer starts at 0x43F02E00, exactly 1MB after the start of the PTP buffer.

If there's no typo here, the difference of the two addresses is only 0xc0000.

reyalp

srsa_4c

reyalp
On d10 (without CHDK exmem enabled) the buffer address is 0x43E42E00
Comments in makefile.inc indicate the video buffer starts at 0x43F02E00, exactly 1MB after the start of the PTP buffer.
If there's no typo here, the difference of the two addresses is only 0xc0000.

Good catch. It was mistake. The exmem buffer is at 0x43e02e00. The
0x43e42e00 is the address returned for slot 4, which I mistakenly thought was at the start.

reyalp

reyalp
A540 (and likely other vxworks cameras) appear to have another, unrelated problem, present in the current trunk. Rapid fire messages like
[code]
!m=require'extras/msgtest'
!m.test{size=1,sizeinc=1,count=10000,gc='step'}
[/code]
occasionally crash (often after 3000-5000 iterations, but variable, sometimes it goes 10k without problems)

I worked on this a bit more. It appears that protecting Canon malloc/free/GetMemInfo with a semaphore avoids the issue.
It didn't crash in ten runs of m.test{size=1,sizeinc=1,count=10000,gc='step'} with the attached patch, where it usually crashes within one or two with the unpatched code.

The patch is just a proof of concept. DryOS malloc, free and GetMemInfo are clearly protected by their own semaphore internally, so the real fix would only need to apply to vxworks. Where to initialize the semaphore is also a bit unclear.

There are a couple things that make this a bit weird:
* Scheduling on vxworks cameras appears to mostly non-preemptive. However, my impression from the vxworks docs is that an interrupt can still result in a task switch (ie interrupt handler changes a semaphore etc that makes a different task the highest priority ready to run), which would make sense with it happening in PTP.
* The semaphore only protects CHDK calls, so presumably the Canon firmware routines could suffer the same problem. Maybe the Canon firmware doesn't do enough rapid fire malloc/frees for it be an issue, or maybe it's only memPartInfo that gets confused when it gets called in the middle of a malloc/free. I didn't come up with a simple way to test this. The (mostly) non-preemptive scheduling makes it hard to come up with a test case that really hammers it.

My inclination is to implement this for vxworks. It's pretty rare in normal usage, but it makes testing the other transfer issues difficult, and I think I have run into with the chdkptp image download commands and lots of files.

canon-mem-sem-work-1.patch

2.2KB

reyalp

Proposed patch. I'll check this in if there aren't objections.

I built all the vx cams and checked the stubs for the semaphore functions seem reasonable (take and give are already used for FileIO, create probably wasn't used in CHDK on most vx cameras)

edit:
Added in trunk 5205

canon-mem-sem-work-3.patch

4.16KB

reyalp

Here's my initial patch to find the PTP handlers in pre-digic 6 cams (corresponding to the d6 one in https://chdk.setepontos.com/d/12714/73

It turns out that on vxworks cams, the compiler canon used pretty much always put 0x1000 or 0x9000 in one register, and then used ORR or ADD to make the operation ID for a bunch of subsequent add_ptp_handler calls. So the finsig_vxworks code only finds handlers that are in a table of object / storage related operations (GetObject, SendObject etc) rather than trying to scrape all the calls.

This is OK since my immediate use for this is to get the PTP buffer related functions.

The dryos firmwares have a few instances of the ADD/ORR logic, but mostly use LDR.

find-ptp-handlers-pre-d6-work-1.patch

8.63KB

srsa_4c

reyalp
It turns out that on vxworks cams, the compiler canon used pretty much always put 0x1000 or 0x9000 in one register, and then used ORR or ADD to make the operation ID for a bunch of subsequent add_ptp_handler calls.

Attached modified patch adds (incomplete) support for this - vx only. Might be duplicated effort.
I found an ID (0x902😎 that seems to have two different handlers in some firmwares.

find-ptp-handlers-pre-d6-work-1mod.patch

12.12KB

reyalp

srsa_4c
Attached modified patch adds (incomplete) support for this - vx only. Might be duplicated effort.

Nice 😁
I wasn't planning to tackle it for the initial version at least, so not duplicated.

I found an ID (0x902😎 that seems to have two different handlers in some firmwares.

The thumb2 sig finder ends up with some event procs and tasks like this too. I was thinking about making it rename them like foo_FW_2 or something.

reyalp

I checked in the ptp handlers stuff, with minor updates. I obviously haven't verified most of them, but I didn't see any false positives in the ones I did check.

« Previous Page Next Page »