On Mon, Oct 07, 2024 at 06:51:16PM +0200, Louis Chauvet wrote: > On 07/10/24 - 14:27, Paz Zcharya wrote: > > Add support for pixel format ABGR8888, which is the default format > > on Android devices. This will allow us to use VKMS as the default > > display driver in Android Emulator (Cuttlefish) and increase VKMS > > adoption. > > Hi Paz, > > Thank you for your contribution! > > I am very happy to see new users for VKMS, and I will be glad to add new > formats to VKMS! > > However, as you can see [1], there is a significant rework of the VKMS > formats and composition that should be merged soon. > Thank you for highlighting this. Great work! > This series introduces two key improvements: performance enhancements and > YUV support. These changes involve substantial modifications to the > vkms_format.c file, which may conflict with your work. > > Additionally, I wrote a few patches [2] and [3] a few months ago to > practice with VKMS, and they did not receive any comments, so I believe I > will be able to merge them quickly after [1]. > > In [2], I added many new formats: ABGR, BGRA, RGBA, XBGR, RGBX, BGRX, > BGR565, P010, P012, P016. > Would you mind testing this version to see if it meets your needs? > Yep, this is perfect! > In [3], I did similar work for writeback, but it is not as complete, so I > need to add a patch, almost identical to your code: > > static void argb_u16_to_ABGR8888(u8 *out_pixel, const struct pixel_argb_u16 *in_pixel) > [...] > > Added: WRITE_LINE(XBGR8888_write_line, argb_u16_to_XBGR8888) > > I need to send a v2 of [3] anyway because of conflicts, do you mind if I > take your argb_u16_to_ABGR8888 to integrate it (with your signed-off-by > obviously)? > Yeah, that would be very helpful. Thank you so much! > In any case, if you have time to test, or even better review [1], [2] or > [3], it could be amazing! > Patches look great. I tested them locally after adding ABGR8888 support, and things seem to be working. Let me know if I can assist you with anything else. Thanks, Paz Zcharya > Thank you, > Louis Chauvet > > [1]:https://lore.kernel.org/all/20241007-yuv-v12-0-01c1ada6fec8@xxxxxxxxxxx/ > [2]:https://lore.kernel.org/all/20241007-b4-new-color-formats-v2-0-d47da50d4674@xxxxxxxxxxx/ > [3]:https://lore.kernel.org/all/20240814-writeback_line_by_line-v2-0-36541c717569@xxxxxxxxxxx/ > > > Signed-off-by: Paz Zcharya <pazz@xxxxxxxxxxxx> > > --- > > > > drivers/gpu/drm/vkms/vkms_formats.c | 20 ++++++++++++++++++++ > > drivers/gpu/drm/vkms/vkms_plane.c | 1 + > > drivers/gpu/drm/vkms/vkms_writeback.c | 1 + > > 3 files changed, 22 insertions(+) > > > > diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c > > index 040b7f113a3b..9e9d7290388e 100644 > > --- a/drivers/gpu/drm/vkms/vkms_formats.c > > +++ b/drivers/gpu/drm/vkms/vkms_formats.c > > @@ -73,6 +73,14 @@ static void XRGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixe > > out_pixel->b = (u16)src_pixels[0] * 257; > > } > > > > +static void ABGR8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) > > +{ > > + out_pixel->a = (u16)src_pixels[3] * 257; > > + out_pixel->b = (u16)src_pixels[2] * 257; > > + out_pixel->g = (u16)src_pixels[1] * 257; > > + out_pixel->r = (u16)src_pixels[0] * 257; > > +} > > + > > static void ARGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) > > { > > __le16 *pixels = (__force __le16 *)src_pixels; > > @@ -176,6 +184,14 @@ static void argb_u16_to_XRGB8888(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel > > dst_pixels[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257); > > } > > > > +static void argb_u16_to_ABGR8888(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) > > +{ > > + dst_pixels[3] = DIV_ROUND_CLOSEST(in_pixel->a, 257); > > + dst_pixels[2] = DIV_ROUND_CLOSEST(in_pixel->b, 257); > > + dst_pixels[1] = DIV_ROUND_CLOSEST(in_pixel->g, 257); > > + dst_pixels[0] = DIV_ROUND_CLOSEST(in_pixel->r, 257); > > +} > > + > > static void argb_u16_to_ARGB16161616(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) > > { > > __le16 *pixels = (__force __le16 *)dst_pixels; > > @@ -234,6 +250,8 @@ void *get_pixel_conversion_function(u32 format) > > return &ARGB8888_to_argb_u16; > > case DRM_FORMAT_XRGB8888: > > return &XRGB8888_to_argb_u16; > > + case DRM_FORMAT_ABGR8888: > > + return &ABGR8888_to_argb_u16; > > case DRM_FORMAT_ARGB16161616: > > return &ARGB16161616_to_argb_u16; > > case DRM_FORMAT_XRGB16161616: > > @@ -252,6 +270,8 @@ void *get_pixel_write_function(u32 format) > > return &argb_u16_to_ARGB8888; > > case DRM_FORMAT_XRGB8888: > > return &argb_u16_to_XRGB8888; > > + case DRM_FORMAT_ABGR8888: > > + return &argb_u16_to_ABGR8888; > > case DRM_FORMAT_ARGB16161616: > > return &argb_u16_to_ARGB16161616; > > case DRM_FORMAT_XRGB16161616: > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > > index e5c625ab8e3e..8efd585fc34c 100644 > > --- a/drivers/gpu/drm/vkms/vkms_plane.c > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > @@ -15,6 +15,7 @@ > > static const u32 vkms_formats[] = { > > DRM_FORMAT_ARGB8888, > > DRM_FORMAT_XRGB8888, > > + DRM_FORMAT_ABGR8888, > > DRM_FORMAT_XRGB16161616, > > DRM_FORMAT_ARGB16161616, > > DRM_FORMAT_RGB565 > > diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c > > index bc724cbd5e3a..04cb9c58e7ad 100644 > > --- a/drivers/gpu/drm/vkms/vkms_writeback.c > > +++ b/drivers/gpu/drm/vkms/vkms_writeback.c > > @@ -17,6 +17,7 @@ > > static const u32 vkms_wb_formats[] = { > > DRM_FORMAT_ARGB8888, > > DRM_FORMAT_XRGB8888, > > + DRM_FORMAT_ABGR8888, > > DRM_FORMAT_XRGB16161616, > > DRM_FORMAT_ARGB16161616, > > DRM_FORMAT_RGB565 > > -- > > 2.47.0.rc0.187.ge670bccf7e-goog > >
