os/arch/arm/src/amebasmart: fix sem_timedwait(&g_read_cmd_done) not b… #6994

zhongnuo-tang · 2025-10-13T01:58:53Z

…eing called

change condition from receive_cmd_done flag to MIPI_LPTX_IS_READ, resolve the issue when rx isr is called earlier than sem_timedwait.
re-initialize semaphore when timeout happen. 3.. add msg.rx_buff = NULL in send cmd that does not require return from LCD module.

os/drivers/lcd/mipi_lcd.c

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

seokhun-eom24

How about enabling the MIPI interrupt when the transfer starts and disabling it when it finishes?

In the current changes, the reason for calling sem_init in the Fail_case is to initialize the semaphore after a timedwait timeout so that it cannot be consumed later.
However, this still does not prevent a situation where an ISR occurs after the semaphore is initialized and performs a post.

Also, if we do it this way, I think send_cmd_done and receive_cmd_done become unnecessary, so please review this approach.

This PR addresses a potential issue that could arise on timeout; even if it isn’t merged quickly, I’d like to proceed in a way that fully resolves the problem.

zhongnuo-tang · 2025-11-05T06:35:45Z

How about enabling the MIPI interrupt when the transfer starts and disabling it when it finishes?

In the current changes, the reason for calling sem_init in the Fail_case is to initialize the semaphore after a timedwait timeout so that it cannot be consumed later. However, this still does not prevent a situation where an ISR occurs after the semaphore is initialized and performs a post.

Also, if we do it this way, I think send_cmd_done and receive_cmd_done become unnecessary, so please review this approach.

This PR addresses a potential issue that could arise on timeout; even if it isn’t merged quickly, I’d like to proceed in a way that fully resolves the problem.

Currently, when we switch to cmd mode to send cmd, we are enabling the interrupt, and disable it after switch to video mode.

static void rtl8730e_mipi_mode_switch(mipi_mode_t mode)
{
	if (mode == CMD_MODE) {
		mipi_mode_switch_to_video(false);
		MIPI_DSI_INT_Config(MIPI, DISABLE, ENABLE, FALSE);
		LCDC_INTConfig(pLCDC, LCDC_BIT_DMA_UN_INTEN, DISABLE);
		DelayMs(140);
	} else {
		MIPI_DSI_INT_Config(MIPI, DISABLE, DISABLE, FALSE);
		LCDC_INTConfig(pLCDC, LCDC_BIT_DMA_UN_INTEN, ENABLE);
		mipi_mode_switch_to_video(true);
	}
}

and it will be disable at the end of cmd sent if the msg->type == MIPI_DSI_END_OF_TRANSMISSION..

TizenRT/os/arch/arm/src/amebasmart/amebasmart_mipi.c

Line 465 in bdf7f99

MIPI_DSI_INT_Config(g_dsi_host.MIPIx, DISABLE, DISABLE, FALSE);

seokhun-eom24 · 2025-11-05T06:50:22Z

How about enabling the MIPI interrupt when the transfer starts and disabling it when it finishes?
In the current changes, the reason for calling sem_init in the Fail_case is to initialize the semaphore after a timedwait timeout so that it cannot be consumed later. However, this still does not prevent a situation where an ISR occurs after the semaphore is initialized and performs a post.
Also, if we do it this way, I think send_cmd_done and receive_cmd_done become unnecessary, so please review this approach.
This PR addresses a potential issue that could arise on timeout; even if it isn’t merged quickly, I’d like to proceed in a way that fully resolves the problem.

