os/arch/arm/src/amebasmart: Fix UART data loss when wakeup from PG #6497

lhenry-realtek · 2024-11-11T05:37:53Z

Required: This commit requires #6482, please apply it as a patch if it is not already merged

Issue is encountered where bytes are lost on UART wakeup source during resume from sleep
Analysis: UART FIFO is started but not drained during resume. When the peripheral is initialized again, it causes the FIFO to also be cleared and cause data loss
Similarly if more than 64 bytes is passed in together, since there is no mechanism to drain the FIFO into RAM it will cause LSR overrun error
This fix drains the FIFO first at KM4, then again at CA32 until it fully wake, then normal Tizen ISR handler resume operation
The drained buffer is also passed to application layer

os/arch/arm/src/amebasmart/amebasmart_serial.c

lhenry-realtek · 2024-11-11T10:25:44Z

Please hold on merge, MAP and ASM file was not updated

os/arch/arm/src/amebasmart/amebasmart_serial.c

edwakuwaku · 2024-11-11T15:09:29Z

os/arch/arm/src/amebasmart/amebasmart_serial.c

+	UART_TypeDef* uartx = UART_DEV_TABLE[uart_index_get(g_uart1priv.tx)].UARTx;
+
+	/* prevent PM transition while fetching remainder FIFO */
+	pm_timedsuspend(pm_domain_register("UART_PG_WAKE"), 5000);


during uart bringup, a domain has been registered, how about refer to that pm id

why 5000, any reason?

So that the board can be observed to actually be awake for a short time and to ensure that anything in the FIFO is fully cleared

Also see tash_main.c it has been defined to 5000ms as well

os/arch/arm/src/amebasmart/amebasmart_serial.c

edwakuwaku · 2024-11-11T15:10:58Z

@lhenry-realtek first commit of this PR shouldn't exist, please rebase it

os/arch/arm/src/amebasmart/amebasmart_serial.c

lhenry-realtek · 2024-11-12T01:28:29Z

bin file will be added after resolving conflicts on our side, do not merge yet

ewoodev

Could you please verify using below tools??
apps/examples/uart_loopback/uart_loopback.c
tools/uart_loopback_test/uart_loopback_test.py
apps/examples/power/power_main.c

TASH >> uart_loopback start
TASH >> power start

ewoodev · 2024-11-14T11:10:48Z

@lhenry-realtek Could you please let me know how many bps are supported?

Based on a 16-byte FIFO: ???
Based on a 64-byte FIFO: ???

zhongnuo-tang · 2024-11-14T11:17:30Z

@lhenry-realtek Could you please let me know how many bps are supported?

Based on a 16-byte FIFO: ??? Based on a 64-byte FIFO: ???

Hi @ewoodev ,
This PR is for UART1 64-byte FIFO, it support up to 115200bps.
For LOGUART 16-byte FIFO, we are not able to prevent data loss for both 115200/1.5M bps.

sunghan-chang · 2024-11-19T00:36:27Z

os/arch/arm/src/amebasmart/amebasmart_serial.c

+	memset(flag, 0, sizeof(flag));
+	memset(uart_data, 0, sizeof(uart_data));


Could you check performance difference between 0 initialization at variable define and memset like this?

What is the flag usage in this function?

You add little bit heavy operation in suspend and resume. How does they affect sleep and wakeup time? Could you check them for before and after?

when it enters sleep, before it takes xx ms, after yyms

when it wakes up, before and after?

changed to = { 0 } initialization

flag is used to check when the IPC operation has finished. In this case the KM4 side will transfer control back to CA32 after it has deinitialized UART1 on its side

measurement in progress

sunghan-chang · 2024-11-19T00:37:57Z

os/arch/arm/src/amebasmart/amebasmart_serial.c

+	ipc_req_msg.rsvd = (u32)flag;
+
+	memset(flag, 0, sizeof(flag));
+	memset(uart_data, 0, sizeof(uart_data));


uart_data type is u32, but memset is working in byte (u8).
Could you compare for loop for each member set with 0 to memset?

