-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Update zc143ac72mipi co5300 driver and display driver example #98778
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
base: main
Are you sure you want to change the base?
Update zc143ac72mipi co5300 driver and display driver example #98778
Conversation
| * area is row 6~472 and line 0~466. So adjust coordinates accordingly. | ||
| */ | ||
| /* First two bytes are starting X coordinate */ | ||
| start_xpos = x; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we update the width and height of the panel in the shield overlay instead of adding this runtime calculations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, it is not the height and width, it is that to the co5300 controller the panel is 472x466 while actually the first 6 rows are cut off physically to make it a round panel, so if the higher app wants to update from 0,0, we need to pass 6,0 to the co5300 controller
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it zc143ac72 panel specific? If this controller is used by other panels, could this value be different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zejiang0jason Yes the driver display_co5300 is used by zc143ac72 only
samples/modules/lvgl/demos/boards/mimxrt700_evk_mimxrt798s_cm33_cpu0.conf
Show resolved
Hide resolved
uLipe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just have a question.
drivers/display/display_co5300.c
Outdated
| data->bytes_per_pixel = 2; | ||
| break; | ||
| case PIXEL_FORMAT_RGB_888: | ||
| data->pixel_format = MIPI_DSI_PIXFMT_RGB888; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm usually the pixel format is set to the data only after the command succeeded otherwise you get inconsistent state.
| } | ||
|
|
||
| rect_w = ROUND_UP(rect_w, CONFIG_SAMPLE_PITCH_ALIGN); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest change:
size_t rect_pitch = ROUND_UP(rect_w, CONFIG_SAMPLE_PITCH_ALIGN);
size_t full_screen_pitch = ROUND_UP(capabilities.x_resolution, CONFIG_SAMPLE_PITCH_ALIGN);
buf_size = MAX(rect_pitch * rect_h, full_screen_pitch * h_step);The variables rect_pitch and full_screen_pitch can be used later in buf_desc setting.
drivers/display/display_co5300.c
Outdated
| local_desc.width = ROUND_UP(local_desc.width, 2U); | ||
| local_desc.height = ROUND_UP(local_desc.height, 2U); | ||
| local_desc.width = desc->width + 1; | ||
| local_desc.height = desc->height + 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why assign the local_desc.width and local_desc.height again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, typo
| * area is row 6~472 and line 0~466. So adjust coordinates accordingly. | ||
| */ | ||
| /* First two bytes are starting X coordinate */ | ||
| start_xpos = x; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it zc143ac72 panel specific? If this controller is used by other panels, could this value be different?
drivers/display/display_co5300.c
Outdated
| local_desc.width = desc->width + 1; | ||
| local_desc.height = desc->height + 1; | ||
| local_desc.pitch = data->frame_pitch; | ||
| local_desc.frame_incomplete = desc->frame_incomplete; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we only send out whole dirty region, when the frame_incomplete is false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that is what we do now in every update, the frame_incomplete param in the descriptor passed down from app level is needed and used by some of low level driver like lcdic or dbi_spi, we just preserve and pass down the param, it is not actually used.
| while (tx_size > 0) { | ||
| msg.tx_len = tx_size; | ||
| msg.tx_buf = src; | ||
| bytes_written = (int)mipi_dsi_transfer(config->mipi_dsi, config->channel, &msg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the tx_size is the whole data size, is the data to be sent continuous in memory? if not, can mipi_dsi_transfer support non-continuous data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it can if using lcdif
drivers/display/display_co5300.c
Outdated
| local_desc.height = desc->height + 1; | ||
| local_desc.pitch = data->frame_pitch; | ||
| local_desc.frame_incomplete = desc->frame_incomplete; | ||
| local_desc.buf_size = local_desc.width * local_desc.height * data->bytes_per_pixel; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if possible, suggest make this area adjustment as a static function, currently this function is very large.
acb7e91 to
7b04e9a
Compare
Add user configuration of buffer address alignment and update area pitch alignment. Signed-off-by: Kate Wang <yumeng.wang@nxp.com>
The mimxrt700_evk uses DC8000 to drive panel which requires a 64 byte align of buffer address and stride. Signed-off-by: Kate Wang <yumeng.wang@nxp.com>
97e0e5c to
9858111
Compare
boards/shields/zc143ac72mipi/boards/mimxrt700_evk_mimxrt798s_cm33_cpu0.overlay
Show resolved
Hide resolved
9858111 to
4daf132
Compare
4daf132 to
64a23e4
Compare
1. Fix wrong backlight pin in driver overlay 2. Remove the power-on pin configuration in code and binding, and add mipi display panel regulator in panel overlay instead. Set regulator-boot-on' to true means the power-on pin will be enabled uring system boot. 3. Remove 'last_known_framebuffer' from panel data structure. It is not used anywhere 4. Fix bug in 'co5300_set_pixel_format' function. 5. Fix the issue that the panel does not support start coordinates and the width/height of the updated area being odd value. Solution: In panel driver, maintain a full screen-sized buffer, its address and pitch alignment is configurable in device tree and shall be compliant with the display controller's requirements. It can be placed in RAM or if the RAM space is not enough it can also be placed in other memory resion. When there is a frame update request, the updated area will be first filled to the buffer, if the area's size or coordinate is odd, adjust the value so the real updated area covers the requested updated area, then use this buffer to send pixel to panel, this can ensure the updated area's size and coordinate are always even. Signed-off-by: Kate Wang <yumeng.wang@nxp.com>
The zc143ac72mipi panel can only accept update area that has even size and coordinates, so the panel driver was updated to maintain a buffer to collect all dirty areas. This buffer shall have address and pitch alignments which compliant to the board's display driver's requirement, and can be placed outside of ram. Update the board specific overlay to add such configurations. Signed-off-by: Kate Wang <yumeng.wang@nxp.com>
64a23e4 to
1e666df
Compare
|



3.1 Fix wrong backlight pin in driver overlay
3.2 Remove the power-on pin configuration in code and binding, and add mipi display panel regulator in panel overlay instead. Set 'regulator-boot-on' to true means the power-on pin will be enabled during system boot.
3.3 Remove 'last_known_framebuffer' from panel data structure. It is not used anywhere
3.4 Fix bug in 'co5300_set_pixel_format' function.
3.5 Fix the issue that the panel does not support start coordinates and the width/height of the updated area being odd value.
Solution: In panel driver, maintain a full screen-sized buffer, its address and pitch alignment is configurable in device tree and shall be compliant with the display controller's requirements. It can be placed in RAM or if the RAM space is not enough it can also be placed in other memory resion. When there is a frame update request, the updated area will be first filled to the buffer, if the area's size or coordinate is odd, adjust the value so the real updated area covers the requested updated area, then use this buffer to send pixel to panel, this can ensure the updated area's size and coordinate are always even.