Currently, when we switch to cmd mode to send cmd, we are enabling the interrupt, and disable it after switch to video mode.
static void rtl8730e_mipi_mode_switch(mipi_mode_t mode)
{
	if (mode == CMD_MODE) {
		mipi_mode_switch_to_video(false);
		MIPI_DSI_INT_Config(MIPI, DISABLE, ENABLE, FALSE);
		LCDC_INTConfig(pLCDC, LCDC_BIT_DMA_UN_INTEN, DISABLE);
		DelayMs(140);
	} else {
		MIPI_DSI_INT_Config(MIPI, DISABLE, DISABLE, FALSE);
		LCDC_INTConfig(pLCDC, LCDC_BIT_DMA_UN_INTEN, ENABLE);
		mipi_mode_switch_to_video(true);
	}
}
and it will be disable at the end of cmd sent if the msg->type == MIPI_DSI_END_OF_TRANSMISSION..

TizenRT/os/arch/arm/src/amebasmart/amebasmart_mipi.c

Line 465 in bdf7f99

MIPI_DSI_INT_Config(g_dsi_host.MIPIx, DISABLE, DISABLE, FALSE);

How about moving the interrupt enable/disable that operates when the mode changes to the point where the transfer occurs?

Doing this would, as mentioned earlier, resolve issues such as an ISR firing after semaphore initialization, and allow us to match each transfer with its ISR one‑to‑one without using send_cmd_done or receive_cmd_done.

Please share your thoughts on this approach.

zhongnuo-tang · 2025-11-05T07:05:11Z

How about enabling the MIPI interrupt when the transfer starts and disabling it when it finishes?
In the current changes, the reason for calling sem_init in the Fail_case is to initialize the semaphore after a timedwait timeout so that it cannot be consumed later. However, this still does not prevent a situation where an ISR occurs after the semaphore is initialized and performs a post.
Also, if we do it this way, I think send_cmd_done and receive_cmd_done become unnecessary, so please review this approach.
This PR addresses a potential issue that could arise on timeout; even if it isn’t merged quickly, I’d like to proceed in a way that fully resolves the problem.

Currently, when we switch to cmd mode to send cmd, we are enabling the interrupt, and disable it after switch to video mode.
static void rtl8730e_mipi_mode_switch(mipi_mode_t mode)
{
	if (mode == CMD_MODE) {
		mipi_mode_switch_to_video(false);
		MIPI_DSI_INT_Config(MIPI, DISABLE, ENABLE, FALSE);
		LCDC_INTConfig(pLCDC, LCDC_BIT_DMA_UN_INTEN, DISABLE);
		DelayMs(140);
	} else {
		MIPI_DSI_INT_Config(MIPI, DISABLE, DISABLE, FALSE);
		LCDC_INTConfig(pLCDC, LCDC_BIT_DMA_UN_INTEN, ENABLE);
		mipi_mode_switch_to_video(true);
	}
}
and it will be disable at the end of cmd sent if the msg->type == MIPI_DSI_END_OF_TRANSMISSION..

TizenRT/os/arch/arm/src/amebasmart/amebasmart_mipi.c

Line 465 in bdf7f99

MIPI_DSI_INT_Config(g_dsi_host.MIPIx, DISABLE, DISABLE, FALSE);
How about moving the interrupt enable/disable that operates when the mode changes to the point where the transfer occurs?

Doing this would, as mentioned earlier, resolve issues such as an ISR firing after semaphore initialization, and allow us to match each transfer with its ISR one‑to‑one without using send_cmd_done or receive_cmd_done.

Please share your thoughts on this approach.

For send cmd, i think it can be done since each send will correspond to a send cmd done isr. But for read cmd, it will trigger minimum two isr, when to disable the irq?

send cmd done isr MIPI_BIT_CMD_TXDONE (for sending this read request)
read cmd isr MIPI_BIT_RCMD1 (replies from LCD)

seokhun-eom24 · 2025-11-05T07:19:36Z

How about enabling the MIPI interrupt when the transfer starts and disabling it when it finishes?
In the current changes, the reason for calling sem_init in the Fail_case is to initialize the semaphore after a timedwait timeout so that it cannot be consumed later. However, this still does not prevent a situation where an ISR occurs after the semaphore is initialized and performs a post.
Also, if we do it this way, I think send_cmd_done and receive_cmd_done become unnecessary, so please review this approach.
This PR addresses a potential issue that could arise on timeout; even if it isn’t merged quickly, I’d like to proceed in a way that fully resolves the problem.

