Skip to content

drivers: display: stm32_ltdc: mask irq if necessary #90972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pavlohamov
Copy link
Contributor

@pavlohamov pavlohamov commented Jun 2, 2025

LTDC interrupt routine is used to reload frame buffer pointer
once full frame is finished flushing. As long as there is no
need to change buffer - there is no need to disturb CPU.
Thus: Enable LTDC interrupt only when new buffer is pending

@pavlohamov pavlohamov changed the title drivers: display: stm32_ltdc: disable irq unnecessary drivers: display: stm32_ltdc: disable unnecessary irq Jun 2, 2025
@pavlohamov pavlohamov force-pushed the stm32_ltdc_isr_dis branch from f16d399 to 8ebd91a Compare June 2, 2025 19:27
@pavlohamov pavlohamov force-pushed the stm32_ltdc_isr_dis branch from 8ebd91a to 22bc468 Compare June 3, 2025 03:45
@pavlohamov
Copy link
Contributor Author

checked on stm32f746g, stm32h7b3i. with lvgl:

  1. 2 ltdc buffers + 50% vdb to force memcpy into ltdc frame buffer
  2. 0 ltdc buffers + 2 100% vdb buffers (with full_refresh=y) to force ltdc driver using isr for buffer alternation

@erwango erwango requested a review from avolmat-st June 6, 2025 07:30
@erwango
Copy link
Member

erwango commented Jun 19, 2025

@ markussmST FYI

Copy link

@avolmat-st avolmat-st left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patch. Makes perfectly sense to me.

Maybe being a bit more explicit in the commit title would help as well to mention that irq is masked when not necessary (not completely) ?

@@ -223,7 +224,10 @@ static int stm32_ltdc_write(const struct device *dev, const uint16_t x,

data->pend_buf = pend_buf;

__HAL_LTDC_CLEAR_FLAG(&data->hltdc, LTDC_FLAG_LI);
irq_enable(config->irqn);
Copy link

@avolmat-st avolmat-st Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than turn off the whole interrupt line at the NVIC level, what about simply disabling at the source, hence at the LTDC level.

This could be something like:

__HAL_LTDC_CLEAR_FLAG(&data->hltdc, LTDC_FLAG_LI);
__HAL_LTDC_ENABLE_IT(&data->hltdc, LTDC_IT_LI);
 k_sem_take(...)
 __HAL_LTDC_DISABLE_IT(&data->hltdc, LTDC_IT_LI);

Avoid calling __HAL_LTDC_ENABLE_IT(&data->hltdc, LTDC_IT_LI); at the ltdc init time is also necessary.

That done, if for some reason others interrupts are still needed it is still possible to handle them and only the LI interrupt is masked.
 

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for such a valuable input. Masking just a particular signal looking much more clean - done
@avolmat-st could you be so kind to take another look?

@pavlohamov pavlohamov force-pushed the stm32_ltdc_isr_dis branch from 22bc468 to fc26efc Compare June 30, 2025 05:29
@pavlohamov pavlohamov changed the title drivers: display: stm32_ltdc: disable unnecessary irq drivers: display: stm32_ltdc: mask irq if necessary Jun 30, 2025
@erwango erwango added this to the v4.3.0 milestone Jun 30, 2025
Copy link

@avolmat-st avolmat-st left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. This LGTM except a very minor point, I think clear of the interrupt flag is no more needed at the end of the init since interrupts are not enabled anyway.

@@ -479,7 +484,6 @@ static int stm32_ltdc_init(const struct device *dev)
LTDC->LIPCR = 0U;

__HAL_LTDC_CLEAR_FLAG(&data->hltdc, LTDC_FLAG_LI);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This interrupt flag clear is most probably useless now since anyway interrupts are not enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, that is making sense. Thanks, done

LTDC interrupt routine is used to reload frame buffer pointer
once full frame is finished flushing. As long as there is no
need to change buffer - there is no need to disturb CPU.
Thus: Enable LTDC interrupt only when new buffer is pending

Signed-off-by: Pavlo Hamov <[email protected]>
@pavlohamov pavlohamov force-pushed the stm32_ltdc_isr_dis branch from fc26efc to b921ab9 Compare July 7, 2025 04:24
Copy link

sonarqubecloud bot commented Jul 7, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants