os/: Add PWM frequency set for MIPI LCD #6958
Conversation
namanjain7
force-pushed
the
lcd_pwm_freq
branch
from
ed3ca8e to
65c63d1
Compare
| # CONFIG_LCD_INIT_TEC_181A is not set | ||
| CONFIG_LCD_XRES=240 | ||
| CONFIG_LCD_YRES=320 | ||
| CONFIG_LCD_BACKLIGHT_PWM_FREQUENCY=100 |
There was a problem hiding this comment.
Let's split the commit for code change and defconfig change.
| #if defined(CONFIG_LCD_ST7785) | ||
| #if CONFIG_LCD_BACKLIGHT_PWM_FREQUENCY > 0 | ||
| pwmout_period_us(&g_rtl8730e_config_dev_s.pwm_led, 1000 / CONFIG_LCD_BACKLIGHT_PWM_FREQUENCY); | ||
| #else | ||
| #error value LCD_BACKLIGHT_PWM_FREQUENCY is invalid | ||
| #endif | ||
| #endif |
There was a problem hiding this comment.
Is this ST7785 specific functionality? Is this necessary here? We should split common part and lcd specific part in file.
There was a problem hiding this comment.
Hello Mr. chang,
as per Mr. nam, This pwm set is only for st7785 LCD.
so pwmout_period_us can't be called for other LCD's and this funciton is defined in low level driver, so we have to call it from rtl8730e_mipi_lcdc.c file, and not from driver.
os/drivers/lcd/Kconfig
Outdated
|
|
||
| if LCD_ST7785 | ||
| config LCD_BACKLIGHT_PWM_FREQUENCY | ||
| int "LCD Backlight PWM Frequency" |
There was a problem hiding this comment.
Could you write the unit of the frequency? in Hz? in MHz?
There was a problem hiding this comment.
it's in kHz, I have added in Kconfig help and message.
| # CONFIG_LCD_INIT_TEC_181A is not set | ||
| CONFIG_LCD_XRES=240 | ||
| CONFIG_LCD_YRES=320 | ||
| CONFIG_LCD_BACKLIGHT_PWM_FREQUENCY=100 |
There was a problem hiding this comment.
100 is a proper value? @ewoodev Could you confirm?
There was a problem hiding this comment.
The vender guidence is 10khz ~ 30khz
So, Let's set 20khz. it is verifiing on other system.
os/drivers/lcd/Kconfig
Outdated
| endchoice | ||
| endif | ||
|
|
||
| if LCD_ST7785 |
There was a problem hiding this comment.
Could you let me know why this is under LCD_ST7785? No way to set this in other LCDs?
There was a problem hiding this comment.
as per Mr. nam, This pwm set is only for st7785 LCD.
There was a problem hiding this comment.
Hello @namanjain7
yes, right, the st7785 lcd is only reqired pwm 20khz setting from vender.
However, this topic should be addressed differently.
The LCD backlight is essentially a separate device from the LCD driver, serving as a light source.
(the LCD IC and backlight IC are hardware-separated)
Therefore, this discussion is more appropriate not to limit to ST7785. If you were adding a frequency setting value for backlight PWM in Kconfig for tuning , it would be better to unrelated to ST7785.
Additionally, we are looking to improve the overall LCD structure. I think, this should also be a goal for inprove the LCD structure.
I suggest separating the backlight control PWM. Furthermore, I suggest that the LCD driver receives app LCD on/off requests as enable/disable commands to control the PWM driver.
It can support various types of LCDs by separating device drivers for each hw.
There was a problem hiding this comment.
Got it.
Currently this lcd set pwm freq will be done during init.
If the application is changing the freq in middle of LCD running, then it's beneficial to add ioctl support for it.
If not then we can add this in st7785.c file (In check lcd vendor and send init command), which will be lcd specific.
what do you think? @ewoodev
There was a problem hiding this comment.
The application never change pwm freq.
✔ the app should not be allowed to control direct lcd hw.
✔ app doesn't know hw each hw spec.
So, current lcd init struct, st7785.c is better. 👀✨
namanjain7
force-pushed
the
lcd_pwm_freq
branch
from
65c63d1 to
12376bd
Compare
Add PWM frequency set for MIPI based lcd, enabled for st7785, initial value is set to 20, can be changed in respective .h defined lcd file
namanjain7
force-pushed
the
lcd_pwm_freq
branch
from
12376bd to
4434ab0
Compare
Hello Mr. Eom, |
4 participants
Add PWM frequency set for MIPI based LCD ST7785, initial value is set to 100, can be changed from Kconfig