Currently, when we switch to cmd mode to send cmd, we are enabling the interrupt, and disable it after switch to video mode.
static void rtl8730e_mipi_mode_switch(mipi_mode_t mode)
{
	if (mode == CMD_MODE) {
		mipi_mode_switch_to_video(false);
		MIPI_DSI_INT_Config(MIPI, DISABLE, ENABLE, FALSE);
		LCDC_INTConfig(pLCDC, LCDC_BIT_DMA_UN_INTEN, DISABLE);
		DelayMs(140);
	} else {
		MIPI_DSI_INT_Config(MIPI, DISABLE, DISABLE, FALSE);
		LCDC_INTConfig(pLCDC, LCDC_BIT_DMA_UN_INTEN, ENABLE);
		mipi_mode_switch_to_video(true);
	}
}
and it will be disable at the end of cmd sent if the msg->type == MIPI_DSI_END_OF_TRANSMISSION..

TizenRT/os/arch/arm/src/amebasmart/amebasmart_mipi.c

Line 465 in bdf7f99

MIPI_DSI_INT_Config(g_dsi_host.MIPIx, DISABLE, DISABLE, FALSE);
How about moving the interrupt enable/disable that operates when the mode changes to the point where the transfer occurs?
Doing this would, as mentioned earlier, resolve issues such as an ISR firing after semaphore initialization, and allow us to match each transfer with its ISR one‑to‑one without using send_cmd_done or receive_cmd_done.
Please share your thoughts on this approach.
For send cmd, i think it can be done since each send will correspond to a send cmd done isr. But for read cmd, it will trigger minimum two isr, when to disable the irq?

send cmd done isr MIPI_BIT_CMD_TXDONE (for sending this read request)

read cmd isr MIPI_BIT_RCMD1 (replies from LCD)

I think it should be disabled after reading the cmd ISR.
And I have one question about it. Is there any possible scenario in which the cmd-to-isr mapping is not one-to-one?

This PR's final implementation goals are

Pair with sem_post and sem_timedwait
Modify it so that after a timeout the semaphore does not affect the next ISR.

zhongnuo-tang · 2025-11-10T07:03:20Z

How about moving the interrupt enable/disable that operates when the mode changes to the point where the transfer occurs?
Doing this would, as mentioned earlier, resolve issues such as an ISR firing after semaphore initialization, and allow us to match each transfer with its ISR one‑to‑one without using send_cmd_done or receive_cmd_done.
Please share your thoughts on this approach.

For send cmd, i think it can be done since each send will correspond to a send cmd done isr. But for read cmd, it will trigger minimum two isr, when to disable the irq?

send cmd done isr MIPI_BIT_CMD_TXDONE (for sending this read request)

read cmd isr MIPI_BIT_RCMD1 (replies from LCD)

I think it should be disabled after reading the cmd ISR. And I have one question about it. Is there any possible scenario in which the cmd-to-isr mapping is not one-to-one?

This PR's final implementation goals are

Pair with sem_post and sem_timedwait

Modify it so that after a timeout the semaphore does not affect the next ISR.

Right now i think they are already paired with the help of 'send_cmd_done' and 'receive_cmd_done'.
Read cmd is one that is not one-to-one mapping in terms of cmd-to-isr, as when we read from lcd, we only send 1 cmd (read), but it will trigger at least 2 isr, MIPI_BIT_CMD_TXDONE and MIPI_BIT_RCMD1.

how about below code when timeout