sunghan-chang · 2024-11-19T00:39:35Z

@lhenry-realtek If this affects sleep and wakeup times, it has huge effect on several features. Could you check it ASAP?

lhenry-realtek · 2024-11-20T10:12:05Z

I have found some bug in the logic, currently refactoring

Initial measurement based on latest commit:

	Enter Sleep (ms)	Exit Sleep (ms)
SMP=0 w/o PR	2.054935	4.875004
SMP=1 w/o PR	2.0377216	5.6893256
SMP=0 w/ PR	PENDING	PENDING
SMP=1 w/ PR	PENDING	PENDING

SMP add roughly 0.8ms when waking up, mainly due to waiting for core1 to stabilize

Readings are taken from average of 10 samples per test

sunghan-chang · 2024-11-20T11:04:46Z

@lhenry-realtek Thank you for sharing status. Let's talk this again with final result.

lhenry-realtek · 2024-11-21T08:56:37Z

Latest measurement (each reading is average of 10 samples)

@sunghan-chang

Base Commit: 646445f
Configuration:

Board: AI_LITE_PF3_V1.0 88x40xT1.2mm
Config: loadable_ext_ddr
Baudrate: 1.5M LOGUART, 115200 UART1
USB=0, GPIO driver=1
Examples: LED, PM, UART Loopback
LED GPIO (PB20) pad used to measure wakeup/sleep at certain points of kernel (in KM0 and in Tizen)

Note: One shot 260 bytes test is done by sending 260 char size data at once into UART1 via Tera Term
Note 2: UART1 buffer is constantly being read by Rx Thread, pm_resume is manually started via TASH command
hello_main.txt
Note 3: Bin file will be uploaded in a force push, this is for update

Flow:

hello start
pm_resume
Begin Test

Before PR

SMP = ON

SMP	Exit Sleep (ms)	Enter Sleep (ms)	Delta Exit<->Next Enter (ms)
Single Key (x10)	5.6541796	1.9748452	1.4811564
Press and Hold Key	5.6341944	2.2721228	1.299164
One Shot 260 bytes	N/A (Data Lost)	N/A (Data Lost)	N/A (Data Lost)

SMP = AMP

AMP	Exit Sleep (ms)	Enter Sleep (ms)	Delta Exit<->Next Enter (ms)
Single Key (x10)	5.6703732	1.9735384	1.475472
Press and Hold Key	5.6339996	2.1854916	1.2922396
One Shot 260 bytes	N/A (Data Lost)	N/A (Data Lost)	N/A (Data Lost)

SMP = OFF

OFF	Exit Sleep (ms)	Enter Sleep (ms)	Delta Exit<->Next Enter (ms)
Single Key (x10)	4.8589508	1.8909452	1.3775044
Press and Hold Key	4.8395508	2.0381852	1.254374
One Shot 260 bytes	N/A (Data Lost)	N/A (Data Lost)	N/A (Data Lost)

After PR

SMP = ON

SMP	Exit Sleep (ms)	Enter Sleep (ms)	Delta Exit<->Next Enter (ms)
Single Key (x10)	5.7609876	1.8512616	1.5626514
Press and Hold Key	5.7422752	2.0995372	1.3290596
One Shot 260 bytes	5.7880912	2.2587168	18.2019352

SMP = AMP

AMP	Exit Sleep (ms)	Enter Sleep (ms)	Delta Exit<->Next Enter (ms)
Single Key (x10)	5.7873244	1.8747712	1.5201764
Press and Hold Key	5.7510772	2.103554	1.3058848
One Shot 260 bytes	5.8428524	2.4325212	18.2866932

SMP = OFF

OFF	Exit Sleep (ms)	Enter Sleep (ms)	Delta Exit<->Next Enter (ms)
Single Key (x10)	4.9601192	1.7771004	1.4247744
Press and Hold Key	4.9740144	1.9423296	1.280094
One Shot 260 bytes	5.0025952	2.4726248	18.2814612

lhenry-realtek · 2024-11-21T09:36:00Z

