Skip to content
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

os/board/rtl8730e/src: disable unnecessary IRQ when backlight power OFF #6505

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

Conversation

zhongnuo-tang
Copy link
Contributor

  1. LCDC irq can be disabled when backlight power OFF
  2. MIPI irq cannot be disabled because it is required to send cmd to turn backlight power ON, eg: ioctl(fd, LCDDEVIO_SETPOWER, x) where x >0.

@zhongnuo-tang zhongnuo-tang force-pushed the disable_lcd_isr_backlight_off branch 2 times, most recently from 91c1395 to 4eff61a Compare November 14, 2024 03:17
@zhongnuo-tang
Copy link
Contributor Author

As requested, to disable unnecessary IRQs when backlight power is set to OFF, and only allow to enter PG when backlight power is OFF.
When backlight power is set more than 0, the module cannot enter PG even there is no updating of LCD screen.
Tested with lcd example, when ioctl(fd, LCDDEVIO_SETPOWER, x), x>0, it will not enter PG. Module will only enter PG when ioctl(fd, LCDDEVIO_SETPOWER, 0);

@ritesh55555
Copy link
Contributor

After using this patch, MIPI domain is getting suspended after booting . I am also using the PR #6500 with it . With only PR #6500 , after booting everything is fine , board is going to sleep and MIPI is not in suspended state.
But if i apply this PR above that , again after booting MIPI domain is in suspended state.
Is it supposed to be like this?

@zhongnuo-tang
Copy link
Contributor Author

After using this patch, MIPI domain is getting suspended after booting . I am also using the PR #6500 with it . With only PR #6500 , after booting everything is fine , board is going to sleep and MIPI is not in suspended state. But if i apply this PR above that , again after booting MIPI domain is in suspended state. Is it supposed to be like this?

Yes, it can only enter PG when backlight is OFF, ioctl(fd, LCDDEVIO_SETPOWER, 0); I received this request as follow

  1. Samsung will control LCD backlight power.
  2. If LCD backlight power OFF, AI-Lite can enter the PG.

@zhongnuo-tang zhongnuo-tang force-pushed the disable_lcd_isr_backlight_off branch from 4eff61a to 65e589c Compare November 22, 2024 01:17
Copy link
Contributor

@ewoodev ewoodev left a comment

Choose a reason for hiding this comment

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

Please verify again with test_fps();

When the LCD backlight is set to 0, if we run the 'test_fps' function, even if you turn on the LCD backlight, the LCD does not work properly.

@ewoodev
Copy link
Contributor

ewoodev commented Nov 25, 2024

As you know, all issues occur in the long term test.

test_fps is also the same as compressing the long term test.
I believe you will fully verify PR.

@zhongnuo-tang
Copy link
Contributor Author

Please verify again with test_fps();

When the LCD backlight is set to 0, if we run the 'test_fps' function, even if you turn on the LCD backlight, the LCD does not work properly.

Hi @ewoodev ,
I don't get this, do you mean the test flow as below?

ioctl(fd, LCDDEVIO_SETPOWER, 0);
test_fps();
ioctl(fd, LCDDEVIO_SETPOWER, 100);

could you share the steps if the above is wrong? Does it involve PM?

@ewoodev
Copy link
Contributor

ewoodev commented Nov 28, 2024

Hi @ewoodev , I don't get this, do you mean the test flow as below?

ioctl(fd, LCDDEVIO_SETPOWER, 0); test_fps(); ioctl(fd, LCDDEVIO_SETPOWER, 100);

could you share the steps if the above is wrong? Does it involve PM?

thread1 : test_fps()
thread2 : control backlight 0

I would say that we need to have a defense logic for the above case.

@zhongnuo-tang zhongnuo-tang force-pushed the disable_lcd_isr_backlight_off branch 2 times, most recently from 6512e6b to 414c3f9 Compare December 2, 2024 01:45
Copy link
Contributor

@ewoodev ewoodev left a comment

Choose a reason for hiding this comment

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

#ifdef CONFIG_PM
void rtl8730e_lcdc_pm(void)
{
rtl8730e_lcd_init();
rtl8730e_enable_lcdc();
rtl8730e_control_backlight(CONFIG_LCD_MAXPOWER);
}
#endif

static void rtl8730e_register_lcdc_isr(void){
InterruptDis(lcdc_irq_info.num);
InterruptUnRegister(lcdc_irq_info.num);
InterruptRegister((IRQ_FUN)rtl8730e_hv_isr, lcdc_irq_info.num, (u32)lcdc_irq_info.data, lcdc_irq_info.priority);
InterruptEn(lcdc_irq_info.num, lcdc_irq_info.priority);
}

Did you check above code?
The IRQ can be enabled when board wakeup

Copy link
Contributor

@ewoodev ewoodev left a comment

Choose a reason for hiding this comment

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

And why backlight is turnd ON when board wakeup??