Fail_case:
 	/* 1. Disable and clear MIPI interrupt sources */
 	MIPI_DSI_INT_Config(priv->MIPIx, DISABLE, DISABLE, FALSE);
 	uint32_t pending = MIPI_DSI_INTS_Get(priv->MIPIx);
	MIPI_DSI_INTS_Clr(priv->MIPIx, pending);

	/* 2. Safe to re-init semaphores */
	if (send_cmd_done == 0) {
		sem_init(&g_send_cmd_done, 0, 0);
	}
	if (receive_cmd_done == 0) {
		sem_init(&g_read_cmd_done, 0, 0);
	}
	if (msg->rx_buf && msg->rx_len > 0 && MIPI_LPTX_IS_READ(msg->type)) {
		memset(msg->rx_buf, 0, msg->rx_len);
		rx_data_ptr = NULL;
		rx_data_len = 0;
	}

	/* 3. Re-enable MIPI interrupts for next transfer */
	MIPI_DSI_INT_Config(priv->MIPIx, DISABLE, ENABLE, FALSE);
	....

seokhun-eom24 · 2025-12-10T00:44:26Z

Hello @zhongnuo-tang

Thank you for the suggestion. This three-step clearance process looks good.

Additionally, I have one more suggestion.
The code uses both semaphores (g_send_cmd_done, g_read_cmd_done) and flags (send_cmd_done, receive_cmd_done) for synchronization. This dual mechanism introduces complexity.
How about removing the flags and calling sem_post directly in the ISR by evaluating reg_val?

amebasmart_mipidsi_isr function like this.

	reg_val2 = MIPI_DSI_INTS_ACPU_Get(MIPIx);
	MIPI_DSI_INTS_ACPU_Clr(MIPIx, reg_val2);
	if (reg_val & MIPI_BIT_CMD_TXDONE) {
		reg_val &= ~MIPI_BIT_CMD_TXDONE;
		sem_post(&g_send_cmd_done);
	}
	if (reg_val & MIPI_BIT_RCMD1) {
		amebasmart_mipidsi_rcmd_decode(MIPIx, 0);
		rcmd_handled = true;
	}
	if (reg_val & MIPI_BIT_RCMD2) {
		amebasmart_mipidsi_rcmd_decode(MIPIx, 1);
		rcmd_handled = true;
	}
	if (reg_val & MIPI_BIT_RCMD3) {
		amebasmart_mipidsi_rcmd_decode(MIPIx, 2);
		rcmd_handled = true;
	}

 	if (reg_val & (MIPI_BIT_RCMD1 | MIPI_BIT_RCMD2 | MIPI_BIT_RCMD3)){
		if (rx_data_ptr) {
			mipillvdbg("RCMD ");
			for (int i = 0; i < rx_data_len; i ++) {
				mipillvdbg("%x ", rx_data_ptr[i]);
			}
			mipillvdbg("\n" );
		}
 	}
	if (rcmd_handled) {
		sem_post(&g_read_cmd_done);
	}

Please let me know if you have any concerns about this approach.

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

ewoodev · 2026-01-02T05:46:32Z

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

