| Message ID | 20220121213831.47229-1-igormtorrente@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add new formats support to vkms | expand |
On 01/21, Igor Torrente wrote: > Summary > ======= > This series of patches refactor some vkms components in order to introduce > new formats to the planes and writeback connector. > > Now in the blend function, the plane's pixels are converted to ARGB16161616 > and then blended together. > > The CRC is calculated based on the ARGB1616161616 buffer. And if required, > this buffer is copied/converted to the writeback buffer format. > > And to handle the pixel conversion, new functions were added to convert > from a specific format to ARGB16161616 (the reciprocal is also true). Hi Igor, Thanks a lot for your work to improve the VKMS. Overall, lgtm. I've pointed out some minor improvements, most of them to better describe changes. It seems that your are using a different version of the kms_cursor_crc test and the test results diverge. Can you update and double-check the statictics? I also consider important to keep the version changes in the body of each commit message. Can you move them to a place that it will not be ignored when applying? > > Tests > ===== > This patch series was tested using the following igt tests: > -t ".*kms_plane.*" > -t ".*kms_writeback.*" > -t ".*kms_cursor_crc*" > -t ".*kms_flip.*" > > New tests passing > ------------------- > - pipe-A-cursor-size-change > - pipe-A-cursor-alpha-transparent > > Performance > ----------- > Further optimizing the code, now it's running slightly faster than the V2. > And it consumes less memory than the current implementation in the common case > (more detail in the commit message). > > Results running the IGT tests `kms_cursor_crc`: > > | Frametime | > |:---------------:|:---------:|:--------------:|:------------:|:-------:| > | implmentation | Current | Per-pixel(V1) | Per-line(V2) | V3 | > | frametime range | 8~22 ms | 32~56 ms | 6~19 ms | 5~18 ms | > | Average | 10.0 ms | 35.8 ms | 8.6 ms | 7.3 ms | > > | Memory consumption (output dimensions) | > |:--------------------------------------:| > | Current | This patch | > |:------------------:|:-----------------:| > | Width * Heigth | 2 * Width | > > XRGB to ARGB behavior > ===================== > During the development, I decided to always fill the alpha channel of > the output pixel whenever the conversion from a format without an alpha > channel to ARGB16161616 is necessary. Therefore, I ignore the value > received from the XRGB and overwrite the value with 0xFFFF. And you can also drop this TO-DO here (Clearing primary plane): https://dri.freedesktop.org/docs/drm/gpu/vkms.html#add-plane-features With these points addressed, you can add my r-b to the entire series: Reviewed-by: Melissa Wen <mwen@igalia.com> > > --- > Igor Torrente (9): > drm: vkms: Replace the deprecated drm_mode_config_init > drm: vkms: Alloc the compose frame using vzalloc > drm: vkms: Replace hardcoded value of `vkms_composer.map` to > DRM_FORMAT_MAX_PLANES > drm: vkms: Rename `vkms_composer` to `vkms_frame_info` > drm: vkms: Add fb information to `vkms_writeback_job` > drm: drm_atomic_helper: Add a new helper to deal with the writeback > connector validation > drm: vkms: Refactor the plane composer to accept new formats > drm: vkms: Adds XRGB_16161616 and ARGB_1616161616 formats > drm: vkms: Add support to the RGB565 format > > drivers/gpu/drm/drm_atomic_helper.c | 39 +++ > drivers/gpu/drm/vkms/Makefile | 1 + > drivers/gpu/drm/vkms/vkms_composer.c | 336 +++++++++++++------------- > drivers/gpu/drm/vkms/vkms_drv.c | 6 +- > drivers/gpu/drm/vkms/vkms_drv.h | 20 +- > drivers/gpu/drm/vkms/vkms_formats.c | 279 +++++++++++++++++++++ > drivers/gpu/drm/vkms/vkms_formats.h | 49 ++++ > drivers/gpu/drm/vkms/vkms_plane.c | 47 ++-- > drivers/gpu/drm/vkms/vkms_writeback.c | 32 ++- > include/drm/drm_atomic_helper.h | 3 + > 10 files changed, 600 insertions(+), 212 deletions(-) > create mode 100644 drivers/gpu/drm/vkms/vkms_formats.c > create mode 100644 drivers/gpu/drm/vkms/vkms_formats.h > > -- > 2.30.2 >
Summary ======= This series of patches refactor some vkms components in order to introduce new formats to the planes and writeback connector. Now in the blend function, the plane's pixels are converted to ARGB16161616 and then blended together. The CRC is calculated based on the ARGB1616161616 buffer. And if required, this buffer is copied/converted to the writeback buffer format. And to handle the pixel conversion, new functions were added to convert from a specific format to ARGB16161616 (the reciprocal is also true). Tests ===== This patch series was tested using the following igt tests: -t ".*kms_plane.*" -t ".*kms_writeback.*" -t ".*kms_cursor_crc*" -t ".*kms_flip.*" New tests passing ------------------- - pipe-A-cursor-size-change - pipe-A-cursor-alpha-transparent Performance ----------- Further optimizing the code, now it's running slightly faster than the V2. And it consumes less memory than the current implementation in the common case (more detail in the commit message). Results running the IGT tests `kms_cursor_crc`: | Frametime | |:---------------:|:---------:|:--------------:|:------------:|:-------:| | implmentation | Current | Per-pixel(V1) | Per-line(V2) | V3 | | frametime range | 8~22 ms | 32~56 ms | 6~19 ms | 5~18 ms | | Average | 10.0 ms | 35.8 ms | 8.6 ms | 7.3 ms | | Memory consumption (output dimensions) | |:--------------------------------------:| | Current | This patch | |:------------------:|:-----------------:| | Width * Heigth | 2 * Width | XRGB to ARGB behavior ===================== During the development, I decided to always fill the alpha channel of the output pixel whenever the conversion from a format without an alpha channel to ARGB16161616 is necessary. Therefore, I ignore the value received from the XRGB and overwrite the value with 0xFFFF. --- Igor Torrente (9): drm: vkms: Replace the deprecated drm_mode_config_init drm: vkms: Alloc the compose frame using vzalloc drm: vkms: Replace hardcoded value of `vkms_composer.map` to DRM_FORMAT_MAX_PLANES drm: vkms: Rename `vkms_composer` to `vkms_frame_info` drm: vkms: Add fb information to `vkms_writeback_job` drm: drm_atomic_helper: Add a new helper to deal with the writeback connector validation drm: vkms: Refactor the plane composer to accept new formats drm: vkms: Adds XRGB_16161616 and ARGB_1616161616 formats drm: vkms: Add support to the RGB565 format drivers/gpu/drm/drm_atomic_helper.c | 39 +++ drivers/gpu/drm/vkms/Makefile | 1 + drivers/gpu/drm/vkms/vkms_composer.c | 336 +++++++++++++------------- drivers/gpu/drm/vkms/vkms_drv.c | 6 +- drivers/gpu/drm/vkms/vkms_drv.h | 20 +- drivers/gpu/drm/vkms/vkms_formats.c | 279 +++++++++++++++++++++ drivers/gpu/drm/vkms/vkms_formats.h | 49 ++++ drivers/gpu/drm/vkms/vkms_plane.c | 47 ++-- drivers/gpu/drm/vkms/vkms_writeback.c | 32 ++- include/drm/drm_atomic_helper.h | 3 + 10 files changed, 600 insertions(+), 212 deletions(-) create mode 100644 drivers/gpu/drm/vkms/vkms_formats.c create mode 100644 drivers/gpu/drm/vkms/vkms_formats.h