[RESEND v6 0/9] Add new formats support to vkms

[Melissa Wen]

On 08/19, 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,

I missed it after taking some weeks off.

The entire series LGTM.
I pointed out some nitpicks, but I'll handle when applying to
drm-misc-next.

The series is:
Reviewed-by: Melissa Wen <mwen at igalia.com>

Thank you,

Melissa

> 
> 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
> -----------
> It's running slightly faster than the current implementation.
> 
> Results running the IGT[1] test
> `igt at kms_cursor_crc@pipe-a-cursor-512x512-onscreen` ten times:
> 
> |                  Frametime                   |
> |:--------------------------------------------:|
> |  Implementation |  Current  |   This commit  |
> |:---------------:|:---------:|:--------------:|
> | frametime range |  9~22 ms  |     10~22 ms   |
> |     Average     |  11.4 ms  |     12.32 ms   |
> 
> Memory consumption
> ==================
> It consumes less memory than the current implementation in
> the common case (more detail in the commit message).
> 
> | Memory consumption (output dimensions) |
> |:--------------------------------------:|
> |       Current      |     This patch    |
> |:------------------:|:-----------------:|
> |   Width * Heigth   |     2 * Width     |
> 
> [1] IGT commit id: bc3f6833a12221a46659535dac06ebb312490eb4
> 
> 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.
> 
> Primary plane and CRTC size
> ===========================
> This patch series reworks the blend function to accept a primary plane with
> a different size and position from CRTC.
> Because now we need to fill the background, we had a loss in
> performance with this change
> 
> Alpha channel output for XRGB formats
> =====================================
> There's still an open question about which value the writeback alpha channel
> should be for XRGB formats.
> The current igt test implementation is expecting the channel to not be change.
> But it's not entirely clear if this should be the behavior followed by vkms
> (or any other driver).
> 
> Open issue: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/118
> ---
> 
> Igor Torrente (9):
>   drm: vkms: Replace hardcoded value of `vkms_composer.map` to
>     DRM_FORMAT_MAX_PLANES
>   drm: vkms: Rename `vkms_composer` to `vkms_frame_info`
>   drm: drm_atomic_helper: Add a new helper to deal with the writeback
>     connector validation
>   drm: vkms: get the reference to `drm_framebuffer` instead if coping it
>   drm: vkms: Add fb information to `vkms_writeback_job`
>   drm: vkms: Refactor the plane composer to accept new formats
>   drm: vkms: Supports to the case where primary plane doesn't match the
>     CRTC
>   drm: vkms: Adds XRGB_16161616 and ARGB_1616161616 formats
>   drm: vkms: Add support to the RGB565 format
> 
>  Documentation/gpu/vkms.rst            |   7 +-
>  drivers/gpu/drm/drm_atomic_helper.c   |  39 ++++
>  drivers/gpu/drm/vkms/Makefile         |   1 +
>  drivers/gpu/drm/vkms/vkms_composer.c  | 314 ++++++++++++--------------
>  drivers/gpu/drm/vkms/vkms_drv.h       |  39 +++-
>  drivers/gpu/drm/vkms/vkms_formats.c   | 302 +++++++++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_formats.h   |  12 +
>  drivers/gpu/drm/vkms/vkms_plane.c     |  50 ++--
>  drivers/gpu/drm/vkms/vkms_writeback.c |  39 +++-
>  include/drm/drm_atomic_helper.h       |   3 +
>  10 files changed, 586 insertions(+), 220 deletions(-)
>  create mode 100644 drivers/gpu/drm/vkms/vkms_formats.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_formats.h
> 
> -- 
> 2.30.2
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220820/ca38f5b7/attachment.sig>