void rtl8730e_lcdc_pm(void)
{
rtl8730e_lcd_init();
rtl8730e_enable_lcdc();
rtl8730e_control_backlight(CONFIG_LCD_MAXPOWER);
}

1. LCDC irq can be disabled when backlight power OFF
2. MIPI irq cannot be disabled because it is required to send cmd to turn backlight power ON, eg: ioctl(fd, LCDDEVIO_SETPOWER, x) where x >0
3. add error handling at lcd_put_area to prevent hang if irq is disabled but put area is called due to timing issue.
4. backlight should set to OFF when wakeup from PG
@zhongnuo-tang zhongnuo-tang force-pushed the disable_lcd_isr_backlight_off branch from 414c3f9 to 8b1b44f Compare December 5, 2024 01:57
@zhongnuo-tang
Copy link
Contributor Author

#ifdef CONFIG_PM void rtl8730e_lcdc_pm(void) { rtl8730e_lcd_init(); rtl8730e_enable_lcdc(); rtl8730e_control_backlight(CONFIG_LCD_MAXPOWER); } #endif

static void rtl8730e_register_lcdc_isr(void){ InterruptDis(lcdc_irq_info.num); InterruptUnRegister(lcdc_irq_info.num); InterruptRegister((IRQ_FUN)rtl8730e_hv_isr, lcdc_irq_info.num, (u32)lcdc_irq_info.data, lcdc_irq_info.priority); InterruptEn(lcdc_irq_info.num, lcdc_irq_info.priority); }

Did you check above code? The IRQ can be enabled when board wakeup

Hi @ewoodev , yes, when wakeup from PG, all status are lost and required re-initialization including IRQ. I think it should be controlled using backlight to disable IRQ after wakeup. I've changed it to

void rtl8730e_lcdc_pm(void)
{
rtl8730e_lcd_init();
rtl8730e_enable_lcdc();
rtl8730e_control_backlight(0);
}

@edwakuwaku
Copy link
Contributor

#ifdef CONFIG_PM void rtl8730e_lcdc_pm(void) { rtl8730e_lcd_init(); rtl8730e_enable_lcdc(); rtl8730e_control_backlight(CONFIG_LCD_MAXPOWER); } #endif

static void rtl8730e_register_lcdc_isr(void){ InterruptDis(lcdc_irq_info.num); InterruptUnRegister(lcdc_irq_info.num); InterruptRegister((IRQ_FUN)rtl8730e_hv_isr, lcdc_irq_info.num, (u32)lcdc_irq_info.data, lcdc_irq_info.priority); InterruptEn(lcdc_irq_info.num, lcdc_irq_info.priority); }

Did you check above code? The IRQ can be enabled when board wakeup

Hi @ewoodev , yes, when wakeup from PG, all status are lost and required re-initialization including IRQ. I think it should be controlled using backlight to disable IRQ after wakeup. I've changed it to

void rtl8730e_lcdc_pm(void)

{

rtl8730e_lcd_init();

rtl8730e_enable_lcdc();

rtl8730e_control_backlight(0);

}

Why IRQ status will lost? GIC regs will be backup and restored during PG flow

@zhongnuo-tang
Copy link
Contributor Author

#ifdef CONFIG_PM void rtl8730e_lcdc_pm(void) { rtl8730e_lcd_init(); rtl8730e_enable_lcdc(); rtl8730e_control_backlight(CONFIG_LCD_MAXPOWER); } #endif

static void rtl8730e_register_lcdc_isr(void){ InterruptDis(lcdc_irq_info.num); InterruptUnRegister(lcdc_irq_info.num); InterruptRegister((IRQ_FUN)rtl8730e_hv_isr, lcdc_irq_info.num, (u32)lcdc_irq_info.data, lcdc_irq_info.priority); InterruptEn(lcdc_irq_info.num, lcdc_irq_info.priority); }

Did you check above code? The IRQ can be enabled when board wakeup

Hi @ewoodev , yes, when wakeup from PG, all status are lost and required re-initialization including IRQ. I think it should be controlled using backlight to disable IRQ after wakeup. I've changed it to
void rtl8730e_lcdc_pm(void)
{
rtl8730e_lcd_init();
rtl8730e_enable_lcdc();
rtl8730e_control_backlight(0);
}

Why IRQ status will lost? GIC regs will be backup and restored during PG flow

understood, but since it is rtl8730e_register_lcdc_isr is called in rtl8730e_lcd_init, and it unregister the irq first before re-register it, it think its ok to leave it as it is?

@ewoodev
Copy link
Contributor

ewoodev commented Dec 5, 2024

if app turns off backlight, the off state must be keep even if wakeup board

@zhongnuo-tang
Copy link
Contributor Author

if app turns off backlight, the off state must be keep even if wakeup board

currently
wakeup->unregister, register, enable IRQ (rtl8730e_lcd_init) -> disable IRQ (rtl8730e_control_backlight(0)), is this ok?

if board can enter PG with backlight on, then need to change the flow.

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

Successfully merging this pull request may close these issues.

4 participants