Le 13/07/24 - 11:38, Maíra Canal a écrit : > On 5/16/24 10:04, Louis Chauvet wrote: > > 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@xxxxxxxxxxxxx> > > Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx> [...] > > /** > > @@ -265,7 +265,7 @@ void vkms_writeback_row(struct vkms_writeback_job *wb, > > * > > * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) > > */ > > -void *get_pixel_conversion_function(u32 format) > > +pixel_read_t get_pixel_read_function(u32 format) > > { > > switch (format) { > > case DRM_FORMAT_ARGB8888: > > @@ -279,7 +279,16 @@ void *get_pixel_conversion_function(u32 format) > > case DRM_FORMAT_RGB565: > > return &RGB565_to_argb_u16; > > default: > > - return NULL; > > + /* > > + * This is a bug in vkms_plane_atomic_check(). All the supported > > + * format must: > > + * - Be listed in vkms_formats in vkms_plane.c > > + * - Have a pixel_read callback defined here > > + */ > > + WARN(true, > > + "Pixel format %p4cc is not supported by VKMS planes. This is a kernel bug, atomic check must forbid this configuration.\n", > > + &format); > > Well, if this is a kernel bug, shouldn't we use a BUG_ON()/BUG() macro? I don't know what is the "rule" for this kind of bug. Yes it is a kernel bug, but it is recoverable and I can avoid a full kernel crash by using a "do nothing" callback. If you prefer I can replace this by: pr_err("Pixel format %p4cc is [...]", &format); BUG(); and drop the next commit. The kernel will crash if there is a mismatch between advertised supported formats and formats listed here. > Best Regards, > - Maíra > > > * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) > > */ > > -void *get_pixel_write_function(u32 format) > > +pixel_write_t get_pixel_write_function(u32 format) > > { > > switch (format) { > > case DRM_FORMAT_ARGB8888: > > @@ -304,6 +313,15 @@ void *get_pixel_write_function(u32 format) > > case DRM_FORMAT_RGB565: > > return &argb_u16_to_RGB565; > > default: > > - return NULL; > > + /* > > + * This is a bug in vkms_writeback_atomic_check. All the supported > > + * format must: > > + * - Be listed in vkms_wb_formats in vkms_writeback.c > > + * - Have a pixel_write callback defined here > > + */ > > + WARN(true, > > + "Pixel format %p4cc is not supported by VKMS writeback. This is a kernel bug, atomic check must forbid this configuration.\n", > > + &format); > > + return (pixel_write_t)NULL; I can do the same here. Thanks, Louis Chauvet [...] -- Louis Chauvet, Bootlin Embedded Linux and Kernel engineering https://bootlin.com