[v2,0/9] drm/vkms: Reimplement line-per-line pixel conversion for plane reading

Message

Louis Chauvet Feb. 23, 2024, 11:37 a.m. UTC

This patchset is the second version of [1]. It is almost a complete 
rewrite to use a line-by-line algorithm for the composition.
It can be divided in three parts:
- PATCH 1 to 4: no functional change is intended, only some formatting and 
documenting
(PATCH 2 is taken from [2])
- PATCH 5: main patch for this series, it reintroduce the 
line-by-line algorithm
- PATCH 6 to 9: taken from Arthur's series [2], with sometimes adaptation 
to use the pixel-by-pixel algorithm.

The PATCH 5 aims to restore the line-by-line pixel reading algorithm. It 
was introduced in 8ba1648567e2 ("drm: vkms: Refactor the plane composer to 
accept new formats") but removed in 8ba1648567e2 ("drm: vkms: Refactor the 
plane composer to accept new formats") in a over-simplification effort. 
At this time, nobody noticed the performance impact of this commit. After 
the first iteration of my series, poeple notice performance impact, and it 
was the case. Pekka suggested to reimplement the line-by-line algorithm.

Expiriments on my side shown great improvement for the line-by-line 
algorithm, and the performances are the same as the original line-by-line 
algorithm. I targeted my effort to make the code working for all the 
rotations and translations. The usage of helpers from drm_rect_* avoid 
reimplementing existing logic.

The only "complex" part remaining is the clipping of the coordinate to 
avoid reading/writing outside of src/dst. Thus I added a lot of comments 
to help when someone will want to add some features (framebuffer resizing 
for example).

The YUV part is not mandatory for this series, but as my first effort was 
to help the integration of YUV, I decided to rebase Arthur's series on 
mine to help. I took [3], [4], [5] and [6] and adapted them to use the 
line-by-line reading. If I did something wrong here, please let me 
know.

My series was mainly tested with:
- kms_plane (for color conversions)
- kms_rotation_crc (for rotations of planes)
- kms_cursor_crc (for translations)
The benchmark used to measure the improvment was done with:
- kms_fb_stress

[1]: https://lore.kernel.org/r/[e-mail]
[2]: https://lore.kernel.org/all/[e-mail]/
[3]: https://lore.kernel.org/all/[e-mail]/
[4]: https://lore.kernel.org/all/[e-mail]/
[5]: https://lore.kernel.org/all/[e-mail]/
[6]: https://lore.kernel.org/all/[e-mail]/

To: Rodrigo Siqueira <[e-mail]>
To: Melissa Wen <[e-mail]>
To: Maíra Canal <[e-mail]>
To: Haneen Mohammed <[e-mail]>
To: Daniel Vetter <[e-mail]>
To: Maarten Lankhorst <[e-mail]>
To: Maxime Ripard <[e-mail]>
To: Thomas Zimmermann <[e-mail]>
To: David Airlie <[e-mail]>
To: [e-mail]
To: Jonathan Corbet <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Signed-off-by: Louis Chauvet <[e-mail]>

Note: after my changes, those tests seems to pass, so [7] may need 
updating (I did not check, it was maybe already the case):
- kms_cursor_legacy@flip-vs-cursor-atomic
- kms_pipe_crc_basic@nonblocking-crc
- kms_pipe_crc_basic@nonblocking-crc-frame-sequence
- kms_writeback@writeback-pixel-formats
- kms_writeback@writeback-invalid-parameters
- kms_flip@flip-vs-absolute-wf_vblank-interruptible
And those tests pass, I did not investigate why the runners fails:
- kms_flip@flip-vs-expired-vblank-interruptible
- kms_flip@flip-vs-expired-vblank
- kms_flip@plain-flip-fb-recreate
- kms_flip@plain-flip-fb-recreate-interruptible
- kms_flip@plain-flip-ts-check-interruptible
- kms_cursor_legacy@cursorA-vs-flipA-toggle
- kms_pipe_crc_basic@nonblocking-crc
- kms_prop_blob@invalid-get-prop
- kms_flip@flip-vs-absolute-wf_vblank-interruptible
- kms_invalid_mode@zero-hdisplay
- kms_invalid_mode@bad-vtotal
- kms_cursor_crc.* (everything is SUCCEED or SKIP, but no fails)

[7]: https://lore.kernel.org/all/[e-mail]/

Changes in v2:
- Rebased the series on top of drm-misc/drm-misc-net
- Extract the typedef for pixel_read/pixel_write
- Introduce the line-by-line algorithm per pixel format
- Add some documentation for existing and new code
- Port the series [1] to use line-by-line algorithm
- Link to v1: https://lore.kernel.org/r/[e-mail]

---
Arthur Grillo (5):
      drm/vkms: Use drm_frame directly
      drm/vkms: Add YUV support
      drm/vkms: Add range and encoding properties to pixel_read function
      drm/vkms: Drop YUV formats TODO
      drm/vkms: Create KUnit tests for YUV conversions

Louis Chauvet (4):
      drm/vkms: Code formatting
      drm/vkms: write/update the documentation for pixel conversion and pixel write functions
      drm/vkms: Add typedef and documentation for pixel_read and pixel_write functions
      drm/vkms: Re-introduce line-per-line composition algorithm

 Documentation/gpu/vkms.rst                    |   3 +-
 drivers/gpu/drm/vkms/Makefile                 |   1 +
 drivers/gpu/drm/vkms/tests/.kunitconfig       |   4 +
 drivers/gpu/drm/vkms/tests/Makefile           |   3 +
 drivers/gpu/drm/vkms/tests/vkms_format_test.c | 155 +++++++
 drivers/gpu/drm/vkms/vkms_composer.c          | 233 ++++++++---
 drivers/gpu/drm/vkms/vkms_crtc.c              |   6 +-
 drivers/gpu/drm/vkms/vkms_drv.c               |   3 +-
 drivers/gpu/drm/vkms/vkms_drv.h               |  56 ++-
 drivers/gpu/drm/vkms/vkms_formats.c           | 565 +++++++++++++++++++++-----
 drivers/gpu/drm/vkms/vkms_formats.h           |  13 +-
 drivers/gpu/drm/vkms/vkms_plane.c             |  50 ++-
 drivers/gpu/drm/vkms/vkms_writeback.c         |  14 +-
 13 files changed, 916 insertions(+), 190 deletions(-)
---
base-commit: aa1267e673fe5307cf00d02add4017d2878598b6
change-id: 20240201-yuv-1337d90d9576

Best regards,

Comments

Maira Canal Feb. 23, 2024, 11:52 a.m. UTC | #1

Hi Louis,

On 2/23/24 08:37, Louis Chauvet wrote:
> This patchset is the second version of [1]. It is almost a complete
> rewrite to use a line-by-line algorithm for the composition.
> It can be divided in three parts:
> - PATCH 1 to 4: no functional change is intended, only some formatting and
> documenting
> (PATCH 2 is taken from [2])
> - PATCH 5: main patch for this series, it reintroduce the
> line-by-line algorithm
> - PATCH 6 to 9: taken from Arthur's series [2], with sometimes adaptation
> to use the pixel-by-pixel algorithm.
> 
> The PATCH 5 aims to restore the line-by-line pixel reading algorithm. It
> was introduced in 8ba1648567e2 ("drm: vkms: Refactor the plane composer to
> accept new formats") but removed in 8ba1648567e2 ("drm: vkms: Refactor the
> plane composer to accept new formats") in a over-simplification effort.
> At this time, nobody noticed the performance impact of this commit. After
> the first iteration of my series, poeple notice performance impact, and it
> was the case. Pekka suggested to reimplement the line-by-line algorithm.
> 
> Expiriments on my side shown great improvement for the line-by-line
> algorithm, and the performances are the same as the original line-by-line
> algorithm. I targeted my effort to make the code working for all the
> rotations and translations. The usage of helpers from drm_rect_* avoid
> reimplementing existing logic.
> 
> The only "complex" part remaining is the clipping of the coordinate to
> avoid reading/writing outside of src/dst. Thus I added a lot of comments
> to help when someone will want to add some features (framebuffer resizing
> for example).
> 
> The YUV part is not mandatory for this series, but as my first effort was
> to help the integration of YUV, I decided to rebase Arthur's series on
> mine to help. I took [3], [4], [5] and [6] and adapted them to use the
> line-by-line reading. If I did something wrong here, please let me
> know.
> 
> My series was mainly tested with:
> - kms_plane (for color conversions)
> - kms_rotation_crc (for rotations of planes)
> - kms_cursor_crc (for translations)
> The benchmark used to measure the improvment was done with:
> - kms_fb_stress
> 
> [1]: https://lore.kernel.org/r/[e-mail]
> [2]: https://lore.kernel.org/all/[e-mail]/
> [3]: https://lore.kernel.org/all/[e-mail]/
> [4]: https://lore.kernel.org/all/[e-mail]/
> [5]: https://lore.kernel.org/all/[e-mail]/
> [6]: https://lore.kernel.org/all/[e-mail]/
> 
> To: Rodrigo Siqueira <[e-mail]>
> To: Melissa Wen <[e-mail]>
> To: Maíra Canal <[e-mail]>
> To: Haneen Mohammed <[e-mail]>
> To: Daniel Vetter <[e-mail]>
> To: Maarten Lankhorst <[e-mail]>
> To: Maxime Ripard <[e-mail]>
> To: Thomas Zimmermann <[e-mail]>
> To: David Airlie <[e-mail]>
> To: [e-mail]
> To: Jonathan Corbet <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Signed-off-by: Louis Chauvet <[e-mail]>
> 
> Note: after my changes, those tests seems to pass, so [7] may need
> updating (I did not check, it was maybe already the case):
> - kms_cursor_legacy@flip-vs-cursor-atomic
> - kms_pipe_crc_basic@nonblocking-crc
> - kms_pipe_crc_basic@nonblocking-crc-frame-sequence
> - kms_writeback@writeback-pixel-formats
> - kms_writeback@writeback-invalid-parameters
> - kms_flip@flip-vs-absolute-wf_vblank-interruptible
> And those tests pass, I did not investigate why the runners fails:
> - kms_flip@flip-vs-expired-vblank-interruptible
> - kms_flip@flip-vs-expired-vblank
> - kms_flip@plain-flip-fb-recreate
> - kms_flip@plain-flip-fb-recreate-interruptible
> - kms_flip@plain-flip-ts-check-interruptible
> - kms_cursor_legacy@cursorA-vs-flipA-toggle
> - kms_pipe_crc_basic@nonblocking-crc
> - kms_prop_blob@invalid-get-prop
> - kms_flip@flip-vs-absolute-wf_vblank-interruptible
> - kms_invalid_mode@zero-hdisplay
> - kms_invalid_mode@bad-vtotal
> - kms_cursor_crc.* (everything is SUCCEED or SKIP, but no fails)

This is great news! Could you just adjust the series fixing the
compiling errors?

Best Regards,
- Maíra

> 
> [7]: https://lore.kernel.org/all/[e-mail]/
> 
> Changes in v2:
> - Rebased the series on top of drm-misc/drm-misc-net
> - Extract the typedef for pixel_read/pixel_write
> - Introduce the line-by-line algorithm per pixel format
> - Add some documentation for existing and new code
> - Port the series [1] to use line-by-line algorithm
> - Link to v1: https://lore.kernel.org/r/[e-mail]
> 
> ---
> Arthur Grillo (5):
>        drm/vkms: Use drm_frame directly
>        drm/vkms: Add YUV support
>        drm/vkms: Add range and encoding properties to pixel_read function
>        drm/vkms: Drop YUV formats TODO
>        drm/vkms: Create KUnit tests for YUV conversions
> 
> Louis Chauvet (4):
>        drm/vkms: Code formatting
>        drm/vkms: write/update the documentation for pixel conversion and pixel write functions
>        drm/vkms: Add typedef and documentation for pixel_read and pixel_write functions
>        drm/vkms: Re-introduce line-per-line composition algorithm
> 
>   Documentation/gpu/vkms.rst                    |   3 +-
>   drivers/gpu/drm/vkms/Makefile                 |   1 +
>   drivers/gpu/drm/vkms/tests/.kunitconfig       |   4 +
>   drivers/gpu/drm/vkms/tests/Makefile           |   3 +
>   drivers/gpu/drm/vkms/tests/vkms_format_test.c | 155 +++++++
>   drivers/gpu/drm/vkms/vkms_composer.c          | 233 ++++++++---
>   drivers/gpu/drm/vkms/vkms_crtc.c              |   6 +-
>   drivers/gpu/drm/vkms/vkms_drv.c               |   3 +-
>   drivers/gpu/drm/vkms/vkms_drv.h               |  56 ++-
>   drivers/gpu/drm/vkms/vkms_formats.c           | 565 +++++++++++++++++++++-----
>   drivers/gpu/drm/vkms/vkms_formats.h           |  13 +-
>   drivers/gpu/drm/vkms/vkms_plane.c             |  50 ++-
>   drivers/gpu/drm/vkms/vkms_writeback.c         |  14 +-
>   13 files changed, 916 insertions(+), 190 deletions(-)
> ---
> base-commit: aa1267e673fe5307cf00d02add4017d2878598b6
> change-id: 20240201-yuv-1337d90d9576
> 
> Best regards,