Hi @ewoodev I have run the tests

CA32PG-KM4PG-Wakeup : HW TIMER
CA32PG-KM4PG-Wakeup : HW TIMER
SEND UART(999): 1234567890abcdefghijklmnopqrstuvwxyz
RECEIVE(999): 1234567890abcdefghijklmnopqrstuvwxyz
RESULT(999): PASSED
CA32PG-KM4PG-Wakeup : HW TIMER
CA32PG-KM4PG-Wakeup : HW TIMER
SEND UART(1000): 1234567890abcdefghijklmnopqrstuvwxyz
RECEIVE(1000): 1234567890abcdefghijklmnopqrstuvwxyz
RESULT(1000): PASSED
CA32PG-KM4PG-Wakeup : HW TIMER
CA32PG-KM4PG-Wakeup : HW TIMER
SEND UART(1001): 1234567890abcdefghijklmnopqrstuvwxyz
RECEIVE(1001): 1234567890abcdefghijklmnopqrstuvwxyz
RESULT(1001): PASSED
CA32PG-KM4PG-Wakeup : HW TIMER
call suspend (11)

lhenry-realtek · 2024-11-22T00:24:17Z

bin/asm/map added

sunghan-chang · 2024-11-22T06:10:25Z

build/tools/amebasmart/gnu_utility/km0_km4_app_RELEASE_NOTE.txt

@@ -1,5 +1,11 @@
 /* == "version" + "Realtek git version" + "compile date" + "compile time" == */

+== version c7e8f3b164 2024/11/22-07:56:05 ==


Could you check PR #6525 ? That also has a change on km0_km4_app. That is missing here.

A rebase is needed, Hnery is working on that.

It has been updated @sunghan-chang

@lhenry-realtek @jwei5 changed them already. Need to update them again.

Hi @sunghan-chang this bin also includes the changes made by @jwei5, it is safe to resolve with this set of files

Issue is encountered where bytes are lost on UART wakeup source during resume from sleep Test Requirements: Please ensure PR Samsung#6482 is applied before testing Analysis: - UART FIFO is started but not drained during resume. When the peripheral is initialized again, it causes the FIFO to also be cleared and cause data loss - Similarly if more than 64 bytes is passed in together, since there is no mechanism to drain the FIFO into RAM it will cause LSR overrun error This fix drains the FIFO first at KM4, then again at CA32 until it fully wake, then normal Tizen ISR handler resume operation - The drained buffer is also passed to application layer - Watchdog is used to monitor UART1 until the FIFO is completely drained

ziyik reviewed Nov 11, 2024

View reviewed changes

os/arch/arm/src/amebasmart/amebasmart_serial.c Outdated Show resolved Hide resolved

lhenry-realtek force-pushed the fix_pg_uart_data_loss branch from f3c28a0 to 5998015 Compare November 11, 2024 06:21

edwakuwaku reviewed Nov 11, 2024

View reviewed changes

os/arch/arm/src/amebasmart/amebasmart_serial.c Outdated Show resolved Hide resolved

edwakuwaku reviewed Nov 11, 2024

View reviewed changes

os/arch/arm/src/amebasmart/amebasmart_serial.c Outdated Show resolved Hide resolved

edwakuwaku reviewed Nov 11, 2024

View reviewed changes

os/arch/arm/src/amebasmart/amebasmart_serial.c Outdated Show resolved Hide resolved

edwakuwaku reviewed Nov 11, 2024

View reviewed changes

os/arch/arm/src/amebasmart/amebasmart_serial.c Outdated Show resolved Hide resolved

edwakuwaku reviewed Nov 11, 2024

View reviewed changes

os/arch/arm/src/amebasmart/amebasmart_serial.c Outdated Show resolved Hide resolved

lhenry-realtek force-pushed the fix_pg_uart_data_loss branch from 5998015 to 7742e37 Compare November 12, 2024 01:27

ewoodev reviewed Nov 14, 2024

View reviewed changes

sunghan-chang reviewed Nov 19, 2024

View reviewed changes

