Re: [PATCH v4 0/9] Add new formats support to vkms

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>

> 
> ---
> 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
> 

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux