[PATCH v4 0/6] drm/vkms: introduce plane rotation property

[Maíra Canal]

On 5/7/23 17:54, Melissa Wen wrote:
> On 04/18, Maíra Canal wrote:
>> This patchset implements all possible rotation value in vkms. All operations
>> were implemented by software by changing the way the pixels are read. The way
>> the blending is performed can be depicted as:
>>
>> - rotate-0:
>>     (x) ---->
>>     ----------------------
>> (y) |                    |
>>   | |                    |
>>   | |                    |
>>   ˇ |                    |
>>     ----------------------
>>
>> - rotate-90:
>>     <---- (y)
>>     ----------------------
>> (x) |                    |
>>   | |                    |
>>   | |                    |
>>   ˇ |                    |
>>     ----------------------
>>
>> - rotate-180:
>>     <---- (x)
>>     ----------------------
>> (y) |                    |
>>   ^ |                    |
>>   | |                    |
>>   | |                    |
>>     ----------------------
>>
>> - rotate-270:
>>     (y) ---->
>>     ----------------------
>> (x) |                    |
>>   ^ |                    |
>>   | |                    |
>>   | |                    |
>>     ----------------------
>>
>> - reflect-x:
>>     <---- (x)
>>     ----------------------
>> (y) |                    |
>>   | |                    |
>>   | |                    |
>>   ˇ |                    |
>>     ----------------------
>>
>> - reflect-y:
>>     (x) ---->
>>     ----------------------
>> (y) |                    |
>>   ^ |                    |
>>   | |                    |
>>   | |                    |
>>     ----------------------
>>
>> The patchset was tested with IGT's kms_rotation_crc tests and also with some
>> additional tests [1] for the reflection operations.
>>
>> In order to avoid code duplication, I introduced a patch that isolates the
>> pixel format convertion and wraps it in a single loop.
>>
>> I tried to apply Ville's suggestion to avoid hand rolled coordinate
>> calculation stuff. Although I couldn't apply all the code suggested by
>> Ville, I was able to remove all the hardcoded code related to the x-offset.
>> As VKMS' composition is performed by line, I still need to indicate the
>> right pixel, which means that I still have some hardcoded code. Thanks for
>> the suggestion, Ville! It really reduced the code complexity.
>> v1 -> v2: https://lore.kernel.org/dri-devel/20230406130138.70752-1-mcanal@igalia.com/T/
>>
>> * Add patch to isolate pixel format conversion (Arthur Grillo).
>>
>> v2 -> v3: https://lore.kernel.org/dri-devel/20230414135151.75975-1-mcanal@igalia.com/T/
>>
>> * Use cpp to calculate pixel size instead of hard-coding (Arthur Grillo).
>> * Don't use the x coordinate in the pixel_read functions (Arthur Grillo).
>> * Use drm_rotate_simplify() to avoid hard-coding rotate-180 (Ville Syrjälä).
>> * Use u8* to input the src_pixels instead of using u16*.
>>
>> v3 -> v4: https://lore.kernel.org/dri-devel/20230417121056.63917-1-mcanal@igalia.com/T/
>>
>> * Create a original rectangle and a rotated rectangle and use the original
>>   rectangle to offset the x-axis (Ville Syrjälä).
>>
>> [1] https://patchwork.freedesktop.org/series/116025/
> 
> Hi Maíra,
> 
> Thanks for adding rotation properties to VKMS!
> Overall, LGTM and this series is:
> 
> Reviewed-by: Melissa Wen <mwen at igalia.com>

Thanks for the review, Melissa! Applied to drm/drm-misc (drm-misc-next).

> 
> As you already applied the first patch, not a big issue, but I see new
> function documentation is missing. Could you send a follow-up patch
> documenting "vkms_compose_row()"? Also, would be good to apply the same
> improvement for the remaining conversion functions too.

I'll try to work on some follow-up patches ASAP.

Best Regards,
- Maíra Canal

> 
> Thanks,
> 
> Melissa
> 
>>
>> Best Regards,
>> - Maíra Canal
>>
>> Maíra Canal (6):
>>   drm/vkms: isolate pixel conversion functionality
>>   drm/vkms: add rotate-0 and reflect-x property
>>   drm/vkms: add reflect-y and rotate-180 property
>>   drm/vkms: add rotate-90 property
>>   drm/vkms: add rotate-270 property
>>   drm/vkms: drop "Rotation" TODO
>>
>>  Documentation/gpu/vkms.rst           |   2 +-
>>  drivers/gpu/drm/vkms/vkms_composer.c |  38 ++++++--
>>  drivers/gpu/drm/vkms/vkms_drv.h      |   6 +-
>>  drivers/gpu/drm/vkms/vkms_formats.c  | 139 +++++++++++++--------------
>>  drivers/gpu/drm/vkms/vkms_formats.h  |   2 +-
>>  drivers/gpu/drm/vkms/vkms_plane.c    |  16 ++-
>>  6 files changed, 117 insertions(+), 86 deletions(-)
>>
>> -- 
>> 2.39.2
>>