+	if (send_cmd_done == 0) {
+		sem_init(&g_send_cmd_done, 0, 0);
+	}
+	if (receive_cmd_done == 0) {
+		sem_init(&g_read_cmd_done, 0, 0);


If you initialize to 0 with this here, the value changes to 1 when IRQ appears.
Do we want this kind of situation?

This appears to be timeout operation handling.
Should we not mask IRQ even though timeout occurred?

I think we should only get IRQ at the time we want it.

Send case, you can throw it and complete it automatically However, the next sending or tx fulled must be queued for tx complate. or send blocking(current)

Recive case, you must be queued or timed out.
If you don't want to wait for read blocking, you have to create flow using static read buffer here.

This BSP code case is send blocking and recive blocking.
If you blocking here, we must control mask mipi IRQ, so that can get IRQ between transfer fuction.

Currently, MIPI isr is enabled when we switch to cmd mode, and disable after switch to video mode OR at the end of cmd sent if the msg->type == MIPI_DSI_END_OF_TRANSMISSION when sending a cmd table. so it should be already handled.

For time out case, if we are using semaphore, it is very hard to handle it when timeout happen as we do not know when the timeout IRQ will fired.. I realized that even if we disable and clear the pending isr, it may not be sufficient..
Do you have any recommendation on this?

Controlling IRQ masking only at the beginning and end means that we have to handle incorrect packets and delayed RX packets.

The incorrect packets are already handled for mipi reset.

When we make an RX request, we send a command via TX. Therefore, the next RX request after a timeout could be for a different command, so the delayed (timed-out) RX packets should be ignored.

i think so, if we disable IRQ, the other cpu can already get IRQ.

The PR code is already notificating to ISR using rx_data_ptr with masking IRQ.

rx_data_ptr = NULL; rx_data_len = 0;

so, i suggest, decide whether if you handling delayed IRQ or not, using checking tx_data_ptr and rx_data_ptr with spin_lock.

like below.

spin_lock(&g_tx); if (send_cmd_done == 0) { send_cmd_done = 1; sem_post(&g_send_cmd_done); } else { spin_unlock(&g_tx); return; } spin_unlock(&g_tx); ... spin_lock(&g_rx); if (receive_cmd_done == 0) { // handle rx; sem_post(&g_read_cmd_done); } spin_unlock(&g_rx);

flags = irqsave(); ... ret = sem_timedwait(&g_send_cmd_done, &abstime); spin_lock(&g_tx); if (send_cmd_done == 0) { send_cmd_done = 1; receive_cmd_done = 1; spin_unlock(&g_tx); return -ETIMEOUT; } spin_unlock(&g_tx); if (MIPI_LPTX_IS_READ(msg->type)) { ... ret = sem_timedwait(&g_read_cmd_done, &abstime); spin_lock(&g_rx); if (receive_cmd_done == 0) { receive_cmd_done = 1; if (msg->rx_buf && msg->rx_len > 0 ) { rx_data_ptr = NULL; rx_data_len = 0; } ret = -ETIMEOUT; } spin_unlock(&g_rx); } ... irqrestore(flags);

//init send_cmd_done = 1; receive_cmd_done = 1; irq enable.

…eing called 1. change condition from receive_cmd_done flag to MIPI_LPTX_IS_READ, resolve the issue when rx isr is called earlier than sem_timedwait. 2. re-initialize semaphore when timeout happen.

1. rx_data_ptr is assigned from msg->rx_buf, it might be freed memory address or using memory for others. So it is not safety to clear in IRQ. 2. assign it to NULL instead

ewoodev · 2026-01-02T08:42:56Z

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

+	if (send_cmd_done == 0) {
+		sem_init(&g_send_cmd_done, 0, 0);
+	}
+	if (receive_cmd_done == 0) {
+		sem_init(&g_read_cmd_done, 0, 0);


Controlling IRQ masking only at the beginning and end means that we have to handle incorrect packets and delayed RX packets.

The incorrect packets are already handled for mipi reset.

When we make an RX request, we send a command via TX. Therefore, the next RX request after a timeout could be for a different command, so the delayed (timed-out) RX packets should be ignored.

i think so, if we disable IRQ, the other cpu can already get IRQ.

The PR code is already notificating to ISR using rx_data_ptr with masking IRQ.

rx_data_ptr = NULL; rx_data_len = 0;

so, i suggest, decide whether if you handling delayed IRQ or not, using checking tx_data_ptr and rx_data_ptr with spin_lock.

ewoodev · 2026-01-02T08:57:05Z

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

 	ret = sem_timedwait(&g_send_cmd_done, &abstime);
-	if (!receive_cmd_done) {
-		ret = sem_timedwait(&g_read_cmd_done, &abstime);
+	if (ret != OK) {
+		goto Fail_case;
 	}


while (sem_timedwait(&g_send_cmd_done, &abstime) != OK) { if (errno == EINTR) { continue; } else { goto Fail_case; } }

please handle EINTR error

ewoodev · 2026-01-02T08:58:02Z

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

+	if (MIPI_LPTX_IS_READ(msg->type)) {
+		(void)clock_gettime(CLOCK_REALTIME, &abstime);
+		abstime.tv_sec += MIPI_TRANSFER_TIMEOUT;
+		ret = sem_timedwait(&g_read_cmd_done, &abstime);


while (sem_timedwait(&g_send_cmd_done, &abstime) != OK) { if (errno == EINTR) { continue; } else { goto Fail_case; } }

please handle EINTR error

seokhun-eom24 approved these changes Oct 13, 2025

View reviewed changes

ewoodev reviewed Oct 13, 2025

View reviewed changes

os/drivers/lcd/mipi_lcd.c Outdated Show resolved Hide resolved

os/arch/arm/src/amebasmart/amebasmart_mipi.c Show resolved Hide resolved

zhongnuo-tang force-pushed the mipi_sem branch 2 times, most recently from 3ba2295 to 2b988ce Compare October 13, 2025 06:38

sunghan-chang reviewed Oct 13, 2025

View reviewed changes

os/arch/arm/src/amebasmart/amebasmart_mipi.c Show resolved Hide resolved

zhongnuo-tang force-pushed the mipi_sem branch from 2b988ce to ed10313 Compare October 13, 2025 10:45

seokhun-eom24 approved these changes Oct 20, 2025

View reviewed changes

pcs1265 approved these changes Oct 21, 2025

View reviewed changes

os/arch/arm/src/amebasmart/amebasmart_mipi.c Show resolved Hide resolved

sunghan-chang reviewed Oct 21, 2025

View reviewed changes

os/arch/arm/src/amebasmart/amebasmart_mipi.c Show resolved Hide resolved

os/arch/arm/src/amebasmart/amebasmart_mipi.c Show resolved Hide resolved

zhongnuo-tang force-pushed the mipi_sem branch from ed10313 to 313e0ab Compare October 22, 2025 05:47

abhinav-s235 reviewed Oct 28, 2025

View reviewed changes

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

zhongnuo-tang force-pushed the mipi_sem branch from 313e0ab to bdf7f99 Compare November 3, 2025 05:58

seokhun-eom24 reviewed Nov 5, 2025

View reviewed changes

ewoodev reviewed Jan 2, 2026

View reviewed changes

zhongnuo-tang added 2 commits January 2, 2026 16:49

os/arch/arm/src/amebasmart: fix sem_timedwait(&g_read_cmd_done) not b…

ca17e45

…eing called 1. change condition from receive_cmd_done flag to MIPI_LPTX_IS_READ, resolve the issue when rx isr is called earlier than sem_timedwait. 2. re-initialize semaphore when timeout happen.

os/arch/arm/src/amebasmart: remove memset of rx_data_ptr in ISR

91a5299

1. rx_data_ptr is assigned from msg->rx_buf, it might be freed memory address or using memory for others. So it is not safety to clear in IRQ. 2. assign it to NULL instead

zhongnuo-tang force-pushed the mipi_sem branch from bdf7f99 to 91a5299 Compare January 2, 2026 08:58

ewoodev reviewed Jan 2, 2026

View reviewed changes

os/arch/arm/src/amebasmart: fix sem_timedwait(&g_read_cmd_done) not b… #6994

Are you sure you want to change the base?

os/arch/arm/src/amebasmart: fix sem_timedwait(&g_read_cmd_done) not b… #6994

Uh oh!

Conversation

zhongnuo-tang commented Oct 13, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seokhun-eom24 left a comment

Choose a reason for hiding this comment

Uh oh!

zhongnuo-tang commented Nov 5, 2025

Uh oh!

seokhun-eom24 commented Nov 5, 2025

Uh oh!

zhongnuo-tang commented Nov 5, 2025

Uh oh!

seokhun-eom24 commented Nov 5, 2025

Uh oh!

zhongnuo-tang commented Nov 10, 2025

Uh oh!

seokhun-eom24 commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

seokhun-eom24 commented Dec 10, 2025 •

edited

Loading