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@xxxxxxxxxxxxx>
Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
---
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;
}
}
