| Message ID | 20240516-yuv-v8-5-cf8d6f86430e@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vkms: Reimplement line-per-line pixel conversion for plane reading | expand |
On 5/16/24 10:04, Louis Chauvet wrote: > Introduce two callbacks which does nothing. They are used in replacement > of NULL and it avoid kernel OOPS if this NULL is called. I don't believe we should avoid a reasonable kernel OOPS. As you noticed, if the user got this kernel OOPS it means that there is a mismatch between what formats are announced by atomic_check and what is really supported by atomic_update. This is a driver error. If this happens, I want the kernel OOPS because it means I need to fix the driver. Sometimes a kernel OOPS can save you some valuable debugging time. I'd probably drop this patch. Best Regards, - Maíra > > If those callback are used, it means that there is a mismatch between > what formats are announced by atomic_check and what is realy supported by > atomic_update. > > Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com> > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> > --- > drivers/gpu/drm/vkms/vkms_formats.c | 45 ++++++++++++++++++++++++++++++++----- > 1 file changed, 39 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c > index 6b3e17374b19..c28c32b00e39 100644 > --- a/drivers/gpu/drm/vkms/vkms_formats.c > +++ b/drivers/gpu/drm/vkms/vkms_formats.c > @@ -135,6 +135,21 @@ static void RGB565_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) > out_pixel->b = drm_fixp2int_round(drm_fixp_mul(fp_b, fp_rb_ratio)); > } > > +/** > + * magenta_to_argb_u16() - pixel_read callback which always read magenta > + * > + * This callback is used when an invalid format is requested for plane reading. > + * It is used to avoid null pointer to be used as a function. In theory, this function should > + * never be called, except if you found a bug in the driver/DRM core. > + */ > +static void magenta_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) > +{ > + out_pixel->a = (u16)0xFFFF; > + out_pixel->r = (u16)0xFFFF; > + out_pixel->g = 0; > + out_pixel->b = (u16)0xFFFF; > +} > + > /** > * vkms_compose_row - compose a single row of a plane > * @stage_buffer: output line with the composed pixels > @@ -237,6 +252,16 @@ static void argb_u16_to_RGB565(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) > *pixel = cpu_to_le16(r << 11 | g << 5 | b); > } > > +/** > + * argb_u16_to_nothing() - pixel_write callback with no effect > + * > + * This callback is used when an invalid format is requested for writeback. > + * It is used to avoid null pointer to be used as a function. In theory, this should never > + * happen, except if there is a bug in the driver > + */ > +static void argb_u16_to_nothing(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) > +{} > + > /** > * vkms_writeback_row() - Generic loop for all supported writeback format. It is executed just > * after the blending to write a line in the writeback buffer. > @@ -260,8 +285,10 @@ void vkms_writeback_row(struct vkms_writeback_job *wb, > > /** > * get_pixel_conversion_function() - Retrieve the correct read_pixel function for a specific > - * format. The returned pointer is NULL for unsupported pixel formats. The caller must ensure that > - * the pointer is valid before using it in a vkms_plane_state. > + * format. > + * > + * If the format is not supported by VKMS a warning is emitted and a dummy "always read magenta" > + * function is returned. > * > * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) > */ > @@ -284,18 +311,21 @@ pixel_read_t get_pixel_read_function(u32 format) > * format must: > * - Be listed in vkms_formats in vkms_plane.c > * - Have a pixel_read callback defined here > + * > + * To avoid kernel crash, a dummy "always read magenta" function is used. It means > + * that during the composition, this plane will always be magenta. > */ > WARN(true, > "Pixel format %p4cc is not supported by VKMS planes. This is a kernel bug, atomic check must forbid this configuration.\n", > &format); > - return (pixel_read_t)NULL; > + return &magenta_to_argb_u16; > } > } > > /** > * get_pixel_write_function() - Retrieve the correct write_pixel function for a specific format. > - * The returned pointer is NULL for unsupported pixel formats. The caller must ensure that the > - * pointer is valid before using it in a vkms_writeback_job. > + * If the format is not supported by VKMS a warning is emitted and a dummy "don't do anything" > + * function is returned. > * > * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) > */ > @@ -318,10 +348,13 @@ pixel_write_t get_pixel_write_function(u32 format) > * format must: > * - Be listed in vkms_wb_formats in vkms_writeback.c > * - Have a pixel_write callback defined here > + * > + * To avoid kernel crash, a dummy "don't do anything" function is used. It means > + * that the resulting writeback buffer is not composed and can contains any values. > */ > 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; > + return &argb_u16_to_nothing; > } > } >
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c index 6b3e17374b19..c28c32b00e39 100644 --- a/drivers/gpu/drm/vkms/vkms_formats.c +++ b/drivers/gpu/drm/vkms/vkms_formats.c @@ -135,6 +135,21 @@ static void RGB565_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) out_pixel->b = drm_fixp2int_round(drm_fixp_mul(fp_b, fp_rb_ratio)); } +/** + * magenta_to_argb_u16() - pixel_read callback which always read magenta + * + * This callback is used when an invalid format is requested for plane reading. + * It is used to avoid null pointer to be used as a function. In theory, this function should + * never be called, except if you found a bug in the driver/DRM core. + */ +static void magenta_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) +{ + out_pixel->a = (u16)0xFFFF; + out_pixel->r = (u16)0xFFFF; + out_pixel->g = 0; + out_pixel->b = (u16)0xFFFF; +} + /** * vkms_compose_row - compose a single row of a plane * @stage_buffer: output line with the composed pixels @@ -237,6 +252,16 @@ static void argb_u16_to_RGB565(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) *pixel = cpu_to_le16(r << 11 | g << 5 | b); } +/** + * argb_u16_to_nothing() - pixel_write callback with no effect + * + * This callback is used when an invalid format is requested for writeback. + * It is used to avoid null pointer to be used as a function. In theory, this should never + * happen, except if there is a bug in the driver + */ +static void argb_u16_to_nothing(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) +{} + /** * vkms_writeback_row() - Generic loop for all supported writeback format. It is executed just * after the blending to write a line in the writeback buffer. @@ -260,8 +285,10 @@ void vkms_writeback_row(struct vkms_writeback_job *wb, /** * get_pixel_conversion_function() - Retrieve the correct read_pixel function for a specific - * format. The returned pointer is NULL for unsupported pixel formats. The caller must ensure that - * the pointer is valid before using it in a vkms_plane_state. + * format. + * + * If the format is not supported by VKMS a warning is emitted and a dummy "always read magenta" + * function is returned. * * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) */ @@ -284,18 +311,21 @@ pixel_read_t get_pixel_read_function(u32 format) * format must: * - Be listed in vkms_formats in vkms_plane.c * - Have a pixel_read callback defined here + * + * To avoid kernel crash, a dummy "always read magenta" function is used. It means + * that during the composition, this plane will always be magenta. */ WARN(true, "Pixel format %p4cc is not supported by VKMS planes. This is a kernel bug, atomic check must forbid this configuration.\n", &format); - return (pixel_read_t)NULL; + return &magenta_to_argb_u16; } } /** * get_pixel_write_function() - Retrieve the correct write_pixel function for a specific format. - * The returned pointer is NULL for unsupported pixel formats. The caller must ensure that the - * pointer is valid before using it in a vkms_writeback_job. + * If the format is not supported by VKMS a warning is emitted and a dummy "don't do anything" + * function is returned. * * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) */ @@ -318,10 +348,13 @@ pixel_write_t get_pixel_write_function(u32 format) * format must: * - Be listed in vkms_wb_formats in vkms_writeback.c * - Have a pixel_write callback defined here + * + * To avoid kernel crash, a dummy "don't do anything" function is used. It means + * that the resulting writeback buffer is not composed and can contains any values. */ 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; + return &argb_u16_to_nothing; } }