lhenry-realtek force-pushed the fix_pg_uart_data_loss branch from 7742e37 to aa6faed Compare November 19, 2024 06:36

lhenry-realtek force-pushed the fix_pg_uart_data_loss branch 2 times, most recently from db928a6 to d17656f Compare November 21, 2024 06:19

lhenry-realtek changed the title ~~os/arch/arm/src/amebasmart: Fix UART data loss when wakeup from PG~~ [DO NOT MERGE] os/arch/arm/src/amebasmart: Fix UART data loss when wakeup from PG Nov 21, 2024

lhenry-realtek force-pushed the fix_pg_uart_data_loss branch from d17656f to 2fb777b Compare November 22, 2024 00:17

lhenry-realtek changed the title ~~[DO NOT MERGE] os/arch/arm/src/amebasmart: Fix UART data loss when wakeup from PG~~ os/arch/arm/src/amebasmart: Fix UART data loss when wakeup from PG Nov 22, 2024

sunghan-chang reviewed Nov 22, 2024

View reviewed changes

lhenry-realtek force-pushed the fix_pg_uart_data_loss branch from 2fb777b to fc81138 Compare November 22, 2024 07:08

lhenry-realtek force-pushed the fix_pg_uart_data_loss branch from fc81138 to f4a9684 Compare November 22, 2024 07:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

os/arch/arm/src/amebasmart: Fix UART data loss when wakeup from PG #6497

os/arch/arm/src/amebasmart: Fix UART data loss when wakeup from PG #6497

lhenry-realtek commented Nov 11, 2024 •

edited

Loading

lhenry-realtek commented Nov 11, 2024

edwakuwaku Nov 11, 2024

edwakuwaku Nov 11, 2024

lhenry-realtek Nov 12, 2024

edwakuwaku commented Nov 11, 2024

lhenry-realtek commented Nov 12, 2024

ewoodev left a comment

ewoodev commented Nov 14, 2024

zhongnuo-tang commented Nov 14, 2024

sunghan-chang Nov 19, 2024

lhenry-realtek Nov 19, 2024

sunghan-chang Nov 19, 2024

lhenry-realtek Nov 19, 2024

sunghan-chang commented Nov 19, 2024

lhenry-realtek commented Nov 20, 2024 •

edited

Loading

sunghan-chang commented Nov 20, 2024

lhenry-realtek commented Nov 21, 2024

lhenry-realtek commented Nov 21, 2024

lhenry-realtek commented Nov 22, 2024

sunghan-chang Nov 22, 2024

ziyik Nov 22, 2024

lhenry-realtek Nov 22, 2024

sunghan-chang Nov 22, 2024

lhenry-realtek Nov 22, 2024

		memset(flag, 0, sizeof(flag));
		memset(uart_data, 0, sizeof(uart_data));

		@@ -1,5 +1,11 @@
		/* == "version" + "Realtek git version" + "compile date" + "compile time" == */

		== version c7e8f3b164 2024/11/22-07:56:05 ==

os/arch/arm/src/amebasmart: Fix UART data loss when wakeup from PG #6497

Are you sure you want to change the base?

os/arch/arm/src/amebasmart: Fix UART data loss when wakeup from PG #6497

Conversation

lhenry-realtek commented Nov 11, 2024 • edited Loading

lhenry-realtek commented Nov 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edwakuwaku commented Nov 11, 2024

lhenry-realtek commented Nov 12, 2024

ewoodev left a comment

Choose a reason for hiding this comment

ewoodev commented Nov 14, 2024

zhongnuo-tang commented Nov 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunghan-chang commented Nov 19, 2024

lhenry-realtek commented Nov 20, 2024 • edited Loading

sunghan-chang commented Nov 20, 2024

lhenry-realtek commented Nov 21, 2024

Before PR

After PR

lhenry-realtek commented Nov 21, 2024

lhenry-realtek commented Nov 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhenry-realtek commented Nov 11, 2024 •

edited

Loading

lhenry-realtek commented Nov 20, 2024 •

edited

Loading