Skip to content

Conversation

@zejiang0jason
Copy link
Contributor

@zejiang0jason zejiang0jason commented Dec 9, 2025

The pitch in struct display_buffer_descriptor is defined as "Number of pixels between consecutive rows in the data buffer".

Limitation:
Some display controller needs the pitch of byte be dividable by an even value (e.g., 4, 64, ...), if the parameter pitch here is the number of pixels, then the RGB888 (3 bytes per pixel) can't work

Change the unit of pitch from pixel to byte.

For easy review:

  • The first patch e621c23 changes the structure definition, it is the key change,
  • The second patch e5229fe updates the display controller and samples accordingly. Currently it is created by AI tool, I am still check the content to make sure no issues.

The `pitch` in `struct display_buffer_descriptor` is defined as
"Number of pixels between consecutive rows in the data buffer".

Limitation:
Some display controller needs the pitch of byte be dividable
by an even value (e.g., 4, 64, ...), if the parameter pitch here
is the number of pixels, then the RGB888 (3 bytes per pixel) can't work

Change the unit of `pitch` from pixel to byte

Signed-off-by: Jason Yu <[email protected]>
The `pitch` in `struct display_buffer_descriptor` is changed from
"Number of pixels between consecutive rows in the data buffer" to
"Number of bytes between consecutive rows in the data buffer".

Update the display controller drivers and samples accordingly.

Signed-off-by: Jason Yu <[email protected]>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 9, 2025

@VynDragon
Copy link
Contributor

VynDragon commented Dec 9, 2025

Thanks, I support this initiative, will try to do a review and testing.

static struct co5300_data co5300_data_##node_id = { \
.pixel_format = DT_INST_PROP(node_id, pixel_format), \
.frame_ptr = CO5300_FRAMEBUFFER(node_id), \
.frame_pitch = ROUND_UP(DT_INST_PROP(node_id, width), \
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not delete this, frame pitch needs a initial value after system init


/* Advance source pointer and decrement remaining */
if (local_desc.pitch > local_desc.width) {
if (local_desc.pitch > local_desc.width * data->bytes_per_pixel) {
Copy link
Contributor

Choose a reason for hiding this comment

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

local_desc.pitch > (local_desc.width * data->bytes_per_pixel)?

LCDIF_SetFrameBufferConfig(config->base, 0, &fbConfig);

if (bytes_per_pixel == 3U) {
/* For RGB888 the stride shall be calculated as
Copy link
Contributor

Choose a reason for hiding this comment

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

For pitch in bytes it shall follow the same law here

.buf_size = (w * h) / 8U,
.width = w,
.pitch = w,
.pitch = DIV_ROUND_UP(w, 8),
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to updatelvgl_display_16bit/24bit/32bit as well?

}

buf_desc.pitch = ROUND_UP(rect_w, CONFIG_SAMPLE_PITCH_ALIGN);
switch (capabilities.current_pixel_format) {
Copy link
Contributor

Choose a reason for hiding this comment

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

May be separate a function?

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