| Age | Commit message (Collapse) | Author |
|---|
|
Re-introduce a line-by-line composition algorithm for each pixel format.
This allows more performance by not requiring an indirection per pixel
read. This patch is focused on readability of the code.
Line-by-line composition was introduced by [1] but rewritten back to
pixel-by-pixel algorithm in [2]. At this time, nobody noticed the impact
on performance, and it was merged.
This patch is almost a revert of [2], but in addition efforts have been
made to increase readability and maintainability of the rotation handling.
The blend function is now divided in two parts:
- Transformation of coordinates from the output referential to the source
referential
- Line conversion and blending
Most of the complexity of the rotation management is avoided by using
drm_rect_* helpers. The remaining complexity is around the clipping, to
avoid reading/writing outside source/destination buffers.
The pixel conversion is now done line-by-line, so the read_pixel_t was
replaced with read_pixel_line_t callback. This way the indirection is only
required once per line and per plane, instead of once per pixel and per
plane.
The read_line_t callbacks are very similar for most pixel format, but it
is required to avoid performance impact. Some helpers for color
conversion were introduced to avoid code repetition:
- *_to_argb_u16: perform colors conversion. They should be inlined by the
compiler, and they are used to avoid repetition between multiple variants
of the same format (argb/xrgb and maybe in the future for formats like
bgr formats).
This new algorithm was tested with:
- kms_plane (for color conversions)
- kms_rotation_crc (for rotations of planes)
- kms_cursor_crc (for translations of planes)
- kms_rotation (for all rotations and formats combinations) [3]
The performance gain was mesured with kms_fb_stress [4] with some
modification to fix the writeback format.
The performance improvement is around 5 to 10%.
[1]: commit 8ba1648567e2 ("drm: vkms: Refactor the plane composer to accept
new formats")
https://lore.kernel.org/all/20220905190811.25024-7-igormtorrente@gmail.com/
[2]: commit 322d716a3e8a ("drm/vkms: isolate pixel conversion
functionality")
https://lore.kernel.org/all/20230418130525.128733-2-mcanal@igalia.com/
[3]: https://lore.kernel.org/igt-dev/20240313-new_rotation-v2-0-6230fd5cae59@bootlin.com/
[4]: https://lore.kernel.org/all/20240422-kms_fb_stress-dev-v5-0-0c577163dc88@riseup.net/
Reviewed-by: José Expósito <jose.exposito89@gmail.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241118-yuv-v14-8-2dbc2f1e222c@bootlin.com
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
|
|
The pixel_read_direction enum is useful to describe the reading direction
in a plane. It avoids using the rotation property of DRM, which not
practical to know the direction of reading.
This patch also introduce two helpers, one to compute the
pixel_read_direction from the DRM rotation property, and one to compute
the step, in byte, between two successive pixel in a specific direction.
Acked-by: Maíra Canal <mairacanal@riseup.net>
Reviewed-by: José Expósito <jose.exposito89@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241118-yuv-v14-7-2dbc2f1e222c@bootlin.com
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
|
|
Introduce the usage of block_h/block_w to compute the offset and the
pointer of a pixel. The previous implementation was specialized for
planes with block_h == block_w == 1. To avoid confusion and allow easier
implementation of tiled formats. It also remove the usage of the
deprecated format field `cpp`.
Introduce the plane_index parameter to get an offset/pointer on a
different plane.
Acked-by: Maíra Canal <mairacanal@riseup.net>
Reviewed-by: José Expósito <jose.exposito89@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241118-yuv-v14-5-2dbc2f1e222c@bootlin.com
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
|
|
As the pixel_read and pixel_write function should never modify the input
buffer, mark those pointers const.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Reviewed-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: José Expósito <jose.exposito89@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241118-yuv-v14-4-2dbc2f1e222c@bootlin.com
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
|
|
Introduce two typedefs: pixel_read_t and pixel_write_t. It allows the
compiler to check if the passed functions take the correct arguments.
Such typedefs will help ensuring consistency across the code base in
case of update of these prototypes.
Rename input/output variable in a consistent way between read_line and
write_line.
A warn has been added in get_pixel_*_function to alert when an unsupported
pixel format is requested. As those formats are checked before
atomic_update callbacks, it should never happen.
Document for those typedefs.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Reviewed-by: José Expósito <jose.exposito89@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241118-yuv-v14-3-2dbc2f1e222c@bootlin.com
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
|
|
Remove intermidiary variables and access the variables directly from
drm_frame. These changes should be noop.
Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Reviewed-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: José Expósito <jose.exposito89@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
[Louis Chauvet: Applied review from Maíra]
Link: https://patchwork.freedesktop.org/patch/msgid/20241118-yuv-v14-2-2dbc2f1e222c@bootlin.com
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
|
|
Add documentation around vkms_output and its initialization.
Add some documentation on pixel conversion functions.
Update of outdated comments for pixel_write functions.
Reviewed-by: José Expósito <jose.exposito89@gmail.com>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240905-google-clarifications-v4-1-e43c1f2e3d87@bootlin.com
|
|
Building with Sparse enabled prints this warning for cpu_to_le16()
calls:
warning: incorrect type in assignment (different base types)
expected unsigned short [usertype]
got restricted __le16 [usertype]
And this warning for le16_to_cpu() calls:
warning: cast to restricted __le16
Declare the target buffer as __le16 to fix both warnings.
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Acked-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240716161725.41408-2-jose.exposito89@gmail.com
|
|
All convertions from the ARGB16161616 format follow the same structure.
Instead of repeting the same structure for each supported format, create
a function to encapsulate the common logic and isolate the pixel
conversion functions in a callback function.
Suggested-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Arthur Grillo <arthurgrillo@riseup.net>
Signed-off-by: Maíra Canal <mairacanal@riseup.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20230515135204.115393-4-mcanal@igalia.com
|
|
The function vkms_compose_row() was introduced in the code without any
documentation. In order to make the function more clear, add a
kernel-doc to it.
Suggested-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Maíra Canal <mairacanal@riseup.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20230508220030.434118-1-mcanal@igalia.com
|
|
Currently, the pixel conversion isn't rounding the fixed-point values
before assigning it to the RGB coefficients, which is causing the IGT
pixel-format tests to fail. So, use the drm_fixp2int_round() fixed-point
helper to round the values when assigning it to the RGB coefficients.
Tested with igt@kms_plane@pixel-format and igt@kms_plane@pixel-format-source-clamping.
[v2]:
* Use drm_fixp2int_round() to fix the pixel conversion instead of
casting the values to s32 (Melissa Wen).
Fixes: 89b03aeaef16 ("drm/vkms: fix 32bit compilation error by replacing macros")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Arthur Grillo <arthurgrillo@riseup.net>
Signed-off-by: Maíra Canal <mairacanal@riseup.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20230512104044.65034-2-mcanal@igalia.com
|
|
Currently, vkms supports the rotate-90, rotate-180, reflect-x and
reflect-y properties. Therefore, improve the vkms IGT test coverage by
adding the rotate-270 property to vkms. The rotation was implement by
software: rotate the way the blending occurs by making the source x axis
be the destination y axis and the source y axis be the destination x
axis and reverse-read the axis.
Now, vkms supports all possible rotation values.
Tested with igt@kms_rotation_crc@primary-rotation-270 [1],
and igt@kms_rotation_crc@sprite-rotation-270 [1].
[1] https://patchwork.freedesktop.org/series/116025/
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mairacanal@riseup.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20230418130525.128733-6-mcanal@igalia.com
|
|
Currently, vkms only supports the rotate-180, reflect-x and reflect-y
properties. Therefore, improve the vkms IGT test coverage by adding the
rotate-90 property to vkms. The rotation was implement by software: rotate
the way the blending occurs by making the source x axis be the destination
y axis and the source y axis be the destination x axis.
Tested with igt@kms_rotation_crc@primary-rotation-90 [1],
igt@kms_rotation_crc@sprite-rotation-90 [1], and
igt@kms_rotation_crc@sprite-rotation-90-pos-100-0 [1].
[1] https://patchwork.freedesktop.org/series/116025/
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mairacanal@riseup.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20230418130525.128733-5-mcanal@igalia.com
|
|
Currently, vkms doesn't support any reflection property. Therefore, add
the reflect-x property to vkms through a software implementation of the
operation. This is possible by reverse reading the x axis during the
blending.
Tested with igt@kms_rotation_crc@primary-reflect-x [1] and
igt@kms_rotation_crc@sprite-reflect-x [1].
[1] https://patchwork.freedesktop.org/series/116025/
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mairacanal@riseup.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20230418130525.128733-3-mcanal@igalia.com
|
|
Currently, the pixel conversion functions repeat the same loop to
iterate the rows. Instead of repeating the same code for each pixel
format, create a function to wrap the loop and isolate the pixel
conversion functionality.
Suggested-by: Arthur Grillo <arthurgrillo@riseup.net>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Arthur Grillo <arthurgrillo@riseup.net>
Signed-off-by: Maíra Canal <mairacanal@riseup.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20230418130525.128733-2-mcanal@igalia.com
|
|
Replace vkms_formats macro for fixed-point operations with functions
from drm/drm_fixed.h to do the same job and fix 32-bit compilation
errors.
v2:
- don't cast results to s32 (Igor)
- add missing drm_fixp2int conversion (Igor)
Fixes: a19c2ac9858 ("drm: vkms: Add support to the RGB565 format")
Tested-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com> (v1)
Reviewed-by: Alex Deucher <alexander.deucher@amd.com> (v1)
Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220910190303.682897-1-mwen@igalia.com
|
|
This commit also adds new helper macros to deal with fixed-point
arithmetic.
It was done to improve the precision of the conversion to ARGB16161616
since the "conversion ratio" is not an integer.
V3: Adapt the handlers to the new format introduced in patch 7 V3.
V5: Minor improvements
V6: Minor improvements (Pekka Paalanen)
Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Igor Torrente <igormtorrente@gmail.com>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220905190811.25024-10-igormtorrente@gmail.com
|
|
This will be useful to write tests that depends on these formats.
ARGB and XRGB follows the a similar implementation of the former formats.
Just adjusting for 16 bits per channel.
V3: Adapt the handlers to the new format introduced in patch 7 V3.
V5: Minor improvements
Added le16_to_cpu/cpu_to_le16 to the 16 bits color read/writes.
Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Igor Torrente <igormtorrente@gmail.com>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220905190811.25024-9-igormtorrente@gmail.com
|
|
Currently the blend function only accepts XRGB_8888 and ARGB_8888
as a color input.
This patch refactors all the functions related to the plane composition
to overcome this limitation.
The pixels blend is done using the new internal format. And new handlers
are being added to convert a specific format to/from this internal format.
So the blend operation depends on these handlers to convert to this common
format. The blended result, if necessary, is converted to the writeback
buffer format.
This patch introduces three major differences to the blend function.
1 - All the planes are blended at once.
2 - The blend calculus is done as per line instead of per pixel.
3 - It is responsible to calculates the CRC and writing the writeback
buffer(if necessary).
These changes allow us to allocate way less memory in the intermediate
buffer to compute these operations. Because now we don't need to
have the entire intermediate image lines at once, just one line is
enough.
| Memory consumption (output dimensions) |
|:--------------------------------------:|
| Current | This patch |
|:------------------:|:-----------------:|
| Width * Heigth | 2 * Width |
Beyond memory, we also have a minor performance benefit from all
these changes. Results running the IGT[1] test
`igt@kms_cursor_crc@pipe-a-cursor-512x512-onscreen` ten times:
| Frametime |
|:------------------------------------------:|
| Implementation | Current | This commit |
|:---------------:|:---------:|:------------:|
| frametime range | 9~22 ms | 5~17 ms |
| Average | 11.4 ms | 7.8 ms |
[1] IGT commit id: bc3f6833a12221a46659535dac06ebb312490eb4
V2: Improves the performance drastically, by performing the operations
per-line and not per-pixel(Pekka Paalanen).
Minor improvements(Pekka Paalanen).
V3: Changes the code to blend the planes all at once. This improves
performance, memory consumption, and removes much of the weirdness
of the V2(Pekka Paalanen and me).
Minor improvements(Pekka Paalanen and me).
V4: Rebase the code and adapt it to the new NUM_OVERLAY_PLANES constant.
V5: Minor checkpatch fixes and the removal of TO-DO item(Melissa Wen).
Several security/robustness improvents(Pekka Paalanen).
Removes check_planes_x_bounds function and allows partial
partly off-screen(Pekka Paalanen).
V6: Fix a mismatch of some variable sizes (Pekka Paalanen).
Several minor improvements (Pekka Paalanen).
Reviewed-by: Melissa Wen <mwen@igalia.com>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Igor Torrente <igormtorrente@gmail.com>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220905190811.25024-7-igormtorrente@gmail.com
|
