[PATCH 3/5] drm/panic: Add a set_pixel() callback to drm_scanout_buffer
This allows drivers to draw the pixel, and handle tiling, or specific color formats. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_panic.c | 120 +++- include/drm/drm_panic.h | 9 +++ 2 files changed, 85 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 09d7d45f80c2..94558087bba3 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -251,40 +251,54 @@ static void drm_panic_blit32(struct iosys_map *dmap, unsigned int dpitch, iosys_map_wr(dmap, y * dpitch + x * sizeof(u32), u32, color); } +static void drm_panic_blit_pixel(struct drm_scanout_buffer *sb, struct drm_rect *clip, +const u8 *sbuf8, unsigned int spitch, u32 color) +{ + unsigned int y, x; + + for (y = 0; y < drm_rect_height(clip); y++) + for (x = 0; x < drm_rect_width(clip); x++) + if (sbuf8[(y * spitch) + x / 8] & (0x80 >> (x % 8))) + sb->set_pixel(sb, clip->x1 + x, clip->y1 + y, color); +} + /* * drm_panic_blit - convert a monochrome image to a linear framebuffer - * @dmap: destination iosys_map - * @dpitch: destination pitch in bytes + * @sb: destination scanout buffer + * @clip: destination rectangle * @sbuf8: source buffer, in monochrome format, 8 pixels per byte. * @spitch: source pitch in bytes - * @height: height of the image to copy, in pixels - * @width: width of the image to copy, in pixels * @color: foreground color, in destination format - * @pixel_width: pixel width in bytes. * * This can be used to draw a font character, which is a monochrome image, to a * framebuffer in other supported format. */ -static void drm_panic_blit(struct iosys_map *dmap, unsigned int dpitch, - const u8 *sbuf8, unsigned int spitch, - unsigned int height, unsigned int width, - u32 color, unsigned int pixel_width) +static void drm_panic_blit(struct drm_scanout_buffer *sb, struct drm_rect *clip, + const u8 *sbuf8, unsigned int spitch, u32 color) { - switch (pixel_width) { + struct iosys_map map; + + if (sb->set_pixel) + return drm_panic_blit_pixel(sb, clip, sbuf8, spitch, color); + + map = sb->map[0]; + iosys_map_incr(, clip->y1 * sb->pitch[0] + clip->x1 * sb->format->cpp[0]); + + switch (sb->format->cpp[0]) { case 2: - drm_panic_blit16(dmap, dpitch, sbuf8, spitch, -height, width, color); + drm_panic_blit16(, sb->pitch[0], sbuf8, spitch, +drm_rect_height(clip), drm_rect_width(clip), color); break; case 3: - drm_panic_blit24(dmap, dpitch, sbuf8, spitch, -height, width, color); + drm_panic_blit24(, sb->pitch[0], sbuf8, spitch, +drm_rect_height(clip), drm_rect_width(clip), color); break; case 4: - drm_panic_blit32(dmap, dpitch, sbuf8, spitch, -height, width, color); + drm_panic_blit32(, sb->pitch[0], sbuf8, spitch, +drm_rect_height(clip), drm_rect_width(clip), color); break; default: - WARN_ONCE(1, "Can't blit with pixel width %d\n", pixel_width); + WARN_ONCE(1, "Can't blit with pixel width %d\n", sb->format->cpp[0]); } } @@ -328,33 +342,51 @@ static void drm_panic_fill32(struct iosys_map *dmap, unsigned int dpitch, iosys_map_wr(dmap, y * dpitch + x * sizeof(u32), u32, color); } +static void drm_panic_fill_pixel(struct drm_scanout_buffer *sb, +struct drm_rect *clip, +u32 color) +{ + unsigned int y, x; + + for (y = 0; y < drm_rect_height(clip); y++) + for (x = 0; x < drm_rect_width(clip); x++) + sb->set_pixel(sb, clip->x1 + x, clip->y1 + y, color); +} + /* * drm_panic_fill - Fill a rectangle with a color - * @dmap: destination iosys_map, pointing to the top left corner of the rectangle - * @dpitch: destination pitch in bytes - * @height: height of the rectangle, in pixels - * @width: width of the rectangle, in pixels - * @color: color to fill the rectangle. - * @pixel_width: pixel width in bytes + * @sb: destination scanout buffer + * @clip: destination rectangle + * @color: foreground color, in destination format * * Fill a rectangle with a color, in a linear framebuffer. */ -static void drm_panic_fill(struct iosys_map *dmap, unsigned int dpitch, - unsigned int height, unsigned
[PATCH 5/5] drm/nouveau: Add drm_panic support for nv50+
Add drm_panic support, for nv50+ cards. It's enough to get the panic screen while running Gnome/Wayland on a GTX 1650. It doesn't support multi-plane or compressed format. Support for other formats and older cards will come later. Tiling is only tested on GTX1650, and might be wrong for other cards. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 127 +++- 1 file changed, 125 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c index 7a2cceaee6e9..dd7aafb9198a 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c @@ -30,11 +30,16 @@ #include #include +#include + #include #include #include -#include #include +#include +#include +#include +#include #include "nouveau_bo.h" #include "nouveau_gem.h" @@ -577,6 +582,113 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) return 0; } +/* Values found on GTX1650 */ +/* blocks level 0, 4x4 pixels */ +#define BL0_W 4 +/* blocks level 1, 8x8 pixels */ +#define BL1_W 8 +/* blocks level 2, Group of Bytes? 16x128 pixels */ +#define BL2_W 16 + +/* get the offset in bytes inside the framebuffer, after taking tiling into account */ +static unsigned int nv50_get_tiled_offset(struct drm_scanout_buffer *sb, unsigned int gobs, + unsigned int x, unsigned int y, unsigned int px_width) +{ + u32 blk2_x, blk2_y, bl2sz; + u32 blk1_x, blk1_y, bl1sz; + u32 blk0_x, blk0_y, bl0sz; + u32 nblk2w, bl2_h, off; + + /* fixme - block2 height depends of the "Group of Bytes" value */ + bl2_h = BL1_W * gobs; + + bl0sz = BL0_W * BL0_W * px_width; + bl1sz = BL1_W * BL1_W * px_width; + bl2sz = BL2_W * bl2_h * px_width; + + /* block level 2 coordinate */ + blk2_x = x / BL2_W; + blk2_y = y / bl2_h; + + x = x % BL2_W; + y = y % bl2_h; + + /* block level 1 coordinate */ + blk1_x = x / BL1_W; + blk1_y = y / BL1_W; + + x = x % BL1_W; + y = y % BL1_W; + + /* block level 0 coordinate */ + blk0_x = x / BL0_W; + blk0_y = y / BL0_W; + + x = x % BL0_W; + y = y % BL0_W; + + nblk2w = DIV_ROUND_UP(sb->width, BL2_W); + + off = ((blk2_y * nblk2w) + blk2_x) * bl2sz; + off += ((blk1_y * 2) + blk1_x) * bl1sz; + off += (blk0_y * 2 + blk0_x) * bl0sz; + off += (x + y * BL0_W) * px_width; + + return off; +} + +static void nv50_set_pixel(struct drm_scanout_buffer *sb, unsigned int x, unsigned int y, u32 color) +{ + struct drm_framebuffer *fb = sb->private; + unsigned int off; + /* According to DRM_FORMAT_MOD_NVIDIA_BLOCK_LINEAR_2D documentation, +* the last 4 bits of modifier is log2(height) of each block, in GOBs +*/ + unsigned int gobs = 1 << (fb->modifier & 0xf); + + off = nv50_get_tiled_offset(sb, gobs, x, y, sb->format->cpp[0]); + iosys_map_wr(>map[0], off, u32, color); +} + +static int +nv50_wndw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb) +{ + struct drm_framebuffer *fb; + struct nouveau_bo *nvbo; + + if (!plane->state || !plane->state->fb) + return -EINVAL; + + fb = plane->state->fb; + nvbo = nouveau_gem_object(fb->obj[0]); + + /* Don't support compressed format, or multiplane yet */ + if (nvbo->comp || fb->format->num_planes != 1) + return -EOPNOTSUPP; + + if (nouveau_bo_map(nvbo)) { + pr_warn("nouveau bo map failed, panic won't be displayed\n"); + return -ENOMEM; + } + + if (nvbo->kmap.bo_kmap_type & TTM_BO_MAP_IOMEM_MASK) + iosys_map_set_vaddr_iomem(>map[0], nvbo->kmap.virtual); + else + iosys_map_set_vaddr(>map[0], nvbo->kmap.virtual); + + sb->height = fb->height; + sb->width = fb->width; + sb->pitch[0] = fb->pitches[0]; + sb->format = fb->format; + + /* If tiling is enabled, use the set_pixel() to display correctly */ + if (fb->modifier & 0xf) { + sb->private = (void *) fb; + sb->set_pixel = nv50_set_pixel; + } + return 0; +} + static const struct drm_plane_helper_funcs nv50_wndw_helper = { .prepare_fb = nv50_wndw_prepare_fb, @@ -584,6 +696,14 @@ nv50_wndw_helper = { .atomic_check = nv50_wndw_atomic_check, }; +static const struct drm_plane_helper_funcs +nv50_wndw_primary_helper = { + .prepare_fb = nv50_wndw_prepare_fb, + .cleanup_fb = nv50_wndw_cleanup_fb, + .atomic_check = nv50_wndw_atomic_check, + .get_scanout_buffer = nv50_wndw_get_scanout_buffer, +};
[PATCH 4/5] drm/panic: add a private pointer to drm_scanout_buffer
So you can pass a parameter to the set_pixel() callback. Signed-off-by: Jocelyn Falempe --- include/drm/drm_panic.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/drm/drm_panic.h b/include/drm/drm_panic.h index 73bb3f3d9ed9..f7c32d64af5f 100644 --- a/include/drm/drm_panic.h +++ b/include/drm/drm_panic.h @@ -51,6 +51,12 @@ struct drm_scanout_buffer { */ unsigned int pitch[DRM_FORMAT_MAX_PLANES]; + /** +* @private: Optional pointer to some private data you want to pass to +* the set_pixel() function. +*/ + void *private; + /** * @set_pixel: Optional function, to set a pixel color on the * framebuffer. It allows to handle special tiling format inside the -- 2.45.0
[PATCH 2/5] drm/panic: only draw the foreground color in drm_panic_blit()
The whole framebuffer is cleared, so it's useless to rewrite the background colored pixels. It allows to simplify the drawing functions, and prepare the work for the set_pixel() callback. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_panic.c | 63 +++-- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 78fdecfede84..09d7d45f80c2 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -207,37 +207,33 @@ static u32 convert_from_xrgb(u32 color, u32 format) static void drm_panic_blit16(struct iosys_map *dmap, unsigned int dpitch, const u8 *sbuf8, unsigned int spitch, unsigned int height, unsigned int width, -u16 fg16, u16 bg16) +u16 color) { unsigned int y, x; - u16 val16; - for (y = 0; y < height; y++) { - for (x = 0; x < width; x++) { - val16 = (sbuf8[(y * spitch) + x / 8] & (0x80 >> (x % 8))) ? fg16 : bg16; - iosys_map_wr(dmap, y * dpitch + x * sizeof(u16), u16, val16); - } - } + for (y = 0; y < height; y++) + for (x = 0; x < width; x++) + if (sbuf8[(y * spitch) + x / 8] & (0x80 >> (x % 8))) + iosys_map_wr(dmap, y * dpitch + x * sizeof(u16), u16, color); } static void drm_panic_blit24(struct iosys_map *dmap, unsigned int dpitch, const u8 *sbuf8, unsigned int spitch, unsigned int height, unsigned int width, -u32 fg32, u32 bg32) +u32 color) { unsigned int y, x; - u32 val32; for (y = 0; y < height; y++) { for (x = 0; x < width; x++) { u32 off = y * dpitch + x * 3; - val32 = (sbuf8[(y * spitch) + x / 8] & (0x80 >> (x % 8))) ? fg32 : bg32; - - /* write blue-green-red to output in little endianness */ - iosys_map_wr(dmap, off, u8, (val32 & 0x00FF) >> 0); - iosys_map_wr(dmap, off + 1, u8, (val32 & 0xFF00) >> 8); - iosys_map_wr(dmap, off + 2, u8, (val32 & 0x00FF) >> 16); + if (sbuf8[(y * spitch) + x / 8] & (0x80 >> (x % 8))) { + /* write blue-green-red to output in little endianness */ + iosys_map_wr(dmap, off, u8, (color & 0x00FF) >> 0); + iosys_map_wr(dmap, off + 1, u8, (color & 0xFF00) >> 8); + iosys_map_wr(dmap, off + 2, u8, (color & 0x00FF) >> 16); + } } } } @@ -245,17 +241,14 @@ static void drm_panic_blit24(struct iosys_map *dmap, unsigned int dpitch, static void drm_panic_blit32(struct iosys_map *dmap, unsigned int dpitch, const u8 *sbuf8, unsigned int spitch, unsigned int height, unsigned int width, -u32 fg32, u32 bg32) +u32 color) { unsigned int y, x; - u32 val32; - for (y = 0; y < height; y++) { - for (x = 0; x < width; x++) { - val32 = (sbuf8[(y * spitch) + x / 8] & (0x80 >> (x % 8))) ? fg32 : bg32; - iosys_map_wr(dmap, y * dpitch + x * sizeof(u32), u32, val32); - } - } + for (y = 0; y < height; y++) + for (x = 0; x < width; x++) + if (sbuf8[(y * spitch) + x / 8] & (0x80 >> (x % 8))) + iosys_map_wr(dmap, y * dpitch + x * sizeof(u32), u32, color); } /* @@ -266,8 +259,7 @@ static void drm_panic_blit32(struct iosys_map *dmap, unsigned int dpitch, * @spitch: source pitch in bytes * @height: height of the image to copy, in pixels * @width: width of the image to copy, in pixels - * @fg_color: foreground color, in destination format - * @bg_color: background color, in destination format + * @color: foreground color, in destination format * @pixel_width: pixel width in bytes. * * This can be used to draw a font character, which is a monochrome image, to a @@ -276,21 +268,20 @@ static void drm_panic_blit32(struct iosys_map *dmap, unsigned int dpitch, static void drm_panic_blit(struct iosys_map *dmap, unsigned int dpitch, const u8 *sbuf8, unsigned int spitch, unsigned int height, unsigned int width, - u32 fg_color, u32 bg_color, -
[PATCH 1/5] drm/panic: Add ABGR2101010 support
Add support for ABGR2101010, used by the nouveau driver. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_panic.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 7ece67086cec..78fdecfede84 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -152,6 +152,14 @@ static u32 convert_xrgb_to_argb2101010(u32 pix) return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); } +static u32 convert_xrgb_to_abgr2101010(u32 pix) +{ + pix = ((pix & 0x00FF) >> 14) | + ((pix & 0xFF00) << 4) | + ((pix & 0x00FF) << 22); + return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); +} + /* * convert_from_xrgb - convert one pixel from xrgb to the desired format * @color: input color, in xrgb format @@ -185,6 +193,8 @@ static u32 convert_from_xrgb(u32 color, u32 format) return convert_xrgb_to_xrgb2101010(color); case DRM_FORMAT_ARGB2101010: return convert_xrgb_to_argb2101010(color); + case DRM_FORMAT_ABGR2101010: + return convert_xrgb_to_abgr2101010(color); default: WARN_ONCE(1, "Can't convert to %p4cc\n", ); return 0; -- 2.45.0
[PATCH 0/5] drm/nouveau: Add drm_panic support for nv50+
This series adds basic drm_panic support for nouveau. Patches 1-4 Add missing bits in drm_panic (ABGR2101010, set_pixel() for tiling, ...) Patch 5 registers nouveau to drm_panic, and handle tiling. I've tested on a GTX1650, while running Gnome/Wayland desktop. I didn't find documentation about nVidia tiling, so it may not work on other GPU than GTX1650. To test it, you need to build your kernel with CONFIG_DRM_PANIC=y, and run echo c > /proc/sysrq-trigger or you can enable CONFIG_DRM_PANIC_DEBUG and run echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 Best regards, -- Jocelyn Jocelyn Falempe (5): drm/panic: Add ABGR2101010 support drm/panic: only draw the foreground color in drm_panic_blit() drm/panic: Add a set_pixel() callback to drm_scanout_buffer drm/panic: add a private pointer to drm_scanout_buffer drm/nouveau: Add drm_panic support for nv50+ drivers/gpu/drm/drm_panic.c | 183 ++-- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 127 +++- include/drm/drm_panic.h | 15 ++ 3 files changed, 247 insertions(+), 78 deletions(-) base-commit: 484436ec5c2bffe8f346a09ae1cbc4cbf5e50005 -- 2.45.0
Re: [PATCH v2 2/2] drm/mgag200: Add an option to disable Write-Combine
On 17/05/2024 17:16, Thomas Zimmermann wrote: Hi Am 17.05.24 um 17:09 schrieb Jocelyn Falempe: Unfortunately, the G200 ioburst workaround doesn't work on some servers like Dell poweredge XR11, XR5610, or HPE XL260. In this case completely disabling WC is the only option to achieve low-latency. So this adds a new Kconfig option to disable WC mapping of the G200. Signed-off-by: Jocelyn Falempe Reviewed-by: Thomas Zimmermann Thanks a lot for the fix. Thanks for the review, I just merged it to drm-misc-next. Best regards Thomas --- drivers/gpu/drm/mgag200/Kconfig | 10 ++ drivers/gpu/drm/mgag200/mgag200_drv.c | 6 ++ 2 files changed, 16 insertions(+) diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index b28c5e4828f47..3096944a8f0ab 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -11,3 +11,13 @@ config DRM_MGAG200 MGA G200 desktop chips and the server variants. It requires 0.3.0 of the modesetting userspace driver, and a version of mga driver that will fail on KMS enabled devices. + +config DRM_MGAG200_DISABLE_WRITECOMBINE + bool "Disable Write Combine mapping of VRAM" + depends on DRM_MGAG200 && PREEMPT_RT + help + The VRAM of the G200 is mapped with Write-Combine to improve + performances. This can interfere with real-time tasks; even if they + are running on other CPU cores than the graphics output. + Enable this option only if you run realtime tasks on a server with a + Matrox G200. \ No newline at end of file diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 3883f25ca4d8b..62080cf0f2da4 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -146,12 +146,18 @@ int mgag200_device_preinit(struct mga_device *mdev) } mdev->vram_res = res; +#if defined(CONFIG_DRM_MGAG200_DISABLE_WRITECOMBINE) + mdev->vram = devm_ioremap(dev->dev, res->start, resource_size(res)); + if (!mdev->vram) + return -ENOMEM; +#else mdev->vram = devm_ioremap_wc(dev->dev, res->start, resource_size(res)); if (!mdev->vram) return -ENOMEM; /* Don't fail on errors, but performance might be reduced. */ devm_arch_phys_wc_add(dev->dev, res->start, resource_size(res)); +#endif return 0; }
[PATCH v2 2/2] drm/mgag200: Add an option to disable Write-Combine
Unfortunately, the G200 ioburst workaround doesn't work on some servers like Dell poweredge XR11, XR5610, or HPE XL260. In this case completely disabling WC is the only option to achieve low-latency. So this adds a new Kconfig option to disable WC mapping of the G200. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/mgag200/Kconfig | 10 ++ drivers/gpu/drm/mgag200/mgag200_drv.c | 6 ++ 2 files changed, 16 insertions(+) diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index b28c5e4828f47..3096944a8f0ab 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -11,3 +11,13 @@ config DRM_MGAG200 MGA G200 desktop chips and the server variants. It requires 0.3.0 of the modesetting userspace driver, and a version of mga driver that will fail on KMS enabled devices. + +config DRM_MGAG200_DISABLE_WRITECOMBINE + bool "Disable Write Combine mapping of VRAM" + depends on DRM_MGAG200 && PREEMPT_RT + help + The VRAM of the G200 is mapped with Write-Combine to improve + performances. This can interfere with real-time tasks; even if they + are running on other CPU cores than the graphics output. + Enable this option only if you run realtime tasks on a server with a + Matrox G200. \ No newline at end of file diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 3883f25ca4d8b..62080cf0f2da4 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -146,12 +146,18 @@ int mgag200_device_preinit(struct mga_device *mdev) } mdev->vram_res = res; +#if defined(CONFIG_DRM_MGAG200_DISABLE_WRITECOMBINE) + mdev->vram = devm_ioremap(dev->dev, res->start, resource_size(res)); + if (!mdev->vram) + return -ENOMEM; +#else mdev->vram = devm_ioremap_wc(dev->dev, res->start, resource_size(res)); if (!mdev->vram) return -ENOMEM; /* Don't fail on errors, but performance might be reduced. */ devm_arch_phys_wc_add(dev->dev, res->start, resource_size(res)); +#endif return 0; } -- 2.45.0
[PATCH v2 1/2] Revert "drm/mgag200: Add a workaround for low-latency"
This reverts commit bfa4437fd3938ae2e186e7664b2db65bb8775670. This workaround doesn't work reliably on all servers. I'll replace it with an option to disable Write-Combine, which has more impact on performance, but fix the latency issue on all hardware. Signed-off-by: Jocelyn Falempe Reviewed-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/Kconfig| 12 drivers/gpu/drm/mgag200/mgag200_drv.c | 17 - drivers/gpu/drm/mgag200/mgag200_mode.c | 8 3 files changed, 37 deletions(-) diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index 5e4d48df4854c..b28c5e4828f47 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -11,15 +11,3 @@ config DRM_MGAG200 MGA G200 desktop chips and the server variants. It requires 0.3.0 of the modesetting userspace driver, and a version of mga driver that will fail on KMS enabled devices. - -config DRM_MGAG200_IOBURST_WORKAROUND - bool "Disable buffer caching" - depends on DRM_MGAG200 && PREEMPT_RT && X86 - help - Enable a workaround to avoid I/O bursts within the mgag200 driver at - the expense of overall display performance. - It restores the map_wc = true; - return >base; -} -#endif - /* * DRM driver */ @@ -113,9 +99,6 @@ static const struct drm_driver mgag200_driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, -#if defined(CONFIG_DRM_MGAG200_IOBURST_WORKAROUND) - .gem_create_object = mgag200_create_object, -#endif DRM_GEM_SHMEM_DRIVER_OPS, }; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index fc54851d3384d..d3d820f7a77d7 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -13,7 +13,6 @@ #include #include -#include #include #include #include @@ -438,13 +437,6 @@ static void mgag200_handle_damage(struct mga_device *mdev, const struct iosys_ma iosys_map_incr(, drm_fb_clip_offset(fb->pitches[0], fb->format, clip)); drm_fb_memcpy(, fb->pitches, vmap, fb, clip); - - /* Flushing the cache greatly improves latency on x86_64 */ -#if defined(CONFIG_DRM_MGAG200_IOBURST_WORKAROUND) - if (!vmap->is_iomem) - drm_clflush_virt_range(vmap->vaddr + clip->y1 * fb->pitches[0], - drm_rect_height(clip) * fb->pitches[0]); -#endif } /* -- 2.45.0
[PATCH v2 0/2] drm/mgag200: Add an option to disable Write-Combine
Unfortunately, the G200 ioburst workaround doesn't work on some servers like Dell poweredge XR11, XR5610, or HPE XL260 In this case completely disabling WC is the only option to achieve low-latency. It's probably useless to maintain two workarounds for this, so I reverted commit bfa4437fd3938 drm/mgag200: Add a workaround for low-latency and added a new and simpler option, to just disable Write-Combine. Jocelyn Falempe (2): Revert "drm/mgag200: Add a workaround for low-latency" drm/mgag200: Add an option to disable Write-Combine drivers/gpu/drm/mgag200/Kconfig| 18 -- drivers/gpu/drm/mgag200/mgag200_drv.c | 23 ++- drivers/gpu/drm/mgag200/mgag200_mode.c | 8 3 files changed, 14 insertions(+), 35 deletions(-) base-commit: 3179338750d83877bbc491493032bdf192266ad9 -- 2.45.0
Re: [PATCH 2/2] drm/mgag200: Add an option to disable Write-Combine
On 17/05/2024 11:39, Thomas Zimmermann wrote: Hi, just nits below. Am 16.05.24 um 18:17 schrieb Jocelyn Falempe: Unfortunately, the G200 ioburst workaround doesn't work on some servers like Dell poweredge XR11, XR5610, or HPE XL260 In this case completely disabling WC is the only option to achieve low-latency. So this adds a new Kconfig option, to disable WC mapping of the G200 The formatting look off. Maybe make this one single paragraph. Ok, No comma after 'option'. Ok, Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/mgag200/Kconfig | 10 ++ drivers/gpu/drm/mgag200/mgag200_drv.c | 7 +++ 2 files changed, 17 insertions(+) diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index b28c5e4828f47..73ab5730b74d9 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -11,3 +11,13 @@ config DRM_MGAG200 MGA G200 desktop chips and the server variants. It requires 0.3.0 of the modesetting userspace driver, and a version of mga driver that will fail on KMS enabled devices. + +config DRM_MGAG200_DISABLE_WRITECOMBINE + bool "Disable Write Combine mapping of VRAM" + depends on DRM_MGAG200 && PREEMPT_RT + help + The VRAM of the G200 is mapped with Write-Combine, to improve No comma after Write-Combine Ok + performances. However this increases the system latency a lot, even Just say "This can interfere with real-time tasks; even if they are running on other CPU cores then the graphics output." Ok + for realtime tasks running on other CPU cores. Typically 40us-80us + latencies are measured with hwlat when Write Combine is enabled. Leave out the next sentence: "Typically ..." The measureed numbers depend on the hardware and everyone is encouraged to test on their own system. You could mention the numbers in the commit description, as you already mention the affected systems there. Ok + Recommended if you run realtime tasks on a server with a Matrox G200. I still think that we should not encourage anyone to use this option. Maybe say "Enable this option only if you run..." Agreed, I will change this. \ No newline at end of file diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 3883f25ca4d8b..7461e3f984eff 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -146,12 +146,19 @@ int mgag200_device_preinit(struct mga_device *mdev) } mdev->vram_res = res; +#if defined(CONFIG_DRM_MGAG200_DISABLE_WRITECOMBINE) + drm_info(dev, "Disable Write Combine\n"); I would not print this drm_info() here. The user has selected the config option, so they should know what happens. It's also listed in /proc/mtrr IIRC. Sure, I can remove the drm_info(). Thanks for the review, I will send a v2 shortly. Best regards Thomas + mdev->vram = devm_ioremap(dev->dev, res->start, resource_size(res)); + if (!mdev->vram) + return -ENOMEM; +#else mdev->vram = devm_ioremap_wc(dev->dev, res->start, resource_size(res)); if (!mdev->vram) return -ENOMEM; /* Don't fail on errors, but performance might be reduced. */ devm_arch_phys_wc_add(dev->dev, res->start, resource_size(res)); +#endif return 0; }
[PATCH 0/2] drm/mgag200: Add an option to disable Write-Combine
Unfortunately, the G200 ioburst workaround doesn't work on some servers like Dell poweredge XR11, XR5610, or HPE XL260 In this case completely disabling WC is the only option to achieve low-latency. It's probably useless to maintain two workarounds for this, so I reverted commit bfa4437fd3938 drm/mgag200: Add a workaround for low-latency and added a new and simpler option, to just disable Write-Combine. Jocelyn Falempe (2): Revert "drm/mgag200: Add a workaround for low-latency" drm/mgag200: Add an option to disable Write-Combine drivers/gpu/drm/mgag200/Kconfig| 18 -- drivers/gpu/drm/mgag200/mgag200_drv.c | 24 +++- drivers/gpu/drm/mgag200/mgag200_mode.c | 8 3 files changed, 15 insertions(+), 35 deletions(-) base-commit: 2c3d1bd284c5141a85188f48e7f42112e81ffcd8 -- 2.45.0
[PATCH 1/2] Revert "drm/mgag200: Add a workaround for low-latency"
This reverts commit bfa4437fd3938ae2e186e7664b2db65bb8775670. This workaround doesn't work reliably on all servers. I'll replace it with an option to disable Write-Combine, which has more impact on performance, but fix the latency issue on all hardware. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/mgag200/Kconfig| 12 drivers/gpu/drm/mgag200/mgag200_drv.c | 17 - drivers/gpu/drm/mgag200/mgag200_mode.c | 8 3 files changed, 37 deletions(-) diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index 5e4d48df4854c..b28c5e4828f47 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -11,15 +11,3 @@ config DRM_MGAG200 MGA G200 desktop chips and the server variants. It requires 0.3.0 of the modesetting userspace driver, and a version of mga driver that will fail on KMS enabled devices. - -config DRM_MGAG200_IOBURST_WORKAROUND - bool "Disable buffer caching" - depends on DRM_MGAG200 && PREEMPT_RT && X86 - help - Enable a workaround to avoid I/O bursts within the mgag200 driver at - the expense of overall display performance. - It restores the map_wc = true; - return >base; -} -#endif - /* * DRM driver */ @@ -113,9 +99,6 @@ static const struct drm_driver mgag200_driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, -#if defined(CONFIG_DRM_MGAG200_IOBURST_WORKAROUND) - .gem_create_object = mgag200_create_object, -#endif DRM_GEM_SHMEM_DRIVER_OPS, }; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index fc54851d3384d..d3d820f7a77d7 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -13,7 +13,6 @@ #include #include -#include #include #include #include @@ -438,13 +437,6 @@ static void mgag200_handle_damage(struct mga_device *mdev, const struct iosys_ma iosys_map_incr(, drm_fb_clip_offset(fb->pitches[0], fb->format, clip)); drm_fb_memcpy(, fb->pitches, vmap, fb, clip); - - /* Flushing the cache greatly improves latency on x86_64 */ -#if defined(CONFIG_DRM_MGAG200_IOBURST_WORKAROUND) - if (!vmap->is_iomem) - drm_clflush_virt_range(vmap->vaddr + clip->y1 * fb->pitches[0], - drm_rect_height(clip) * fb->pitches[0]); -#endif } /* -- 2.45.0
[PATCH 2/2] drm/mgag200: Add an option to disable Write-Combine
Unfortunately, the G200 ioburst workaround doesn't work on some servers like Dell poweredge XR11, XR5610, or HPE XL260 In this case completely disabling WC is the only option to achieve low-latency. So this adds a new Kconfig option, to disable WC mapping of the G200 Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/mgag200/Kconfig | 10 ++ drivers/gpu/drm/mgag200/mgag200_drv.c | 7 +++ 2 files changed, 17 insertions(+) diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index b28c5e4828f47..73ab5730b74d9 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -11,3 +11,13 @@ config DRM_MGAG200 MGA G200 desktop chips and the server variants. It requires 0.3.0 of the modesetting userspace driver, and a version of mga driver that will fail on KMS enabled devices. + +config DRM_MGAG200_DISABLE_WRITECOMBINE + bool "Disable Write Combine mapping of VRAM" + depends on DRM_MGAG200 && PREEMPT_RT + help + The VRAM of the G200 is mapped with Write-Combine, to improve + performances. However this increases the system latency a lot, even + for realtime tasks running on other CPU cores. Typically 40us-80us + latencies are measured with hwlat when Write Combine is enabled. + Recommended if you run realtime tasks on a server with a Matrox G200. \ No newline at end of file diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 3883f25ca4d8b..7461e3f984eff 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -146,12 +146,19 @@ int mgag200_device_preinit(struct mga_device *mdev) } mdev->vram_res = res; +#if defined(CONFIG_DRM_MGAG200_DISABLE_WRITECOMBINE) + drm_info(dev, "Disable Write Combine\n"); + mdev->vram = devm_ioremap(dev->dev, res->start, resource_size(res)); + if (!mdev->vram) + return -ENOMEM; +#else mdev->vram = devm_ioremap_wc(dev->dev, res->start, resource_size(res)); if (!mdev->vram) return -ENOMEM; /* Don't fail on errors, but performance might be reduced. */ devm_arch_phys_wc_add(dev->dev, res->start, resource_size(res)); +#endif return 0; } -- 2.45.0
Re: [PATCH 00/10] drm/mgag200: Refactor DDC code
Thanks for this refactor of mgag200. for the whole series: Reviewed-by: Jocelyn Falempe -- Jocelyn On 13/05/2024 14:51, Thomas Zimmermann wrote: Clean up a the driver's DDC code, make it simpler, more robust, and mostly self contained. The patches in this patchset have previously been sent as part of rev 1 of [1]. Patches 1 and 2 fix long-standing problems in the DDC code. Patches 3 to 9 refactor the DDC code. The code then keeps its data structures internal, acquires locks automatically and is much more readable overall. Patch 10 replaces driver code with an equivalent helper. Tested on various Matrox hardware. [1] https://patchwork.freedesktop.org/series/131977/ Thomas Zimmermann (10): drm/mgag200: Set DDC timeout in milliseconds drm/mgag200: Bind I2C lifetime to DRM device drm/mgag200: Store pointer to struct mga_device in struct mga_i2c_chan drm/mgag200: Allocate instance of struct mga_i2c_chan dynamically drm/mgag200: Inline mgag200_i2c_init() drm/mgag200: Replace struct mga_i2c_chan with struct mgag200_ddc drm/mgag200: Rename mgag200_i2c.c to mgag200_ddc.c drm/mgag200: Rename struct i2c_algo_bit_data callbacks drm/mgag200: Acquire I/O-register lock in DDC code drm/mgag200: Use drm_connector_helper_get_modes() drivers/gpu/drm/mgag200/Makefile | 2 +- drivers/gpu/drm/mgag200/mgag200_ddc.c | 179 ++ drivers/gpu/drm/mgag200/mgag200_ddc.h | 11 ++ drivers/gpu/drm/mgag200/mgag200_drv.h | 18 +-- drivers/gpu/drm/mgag200/mgag200_g200.c| 11 +- drivers/gpu/drm/mgag200/mgag200_g200eh.c | 11 +- drivers/gpu/drm/mgag200/mgag200_g200eh3.c | 11 +- drivers/gpu/drm/mgag200/mgag200_g200er.c | 11 +- drivers/gpu/drm/mgag200/mgag200_g200ev.c | 11 +- drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 11 +- drivers/gpu/drm/mgag200/mgag200_g200se.c | 11 +- drivers/gpu/drm/mgag200/mgag200_g200wb.c | 11 +- drivers/gpu/drm/mgag200/mgag200_i2c.c | 129 drivers/gpu/drm/mgag200/mgag200_mode.c| 27 +--- 14 files changed, 241 insertions(+), 213 deletions(-) create mode 100644 drivers/gpu/drm/mgag200/mgag200_ddc.c create mode 100644 drivers/gpu/drm/mgag200/mgag200_ddc.h delete mode 100644 drivers/gpu/drm/mgag200/mgag200_i2c.c
Re: [PATCH] lib/fonts: Allow to select fonts for drm_panic
On 30/04/2024 14:58, Daniel Vetter wrote: On Fri, Apr 19, 2024 at 03:20:19PM +0200, Jocelyn Falempe wrote: drm_panic has been introduced recently, and uses the same fonts as FRAMEBUFFER_CONSOLE. Signed-off-by: Jocelyn Falempe Acked-by: Daniel Vetter lib/fonts/ doesn't seem to have a designated maintainer, so please push this through drm-misc. Thanks for the review, I just merged it to drm-misc-next. -- Jocelyn Thanks, Sima --- lib/fonts/Kconfig | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/fonts/Kconfig b/lib/fonts/Kconfig index 7e945fdcbf11..befcb463f738 100644 --- a/lib/fonts/Kconfig +++ b/lib/fonts/Kconfig @@ -10,7 +10,7 @@ if FONT_SUPPORT config FONTS bool "Select compiled-in fonts" - depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE + depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE || DRM_PANIC help Say Y here if you would like to use fonts other than the default your frame buffer console usually use. @@ -23,7 +23,7 @@ config FONTS config FONT_8x8 bool "VGA 8x8 font" if FONTS - depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE + depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE || DRM_PANIC default y if !SPARC && !FONTS help This is the "high resolution" font for the VGA frame buffer (the one @@ -46,7 +46,7 @@ config FONT_8x16 config FONT_6x11 bool "Mac console 6x11 font (not supported by all drivers)" if FONTS - depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE + depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE || DRM_PANIC default y if !SPARC && !FONTS && MAC help Small console font with Macintosh-style high-half glyphs. Some Mac @@ -54,7 +54,7 @@ config FONT_6x11 config FONT_7x14 bool "console 7x14 font (not supported by all drivers)" if FONTS - depends on FRAMEBUFFER_CONSOLE + depends on FRAMEBUFFER_CONSOLE || DRM_PANIC help Console font with characters just a bit smaller than the default. If the standard 8x16 font is a little too big for you, say Y. @@ -62,7 +62,7 @@ config FONT_7x14 config FONT_PEARL_8x8 bool "Pearl (old m68k) console 8x8 font" if FONTS - depends on FRAMEBUFFER_CONSOLE + depends on FRAMEBUFFER_CONSOLE || DRM_PANIC default y if !SPARC && !FONTS && AMIGA help Small console font with PC-style control-character and high-half @@ -70,7 +70,7 @@ config FONT_PEARL_8x8 config FONT_ACORN_8x8 bool "Acorn console 8x8 font" if FONTS - depends on FRAMEBUFFER_CONSOLE + depends on FRAMEBUFFER_CONSOLE || DRM_PANIC default y if !SPARC && !FONTS && ARM && ARCH_ACORN help Small console font with PC-style control characters and high-half @@ -90,7 +90,7 @@ config FONT_6x10 config FONT_10x18 bool "console 10x18 font (not supported by all drivers)" if FONTS - depends on FRAMEBUFFER_CONSOLE + depends on FRAMEBUFFER_CONSOLE || DRM_PANIC help This is a high resolution console font for machines with very big letters. It fits between the sun 12x22 and the normal 8x16 font. @@ -105,7 +105,7 @@ config FONT_SUN8x16 config FONT_SUN12x22 bool "Sparc console 12x22 font (not supported by all drivers)" - depends on FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC) + depends on (FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC)) || DRM_PANIC help This is the high resolution console font for Sun machines with very big letters (like the letters used in the SPARC PROM). If the @@ -113,7 +113,7 @@ config FONT_SUN12x22 config FONT_TER16x32 bool "Terminus 16x32 font (not supported by all drivers)" - depends on FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC) + depends on (FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC)) || DRM_PANIC help Terminus Font is a clean, fixed width bitmap font, designed for long (8 and more hours per day) work with computers. @@ -122,7 +122,7 @@ config FONT_TER16x32 config FONT_6x8 bool "OLED 6x8 font" if FONTS - depends on FRAMEBUFFER_CONSOLE + depends on FRAMEBUFFER_CONSOLE || DRM_PANIC help This font is useful for small displays (OLED). base-commit: a35e92ef04c07bd473404b9b73d489aea19a60a8 -- 2.44.0
Re: [PATCH] drm/fb_dma: Add checks in drm_fb_dma_get_scanout_buffer()
On 29/04/2024 09:24, Thomas Zimmermann wrote: Am 26.04.24 um 14:10 schrieb Jocelyn Falempe: plane->state and plane->state->fb can be NULL, so add a check before dereferencing them. Found by testing with the imx driver. Fixes: 879b3b6511fe9 ("drm/fb_dma: Add generic get_scanout_buffer() for drm_panic") Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_dma_helper.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index 96e5ab960f12..d7bffde94cc5 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -167,6 +167,9 @@ int drm_fb_dma_get_scanout_buffer(struct drm_plane *plane, struct drm_gem_dma_object *dma_obj; struct drm_framebuffer *fb; + if (!plane->state || !plane->state->fb) + return -ENODEV; + It's EINVAL here. With that fixed, free free to add Thanks, I just merged it to drm-misc-next with that fixed. Reviewed-by: Thomas Zimmermann Best regards Thomas fb = plane->state->fb; /* Only support linear modifier */ if (fb->modifier != DRM_FORMAT_MOD_LINEAR) base-commit: 2e3f08a1ac99cb9a19a5cb151593d4f9df5cc6a7 -- Jocelyn
[PATCH] drm/fb_dma: Add checks in drm_fb_dma_get_scanout_buffer()
plane->state and plane->state->fb can be NULL, so add a check before dereferencing them. Found by testing with the imx driver. Fixes: 879b3b6511fe9 ("drm/fb_dma: Add generic get_scanout_buffer() for drm_panic") Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_dma_helper.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index 96e5ab960f12..d7bffde94cc5 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -167,6 +167,9 @@ int drm_fb_dma_get_scanout_buffer(struct drm_plane *plane, struct drm_gem_dma_object *dma_obj; struct drm_framebuffer *fb; + if (!plane->state || !plane->state->fb) + return -ENODEV; + fb = plane->state->fb; /* Only support linear modifier */ if (fb->modifier != DRM_FORMAT_MOD_LINEAR) base-commit: 2e3f08a1ac99cb9a19a5cb151593d4f9df5cc6a7 -- 2.44.0
[PATCH 1/2] drm/fb-helper: Add drm_fb_helper_emergency_disable
This is needed for drm_panic, to disable fbcon output that will otherwise mix with drm_panic output. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_helper.c | 17 + include/drm/drm_fb_helper.h | 5 + 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index d612133e2cf7e..e2e681e252e73 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -798,6 +798,23 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper, } EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked); +/** + * drm_fb_helper_emergency_disable - disable fb output in panic situation + * @fb_helper: driver-allocated fbdev helper, can be NULL + * + * A wrapper around fb_set_suspend, to disable fb emulation when a panic occurs. + */ +void drm_fb_helper_emergency_disable(struct drm_fb_helper *fb_helper) +{ + if (fb_helper && fb_helper->info && fb_helper->info->state == FBINFO_STATE_RUNNING) { + if (console_trylock()) { + fb_set_suspend(fb_helper->info, FBINFO_STATE_SUSPENDED); + console_unlock(); + } + } +} +EXPORT_SYMBOL(drm_fb_helper_emergency_disable); + static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info) { u32 *palette = (u32 *)info->pseudo_palette; diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 375737fd6c36e..8c61c4fe93e3b 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -261,6 +261,7 @@ void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagerefli void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, bool suspend); void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper, bool suspend); +void drm_fb_helper_emergency_disable(struct drm_fb_helper *fb_helper); int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info); @@ -378,6 +379,10 @@ drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper, bool suspend { } +static inline void drm_fb_helper_emergency_disable(struct drm_fb_helper *fb_helper) +{ +} + static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) { return 0; -- 2.44.0
[PATCH 2/2] drm/panic: Allows to run with fbcon
Disable the framebuffer emulation when a panic occurs, to avoid fbcon to overwrite the panic screen. So it's now safe to enable DRM_PANIC and FRAMEBUFFER_CONSOLE. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/Kconfig | 2 +- drivers/gpu/drm/drm_panic.c | 4 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 959b19a041018..7a8b1ef4c6bcf 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -106,7 +106,7 @@ config DRM_KMS_HELPER config DRM_PANIC bool "Display a user-friendly message when a kernel panic occurs" - depends on DRM && !FRAMEBUFFER_CONSOLE + depends on DRM select DRM_KMS_HELPER select FONT_SUPPORT help diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 7ece67086cecb..c46a2b878e759 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -470,6 +471,9 @@ static void draw_panic_plane(struct drm_plane *plane) int ret; unsigned long flags; + /* Prevent fbcon from overwriting the panic screen */ + drm_fb_helper_emergency_disable(plane->dev->fb_helper); + if (!drm_panic_trylock(plane->dev, flags)) return; -- 2.44.0
[PATCH 0/2] drm/panic: Allows to run with fbcon
fbcon and drm_panic cannot be built together because they both write to the screen when a panic occurs, leading to a corrupted panic image. With this 2 patches, drm_panic will disable the fbdev output before writing the panic screen, so only drm_panic screen will be shown. Jocelyn Falempe (2): drm/fb-helper: Add drm_fb_helper_emergency_disable drm/panic: Allows to run with fbcon drivers/gpu/drm/Kconfig | 2 +- drivers/gpu/drm/drm_fb_helper.c | 17 + drivers/gpu/drm/drm_panic.c | 4 include/drm/drm_fb_helper.h | 5 + 4 files changed, 27 insertions(+), 1 deletion(-) base-commit: 105aa4c65b76c3a344ca89a2d2dc96c84cca557f -- 2.44.0
[PATCH] lib/fonts: Allow to select fonts for drm_panic
drm_panic has been introduced recently, and uses the same fonts as FRAMEBUFFER_CONSOLE. Signed-off-by: Jocelyn Falempe --- lib/fonts/Kconfig | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/fonts/Kconfig b/lib/fonts/Kconfig index 7e945fdcbf11..befcb463f738 100644 --- a/lib/fonts/Kconfig +++ b/lib/fonts/Kconfig @@ -10,7 +10,7 @@ if FONT_SUPPORT config FONTS bool "Select compiled-in fonts" - depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE + depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE || DRM_PANIC help Say Y here if you would like to use fonts other than the default your frame buffer console usually use. @@ -23,7 +23,7 @@ config FONTS config FONT_8x8 bool "VGA 8x8 font" if FONTS - depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE + depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE || DRM_PANIC default y if !SPARC && !FONTS help This is the "high resolution" font for the VGA frame buffer (the one @@ -46,7 +46,7 @@ config FONT_8x16 config FONT_6x11 bool "Mac console 6x11 font (not supported by all drivers)" if FONTS - depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE + depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE || DRM_PANIC default y if !SPARC && !FONTS && MAC help Small console font with Macintosh-style high-half glyphs. Some Mac @@ -54,7 +54,7 @@ config FONT_6x11 config FONT_7x14 bool "console 7x14 font (not supported by all drivers)" if FONTS - depends on FRAMEBUFFER_CONSOLE + depends on FRAMEBUFFER_CONSOLE || DRM_PANIC help Console font with characters just a bit smaller than the default. If the standard 8x16 font is a little too big for you, say Y. @@ -62,7 +62,7 @@ config FONT_7x14 config FONT_PEARL_8x8 bool "Pearl (old m68k) console 8x8 font" if FONTS - depends on FRAMEBUFFER_CONSOLE + depends on FRAMEBUFFER_CONSOLE || DRM_PANIC default y if !SPARC && !FONTS && AMIGA help Small console font with PC-style control-character and high-half @@ -70,7 +70,7 @@ config FONT_PEARL_8x8 config FONT_ACORN_8x8 bool "Acorn console 8x8 font" if FONTS - depends on FRAMEBUFFER_CONSOLE + depends on FRAMEBUFFER_CONSOLE || DRM_PANIC default y if !SPARC && !FONTS && ARM && ARCH_ACORN help Small console font with PC-style control characters and high-half @@ -90,7 +90,7 @@ config FONT_6x10 config FONT_10x18 bool "console 10x18 font (not supported by all drivers)" if FONTS - depends on FRAMEBUFFER_CONSOLE + depends on FRAMEBUFFER_CONSOLE || DRM_PANIC help This is a high resolution console font for machines with very big letters. It fits between the sun 12x22 and the normal 8x16 font. @@ -105,7 +105,7 @@ config FONT_SUN8x16 config FONT_SUN12x22 bool "Sparc console 12x22 font (not supported by all drivers)" - depends on FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC) + depends on (FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC)) || DRM_PANIC help This is the high resolution console font for Sun machines with very big letters (like the letters used in the SPARC PROM). If the @@ -113,7 +113,7 @@ config FONT_SUN12x22 config FONT_TER16x32 bool "Terminus 16x32 font (not supported by all drivers)" - depends on FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC) + depends on (FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC)) || DRM_PANIC help Terminus Font is a clean, fixed width bitmap font, designed for long (8 and more hours per day) work with computers. @@ -122,7 +122,7 @@ config FONT_TER16x32 config FONT_6x8 bool "OLED 6x8 font" if FONTS - depends on FRAMEBUFFER_CONSOLE + depends on FRAMEBUFFER_CONSOLE || DRM_PANIC help This font is useful for small displays (OLED). base-commit: a35e92ef04c07bd473404b9b73d489aea19a60a8 -- 2.44.0
Re: [PATCH] drm/fb_dma: Fix parameter name in htmldocs
On 16/04/2024 14:12, Thomas Zimmermann wrote: Hi Am 16.04.24 um 11:05 schrieb Jocelyn Falempe: The parameter name is 'sb' and not 'drm_scanout_buffer'. It fixes the following warnings: drivers/gpu/drm/drm_fb_dma_helper.c:166: warning: Excess function parameter 'drm_scanout_buffer' description in 'drm_fb_dma_get_scanout_buffer' drivers/gpu/drm/drm_fb_dma_helper.c:166: warning: Function parameter or struct member 'sb' not described in 'drm_fb_dma_get_scanout_buffer' drivers/gpu/drm/drm_fb_dma_helper.c:166: warning: Excess function parameter 'drm_scanout_buffer' description in 'drm_fb_dma_get_scanout_buffer' Fixes: 879b3b6511fe ("drm/fb_dma: Add generic get_scanout_buffer() for drm_panic") Reported-by: Stephen Rothwell Signed-off-by: Jocelyn Falempe Reviewed-by: Thomas Zimmermann pushed to drm-misc-next Thanks, -- Jocelyn Best regards Thomas --- drivers/gpu/drm/drm_fb_dma_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index c79b5078303f..96e5ab960f12 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -153,7 +153,7 @@ EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); /** * drm_fb_dma_get_scanout_buffer - Provide a scanout buffer in case of panic * @plane: DRM primary plane - * @drm_scanout_buffer: scanout buffer for the panic handler + * @sb: scanout buffer for the panic handler * Returns: 0 or negative error code * * Generic get_scanout_buffer() implementation, for drivers that uses the base-commit: 7b0062036c3b71b4a69e244ecf0502c06c4cf5f0
[PATCH] drm/fb_dma: Fix parameter name in htmldocs
The parameter name is 'sb' and not 'drm_scanout_buffer'. It fixes the following warnings: drivers/gpu/drm/drm_fb_dma_helper.c:166: warning: Excess function parameter 'drm_scanout_buffer' description in 'drm_fb_dma_get_scanout_buffer' drivers/gpu/drm/drm_fb_dma_helper.c:166: warning: Function parameter or struct member 'sb' not described in 'drm_fb_dma_get_scanout_buffer' drivers/gpu/drm/drm_fb_dma_helper.c:166: warning: Excess function parameter 'drm_scanout_buffer' description in 'drm_fb_dma_get_scanout_buffer' Fixes: 879b3b6511fe ("drm/fb_dma: Add generic get_scanout_buffer() for drm_panic") Reported-by: Stephen Rothwell Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_dma_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index c79b5078303f..96e5ab960f12 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -153,7 +153,7 @@ EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); /** * drm_fb_dma_get_scanout_buffer - Provide a scanout buffer in case of panic * @plane: DRM primary plane - * @drm_scanout_buffer: scanout buffer for the panic handler + * @sb: scanout buffer for the panic handler * Returns: 0 or negative error code * * Generic get_scanout_buffer() implementation, for drivers that uses the base-commit: 7b0062036c3b71b4a69e244ecf0502c06c4cf5f0 -- 2.44.0
Re: linux-next: build warnings after merge of the drm-misc tree
On 16/04/2024 09:31, Stephen Rothwell wrote: Hi all, After merging the drm-misc tree, today's linux-next build (htmldocs) produced these warnings: drivers/gpu/drm/drm_fb_dma_helper.c:166: warning: Excess function parameter 'drm_scanout_buffer' description in 'drm_fb_dma_get_scanout_buffer' drivers/gpu/drm/drm_fb_dma_helper.c:166: warning: Function parameter or struct member 'sb' not described in 'drm_fb_dma_get_scanout_buffer' drivers/gpu/drm/drm_fb_dma_helper.c:166: warning: Excess function parameter 'drm_scanout_buffer' description in 'drm_fb_dma_get_scanout_buffer' Hi, Thanks for pointing that out. The parameter name is 'sb' and not 'drm_scanout_buffer', I will send a fix. Best regards, -- Jocelyn Introduced by commit 879b3b6511fe ("drm/fb_dma: Add generic get_scanout_buffer() for drm_panic")
Re: [PATCH] drm/fb_dma: s/drm_panic_gem_get_scanout_buffer/drm_fb_dma_get_scanout_buffer
Hi, You're right, I messed up the rename, and I mostly test on x86, where I don't build the imx driver. Reviewed-by: Jocelyn Falempe Best regards, -- Jocelyn On 15/04/2024 17:09, Maíra Canal wrote: On version 11, Thomas suggested to change the name of the function and this request was applied on version 12, which is the version that landed. Although the name of the function changed on the C file, it didn't changed on the header file, leading to a compilation error as such: drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c:780:24: error: use of undeclared identifier 'drm_fb_dma_get_scanout_buffer'; did you mean 'drm_panic_gem_get_scanout_buffer'? 780 | .get_scanout_buffer = drm_fb_dma_get_scanout_buffer, | ^ | drm_panic_gem_get_scanout_buffer ./include/drm/drm_fb_dma_helper.h:23:5: note: 'drm_panic_gem_get_scanout_buffer' declared here 23 | int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, | ^ 1 error generated. Best Regards, - Maíra Fixes: 879b3b6511fe ("drm/fb_dma: Add generic get_scanout_buffer() for drm_panic" Signed-off-by: Maíra Canal --- include/drm/drm_fb_dma_helper.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h index 61f24c2aba2f..c950732c6d36 100644 --- a/include/drm/drm_fb_dma_helper.h +++ b/include/drm/drm_fb_dma_helper.h @@ -6,6 +6,7 @@ struct drm_device; struct drm_framebuffer; +struct drm_plane; struct drm_plane_state; struct drm_scanout_buffer; @@ -20,8 +21,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, struct drm_plane_state *old_state, struct drm_plane_state *state); -int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, -struct drm_scanout_buffer *sb); +int drm_fb_dma_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb); #endif
Re: [PATCH v12 0/9] drm/panic: Add a drm panic handler
On 10/04/2024 10:12, Daniel Vetter wrote: On Tue, Apr 09, 2024 at 06:30:39PM +0200, Jocelyn Falempe wrote: drm/panic: Add a drm panic handler This introduces a new drm panic handler, which displays a message when a panic occurs. So when fbcon is disabled, you can still see a kernel panic. This is one of the missing feature, when disabling VT/fbcon in the kernel: https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/ Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel. It works with simpledrm, mgag200, ast, and imx. To test it, make sure you're using one of the supported driver, and trigger a panic: echo c > /proc/sysrq-trigger or you can enable CONFIG_DRM_PANIC_DEBUG and echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 Even if this is not yet useful, it will allows to work on more driver support, and better debug information to be added. v2: * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) * Add the panic reason to the panic message (Nerdopolis) * Add an exclamation mark (Nerdopolis) v3: * Rework the drawing functions, to write the pixels line by line and to use the drm conversion helper to support other formats. (Thomas Zimmermann) v4: * Fully support all simpledrm formats using drm conversion helpers * Rename dpanic_* to drm_panic_*, and have more coherent function name. (Thomas Zimmermann) * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) * Add foreground/background color config option * Fix the bottom lines not painted if the framebuffer height is not a multiple of the font height. * Automatically register the driver to drm_panic, if the function get_scanout_buffer() exists. (Thomas Zimmermann) * Add mgag200 support. v5: * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. (Thomas Zimmermann) * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). * Add ast support. * Add experimental imx/ipuv3 support, to test on an ARM hw. (Maxime Ripard) v6: * Fix sparse and __le32 warnings * Drop the IMX/IPUV3 experiment, it was just to show that it works also on ARM devices. v7: * Add a check to see if the 4cc format is supported by drm_panic. * Add a drm/plane helper to loop over all visible primary buffer, simplifying the get_scanout_buffer implementations * Add a generic implementation for drivers that uses drm_fb_dma. (Maxime Ripard) * Add back the IMX/IPUV3 support, and use the generic implementation. (Maxime Ripard) v8: * Directly register each plane to the panic notifier (Sima) * Replace get_scanout_buffer() with set_scanout_buffer() to simplify the locking. (Thomas Zimmermann) * Add a debugfs entry, to trigger the drm_panic without a real panic (Sima) * Fix the drm_panic Documentation, and include it in drm-kms.rst v9: * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) * Create a debugfs file for each plane in the device's debugfs directory. This allows to test for each plane of each GPU independently. v10: * Move blit and fill functions back in drm_panic (Thomas Zimmermann). * Simplify the text drawing functions. * Use kmsg_dumper instead of panic_notifier (Sima). * Use spinlock_irqsave/restore (John Ogness) v11: * Use macro instead of inline functions for drm_panic_lock/unlock (John Ogness) v12: * Use array for map and pitch in struct drm_scanout_buffer to support multi-planar format later. (Thomas Zimmermann) * Rename drm_panic_gem_get_scanout_buffer to drm_fb_dma_get_scanout_buffer and build it unconditionally (Thomas Zimmermann) * Better indent struct drm_scanout_buffer declaration. (Thomas Zimmermann) On the series: Acked-by: Daniel Vetter And apologies for the detours this patch set took and my part in the (too many) revisions. I think we should land this and do anything more once it's merged and we extend the panic support to more drivers. Thanks for your help, I just pushed it to drm-misc-next. Best regards, -- Jocelyn Thanks a lot to you for seeing this through! Cheers, Sima Best regards, Daniel Vetter (1): drm/panic: Add drm panic locking Jocelyn Falempe (8): drm/panic: Add a drm panic handler drm/panic: Add support for color format conversion drm/panic: Add debugfs entry to test without triggering panic. drm/fb_dma: Add generic get_scanout_
Re: [PATCH v12 2/9] drm/panic: Add a drm panic handler
Hi, On 10/04/2024 10:10, Daniel Vetter wrote: On Tue, Apr 09, 2024 at 06:30:41PM +0200, Jocelyn Falempe wrote: This module displays a user friendly message when a kernel panic occurs. It currently doesn't contain any debug information, but that can be added later. v2 * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) * Add the panic reason to the panic message (Nerdopolis) * Add an exclamation mark (Nerdopolis) v3 * Rework the drawing functions, to write the pixels line by line and to use the drm conversion helper to support other formats. (Thomas Zimmermann) v4 * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) * Add foreground/background color config option * Fix the bottom lines not painted if the framebuffer height is not a multiple of the font height. * Automatically register the device to drm_panic, if the function get_scanout_buffer exists. (Thomas Zimmermann) v5 * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). v6 * Fix sparse warning for panic_msg and logo. v7 * Add select DRM_KMS_HELPER for the color conversion functions. v8 * Register directly each plane to the panic notifier (Sima) * Add raw_spinlock to properly handle concurrency (Sima) * Register plane instead of device, to avoid looping through plane list, and simplify code. * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) * Removed the draw_pixel_xy() API, will see later if it can be added back. v9 * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) v10 * Move blit and fill functions back in drm_panic (Thomas Zimmermann). * Simplify the text drawing functions. * Use kmsg_dumper instead of panic_notifier (Sima). v12 * Use array for map and pitch in struct drm_scanout_buffer to support multi-planar format later. (Thomas Zimmermann) * Better indent struct drm_scanout_buffer declaration. (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe Some detail suggestions for the kerneldoc but those aside Acked-by: Daniel Vetter Thanks for the review. I fixed the documentation, and make sure the links are working. --- Documentation/gpu/drm-kms.rst| 12 + drivers/gpu/drm/Kconfig | 23 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_drv.c| 4 + drivers/gpu/drm/drm_panic.c | 289 +++ include/drm/drm_modeset_helper_vtables.h | 37 +++ include/drm/drm_panic.h | 57 + include/drm/drm_plane.h | 6 + 8 files changed, 429 insertions(+) create mode 100644 drivers/gpu/drm/drm_panic.c diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 13d3627d8bc0..b64334661aeb 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -398,6 +398,18 @@ Plane Damage Tracking Functions Reference .. kernel-doc:: include/drm/drm_damage_helper.h :internal: +Plane Panic Feature +--- + +.. kernel-doc:: drivers/gpu/drm/drm_panic.c + :doc: overview + +Plane Panic Functions Reference +--- + +.. kernel-doc:: drivers/gpu/drm/drm_panic.c + :export: + Display Modes Function Reference diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 3914aaf443a8..f8a26423369e 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -104,6 +104,29 @@ config DRM_KMS_HELPER help CRTC helpers for KMS drivers. +config DRM_PANIC + bool "Display a user-friendly message when a kernel panic occurs" + depends on DRM && !FRAMEBUFFER_CONSOLE + select DRM_KMS_HELPER + select FONT_SUPPORT + help + Enable a drm panic handler, which will display a user-friendly message + when a kernel panic occurs. It's useful when using a user-space + console instead of fbcon. + It will only work if your graphic driver supports this feature. + To support Hi-DPI Display, you can enable bigger fonts like + FONT_TER16x32 + +config DRM_PANIC_FOREGROUND_COLOR + hex "Drm panic screen foreground color, in RGB" + depends on DRM_PANIC + default 0xff + +config DRM_PANIC_BACKGROUND_COLOR +
Re: [PATCH v12 1/9] drm/panic: Add drm panic locking
Hi, On 10/04/2024 09:59, Daniel Vetter wrote: On Tue, Apr 09, 2024 at 06:30:40PM +0200, Jocelyn Falempe wrote: From: Daniel Vetter Rough sketch for the locking of drm panic printing code. The upshot of this approach is that we can pretty much entirely rely on the atomic commit flow, with the pair of raw_spin_lock/unlock providing any barriers we need, without having to create really big critical sections in code. This also avoids the need that drivers must explicitly update the panic handler state, which they might forget to do, or not do consistently, and then we blow up in the worst possible times. It is somewhat racy against a concurrent atomic update, and we might write into a buffer which the hardware will never display. But there's fundamentally no way to avoid that - if we do the panic state update explicitly after writing to the hardware, we might instead write to an old buffer that the user will barely ever see. Note that an rcu protected deference of plane->state would give us the the same guarantees, but it has the downside that we then need to protect the plane state freeing functions with call_rcu too. Which would very widely impact a lot of code and therefore doesn't seem worth the complexity compared to a raw spinlock with very tiny critical sections. Plus rcu cannot be used to protect access to peek/poke registers anyway, so we'd still need it for those cases. Peek/poke registers for vram access (or a gart pte reserved just for panic code) are also the reason I've gone with a per-device and not per-plane spinlock, since usually these things are global for the entire display. Going with per-plane locks would mean drivers for such hardware would need additional locks, which we don't want, since it deviates from the per-console takeoverlocks design. Longer term it might be useful if the panic notifiers grow a bit more structure than just the absolute bare EXPORT_SYMBOL(panic_notifier_list) - somewhat aside, why is that not EXPORT_SYMBOL_GPL ... If panic notifiers would be more like console drivers with proper register/unregister interfaces we could perhaps reuse the very fancy console lock with all it's check and takeover semantics that John Ogness is developing to fix the console_lock mess. But for the initial cut of a drm panic printing support I don't think we need that, because the critical sections are extremely small and only happen once per display refresh. So generally just 60 tiny locked sections per second, which is nothing compared to a serial console running a 115kbaud doing really slow mmio writes for each byte. So for now the raw spintrylock in drm panic notifier callback should be good enough. Another benefit of making panic notifiers more like full blown consoles (that are used in panics only) would be that we get the two stage design, where first all the safe outputs are used. And then the dangerous takeover tricks are deployed (where for display drivers we also might try to intercept any in-flight display buffer flips, which if we race and misprogram fifos and watermarks can hang the memory controller on some hw). For context the actual implementation on the drm side is by Jocelyn and this patch is meant to be combined with the overall approach in v7 (v8 is a bit less flexible, which I think is the wrong direction): https://lore.kernel.org/dri-devel/20240104160301.185915-1-jfale...@redhat.com/ Note that the locking is very much not correct there, hence this separate rfc. v2: - fix authorship, this was all my typing - some typo oopsies - link to the drm panic work by Jocelyn for context Please annotate that v10 and later is your work, credit where credit is due and all that :-) Sure, I'll add a comment about this. v10: - Use spinlock_irqsave/restore (John Ogness) v11: - Use macro instead of inline functions for drm_panic_lock/unlock (John Ogness) Signed-off-by: Daniel Vetter Cc: Jocelyn Falempe Cc: Andrew Morton Cc: "Peter Zijlstra (Intel)" Cc: Lukas Wunner Cc: Petr Mladek Cc: Steven Rostedt Cc: John Ogness Cc: Sergey Senozhatsky Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++ drivers/gpu/drm/drm_drv.c | 1 + include/drm/drm_mode_config.h | 10 +++ include/drm/drm_panic.h | 100 4 files changed, 115 insertions(+) create mode 100644 include/drm/drm_panic.h diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 39ef0a6addeb..fb97b51b38f1 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -3016,6 +3017,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall) { int i, ret; +
Re: [PATCH 3/3] drm/ast: Define struct ast_ddc in ast_ddc.c
Hi, Thanks for the patch, it looks good to me. Reviewed-by: Jocelyn Falempe -- Jocelyn On 03/04/2024 12:31, Thomas Zimmermann wrote: Move the definition of struct ast_ddc to ast_ddc.c and return the i2c adapter from ast_ddc_create(). Update callers accordingly. Avoids including Linux i2c header files, except where required. No functional changes. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_ddc.c | 14 -- drivers/gpu/drm/ast/ast_ddc.h | 13 ++--- drivers/gpu/drm/ast/ast_mode.c | 8 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_ddc.c b/drivers/gpu/drm/ast/ast_ddc.c index 4df52aeba4f7e..29cf5d157f344 100644 --- a/drivers/gpu/drm/ast/ast_ddc.c +++ b/drivers/gpu/drm/ast/ast_ddc.c @@ -21,12 +21,22 @@ * of the Software. */ +#include +#include + #include #include #include "ast_ddc.h" #include "ast_drv.h" +struct ast_ddc { + struct ast_device *ast; + + struct i2c_algo_bit_data bit; + struct i2c_adapter adapter; +}; + static void ast_ddc_algo_bit_data_setsda(void *data, int state) { struct ast_ddc *ddc = data; @@ -132,7 +142,7 @@ static void ast_ddc_release(struct drm_device *dev, void *res) i2c_del_adapter(>adapter); } -struct ast_ddc *ast_ddc_create(struct ast_device *ast) +struct i2c_adapter *ast_ddc_create(struct ast_device *ast) { struct drm_device *dev = >base; struct ast_ddc *ddc; @@ -173,5 +183,5 @@ struct ast_ddc *ast_ddc_create(struct ast_device *ast) if (ret) return ERR_PTR(ret); - return ddc; + return >adapter; } diff --git a/drivers/gpu/drm/ast/ast_ddc.h b/drivers/gpu/drm/ast/ast_ddc.h index 08f3994e09ccd..85c93edc9ae12 100644 --- a/drivers/gpu/drm/ast/ast_ddc.h +++ b/drivers/gpu/drm/ast/ast_ddc.h @@ -3,18 +3,9 @@ #ifndef __AST_DDC_H__ #define __AST_DDC_H__ -#include -#include - struct ast_device; +struct i2c_adapter; -struct ast_ddc { - struct ast_device *ast; - - struct i2c_adapter adapter; - struct i2c_algo_bit_data bit; -}; - -struct ast_ddc *ast_ddc_create(struct ast_device *ast); +struct i2c_adapter *ast_ddc_create(struct ast_device *ast); #endif diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index bb9b66aba9ee9..ae21d89b899c8 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1360,7 +1360,7 @@ static const struct drm_connector_funcs ast_vga_connector_funcs = { static int ast_vga_connector_init(struct drm_device *dev, struct drm_connector *connector) { struct ast_device *ast = to_ast_device(dev); - struct ast_ddc *ddc; + struct i2c_adapter *ddc; int ret; ddc = ast_ddc_create(ast); @@ -1371,7 +1371,7 @@ static int ast_vga_connector_init(struct drm_device *dev, struct drm_connector * } ret = drm_connector_init_with_ddc(dev, connector, _vga_connector_funcs, - DRM_MODE_CONNECTOR_VGA, >adapter); + DRM_MODE_CONNECTOR_VGA, ddc); if (ret) return ret; @@ -1429,7 +1429,7 @@ static const struct drm_connector_funcs ast_sil164_connector_funcs = { static int ast_sil164_connector_init(struct drm_device *dev, struct drm_connector *connector) { struct ast_device *ast = to_ast_device(dev); - struct ast_ddc *ddc; + struct i2c_adapter *ddc; int ret; ddc = ast_ddc_create(ast); @@ -1440,7 +1440,7 @@ static int ast_sil164_connector_init(struct drm_device *dev, struct drm_connecto } ret = drm_connector_init_with_ddc(dev, connector, _sil164_connector_funcs, - DRM_MODE_CONNECTOR_DVII, >adapter); + DRM_MODE_CONNECTOR_DVII, ddc); if (ret) return ret;
Re: [PATCH 2/3] drm/ast: Group DDC init code by data structure
Hi, Thanks for the patch, it looks good to me. Reviewed-by: Jocelyn Falempe -- Jocelyn On 03/04/2024 12:31, Thomas Zimmermann wrote: Reorder the code to set up the DDC channel by data structure, so that each data structure's init is in a separate block: first the bit algo then the i2c adapter. Makes the code more readable. No functional changes. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_ddc.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_ddc.c b/drivers/gpu/drm/ast/ast_ddc.c index 3e156a6b6831d..4df52aeba4f7e 100644 --- a/drivers/gpu/drm/ast/ast_ddc.c +++ b/drivers/gpu/drm/ast/ast_ddc.c @@ -145,15 +145,7 @@ struct ast_ddc *ast_ddc_create(struct ast_device *ast) return ERR_PTR(-ENOMEM); ddc->ast = ast; - adapter = >adapter; - adapter->owner = THIS_MODULE; - adapter->dev.parent = dev->dev; - i2c_set_adapdata(adapter, ddc); - snprintf(adapter->name, sizeof(adapter->name), "AST DDC bus"); - bit = >bit; - bit->udelay = 20; - bit->timeout = usecs_to_jiffies(2200); bit->data = ddc; bit->setsda = ast_ddc_algo_bit_data_setsda; bit->setscl = ast_ddc_algo_bit_data_setscl; @@ -161,8 +153,16 @@ struct ast_ddc *ast_ddc_create(struct ast_device *ast) bit->getscl = ast_ddc_algo_bit_data_getscl; bit->pre_xfer = ast_ddc_algo_bit_data_pre_xfer; bit->post_xfer = ast_ddc_algo_bit_data_post_xfer; + bit->udelay = 20; + bit->timeout = usecs_to_jiffies(2200); + adapter = >adapter; + adapter->owner = THIS_MODULE; adapter->algo_data = bit; + adapter->dev.parent = dev->dev; + snprintf(adapter->name, sizeof(adapter->name), "AST DDC bus"); + i2c_set_adapdata(adapter, ddc); + ret = i2c_bit_add_bus(adapter); if (ret) { drm_err(dev, "Failed to register bit i2c\n");
Re: [PATCH 1/3] drm/ast: Set DDC timeout in milliseconds
Hi, Thanks for the patch, it looks good to me. Reviewed-by: Jocelyn Falempe -- Jocelyn On 03/04/2024 12:31, Thomas Zimmermann wrote: Compute the i2c timeout in jiffies from a value in milliseconds. The original values of 2 jiffies equals 2 milliseconds if HZ has been configured to a value of 1000. This corresponds to 2.2 milliseconds used by most other DRM drivers. Update ast accordingly. Signed-off-by: Thomas Zimmermann Fixes: 312fec1405dd ("drm: Initial KMS driver for AST (ASpeed Technologies) 2000 series (v2)") Cc: Dave Airlie Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: Jocelyn Falempe Cc: dri-devel@lists.freedesktop.org Cc: # v3.5+ --- drivers/gpu/drm/ast/ast_ddc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ast/ast_ddc.c b/drivers/gpu/drm/ast/ast_ddc.c index b7718084422f3..3e156a6b6831d 100644 --- a/drivers/gpu/drm/ast/ast_ddc.c +++ b/drivers/gpu/drm/ast/ast_ddc.c @@ -153,7 +153,7 @@ struct ast_ddc *ast_ddc_create(struct ast_device *ast) bit = >bit; bit->udelay = 20; - bit->timeout = 2; + bit->timeout = usecs_to_jiffies(2200); bit->data = ddc; bit->setsda = ast_ddc_algo_bit_data_setsda; bit->setscl = ast_ddc_algo_bit_data_setscl;
[PATCH v12 8/9] drm/imx: Add drm_panic support
Add support for the drm_panic module, which displays a user-friendly message to the screen when a kernel panic occurs. v7: * use drm_panic_gem_get_scanout_buffer() helper v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions v12: * Rename drm_panic_gem_get_scanout_buffer to drm_fb_dma_get_scanout_buffer (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c index dade8b59feae..de010964ee7d 100644 --- a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c @@ -772,6 +772,12 @@ static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = { .atomic_disable = ipu_plane_atomic_disable, .atomic_update = ipu_plane_atomic_update, }; +static const struct drm_plane_helper_funcs ipu_primary_plane_helper_funcs = { + .atomic_check = ipu_plane_atomic_check, + .atomic_disable = ipu_plane_atomic_disable, + .atomic_update = ipu_plane_atomic_update, + .get_scanout_buffer = drm_fb_dma_get_scanout_buffer, +}; bool ipu_plane_atomic_update_pending(struct drm_plane *plane) { @@ -916,7 +922,10 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, ipu_plane->dma = dma; ipu_plane->dp_flow = dp; - drm_plane_helper_add(_plane->base, _plane_helper_funcs); + if (type == DRM_PLANE_TYPE_PRIMARY) + drm_plane_helper_add(_plane->base, _primary_plane_helper_funcs); + else + drm_plane_helper_add(_plane->base, _plane_helper_funcs); if (dp == IPU_DP_FLOW_SYNC_BG || dp == IPU_DP_FLOW_SYNC_FG) ret = drm_plane_create_zpos_property(_plane->base, zpos, 0, -- 2.44.0
[PATCH v12 4/9] drm/panic: Add debugfs entry to test without triggering panic.
Add a debugfs file, so you can test drm_panic without freezing your machine. This is unsafe, and should be enabled only for developer or tester. To display the drm_panic screen on the device 0: echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 v9: * Create a debugfs file for each plane in the device's debugfs directory. This allows to test for each plane of each GPU independently. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/Kconfig | 9 drivers/gpu/drm/drm_panic.c | 43 - 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index f8a26423369e..959b19a04101 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -127,6 +127,15 @@ config DRM_PANIC_BACKGROUND_COLOR depends on DRM_PANIC default 0x00 +config DRM_PANIC_DEBUG + bool "Add a debug fs entry to trigger drm_panic" + depends on DRM_PANIC && DEBUG_FS + help + Add dri/[device]/drm_panic_plane_x in the kernel debugfs, to force the + panic handler to write the panic message to this plane scanout buffer. + This is unsafe and should not be enabled on a production build. + If in doubt, say "N". + config DRM_DEBUG_DP_MST_TOPOLOGY_REFS bool "Enable refcount backtrace history in the DP MST helpers" depends on STACKTRACE_SUPPORT diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index e1ec30b6c04a..78fd6d5d7adc 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -495,6 +495,45 @@ static void drm_panic(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason) draw_panic_plane(plane); } + +/* + * DEBUG FS, This is currently unsafe. + * Create one file per plane, so it's possible to debug one plane at a time. + */ +#ifdef CONFIG_DRM_PANIC_DEBUG +#include + +static ssize_t debugfs_trigger_write(struct file *file, const char __user *user_buf, +size_t count, loff_t *ppos) +{ + bool run; + + if (kstrtobool_from_user(user_buf, count, ) == 0 && run) { + struct drm_plane *plane = file->private_data; + + draw_panic_plane(plane); + } + return count; +} + +static const struct file_operations dbg_drm_panic_ops = { + .owner = THIS_MODULE, + .write = debugfs_trigger_write, + .open = simple_open, +}; + +static void debugfs_register_plane(struct drm_plane *plane, int index) +{ + char fname[32]; + + snprintf(fname, 32, "drm_panic_plane_%d", index); + debugfs_create_file(fname, 0200, plane->dev->debugfs_root, + plane, _drm_panic_ops); +} +#else +static void debugfs_register_plane(struct drm_plane *plane, int index) {} +#endif /* CONFIG_DRM_PANIC_DEBUG */ + /** * drm_panic_register() - Initialize DRM panic for a device * @dev: the drm device on which the panic screen will be displayed. @@ -514,8 +553,10 @@ void drm_panic_register(struct drm_device *dev) plane->kmsg_panic.max_reason = KMSG_DUMP_PANIC; if (kmsg_dump_register(>kmsg_panic)) drm_warn(dev, "Failed to register panic handler\n"); - else + else { + debugfs_register_plane(plane, registered_plane); registered_plane++; + } } if (registered_plane) drm_info(dev, "Registered %d planes with drm panic\n", registered_plane); -- 2.44.0
[PATCH v12 9/9] drm/ast: Add drm_panic support
Add support for the drm_panic module, which displays a message to the screen when a kernel panic occurs. v7 * Use drm_for_each_primary_visible_plane() v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions v12: * Use array for map and pitch in struct drm_scanout_buffer to support multi-planar format later. (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe Acked-by: Sui Jingfeng Tested-by: Sui Jingfeng Reviewed-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_mode.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index bb9b66aba9ee..417b111d90fa 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -701,12 +702,29 @@ static void ast_primary_plane_helper_atomic_disable(struct drm_plane *plane, ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x1, 0xdf, 0x20); } +static int ast_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct ast_plane *ast_plane = to_ast_plane(plane); + + if (plane->state && plane->state->fb && ast_plane->vaddr) { + sb->format = plane->state->fb->format; + sb->width = plane->state->fb->width; + sb->height = plane->state->fb->height; + sb->pitch[0] = plane->state->fb->pitches[0]; + iosys_map_set_vaddr_iomem(>map[0], ast_plane->vaddr); + return 0; + } + return -ENODEV; +} + static const struct drm_plane_helper_funcs ast_primary_plane_helper_funcs = { DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, .atomic_check = ast_primary_plane_helper_atomic_check, .atomic_update = ast_primary_plane_helper_atomic_update, .atomic_enable = ast_primary_plane_helper_atomic_enable, .atomic_disable = ast_primary_plane_helper_atomic_disable, + .get_scanout_buffer = ast_primary_plane_helper_get_scanout_buffer, }; static const struct drm_plane_funcs ast_primary_plane_funcs = { -- 2.44.0
[PATCH v12 6/9] drm/simpledrm: Add drm_panic support
Add support for the drm_panic module, which displays a user-friendly message to the screen when a kernel panic occurs. v8: * Replace get_scanout_buffer() with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions (Thomas Zimmermann) v12: * Use array for map and pitch in struct drm_scanout_buffer to support multi-planar format later. (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe Reviewed-by: Thomas Zimmermann --- drivers/gpu/drm/tiny/simpledrm.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 7ce1c4617675..1d8fa07572c5 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #define DRIVER_NAME"simpledrm" @@ -671,11 +672,26 @@ static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plan drm_dev_exit(idx); } +static int simpledrm_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb) +{ + struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev); + + sb->width = sdev->mode.hdisplay; + sb->height = sdev->mode.vdisplay; + sb->format = sdev->format; + sb->pitch[0] = sdev->pitch; + sb->map[0] = sdev->screen_base; + + return 0; +} + static const struct drm_plane_helper_funcs simpledrm_primary_plane_helper_funcs = { DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, .atomic_check = simpledrm_primary_plane_helper_atomic_check, .atomic_update = simpledrm_primary_plane_helper_atomic_update, .atomic_disable = simpledrm_primary_plane_helper_atomic_disable, + .get_scanout_buffer = simpledrm_primary_plane_helper_get_scanout_buffer, }; static const struct drm_plane_funcs simpledrm_primary_plane_funcs = { -- 2.44.0
[PATCH v12 7/9] drm/mgag200: Add drm_panic support
Add support for the drm_panic module, which displays a message to the screen when a kernel panic occurs. v5: * Also check that the plane is visible and primary. (Thomas Zimmermann) v7: * use drm_for_each_primary_visible_plane() v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions (Thomas Zimmermann) v12: * Use array for map and pitch in struct drm_scanout_buffer to support multi-planar format later. (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe Reviewed-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.h | 7 ++- drivers/gpu/drm/mgag200/mgag200_mode.c | 18 ++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 765e49fd8911..58a0e62eaf18 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -366,6 +366,7 @@ struct drm_crtc_state; struct drm_display_mode; struct drm_plane; struct drm_atomic_state; +struct drm_scanout_buffer; extern const uint32_t mgag200_primary_plane_formats[]; extern const size_t mgag200_primary_plane_formats_size; @@ -379,12 +380,16 @@ void mgag200_primary_plane_helper_atomic_enable(struct drm_plane *plane, struct drm_atomic_state *state); void mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, struct drm_atomic_state *old_state); +int mgag200_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb); + #define MGAG200_PRIMARY_PLANE_HELPER_FUNCS \ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, \ .atomic_check = mgag200_primary_plane_helper_atomic_check, \ .atomic_update = mgag200_primary_plane_helper_atomic_update, \ .atomic_enable = mgag200_primary_plane_helper_atomic_enable, \ - .atomic_disable = mgag200_primary_plane_helper_atomic_disable + .atomic_disable = mgag200_primary_plane_helper_atomic_disable, \ + .get_scanout_buffer = mgag200_primary_plane_helper_get_scanout_buffer #define MGAG200_PRIMARY_PLANE_FUNCS \ .update_plane = drm_atomic_helper_update_plane, \ diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index e17cb4c5f774..fc54851d3384 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "mgag200_drv.h" @@ -546,6 +547,23 @@ void mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, msleep(20); } +int mgag200_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct mga_device *mdev = to_mga_device(plane->dev); + struct iosys_map map = IOSYS_MAP_INIT_VADDR_IOMEM(mdev->vram); + + if (plane->state && plane->state->fb) { + sb->format = plane->state->fb->format; + sb->width = plane->state->fb->width; + sb->height = plane->state->fb->height; + sb->pitch[0] = plane->state->fb->pitches[0]; + sb->map[0] = map; + return 0; + } + return -ENODEV; +} + /* * CRTC */ -- 2.44.0
[PATCH v12 5/9] drm/fb_dma: Add generic get_scanout_buffer() for drm_panic
This was initialy done for imx6, but should work on most drivers using drm_fb_dma_helper. v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * go back to get_scanout_buffer() * move get_scanout_buffer() to plane helper functions v12: * Rename drm_panic_gem_get_scanout_buffer to drm_fb_dma_get_scanout_buffer (Thomas Zimmermann) * Remove the #ifdef CONFIG_DRM_PANIC, and build it unconditionnaly, as it's a small function. (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_dma_helper.c | 42 + include/drm/drm_fb_dma_helper.h | 4 +++ 2 files changed, 46 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index 3b535ad1b07c..c79b5078303f 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -148,3 +149,44 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, } } EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); + +/** + * drm_fb_dma_get_scanout_buffer - Provide a scanout buffer in case of panic + * @plane: DRM primary plane + * @drm_scanout_buffer: scanout buffer for the panic handler + * Returns: 0 or negative error code + * + * Generic get_scanout_buffer() implementation, for drivers that uses the + * drm_fb_dma_helper. It won't call vmap in the panic context, so the driver + * should make sure the primary plane is vmapped, otherwise the panic screen + * won't get displayed. + */ +int drm_fb_dma_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct drm_gem_dma_object *dma_obj; + struct drm_framebuffer *fb; + + fb = plane->state->fb; + /* Only support linear modifier */ + if (fb->modifier != DRM_FORMAT_MOD_LINEAR) + return -ENODEV; + + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); + + /* Buffer should be accessible from the CPU */ + if (dma_obj->base.import_attach) + return -ENODEV; + + /* Buffer should be already mapped to CPU */ + if (!dma_obj->vaddr) + return -ENODEV; + + iosys_map_set_vaddr(>map[0], dma_obj->vaddr); + sb->format = fb->format; + sb->height = fb->height; + sb->width = fb->width; + sb->pitch[0] = fb->pitches[0]; + return 0; +} +EXPORT_SYMBOL(drm_fb_dma_get_scanout_buffer); diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h index d5e036c57801..61f24c2aba2f 100644 --- a/include/drm/drm_fb_dma_helper.h +++ b/include/drm/drm_fb_dma_helper.h @@ -7,6 +7,7 @@ struct drm_device; struct drm_framebuffer; struct drm_plane_state; +struct drm_scanout_buffer; struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb, unsigned int plane); @@ -19,5 +20,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, struct drm_plane_state *old_state, struct drm_plane_state *state); +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb); + #endif -- 2.44.0
[PATCH v12 0/9] drm/panic: Add a drm panic handler
drm/panic: Add a drm panic handler This introduces a new drm panic handler, which displays a message when a panic occurs. So when fbcon is disabled, you can still see a kernel panic. This is one of the missing feature, when disabling VT/fbcon in the kernel: https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/ Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel. It works with simpledrm, mgag200, ast, and imx. To test it, make sure you're using one of the supported driver, and trigger a panic: echo c > /proc/sysrq-trigger or you can enable CONFIG_DRM_PANIC_DEBUG and echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 Even if this is not yet useful, it will allows to work on more driver support, and better debug information to be added. v2: * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) * Add the panic reason to the panic message (Nerdopolis) * Add an exclamation mark (Nerdopolis) v3: * Rework the drawing functions, to write the pixels line by line and to use the drm conversion helper to support other formats. (Thomas Zimmermann) v4: * Fully support all simpledrm formats using drm conversion helpers * Rename dpanic_* to drm_panic_*, and have more coherent function name. (Thomas Zimmermann) * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) * Add foreground/background color config option * Fix the bottom lines not painted if the framebuffer height is not a multiple of the font height. * Automatically register the driver to drm_panic, if the function get_scanout_buffer() exists. (Thomas Zimmermann) * Add mgag200 support. v5: * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. (Thomas Zimmermann) * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). * Add ast support. * Add experimental imx/ipuv3 support, to test on an ARM hw. (Maxime Ripard) v6: * Fix sparse and __le32 warnings * Drop the IMX/IPUV3 experiment, it was just to show that it works also on ARM devices. v7: * Add a check to see if the 4cc format is supported by drm_panic. * Add a drm/plane helper to loop over all visible primary buffer, simplifying the get_scanout_buffer implementations * Add a generic implementation for drivers that uses drm_fb_dma. (Maxime Ripard) * Add back the IMX/IPUV3 support, and use the generic implementation. (Maxime Ripard) v8: * Directly register each plane to the panic notifier (Sima) * Replace get_scanout_buffer() with set_scanout_buffer() to simplify the locking. (Thomas Zimmermann) * Add a debugfs entry, to trigger the drm_panic without a real panic (Sima) * Fix the drm_panic Documentation, and include it in drm-kms.rst v9: * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) * Create a debugfs file for each plane in the device's debugfs directory. This allows to test for each plane of each GPU independently. v10: * Move blit and fill functions back in drm_panic (Thomas Zimmermann). * Simplify the text drawing functions. * Use kmsg_dumper instead of panic_notifier (Sima). * Use spinlock_irqsave/restore (John Ogness) v11: * Use macro instead of inline functions for drm_panic_lock/unlock (John Ogness) v12: * Use array for map and pitch in struct drm_scanout_buffer to support multi-planar format later. (Thomas Zimmermann) * Rename drm_panic_gem_get_scanout_buffer to drm_fb_dma_get_scanout_buffer and build it unconditionally (Thomas Zimmermann) * Better indent struct drm_scanout_buffer declaration. (Thomas Zimmermann) Best regards, Daniel Vetter (1): drm/panic: Add drm panic locking Jocelyn Falempe (8): drm/panic: Add a drm panic handler drm/panic: Add support for color format conversion drm/panic: Add debugfs entry to test without triggering panic. drm/fb_dma: Add generic get_scanout_buffer() for drm_panic drm/simpledrm: Add drm_panic support drm/mgag200: Add drm_panic support drm/imx: Add drm_panic support drm/ast: Add drm_panic support Documentation/gpu/drm-kms.rst| 12 + drivers/gpu/drm/Kconfig | 32 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/ast/ast_mode.c | 18 + drivers/gpu/drm/drm_atomic_helper.c | 4 + drivers/gpu/drm/drm_drv.c| 5 + drivers/gpu/drm/drm_fb_dma_helper.c | 42 ++ drivers/gpu/drm/drm_panic.c
[PATCH v12 3/9] drm/panic: Add support for color format conversion
Add support for the following formats: DRM_FORMAT_RGB565 DRM_FORMAT_RGBA5551 DRM_FORMAT_XRGB1555 DRM_FORMAT_ARGB1555 DRM_FORMAT_RGB888 DRM_FORMAT_XRGB DRM_FORMAT_ARGB DRM_FORMAT_XBGR DRM_FORMAT_XRGB2101010 DRM_FORMAT_ARGB2101010 v10: * move and simplify the functions from the drm format helper to drm_panic v12: * Use array for map and pitch in struct drm_scanout_buffer to support multi-planar format later. (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_panic.c | 273 ++-- 1 file changed, 263 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index fa4b220534a4..e1ec30b6c04a 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -81,15 +81,155 @@ static const struct drm_panic_line logo[] = { PANIC_LINE(" \\___)=(___/"), }; -static void drm_panic_fill32(struct iosys_map *map, unsigned int pitch, +/* + * Color conversion + */ + +static u16 convert_xrgb_to_rgb565(u32 pix) +{ + return ((pix & 0x00F8) >> 8) | + ((pix & 0xFC00) >> 5) | + ((pix & 0x00F8) >> 3); +} + +static u16 convert_xrgb_to_rgba5551(u32 pix) +{ + return ((pix & 0x00f8) >> 8) | + ((pix & 0xf800) >> 5) | + ((pix & 0x00f8) >> 2) | + BIT(0); /* set alpha bit */ +} + +static u16 convert_xrgb_to_xrgb1555(u32 pix) +{ + return ((pix & 0x00f8) >> 9) | + ((pix & 0xf800) >> 6) | + ((pix & 0x00f8) >> 3); +} + +static u16 convert_xrgb_to_argb1555(u32 pix) +{ + return BIT(15) | /* set alpha bit */ + ((pix & 0x00f8) >> 9) | + ((pix & 0xf800) >> 6) | + ((pix & 0x00f8) >> 3); +} + +static u32 convert_xrgb_to_argb(u32 pix) +{ + return pix | GENMASK(31, 24); /* fill alpha bits */ +} + +static u32 convert_xrgb_to_xbgr(u32 pix) +{ + return ((pix & 0x00ff) >> 16) << 0 | + ((pix & 0xff00) >> 8) << 8 | + ((pix & 0x00ff) >> 0) << 16 | + ((pix & 0xff00) >> 24) << 24; +} + +static u32 convert_xrgb_to_abgr(u32 pix) +{ + return ((pix & 0x00ff) >> 16) << 0 | + ((pix & 0xff00) >> 8) << 8 | + ((pix & 0x00ff) >> 0) << 16 | + GENMASK(31, 24); /* fill alpha bits */ +} + +static u32 convert_xrgb_to_xrgb2101010(u32 pix) +{ + pix = ((pix & 0x00FF) << 2) | + ((pix & 0xFF00) << 4) | + ((pix & 0x00FF) << 6); + return pix | ((pix >> 8) & 0x00300C03); +} + +static u32 convert_xrgb_to_argb2101010(u32 pix) +{ + pix = ((pix & 0x00FF) << 2) | + ((pix & 0xFF00) << 4) | + ((pix & 0x00FF) << 6); + return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); +} + +/* + * convert_from_xrgb - convert one pixel from xrgb to the desired format + * @color: input color, in xrgb format + * @format: output format + * + * Returns: + * Color in the format specified, casted to u32. + * Or 0 if the format is not supported. + */ +static u32 convert_from_xrgb(u32 color, u32 format) +{ + switch (format) { + case DRM_FORMAT_RGB565: + return convert_xrgb_to_rgb565(color); + case DRM_FORMAT_RGBA5551: + return convert_xrgb_to_rgba5551(color); + case DRM_FORMAT_XRGB1555: + return convert_xrgb_to_xrgb1555(color); + case DRM_FORMAT_ARGB1555: + return convert_xrgb_to_argb1555(color); + case DRM_FORMAT_RGB888: + case DRM_FORMAT_XRGB: + return color; + case DRM_FORMAT_ARGB: + return convert_xrgb_to_argb(color); + case DRM_FORMAT_XBGR: + return convert_xrgb_to_xbgr(color); + case DRM_FORMAT_ABGR: + return convert_xrgb_to_abgr(color); + case DRM_FORMAT_XRGB2101010: + return convert_xrgb_to_xrgb2101010(color); + case DRM_FORMAT_ARGB2101010: + return convert_xrgb_to_argb2101010(color); + default: + WARN_ONCE(1, "Can't convert to %p4cc\n", ); + return 0; + } +} + +/* + * Blit & Fill + */ +static void drm_panic_blit16(struct iosys_map *dmap, unsigned int dpitch, +const u8 *sbuf8, unsigned int spitch, unsigned int height, unsigned int width, -
[PATCH v12 2/9] drm/panic: Add a drm panic handler
This module displays a user friendly message when a kernel panic occurs. It currently doesn't contain any debug information, but that can be added later. v2 * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) * Add the panic reason to the panic message (Nerdopolis) * Add an exclamation mark (Nerdopolis) v3 * Rework the drawing functions, to write the pixels line by line and to use the drm conversion helper to support other formats. (Thomas Zimmermann) v4 * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) * Add foreground/background color config option * Fix the bottom lines not painted if the framebuffer height is not a multiple of the font height. * Automatically register the device to drm_panic, if the function get_scanout_buffer exists. (Thomas Zimmermann) v5 * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). v6 * Fix sparse warning for panic_msg and logo. v7 * Add select DRM_KMS_HELPER for the color conversion functions. v8 * Register directly each plane to the panic notifier (Sima) * Add raw_spinlock to properly handle concurrency (Sima) * Register plane instead of device, to avoid looping through plane list, and simplify code. * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) * Removed the draw_pixel_xy() API, will see later if it can be added back. v9 * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) v10 * Move blit and fill functions back in drm_panic (Thomas Zimmermann). * Simplify the text drawing functions. * Use kmsg_dumper instead of panic_notifier (Sima). v12 * Use array for map and pitch in struct drm_scanout_buffer to support multi-planar format later. (Thomas Zimmermann) * Better indent struct drm_scanout_buffer declaration. (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- Documentation/gpu/drm-kms.rst| 12 + drivers/gpu/drm/Kconfig | 23 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_drv.c| 4 + drivers/gpu/drm/drm_panic.c | 289 +++ include/drm/drm_modeset_helper_vtables.h | 37 +++ include/drm/drm_panic.h | 57 + include/drm/drm_plane.h | 6 + 8 files changed, 429 insertions(+) create mode 100644 drivers/gpu/drm/drm_panic.c diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 13d3627d8bc0..b64334661aeb 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -398,6 +398,18 @@ Plane Damage Tracking Functions Reference .. kernel-doc:: include/drm/drm_damage_helper.h :internal: +Plane Panic Feature +--- + +.. kernel-doc:: drivers/gpu/drm/drm_panic.c + :doc: overview + +Plane Panic Functions Reference +--- + +.. kernel-doc:: drivers/gpu/drm/drm_panic.c + :export: + Display Modes Function Reference diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 3914aaf443a8..f8a26423369e 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -104,6 +104,29 @@ config DRM_KMS_HELPER help CRTC helpers for KMS drivers. +config DRM_PANIC + bool "Display a user-friendly message when a kernel panic occurs" + depends on DRM && !FRAMEBUFFER_CONSOLE + select DRM_KMS_HELPER + select FONT_SUPPORT + help + Enable a drm panic handler, which will display a user-friendly message + when a kernel panic occurs. It's useful when using a user-space + console instead of fbcon. + It will only work if your graphic driver supports this feature. + To support Hi-DPI Display, you can enable bigger fonts like + FONT_TER16x32 + +config DRM_PANIC_FOREGROUND_COLOR + hex "Drm panic screen foreground color, in RGB" + depends on DRM_PANIC + default 0xff + +config DRM_PANIC_BACKGROUND_COLOR + hex "Drm panic screen background color, in RGB" + depends on DRM_PANIC + default 0x00 + config DRM_DEBUG_DP_MST_TOPOLOGY_REFS bool "Enable refcount backtrace history in the DP MST helpers" depends on STACKTRACE_SUPPORT diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index
[PATCH v12 1/9] drm/panic: Add drm panic locking
From: Daniel Vetter Rough sketch for the locking of drm panic printing code. The upshot of this approach is that we can pretty much entirely rely on the atomic commit flow, with the pair of raw_spin_lock/unlock providing any barriers we need, without having to create really big critical sections in code. This also avoids the need that drivers must explicitly update the panic handler state, which they might forget to do, or not do consistently, and then we blow up in the worst possible times. It is somewhat racy against a concurrent atomic update, and we might write into a buffer which the hardware will never display. But there's fundamentally no way to avoid that - if we do the panic state update explicitly after writing to the hardware, we might instead write to an old buffer that the user will barely ever see. Note that an rcu protected deference of plane->state would give us the the same guarantees, but it has the downside that we then need to protect the plane state freeing functions with call_rcu too. Which would very widely impact a lot of code and therefore doesn't seem worth the complexity compared to a raw spinlock with very tiny critical sections. Plus rcu cannot be used to protect access to peek/poke registers anyway, so we'd still need it for those cases. Peek/poke registers for vram access (or a gart pte reserved just for panic code) are also the reason I've gone with a per-device and not per-plane spinlock, since usually these things are global for the entire display. Going with per-plane locks would mean drivers for such hardware would need additional locks, which we don't want, since it deviates from the per-console takeoverlocks design. Longer term it might be useful if the panic notifiers grow a bit more structure than just the absolute bare EXPORT_SYMBOL(panic_notifier_list) - somewhat aside, why is that not EXPORT_SYMBOL_GPL ... If panic notifiers would be more like console drivers with proper register/unregister interfaces we could perhaps reuse the very fancy console lock with all it's check and takeover semantics that John Ogness is developing to fix the console_lock mess. But for the initial cut of a drm panic printing support I don't think we need that, because the critical sections are extremely small and only happen once per display refresh. So generally just 60 tiny locked sections per second, which is nothing compared to a serial console running a 115kbaud doing really slow mmio writes for each byte. So for now the raw spintrylock in drm panic notifier callback should be good enough. Another benefit of making panic notifiers more like full blown consoles (that are used in panics only) would be that we get the two stage design, where first all the safe outputs are used. And then the dangerous takeover tricks are deployed (where for display drivers we also might try to intercept any in-flight display buffer flips, which if we race and misprogram fifos and watermarks can hang the memory controller on some hw). For context the actual implementation on the drm side is by Jocelyn and this patch is meant to be combined with the overall approach in v7 (v8 is a bit less flexible, which I think is the wrong direction): https://lore.kernel.org/dri-devel/20240104160301.185915-1-jfale...@redhat.com/ Note that the locking is very much not correct there, hence this separate rfc. v2: - fix authorship, this was all my typing - some typo oopsies - link to the drm panic work by Jocelyn for context v10: - Use spinlock_irqsave/restore (John Ogness) v11: - Use macro instead of inline functions for drm_panic_lock/unlock (John Ogness) Signed-off-by: Daniel Vetter Cc: Jocelyn Falempe Cc: Andrew Morton Cc: "Peter Zijlstra (Intel)" Cc: Lukas Wunner Cc: Petr Mladek Cc: Steven Rostedt Cc: John Ogness Cc: Sergey Senozhatsky Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++ drivers/gpu/drm/drm_drv.c | 1 + include/drm/drm_mode_config.h | 10 +++ include/drm/drm_panic.h | 100 4 files changed, 115 insertions(+) create mode 100644 include/drm/drm_panic.h diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 39ef0a6addeb..fb97b51b38f1 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -3016,6 +3017,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall) { int i, ret; + unsigned long flags; struct drm_connector *connector; struct drm_connector_state *old_conn_state, *new_conn_state; struct drm_crtc *crtc; @@ -3099,6 +3101,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_
Re: [PATCH v11 5/9] drm/fb_dma: Add generic get_scanout_buffer() for drm_panic
On 09/04/2024 10:21, Thomas Zimmermann wrote: Hi Am 28.03.24 um 13:03 schrieb Jocelyn Falempe: This was initialy done for imx6, but should work on most drivers using drm_fb_dma_helper. v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * go back to get_scanout_buffer() * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_dma_helper.c | 47 + include/drm/drm_fb_dma_helper.h | 4 +++ 2 files changed, 51 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index 3b535ad1b07c..010327069ad4 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -148,3 +149,49 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, } } EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); + +#if defined(CONFIG_DRM_PANIC) I would not bother with CONFIG_DRM_PANIC for now and make the helper generally available. yes, that's a small function, let's keep it simple. Done in v12 +/** + * @plane: DRM primary plane + * @drm_scanout_buffer: scanout buffer for the panic handler + * Returns: 0 or negative error code + * + * Generic get_scanout_buffer() implementation, for drivers that uses the + * drm_fb_dma_helper. + */ +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) It's neither really a function for panic handling nor a GEM function. This helper needs to be called drm_fb_dma_get_scanout_buffer() IMHO. I agree, it matches the other functions prefixes in this file. Done in v12 +{ + struct drm_gem_dma_object *dma_obj; + struct drm_framebuffer *fb; + + fb = plane->state->fb; + /* Only support linear modifier */ + if (fb->modifier != DRM_FORMAT_MOD_LINEAR) + return -ENODEV; + + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); + + /* Buffer should be accessible from the CPU */ + if (dma_obj->base.import_attach) + return -ENODEV; + + /* Buffer should be already mapped to CPU */ + if (!dma_obj->vaddr) + return -ENODEV; + + iosys_map_set_vaddr(>map, dma_obj->vaddr); + sb->format = fb->format; + sb->height = fb->height; + sb->width = fb->width; + sb->pitch = fb->pitches[0]; + return 0; +} +#else +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + return -ENODEV; +} +#endif +EXPORT_SYMBOL(drm_panic_gem_get_scanout_buffer); diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h index d5e036c57801..61f24c2aba2f 100644 --- a/include/drm/drm_fb_dma_helper.h +++ b/include/drm/drm_fb_dma_helper.h @@ -7,6 +7,7 @@ struct drm_device; struct drm_framebuffer; struct drm_plane_state; +struct drm_scanout_buffer; struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb, unsigned int plane); @@ -19,5 +20,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, struct drm_plane_state *old_state, struct drm_plane_state *state); +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb); + #endif Thanks for the reviews, -- Jocelyn
Re: [PATCH v11 2/9] drm/panic: Add a drm panic handler
Hi, On 09/04/2024 10:30, Thomas Zimmermann wrote: Hi Am 28.03.24 um 13:03 schrieb Jocelyn Falempe: +/** + * struct drm_scanout_buffer - DRM scanout buffer + * + * This structure holds the information necessary for drm_panic to draw the + * panic screen, and display it. + */ +struct drm_scanout_buffer { + /** + * @format: + * + * drm format of the scanout buffer. + */ + const struct drm_format_info *format; Newline here and among the other fields please. Done in v12. + /** + * @map: + * + * Virtual address of the scanout buffer, either in memory or iomem. + * The scanout buffer should be in linear format, and can be directly + * sent to the display hardware. Tearing is not an issue for the panic + * screen. + */ + struct iosys_map map; I would make this an array of DRM_FORMAT_MAX_PLANES. Its functionality is then equivalent to the fields in struct drm_framebuffer. Supporting multiple color planes is the general expectation in the DRM code, even if not all parts actually implement it. In the panic code, you simply test that the scan-out format has only a single plane. struct iosys_map map[DRM_FORMAT_MAX_PLANES] Sure, that was on my todo list, done in v12. + /** + * @width: Width of the scanout buffer, in pixels. + */ + unsigned int width; + /** + * @height: Height of the scanout buffer, in pixels. + */ + unsigned int height; + /** + * @pitch: Length in bytes between the start of two consecutive lines. + */ + unsigned int pitch; Also use an array of DRM_FORMAT_MAX_PLANES. +}; This data structure looks like something I could use for the shadow-plane helpers. Expect it to be moved elsewhere at some point. Yes, this can even be part of the struct drm_framebuffer. Best regards Thomas Thanks for the reviews, -- Jocelyn
Re: [PATCH 00/11] drm/mgag200: Detect connector status
Hi Thomas, I've tested this series on my Dell T310, and it works well, when I plug/unplug the VGA screen, it's reflected in /sys/class/drm/.../status I've also tested it remotely on a Dell R640, which doesn't have a VGA monitor connected. After the patch, on the iDrac console, I only get a green screen saying: "Out of Range Reason: Video Capture Failure Detected Resolution: 0x768 Detected Color Depth-1bpp" and the file: /sys/class/drm/card0-VGA-1/modes is empty Before the patch, the driver reports VGA as connected, and modes contains 1024x768 and others. I think we may need to add a virtual connector for BMC, like I've done for AST ? So that when no VGA monitor is available, you can still choose an appropriate resolution. Best regards, -- Jocelyn On 03/04/2024 11:24, Thomas Zimmermann wrote: Detect the connector status by polling the DDC. Update the status at runtime. Clean up a the driver's DDC code in the process. Patches 1 and 2 fix long-standing problems in the DDC code. Patches 3 to 9 refactor the DDC code. The code then keeps its data structures internal, acquires locks automatically and it much more readable overall. With patches 10 and 11, mgag200 makes use of existing helpers for reading and probing the DDC. It then correctly updates the status and EDID at runtime. Tested on various Matrox hardware. Thomas Zimmermann (11): drm/mgag200: Set DDC timeout in milliseconds drm/mgag200: Bind I2C lifetime to DRM device drm/mgag200: Store pointer to struct mga_device in struct mga_i2c_chan drm/mgag200: Allocate instance of struct mga_i2c_chan dynamically drm/mgag200: Inline mgag200_i2c_init() drm/mgag200: Replace struct mga_i2c_chan with struct mgag200_ddc drm/mgag200: Rename mgag200_i2c.c to mgag200_ddc.c drm/mgag200: Rename struct i2c_algo_bit_data callbacks drm/mgag200: Acquire I/O-register lock in DDC code drm/mgag200: Use drm_connector_helper_get_modes() drm/mgag200: Set .detect_ctx() and enable connector polling drivers/gpu/drm/mgag200/Makefile | 2 +- drivers/gpu/drm/mgag200/mgag200_ddc.c | 179 ++ drivers/gpu/drm/mgag200/mgag200_ddc.h | 11 ++ drivers/gpu/drm/mgag200/mgag200_drv.h | 19 +-- drivers/gpu/drm/mgag200/mgag200_g200.c| 15 +- drivers/gpu/drm/mgag200/mgag200_g200eh.c | 15 +- drivers/gpu/drm/mgag200/mgag200_g200eh3.c | 15 +- drivers/gpu/drm/mgag200/mgag200_g200er.c | 15 +- drivers/gpu/drm/mgag200/mgag200_g200ev.c | 15 +- drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 15 +- drivers/gpu/drm/mgag200/mgag200_g200se.c | 15 +- drivers/gpu/drm/mgag200/mgag200_g200wb.c | 15 +- drivers/gpu/drm/mgag200/mgag200_i2c.c | 129 drivers/gpu/drm/mgag200/mgag200_mode.c| 27 +--- 14 files changed, 274 insertions(+), 213 deletions(-) create mode 100644 drivers/gpu/drm/mgag200/mgag200_ddc.c create mode 100644 drivers/gpu/drm/mgag200/mgag200_ddc.h delete mode 100644 drivers/gpu/drm/mgag200/mgag200_i2c.c
[PATCH v11 5/9] drm/fb_dma: Add generic get_scanout_buffer() for drm_panic
This was initialy done for imx6, but should work on most drivers using drm_fb_dma_helper. v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * go back to get_scanout_buffer() * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_dma_helper.c | 47 + include/drm/drm_fb_dma_helper.h | 4 +++ 2 files changed, 51 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index 3b535ad1b07c..010327069ad4 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -148,3 +149,49 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, } } EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); + +#if defined(CONFIG_DRM_PANIC) +/** + * @plane: DRM primary plane + * @drm_scanout_buffer: scanout buffer for the panic handler + * Returns: 0 or negative error code + * + * Generic get_scanout_buffer() implementation, for drivers that uses the + * drm_fb_dma_helper. + */ +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb) +{ + struct drm_gem_dma_object *dma_obj; + struct drm_framebuffer *fb; + + fb = plane->state->fb; + /* Only support linear modifier */ + if (fb->modifier != DRM_FORMAT_MOD_LINEAR) + return -ENODEV; + + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); + + /* Buffer should be accessible from the CPU */ + if (dma_obj->base.import_attach) + return -ENODEV; + + /* Buffer should be already mapped to CPU */ + if (!dma_obj->vaddr) + return -ENODEV; + + iosys_map_set_vaddr(>map, dma_obj->vaddr); + sb->format = fb->format; + sb->height = fb->height; + sb->width = fb->width; + sb->pitch = fb->pitches[0]; + return 0; +} +#else +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb) +{ + return -ENODEV; +} +#endif +EXPORT_SYMBOL(drm_panic_gem_get_scanout_buffer); diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h index d5e036c57801..61f24c2aba2f 100644 --- a/include/drm/drm_fb_dma_helper.h +++ b/include/drm/drm_fb_dma_helper.h @@ -7,6 +7,7 @@ struct drm_device; struct drm_framebuffer; struct drm_plane_state; +struct drm_scanout_buffer; struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb, unsigned int plane); @@ -19,5 +20,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, struct drm_plane_state *old_state, struct drm_plane_state *state); +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb); + #endif -- 2.44.0
[PATCH v11 9/9] drm/ast: Add drm_panic support
Add support for the drm_panic module, which displays a message to the screen when a kernel panic occurs. v7 * Use drm_for_each_primary_visible_plane() v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe Acked-by: Sui Jingfeng Tested-by: Sui Jingfeng --- drivers/gpu/drm/ast/ast_mode.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index a718646a66b8..49f2d8bd3377 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -700,12 +701,29 @@ static void ast_primary_plane_helper_atomic_disable(struct drm_plane *plane, ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x1, 0xdf, 0x20); } +static int ast_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct ast_plane *ast_plane = to_ast_plane(plane); + + if (plane->state && plane->state->fb && ast_plane->vaddr) { + sb->format = plane->state->fb->format; + sb->width = plane->state->fb->width; + sb->height = plane->state->fb->height; + sb->pitch = plane->state->fb->pitches[0]; + iosys_map_set_vaddr_iomem(>map, ast_plane->vaddr); + return 0; + } + return -ENODEV; +} + static const struct drm_plane_helper_funcs ast_primary_plane_helper_funcs = { DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, .atomic_check = ast_primary_plane_helper_atomic_check, .atomic_update = ast_primary_plane_helper_atomic_update, .atomic_enable = ast_primary_plane_helper_atomic_enable, .atomic_disable = ast_primary_plane_helper_atomic_disable, + .get_scanout_buffer = ast_primary_plane_helper_get_scanout_buffer, }; static const struct drm_plane_funcs ast_primary_plane_funcs = { -- 2.44.0
[PATCH v11 6/9] drm/simpledrm: Add drm_panic support
Add support for the drm_panic module, which displays a user-friendly message to the screen when a kernel panic occurs. v8: * Replace get_scanout_buffer() with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/tiny/simpledrm.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 7ce1c4617675..f498665be8c4 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #define DRIVER_NAME"simpledrm" @@ -671,11 +672,26 @@ static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plan drm_dev_exit(idx); } +static int simpledrm_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb) +{ + struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev); + + sb->width = sdev->mode.hdisplay; + sb->height = sdev->mode.vdisplay; + sb->format = sdev->format; + sb->pitch = sdev->pitch; + sb->map = sdev->screen_base; + + return 0; +} + static const struct drm_plane_helper_funcs simpledrm_primary_plane_helper_funcs = { DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, .atomic_check = simpledrm_primary_plane_helper_atomic_check, .atomic_update = simpledrm_primary_plane_helper_atomic_update, .atomic_disable = simpledrm_primary_plane_helper_atomic_disable, + .get_scanout_buffer = simpledrm_primary_plane_helper_get_scanout_buffer, }; static const struct drm_plane_funcs simpledrm_primary_plane_funcs = { -- 2.44.0
[PATCH v11 7/9] drm/mgag200: Add drm_panic support
Add support for the drm_panic module, which displays a message to the screen when a kernel panic occurs. v5: * Also check that the plane is visible and primary. (Thomas Zimmermann) v7: * use drm_for_each_primary_visible_plane() v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/mgag200/mgag200_drv.h | 7 ++- drivers/gpu/drm/mgag200/mgag200_mode.c | 18 ++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 765e49fd8911..58a0e62eaf18 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -366,6 +366,7 @@ struct drm_crtc_state; struct drm_display_mode; struct drm_plane; struct drm_atomic_state; +struct drm_scanout_buffer; extern const uint32_t mgag200_primary_plane_formats[]; extern const size_t mgag200_primary_plane_formats_size; @@ -379,12 +380,16 @@ void mgag200_primary_plane_helper_atomic_enable(struct drm_plane *plane, struct drm_atomic_state *state); void mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, struct drm_atomic_state *old_state); +int mgag200_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb); + #define MGAG200_PRIMARY_PLANE_HELPER_FUNCS \ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, \ .atomic_check = mgag200_primary_plane_helper_atomic_check, \ .atomic_update = mgag200_primary_plane_helper_atomic_update, \ .atomic_enable = mgag200_primary_plane_helper_atomic_enable, \ - .atomic_disable = mgag200_primary_plane_helper_atomic_disable + .atomic_disable = mgag200_primary_plane_helper_atomic_disable, \ + .get_scanout_buffer = mgag200_primary_plane_helper_get_scanout_buffer #define MGAG200_PRIMARY_PLANE_FUNCS \ .update_plane = drm_atomic_helper_update_plane, \ diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index e17cb4c5f774..8f368ecca4d4 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "mgag200_drv.h" @@ -546,6 +547,23 @@ void mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, msleep(20); } +int mgag200_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct mga_device *mdev = to_mga_device(plane->dev); + struct iosys_map map = IOSYS_MAP_INIT_VADDR_IOMEM(mdev->vram); + + if (plane->state && plane->state->fb) { + sb->format = plane->state->fb->format; + sb->width = plane->state->fb->width; + sb->height = plane->state->fb->height; + sb->pitch = plane->state->fb->pitches[0]; + sb->map = map; + return 0; + } + return -ENODEV; +} + /* * CRTC */ -- 2.44.0
[PATCH v11 8/9] drm/imx: Add drm_panic support
Add support for the drm_panic module, which displays a user-friendly message to the screen when a kernel panic occurs. v7: * use drm_panic_gem_get_scanout_buffer() helper v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c index dade8b59feae..3e21d2a1a124 100644 --- a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c @@ -772,6 +772,12 @@ static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = { .atomic_disable = ipu_plane_atomic_disable, .atomic_update = ipu_plane_atomic_update, }; +static const struct drm_plane_helper_funcs ipu_primary_plane_helper_funcs = { + .atomic_check = ipu_plane_atomic_check, + .atomic_disable = ipu_plane_atomic_disable, + .atomic_update = ipu_plane_atomic_update, + .get_scanout_buffer = drm_panic_gem_get_scanout_buffer, +}; bool ipu_plane_atomic_update_pending(struct drm_plane *plane) { @@ -916,7 +922,10 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, ipu_plane->dma = dma; ipu_plane->dp_flow = dp; - drm_plane_helper_add(_plane->base, _plane_helper_funcs); + if (type == DRM_PLANE_TYPE_PRIMARY) + drm_plane_helper_add(_plane->base, _primary_plane_helper_funcs); + else + drm_plane_helper_add(_plane->base, _plane_helper_funcs); if (dp == IPU_DP_FLOW_SYNC_BG || dp == IPU_DP_FLOW_SYNC_FG) ret = drm_plane_create_zpos_property(_plane->base, zpos, 0, -- 2.44.0
[PATCH v11 3/9] drm/panic: Add support for color format conversion
Add support for the following formats: DRM_FORMAT_RGB565 DRM_FORMAT_RGBA5551 DRM_FORMAT_XRGB1555 DRM_FORMAT_ARGB1555 DRM_FORMAT_RGB888 DRM_FORMAT_XRGB DRM_FORMAT_ARGB DRM_FORMAT_XBGR DRM_FORMAT_XRGB2101010 DRM_FORMAT_ARGB2101010 v10: * move and simplify the functions from the drm format helper to drm_panic Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_panic.c | 272 ++-- 1 file changed, 262 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 72b5b363bcee..7761779a884b 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -81,15 +81,155 @@ static const struct drm_panic_line logo[] = { PANIC_LINE(" \\___)=(___/"), }; -static void drm_panic_fill32(struct iosys_map *map, unsigned int pitch, +/* + * Color conversion + */ + +static u16 convert_xrgb_to_rgb565(u32 pix) +{ + return ((pix & 0x00F8) >> 8) | + ((pix & 0xFC00) >> 5) | + ((pix & 0x00F8) >> 3); +} + +static u16 convert_xrgb_to_rgba5551(u32 pix) +{ + return ((pix & 0x00f8) >> 8) | + ((pix & 0xf800) >> 5) | + ((pix & 0x00f8) >> 2) | + BIT(0); /* set alpha bit */ +} + +static u16 convert_xrgb_to_xrgb1555(u32 pix) +{ + return ((pix & 0x00f8) >> 9) | + ((pix & 0xf800) >> 6) | + ((pix & 0x00f8) >> 3); +} + +static u16 convert_xrgb_to_argb1555(u32 pix) +{ + return BIT(15) | /* set alpha bit */ + ((pix & 0x00f8) >> 9) | + ((pix & 0xf800) >> 6) | + ((pix & 0x00f8) >> 3); +} + +static u32 convert_xrgb_to_argb(u32 pix) +{ + return pix | GENMASK(31, 24); /* fill alpha bits */ +} + +static u32 convert_xrgb_to_xbgr(u32 pix) +{ + return ((pix & 0x00ff) >> 16) << 0 | + ((pix & 0xff00) >> 8) << 8 | + ((pix & 0x00ff) >> 0) << 16 | + ((pix & 0xff00) >> 24) << 24; +} + +static u32 convert_xrgb_to_abgr(u32 pix) +{ + return ((pix & 0x00ff) >> 16) << 0 | + ((pix & 0xff00) >> 8) << 8 | + ((pix & 0x00ff) >> 0) << 16 | + GENMASK(31, 24); /* fill alpha bits */ +} + +static u32 convert_xrgb_to_xrgb2101010(u32 pix) +{ + pix = ((pix & 0x00FF) << 2) | + ((pix & 0xFF00) << 4) | + ((pix & 0x00FF) << 6); + return pix | ((pix >> 8) & 0x00300C03); +} + +static u32 convert_xrgb_to_argb2101010(u32 pix) +{ + pix = ((pix & 0x00FF) << 2) | + ((pix & 0xFF00) << 4) | + ((pix & 0x00FF) << 6); + return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); +} + +/* + * convert_from_xrgb - convert one pixel from xrgb to the desired format + * @color: input color, in xrgb format + * @format: output format + * + * Returns: + * Color in the format specified, casted to u32. + * Or 0 if the format is not supported. + */ +static u32 convert_from_xrgb(u32 color, u32 format) +{ + switch (format) { + case DRM_FORMAT_RGB565: + return convert_xrgb_to_rgb565(color); + case DRM_FORMAT_RGBA5551: + return convert_xrgb_to_rgba5551(color); + case DRM_FORMAT_XRGB1555: + return convert_xrgb_to_xrgb1555(color); + case DRM_FORMAT_ARGB1555: + return convert_xrgb_to_argb1555(color); + case DRM_FORMAT_RGB888: + case DRM_FORMAT_XRGB: + return color; + case DRM_FORMAT_ARGB: + return convert_xrgb_to_argb(color); + case DRM_FORMAT_XBGR: + return convert_xrgb_to_xbgr(color); + case DRM_FORMAT_ABGR: + return convert_xrgb_to_abgr(color); + case DRM_FORMAT_XRGB2101010: + return convert_xrgb_to_xrgb2101010(color); + case DRM_FORMAT_ARGB2101010: + return convert_xrgb_to_argb2101010(color); + default: + WARN_ONCE(1, "Can't convert to %p4cc\n", ); + return 0; + } +} + +/* + * Blit & Fill + */ +static void drm_panic_blit16(struct iosys_map *dmap, unsigned int dpitch, +const u8 *sbuf8, unsigned int spitch, unsigned int height, unsigned int width, -u32 color) +u16 fg16, u16 bg16) { unsigned int y, x; + u16 val16; -
[PATCH v11 4/9] drm/panic: Add debugfs entry to test without triggering panic.
Add a debugfs file, so you can test drm_panic without freezing your machine. This is unsafe, and should be enabled only for developer or tester. To display the drm_panic screen on the device 0: echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 v9: * Create a debugfs file for each plane in the device's debugfs directory. This allows to test for each plane of each GPU independently. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/Kconfig | 9 drivers/gpu/drm/drm_panic.c | 43 - 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index f07ca38d3f98..6e41fbd16b3d 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR depends on DRM_PANIC default 0x00 +config DRM_PANIC_DEBUG + bool "Add a debug fs entry to trigger drm_panic" + depends on DRM_PANIC && DEBUG_FS + help + Add dri/[device]/drm_panic_plane_x in the kernel debugfs, to force the + panic handler to write the panic message to this plane scanout buffer. + This is unsafe and should not be enabled on a production build. + If in doubt, say "N". + config DRM_DEBUG_DP_MST_TOPOLOGY_REFS bool "Enable refcount backtrace history in the DP MST helpers" depends on STACKTRACE_SUPPORT diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 7761779a884b..b75e90da7f39 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -493,6 +493,45 @@ static void drm_panic(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason) draw_panic_plane(plane); } + +/* + * DEBUG FS, This is currently unsafe. + * Create one file per plane, so it's possible to debug one plane at a time. + */ +#ifdef CONFIG_DRM_PANIC_DEBUG +#include + +static ssize_t debugfs_trigger_write(struct file *file, const char __user *user_buf, +size_t count, loff_t *ppos) +{ + bool run; + + if (kstrtobool_from_user(user_buf, count, ) == 0 && run) { + struct drm_plane *plane = file->private_data; + + draw_panic_plane(plane); + } + return count; +} + +static const struct file_operations dbg_drm_panic_ops = { + .owner = THIS_MODULE, + .write = debugfs_trigger_write, + .open = simple_open, +}; + +static void debugfs_register_plane(struct drm_plane *plane, int index) +{ + char fname[32]; + + snprintf(fname, 32, "drm_panic_plane_%d", index); + debugfs_create_file(fname, 0200, plane->dev->debugfs_root, + plane, _drm_panic_ops); +} +#else +static void debugfs_register_plane(struct drm_plane *plane, int index) {} +#endif /* CONFIG_DRM_PANIC_DEBUG */ + /** * drm_panic_register() - Initialize DRM panic for a device * @dev: the drm device on which the panic screen will be displayed. @@ -512,8 +551,10 @@ void drm_panic_register(struct drm_device *dev) plane->kmsg_panic.max_reason = KMSG_DUMP_PANIC; if (kmsg_dump_register(>kmsg_panic)) drm_warn(dev, "Failed to register panic handler\n"); - else + else { + debugfs_register_plane(plane, registered_plane); registered_plane++; + } } if (registered_plane) drm_info(dev, "Registered %d planes with drm panic\n", registered_plane); -- 2.44.0
[PATCH v11 2/9] drm/panic: Add a drm panic handler
This module displays a user friendly message when a kernel panic occurs. It currently doesn't contain any debug information, but that can be added later. v2 * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) * Add the panic reason to the panic message (Nerdopolis) * Add an exclamation mark (Nerdopolis) v3 * Rework the drawing functions, to write the pixels line by line and to use the drm conversion helper to support other formats. (Thomas Zimmermann) v4 * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) * Add foreground/background color config option * Fix the bottom lines not painted if the framebuffer height is not a multiple of the font height. * Automatically register the device to drm_panic, if the function get_scanout_buffer exists. (Thomas Zimmermann) v5 * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). v6 * Fix sparse warning for panic_msg and logo. v7 * Add select DRM_KMS_HELPER for the color conversion functions. v8 * Register directly each plane to the panic notifier (Sima) * Add raw_spinlock to properly handle concurrency (Sima) * Register plane instead of device, to avoid looping through plane list, and simplify code. * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) * Removed the draw_pixel_xy() API, will see later if it can be added back. v9 * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) v10 * Move blit and fill functions back in drm_panic (Thomas Zimmermann). * Simplify the text drawing functions. * Use kmsg_dumper instead of panic_notifier (Sima). Signed-off-by: Jocelyn Falempe --- Documentation/gpu/drm-kms.rst| 12 + drivers/gpu/drm/Kconfig | 23 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_drv.c| 4 + drivers/gpu/drm/drm_panic.c | 288 +++ include/drm/drm_modeset_helper_vtables.h | 37 +++ include/drm/drm_panic.h | 52 include/drm/drm_plane.h | 6 + 8 files changed, 423 insertions(+) create mode 100644 drivers/gpu/drm/drm_panic.c diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 13d3627d8bc0..b64334661aeb 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -398,6 +398,18 @@ Plane Damage Tracking Functions Reference .. kernel-doc:: include/drm/drm_damage_helper.h :internal: +Plane Panic Feature +--- + +.. kernel-doc:: drivers/gpu/drm/drm_panic.c + :doc: overview + +Plane Panic Functions Reference +--- + +.. kernel-doc:: drivers/gpu/drm/drm_panic.c + :export: + Display Modes Function Reference diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 16029435b750..f07ca38d3f98 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -102,6 +102,29 @@ config DRM_KMS_HELPER help CRTC helpers for KMS drivers. +config DRM_PANIC + bool "Display a user-friendly message when a kernel panic occurs" + depends on DRM && !FRAMEBUFFER_CONSOLE + select DRM_KMS_HELPER + select FONT_SUPPORT + help + Enable a drm panic handler, which will display a user-friendly message + when a kernel panic occurs. It's useful when using a user-space + console instead of fbcon. + It will only work if your graphic driver supports this feature. + To support Hi-DPI Display, you can enable bigger fonts like + FONT_TER16x32 + +config DRM_PANIC_FOREGROUND_COLOR + hex "Drm panic screen foreground color, in RGB" + depends on DRM_PANIC + default 0xff + +config DRM_PANIC_BACKGROUND_COLOR + hex "Drm panic screen background color, in RGB" + depends on DRM_PANIC + default 0x00 + config DRM_DEBUG_DP_MST_TOPOLOGY_REFS bool "Enable refcount backtrace history in the DP MST helpers" depends on STACKTRACE_SUPPORT diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index a73c04d2d7a3..f9ca4f8fa6c5 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -88,6 +88,7 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += \ drm_privacy_screen.o \
[PATCH v11 1/9] drm/panic: Add drm panic locking
From: Daniel Vetter Rough sketch for the locking of drm panic printing code. The upshot of this approach is that we can pretty much entirely rely on the atomic commit flow, with the pair of raw_spin_lock/unlock providing any barriers we need, without having to create really big critical sections in code. This also avoids the need that drivers must explicitly update the panic handler state, which they might forget to do, or not do consistently, and then we blow up in the worst possible times. It is somewhat racy against a concurrent atomic update, and we might write into a buffer which the hardware will never display. But there's fundamentally no way to avoid that - if we do the panic state update explicitly after writing to the hardware, we might instead write to an old buffer that the user will barely ever see. Note that an rcu protected deference of plane->state would give us the the same guarantees, but it has the downside that we then need to protect the plane state freeing functions with call_rcu too. Which would very widely impact a lot of code and therefore doesn't seem worth the complexity compared to a raw spinlock with very tiny critical sections. Plus rcu cannot be used to protect access to peek/poke registers anyway, so we'd still need it for those cases. Peek/poke registers for vram access (or a gart pte reserved just for panic code) are also the reason I've gone with a per-device and not per-plane spinlock, since usually these things are global for the entire display. Going with per-plane locks would mean drivers for such hardware would need additional locks, which we don't want, since it deviates from the per-console takeoverlocks design. Longer term it might be useful if the panic notifiers grow a bit more structure than just the absolute bare EXPORT_SYMBOL(panic_notifier_list) - somewhat aside, why is that not EXPORT_SYMBOL_GPL ... If panic notifiers would be more like console drivers with proper register/unregister interfaces we could perhaps reuse the very fancy console lock with all it's check and takeover semantics that John Ogness is developing to fix the console_lock mess. But for the initial cut of a drm panic printing support I don't think we need that, because the critical sections are extremely small and only happen once per display refresh. So generally just 60 tiny locked sections per second, which is nothing compared to a serial console running a 115kbaud doing really slow mmio writes for each byte. So for now the raw spintrylock in drm panic notifier callback should be good enough. Another benefit of making panic notifiers more like full blown consoles (that are used in panics only) would be that we get the two stage design, where first all the safe outputs are used. And then the dangerous takeover tricks are deployed (where for display drivers we also might try to intercept any in-flight display buffer flips, which if we race and misprogram fifos and watermarks can hang the memory controller on some hw). For context the actual implementation on the drm side is by Jocelyn and this patch is meant to be combined with the overall approach in v7 (v8 is a bit less flexible, which I think is the wrong direction): https://lore.kernel.org/dri-devel/20240104160301.185915-1-jfale...@redhat.com/ Note that the locking is very much not correct there, hence this separate rfc. v2: - fix authorship, this was all my typing - some typo oopsies - link to the drm panic work by Jocelyn for context v10: - Use spinlock_irqsave/restore (John Ogness) v11: - Use macro instead of inline functions for drm_panic_lock/unlock (John Ogness) Signed-off-by: Daniel Vetter Cc: Jocelyn Falempe Cc: Andrew Morton Cc: "Peter Zijlstra (Intel)" Cc: Lukas Wunner Cc: Petr Mladek Cc: Steven Rostedt Cc: John Ogness Cc: Sergey Senozhatsky Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++ drivers/gpu/drm/drm_drv.c | 1 + include/drm/drm_mode_config.h | 10 +++ include/drm/drm_panic.h | 100 4 files changed, 115 insertions(+) create mode 100644 include/drm/drm_panic.h diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 39ef0a6addeb..fb97b51b38f1 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -3016,6 +3017,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall) { int i, ret; + unsigned long flags; struct drm_connector *connector; struct drm_connector_state *old_conn_state, *new_conn_state; struct drm_crtc *crtc; @@ -3099,6 +3101,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_
[PATCH v11 0/9] drm/panic: Add a drm panic handler
This introduces a new drm panic handler, which displays a message when a panic occurs. So when fbcon is disabled, you can still see a kernel panic. This is one of the missing feature, when disabling VT/fbcon in the kernel: https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/ Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel. It works with simpledrm, mgag200, ast, and imx. To test it, make sure you're using one of the supported driver, and trigger a panic: echo c > /proc/sysrq-trigger or you can enable CONFIG_DRM_PANIC_DEBUG and echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 There were not many comments on v10, so I think we're close to something that can be merged. Even if this is not yet useful, it will allows to work on more driver support, and better debug information. v2: * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) * Add the panic reason to the panic message (Nerdopolis) * Add an exclamation mark (Nerdopolis) v3: * Rework the drawing functions, to write the pixels line by line and to use the drm conversion helper to support other formats. (Thomas Zimmermann) v4: * Fully support all simpledrm formats using drm conversion helpers * Rename dpanic_* to drm_panic_*, and have more coherent function name. (Thomas Zimmermann) * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) * Add foreground/background color config option * Fix the bottom lines not painted if the framebuffer height is not a multiple of the font height. * Automatically register the driver to drm_panic, if the function get_scanout_buffer() exists. (Thomas Zimmermann) * Add mgag200 support. v5: * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. (Thomas Zimmermann) * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). * Add ast support. * Add experimental imx/ipuv3 support, to test on an ARM hw. (Maxime Ripard) v6: * Fix sparse and __le32 warnings * Drop the IMX/IPUV3 experiment, it was just to show that it works also on ARM devices. v7: * Add a check to see if the 4cc format is supported by drm_panic. * Add a drm/plane helper to loop over all visible primary buffer, simplifying the get_scanout_buffer implementations * Add a generic implementation for drivers that uses drm_fb_dma. (Maxime Ripard) * Add back the IMX/IPUV3 support, and use the generic implementation. (Maxime Ripard) v8: * Directly register each plane to the panic notifier (Sima) * Replace get_scanout_buffer() with set_scanout_buffer() to simplify the locking. (Thomas Zimmermann) * Add a debugfs entry, to trigger the drm_panic without a real panic (Sima) * Fix the drm_panic Documentation, and include it in drm-kms.rst v9: * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) * Create a debugfs file for each plane in the device's debugfs directory. This allows to test for each plane of each GPU independently. v10: * Move blit and fill functions back in drm_panic (Thomas Zimmermann). * Simplify the text drawing functions. * Use kmsg_dumper instead of panic_notifier (Sima). * Use spinlock_irqsave/restore (John Ogness) v11: * Use macro instead of inline functions for drm_panic_lock/unlock (John Ogness) Best regards, Daniel Vetter (1): drm/panic: Add drm panic locking Jocelyn Falempe (8): drm/panic: Add a drm panic handler drm/panic: Add support for color format conversion drm/panic: Add debugfs entry to test without triggering panic. drm/fb_dma: Add generic get_scanout_buffer() for drm_panic drm/simpledrm: Add drm_panic support drm/mgag200: Add drm_panic support drm/imx: Add drm_panic support drm/ast: Add drm_panic support Documentation/gpu/drm-kms.rst| 12 + drivers/gpu/drm/Kconfig | 32 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/ast/ast_mode.c | 18 + drivers/gpu/drm/drm_atomic_helper.c | 4 + drivers/gpu/drm/drm_drv.c| 5 + drivers/gpu/drm/drm_fb_dma_helper.c | 47 ++ drivers/gpu/drm/drm_panic.c | 581 +++ drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 11 +- drivers/gpu/drm/mgag200/mgag200_drv.h| 7 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 18 + drivers/gpu/drm/tiny/simpledrm.c | 16 + include/drm/drm_fb_dma_helper.h | 4 + include/drm/drm_mode_co
Re: [PATCH] vmwgfx: Create debugfs ttm_resource_manager entry only if needed
I just pushed it to drm-misc-fixes. Thanks for your reviews, -- Jocelyn On 14/03/2024 09:14, Jocelyn Falempe wrote: On 13/03/2024 18:57, Zack Rusin wrote: On Tue, Mar 12, 2024 at 5:36 AM Jocelyn Falempe wrote: [...] Thanks! That looks great. I can push it through drm-misc-fixes. Thanks, I think I only forget the "drm/" in the commit title, but yes you can push it with this small correction. Reviewed-by: Zack Rusin z Best regards,
Re: [PATCH] drm/ast: Fix soft lockup
Hi, Thanks for your patch. I'm wondering how you can trigger this infinite loop ? Also this looks like a simple fix, that can be easily backported, so I'm adding stable in Cc. If Thomas has no objections, I can push it to drm-misc-fixes. Reviewed-by: Jocelyn Falempe -- Jocelyn On 25/03/2024 04:35, Jammy Huang wrote: Avoid infinite-loop in ast_dp_set_on_off(). Signed-off-by: Jammy Huang --- drivers/gpu/drm/ast/ast_dp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c index ebb6d8ebd44e..1e9259416980 100644 --- a/drivers/gpu/drm/ast/ast_dp.c +++ b/drivers/gpu/drm/ast/ast_dp.c @@ -180,6 +180,7 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on) { struct ast_device *ast = to_ast_device(dev); u8 video_on_off = on; + u32 i = 0; // Video On/Off ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE3, (u8) ~AST_DP_VIDEO_ENABLE, on); @@ -192,6 +193,8 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on) ASTDP_MIRROR_VIDEO_ENABLE) != video_on_off) { // wait 1 ms mdelay(1); + if (++i > 200) + break; } } } base-commit: b0546776ad3f332e215cebc0b063ba4351971cca
Re: [PATCH v5 00/13] drm/ast: Detect connector status for VGA and SIL164
Hi, Thanks for those patches, that's really a good enhancement to the ast driver, and fits well with the BMC virtual connector. I've reviewed the whole series, and it looks good to me. Reviewed-by: Jocelyn Falempe -- Jocelyn On 20/03/2024 10:34, Thomas Zimmermann wrote: Detect the status of the VGA or SIL164 conenctor by polling the DDC channel. Update the status at runtime and flip the BMC status as well. Patches 1 adds a missing include statement that will be required to make later patches compile. Patches 2 and 3 simplify the VGA and SIL164 connectors, such that no additional data structure is required. These patches have been reviewed before as part of the series at [1]. Patches 4 to 10 improve the I2C code that is used to retrieve the monitor's EDID data. It's now fully managed, it acquires the necessary lock automatically and it is called DDC, which better represents its purpose than I2C. Patches 11 to 13 finally implement polling. Patch 11 updates ast's EDID code to be up-to-date. The helper drm_connector_get_modes() reads the EDID via DDC and updates the property. No driver code is required. Patch 12 uses a similar pattern to detect the presence of the monitor and sets the connector status accordingly. As polling also needs to be cleaned up, patch 13 adds the necessary helpers to do so. Tested on AST2500 hardware and BMC output. The BMC connector now also flips its status correctly at runtime. [1] https://patchwork.freedesktop.org/series/104547/ v5: - share implementation in drm_connector_helper_detect_ctx() (Maxime) - test for DDC presence with drm_probe_ddc() (Maxime, Jani) - perform managed cleanup of poll thread Thomas Zimmermann (13): drm/ast: Include where necessary drm/ast: Fail probing if DDC channel could not be initialized drm/ast: Remove struct ast_{vga,sil165}_connector drm/ast: Allocate instance of struct ast_i2c_chan with managed helpers drm/ast: Move DDC code to ast_ddc.{c,h} drm/ast: Rename struct ast_i2c_chan to struct ast_ddc drm/ast: Pass AST device to ast_ddc_create() drm/ast: Store AST device in struct ast_ddc drm/ast: Rename struct i2c_algo_bit_data callbacks and their parameters drm/ast: Acquire I/O-register lock in DDC code drm/ast: Use drm_connector_helper_get_modes() drm/ast: Implement polling for VGA and SIL164 connectors drm/ast: Automatically clean up poll helper drivers/gpu/drm/ast/Makefile | 10 +- drivers/gpu/drm/ast/{ast_i2c.c => ast_ddc.c} | 120 +-- drivers/gpu/drm/ast/ast_ddc.h| 20 +++ drivers/gpu/drm/ast/ast_drv.c| 1 + drivers/gpu/drm/ast/ast_drv.h| 39 + drivers/gpu/drm/ast/ast_main.c | 1 + drivers/gpu/drm/ast/ast_mode.c | 147 +-- drivers/gpu/drm/drm_probe_helper.c | 56 +++ include/drm/drm_probe_helper.h | 5 + 9 files changed, 206 insertions(+), 193 deletions(-) rename drivers/gpu/drm/ast/{ast_i2c.c => ast_ddc.c} (54%) create mode 100644 drivers/gpu/drm/ast/ast_ddc.h
Re: [PATCH 12/43] drm/mgag200: Use fbdev-shmem
On 18/03/2024 08:56, Thomas Zimmermann wrote: Hi Am 13.03.24 um 15:03 schrieb Jocelyn Falempe: Hi, Thanks, it looks good to me. Reviewed-by: Jocelyn Falempe Thanks. Do you still have access to that broken realtime system? I wonder if this patch makes a difference, as there's now one large memcpy() less. Hi, Sure, I'll do some latency tests if I can get access to that server again. Best regards, Best regards Thomas
Re: [PATCH v10 1/9] drm/panic: Add drm panic locking
On 15/03/2024 16:28, John Ogness wrote: On 2024-03-15, Jocelyn Falempe wrote: +static inline int drm_panic_trylock(struct drm_device *dev, unsigned long *flags) +{ + return raw_spin_trylock_irqsave(>mode_config.panic_lock, *flags); +} [...] +static inline unsigned long drm_panic_lock(struct drm_device *dev) +{ + unsigned long flags; + + raw_spin_lock_irqsave(>mode_config.panic_lock, flags); + return flags; +} I find it a bit strange that @flags is passed as an argument in the trylock variant but returned in the lock variant. I would pass it as an argument in both variants. Just a suggestion. Or maybe I can do it as a macro instead of a static inline function, to have the same semantic as raw_spin_lock_irqsave() which modify the flags, without passing it as pointer, because it's also a macro. https://elixir.bootlin.com/linux/latest/source/include/linux/spinlock.h#L296 #define drm_panic_lock(dev, flags) \ raw_spin_lock_irqsave(>mode_config.panic_lock, flags); John Ogness -- Jocelyn
[PATCH v10 9/9] drm/ast: Add drm_panic support
Add support for the drm_panic module, which displays a message to the screen when a kernel panic occurs. v7 * Use drm_for_each_primary_visible_plane() v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/ast/ast_mode.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index a718646a66b8..49f2d8bd3377 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -700,12 +701,29 @@ static void ast_primary_plane_helper_atomic_disable(struct drm_plane *plane, ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x1, 0xdf, 0x20); } +static int ast_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct ast_plane *ast_plane = to_ast_plane(plane); + + if (plane->state && plane->state->fb && ast_plane->vaddr) { + sb->format = plane->state->fb->format; + sb->width = plane->state->fb->width; + sb->height = plane->state->fb->height; + sb->pitch = plane->state->fb->pitches[0]; + iosys_map_set_vaddr_iomem(>map, ast_plane->vaddr); + return 0; + } + return -ENODEV; +} + static const struct drm_plane_helper_funcs ast_primary_plane_helper_funcs = { DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, .atomic_check = ast_primary_plane_helper_atomic_check, .atomic_update = ast_primary_plane_helper_atomic_update, .atomic_enable = ast_primary_plane_helper_atomic_enable, .atomic_disable = ast_primary_plane_helper_atomic_disable, + .get_scanout_buffer = ast_primary_plane_helper_get_scanout_buffer, }; static const struct drm_plane_funcs ast_primary_plane_funcs = { -- 2.44.0
[PATCH v10 4/9] drm/panic: Add debugfs entry to test without triggering panic.
Add a debugfs file, so you can test drm_panic without freezing your machine. This is unsafe, and should be enabled only for developer or tester. To display the drm_panic screen on the device 0: echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 v9: * Create a debugfs file for each plane in the device's debugfs directory. This allows to test for each plane of each GPU independently. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/Kconfig | 9 drivers/gpu/drm/drm_panic.c | 43 - 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index f07ca38d3f98..6e41fbd16b3d 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR depends on DRM_PANIC default 0x00 +config DRM_PANIC_DEBUG + bool "Add a debug fs entry to trigger drm_panic" + depends on DRM_PANIC && DEBUG_FS + help + Add dri/[device]/drm_panic_plane_x in the kernel debugfs, to force the + panic handler to write the panic message to this plane scanout buffer. + This is unsafe and should not be enabled on a production build. + If in doubt, say "N". + config DRM_DEBUG_DP_MST_TOPOLOGY_REFS bool "Enable refcount backtrace history in the DP MST helpers" depends on STACKTRACE_SUPPORT diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 2c3b2fffe659..cd2c7c2d1c59 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -493,6 +493,45 @@ static void drm_panic(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason) draw_panic_plane(plane); } + +/* + * DEBUG FS, This is currently unsafe. + * Create one file per plane, so it's possible to debug one plane at a time. + */ +#ifdef CONFIG_DRM_PANIC_DEBUG +#include + +static ssize_t debugfs_trigger_write(struct file *file, const char __user *user_buf, +size_t count, loff_t *ppos) +{ + bool run; + + if (kstrtobool_from_user(user_buf, count, ) == 0 && run) { + struct drm_plane *plane = file->private_data; + + draw_panic_plane(plane); + } + return count; +} + +static const struct file_operations dbg_drm_panic_ops = { + .owner = THIS_MODULE, + .write = debugfs_trigger_write, + .open = simple_open, +}; + +static void debugfs_register_plane(struct drm_plane *plane, int index) +{ + char fname[32]; + + snprintf(fname, 32, "drm_panic_plane_%d", index); + debugfs_create_file(fname, 0200, plane->dev->debugfs_root, + plane, _drm_panic_ops); +} +#else +static void debugfs_register_plane(struct drm_plane *plane, int index) {} +#endif /* CONFIG_DRM_PANIC_DEBUG */ + /** * drm_panic_register() - Initialize DRM panic for a device * @dev: the drm device on which the panic screen will be displayed. @@ -512,8 +551,10 @@ void drm_panic_register(struct drm_device *dev) plane->kmsg_panic.max_reason = KMSG_DUMP_PANIC; if (kmsg_dump_register(>kmsg_panic)) drm_warn(dev, "Failed to register panic handler\n"); - else + else { + debugfs_register_plane(plane, registered_plane); registered_plane++; + } } if (registered_plane) drm_info(dev, "Registered %d planes with drm panic\n", registered_plane); -- 2.44.0
[PATCH v10 8/9] drm/imx: Add drm_panic support
Add support for the drm_panic module, which displays a user-friendly message to the screen when a kernel panic occurs. v7: * use drm_panic_gem_get_scanout_buffer() helper v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c index dade8b59feae..3e21d2a1a124 100644 --- a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c @@ -772,6 +772,12 @@ static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = { .atomic_disable = ipu_plane_atomic_disable, .atomic_update = ipu_plane_atomic_update, }; +static const struct drm_plane_helper_funcs ipu_primary_plane_helper_funcs = { + .atomic_check = ipu_plane_atomic_check, + .atomic_disable = ipu_plane_atomic_disable, + .atomic_update = ipu_plane_atomic_update, + .get_scanout_buffer = drm_panic_gem_get_scanout_buffer, +}; bool ipu_plane_atomic_update_pending(struct drm_plane *plane) { @@ -916,7 +922,10 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, ipu_plane->dma = dma; ipu_plane->dp_flow = dp; - drm_plane_helper_add(_plane->base, _plane_helper_funcs); + if (type == DRM_PLANE_TYPE_PRIMARY) + drm_plane_helper_add(_plane->base, _primary_plane_helper_funcs); + else + drm_plane_helper_add(_plane->base, _plane_helper_funcs); if (dp == IPU_DP_FLOW_SYNC_BG || dp == IPU_DP_FLOW_SYNC_FG) ret = drm_plane_create_zpos_property(_plane->base, zpos, 0, -- 2.44.0
[PATCH v10 6/9] drm/simpledrm: Add drm_panic support
Add support for the drm_panic module, which displays a user-friendly message to the screen when a kernel panic occurs. v8: * Replace get_scanout_buffer() with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/tiny/simpledrm.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 7ce1c4617675..f498665be8c4 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #define DRIVER_NAME"simpledrm" @@ -671,11 +672,26 @@ static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plan drm_dev_exit(idx); } +static int simpledrm_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb) +{ + struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev); + + sb->width = sdev->mode.hdisplay; + sb->height = sdev->mode.vdisplay; + sb->format = sdev->format; + sb->pitch = sdev->pitch; + sb->map = sdev->screen_base; + + return 0; +} + static const struct drm_plane_helper_funcs simpledrm_primary_plane_helper_funcs = { DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, .atomic_check = simpledrm_primary_plane_helper_atomic_check, .atomic_update = simpledrm_primary_plane_helper_atomic_update, .atomic_disable = simpledrm_primary_plane_helper_atomic_disable, + .get_scanout_buffer = simpledrm_primary_plane_helper_get_scanout_buffer, }; static const struct drm_plane_funcs simpledrm_primary_plane_funcs = { -- 2.44.0
[PATCH v10 7/9] drm/mgag200: Add drm_panic support
Add support for the drm_panic module, which displays a message to the screen when a kernel panic occurs. v5: * Also check that the plane is visible and primary. (Thomas Zimmermann) v7: * use drm_for_each_primary_visible_plane() v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/mgag200/mgag200_drv.h | 7 ++- drivers/gpu/drm/mgag200/mgag200_mode.c | 18 ++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 765e49fd8911..58a0e62eaf18 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -366,6 +366,7 @@ struct drm_crtc_state; struct drm_display_mode; struct drm_plane; struct drm_atomic_state; +struct drm_scanout_buffer; extern const uint32_t mgag200_primary_plane_formats[]; extern const size_t mgag200_primary_plane_formats_size; @@ -379,12 +380,16 @@ void mgag200_primary_plane_helper_atomic_enable(struct drm_plane *plane, struct drm_atomic_state *state); void mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, struct drm_atomic_state *old_state); +int mgag200_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb); + #define MGAG200_PRIMARY_PLANE_HELPER_FUNCS \ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, \ .atomic_check = mgag200_primary_plane_helper_atomic_check, \ .atomic_update = mgag200_primary_plane_helper_atomic_update, \ .atomic_enable = mgag200_primary_plane_helper_atomic_enable, \ - .atomic_disable = mgag200_primary_plane_helper_atomic_disable + .atomic_disable = mgag200_primary_plane_helper_atomic_disable, \ + .get_scanout_buffer = mgag200_primary_plane_helper_get_scanout_buffer #define MGAG200_PRIMARY_PLANE_FUNCS \ .update_plane = drm_atomic_helper_update_plane, \ diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index e17cb4c5f774..8f368ecca4d4 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "mgag200_drv.h" @@ -546,6 +547,23 @@ void mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, msleep(20); } +int mgag200_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct mga_device *mdev = to_mga_device(plane->dev); + struct iosys_map map = IOSYS_MAP_INIT_VADDR_IOMEM(mdev->vram); + + if (plane->state && plane->state->fb) { + sb->format = plane->state->fb->format; + sb->width = plane->state->fb->width; + sb->height = plane->state->fb->height; + sb->pitch = plane->state->fb->pitches[0]; + sb->map = map; + return 0; + } + return -ENODEV; +} + /* * CRTC */ -- 2.44.0
[PATCH v10 5/9] drm/fb_dma: Add generic get_scanout_buffer() for drm_panic
This was initialy done for imx6, but should work on most drivers using drm_fb_dma_helper. v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * go back to get_scanout_buffer() * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_dma_helper.c | 47 + include/drm/drm_fb_dma_helper.h | 4 +++ 2 files changed, 51 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index 3b535ad1b07c..010327069ad4 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -148,3 +149,49 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, } } EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); + +#if defined(CONFIG_DRM_PANIC) +/** + * @plane: DRM primary plane + * @drm_scanout_buffer: scanout buffer for the panic handler + * Returns: 0 or negative error code + * + * Generic get_scanout_buffer() implementation, for drivers that uses the + * drm_fb_dma_helper. + */ +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb) +{ + struct drm_gem_dma_object *dma_obj; + struct drm_framebuffer *fb; + + fb = plane->state->fb; + /* Only support linear modifier */ + if (fb->modifier != DRM_FORMAT_MOD_LINEAR) + return -ENODEV; + + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); + + /* Buffer should be accessible from the CPU */ + if (dma_obj->base.import_attach) + return -ENODEV; + + /* Buffer should be already mapped to CPU */ + if (!dma_obj->vaddr) + return -ENODEV; + + iosys_map_set_vaddr(>map, dma_obj->vaddr); + sb->format = fb->format; + sb->height = fb->height; + sb->width = fb->width; + sb->pitch = fb->pitches[0]; + return 0; +} +#else +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb) +{ + return -ENODEV; +} +#endif +EXPORT_SYMBOL(drm_panic_gem_get_scanout_buffer); diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h index d5e036c57801..61f24c2aba2f 100644 --- a/include/drm/drm_fb_dma_helper.h +++ b/include/drm/drm_fb_dma_helper.h @@ -7,6 +7,7 @@ struct drm_device; struct drm_framebuffer; struct drm_plane_state; +struct drm_scanout_buffer; struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb, unsigned int plane); @@ -19,5 +20,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, struct drm_plane_state *old_state, struct drm_plane_state *state); +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb); + #endif -- 2.44.0
[PATCH v10 3/9] drm/panic: Add support for color format conversion
Add support for the following formats: DRM_FORMAT_RGB565 DRM_FORMAT_RGBA5551 DRM_FORMAT_XRGB1555 DRM_FORMAT_ARGB1555 DRM_FORMAT_RGB888 DRM_FORMAT_XRGB DRM_FORMAT_ARGB DRM_FORMAT_XBGR DRM_FORMAT_XRGB2101010 DRM_FORMAT_ARGB2101010 v10: * move and simplify the functions from the drm format helper to drm_panic Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_panic.c | 272 ++-- 1 file changed, 262 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index faa64961c0f2..2c3b2fffe659 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -81,15 +81,155 @@ static const struct drm_panic_line logo[] = { PANIC_LINE(" \\___)=(___/"), }; -static void drm_panic_fill32(struct iosys_map *map, unsigned int pitch, +/* + * Color conversion + */ + +static u16 convert_xrgb_to_rgb565(u32 pix) +{ + return ((pix & 0x00F8) >> 8) | + ((pix & 0xFC00) >> 5) | + ((pix & 0x00F8) >> 3); +} + +static u16 convert_xrgb_to_rgba5551(u32 pix) +{ + return ((pix & 0x00f8) >> 8) | + ((pix & 0xf800) >> 5) | + ((pix & 0x00f8) >> 2) | + BIT(0); /* set alpha bit */ +} + +static u16 convert_xrgb_to_xrgb1555(u32 pix) +{ + return ((pix & 0x00f8) >> 9) | + ((pix & 0xf800) >> 6) | + ((pix & 0x00f8) >> 3); +} + +static u16 convert_xrgb_to_argb1555(u32 pix) +{ + return BIT(15) | /* set alpha bit */ + ((pix & 0x00f8) >> 9) | + ((pix & 0xf800) >> 6) | + ((pix & 0x00f8) >> 3); +} + +static u32 convert_xrgb_to_argb(u32 pix) +{ + return pix | GENMASK(31, 24); /* fill alpha bits */ +} + +static u32 convert_xrgb_to_xbgr(u32 pix) +{ + return ((pix & 0x00ff) >> 16) << 0 | + ((pix & 0xff00) >> 8) << 8 | + ((pix & 0x00ff) >> 0) << 16 | + ((pix & 0xff00) >> 24) << 24; +} + +static u32 convert_xrgb_to_abgr(u32 pix) +{ + return ((pix & 0x00ff) >> 16) << 0 | + ((pix & 0xff00) >> 8) << 8 | + ((pix & 0x00ff) >> 0) << 16 | + GENMASK(31, 24); /* fill alpha bits */ +} + +static u32 convert_xrgb_to_xrgb2101010(u32 pix) +{ + pix = ((pix & 0x00FF) << 2) | + ((pix & 0xFF00) << 4) | + ((pix & 0x00FF) << 6); + return pix | ((pix >> 8) & 0x00300C03); +} + +static u32 convert_xrgb_to_argb2101010(u32 pix) +{ + pix = ((pix & 0x00FF) << 2) | + ((pix & 0xFF00) << 4) | + ((pix & 0x00FF) << 6); + return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); +} + +/* + * convert_from_xrgb - convert one pixel from xrgb to the desired format + * @color: input color, in xrgb format + * @format: output format + * + * Returns: + * Color in the format specified, casted to u32. + * Or 0 if the format is not supported. + */ +static u32 convert_from_xrgb(u32 color, u32 format) +{ + switch (format) { + case DRM_FORMAT_RGB565: + return convert_xrgb_to_rgb565(color); + case DRM_FORMAT_RGBA5551: + return convert_xrgb_to_rgba5551(color); + case DRM_FORMAT_XRGB1555: + return convert_xrgb_to_xrgb1555(color); + case DRM_FORMAT_ARGB1555: + return convert_xrgb_to_argb1555(color); + case DRM_FORMAT_RGB888: + case DRM_FORMAT_XRGB: + return color; + case DRM_FORMAT_ARGB: + return convert_xrgb_to_argb(color); + case DRM_FORMAT_XBGR: + return convert_xrgb_to_xbgr(color); + case DRM_FORMAT_ABGR: + return convert_xrgb_to_abgr(color); + case DRM_FORMAT_XRGB2101010: + return convert_xrgb_to_xrgb2101010(color); + case DRM_FORMAT_ARGB2101010: + return convert_xrgb_to_argb2101010(color); + default: + WARN_ONCE(1, "Can't convert to %p4cc\n", ); + return 0; + } +} + +/* + * Blit & Fill + */ +static void drm_panic_blit16(struct iosys_map *dmap, unsigned int dpitch, +const u8 *sbuf8, unsigned int spitch, unsigned int height, unsigned int width, -u32 color) +u16 fg16, u16 bg16) { unsigned int y, x; + u16 val16; -
[PATCH v10 2/9] drm/panic: Add a drm panic handler
This module displays a user friendly message when a kernel panic occurs. It currently doesn't contain any debug information, but that can be added later. v2 * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) * Add the panic reason to the panic message (Nerdopolis) * Add an exclamation mark (Nerdopolis) v3 * Rework the drawing functions, to write the pixels line by line and to use the drm conversion helper to support other formats. (Thomas Zimmermann) v4 * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) * Add foreground/background color config option * Fix the bottom lines not painted if the framebuffer height is not a multiple of the font height. * Automatically register the device to drm_panic, if the function get_scanout_buffer exists. (Thomas Zimmermann) v5 * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). v6 * Fix sparse warning for panic_msg and logo. v7 * Add select DRM_KMS_HELPER for the color conversion functions. v8 * Register directly each plane to the panic notifier (Sima) * Add raw_spinlock to properly handle concurrency (Sima) * Register plane instead of device, to avoid looping through plane list, and simplify code. * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) * Removed the draw_pixel_xy() API, will see later if it can be added back. v9 * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) v10 * Move blit and fill functions back in drm_panic (Thomas Zimmermann). * Simplify the text drawing functions. * Use kmsg_dumper instead of panic_notifier (Sima). Signed-off-by: Jocelyn Falempe --- Documentation/gpu/drm-kms.rst| 12 + drivers/gpu/drm/Kconfig | 23 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_drv.c| 4 + drivers/gpu/drm/drm_panic.c | 288 +++ drivers/gpu/drm/drm_plane.c | 1 + include/drm/drm_modeset_helper_vtables.h | 37 +++ include/drm/drm_panic.h | 52 include/drm/drm_plane.h | 6 + 9 files changed, 424 insertions(+) create mode 100644 drivers/gpu/drm/drm_panic.c diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 13d3627d8bc0..b64334661aeb 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -398,6 +398,18 @@ Plane Damage Tracking Functions Reference .. kernel-doc:: include/drm/drm_damage_helper.h :internal: +Plane Panic Feature +--- + +.. kernel-doc:: drivers/gpu/drm/drm_panic.c + :doc: overview + +Plane Panic Functions Reference +--- + +.. kernel-doc:: drivers/gpu/drm/drm_panic.c + :export: + Display Modes Function Reference diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 16029435b750..f07ca38d3f98 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -102,6 +102,29 @@ config DRM_KMS_HELPER help CRTC helpers for KMS drivers. +config DRM_PANIC + bool "Display a user-friendly message when a kernel panic occurs" + depends on DRM && !FRAMEBUFFER_CONSOLE + select DRM_KMS_HELPER + select FONT_SUPPORT + help + Enable a drm panic handler, which will display a user-friendly message + when a kernel panic occurs. It's useful when using a user-space + console instead of fbcon. + It will only work if your graphic driver supports this feature. + To support Hi-DPI Display, you can enable bigger fonts like + FONT_TER16x32 + +config DRM_PANIC_FOREGROUND_COLOR + hex "Drm panic screen foreground color, in RGB" + depends on DRM_PANIC + default 0xff + +config DRM_PANIC_BACKGROUND_COLOR + hex "Drm panic screen background color, in RGB" + depends on DRM_PANIC + default 0x00 + config DRM_DEBUG_DP_MST_TOPOLOGY_REFS bool "Enable refcount backtrace history in the DP MST helpers" depends on STACKTRACE_SUPPORT diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index a73c04d2d7a3..f9ca4f8fa6c5 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -88,6 +88,7 @@ drm-$(CONFIG_DRM_P
[PATCH v10 1/9] drm/panic: Add drm panic locking
From: Daniel Vetter Rough sketch for the locking of drm panic printing code. The upshot of this approach is that we can pretty much entirely rely on the atomic commit flow, with the pair of raw_spin_lock/unlock providing any barriers we need, without having to create really big critical sections in code. This also avoids the need that drivers must explicitly update the panic handler state, which they might forget to do, or not do consistently, and then we blow up in the worst possible times. It is somewhat racy against a concurrent atomic update, and we might write into a buffer which the hardware will never display. But there's fundamentally no way to avoid that - if we do the panic state update explicitly after writing to the hardware, we might instead write to an old buffer that the user will barely ever see. Note that an rcu protected deference of plane->state would give us the the same guarantees, but it has the downside that we then need to protect the plane state freeing functions with call_rcu too. Which would very widely impact a lot of code and therefore doesn't seem worth the complexity compared to a raw spinlock with very tiny critical sections. Plus rcu cannot be used to protect access to peek/poke registers anyway, so we'd still need it for those cases. Peek/poke registers for vram access (or a gart pte reserved just for panic code) are also the reason I've gone with a per-device and not per-plane spinlock, since usually these things are global for the entire display. Going with per-plane locks would mean drivers for such hardware would need additional locks, which we don't want, since it deviates from the per-console takeoverlocks design. Longer term it might be useful if the panic notifiers grow a bit more structure than just the absolute bare EXPORT_SYMBOL(panic_notifier_list) - somewhat aside, why is that not EXPORT_SYMBOL_GPL ... If panic notifiers would be more like console drivers with proper register/unregister interfaces we could perhaps reuse the very fancy console lock with all it's check and takeover semantics that John Ogness is developing to fix the console_lock mess. But for the initial cut of a drm panic printing support I don't think we need that, because the critical sections are extremely small and only happen once per display refresh. So generally just 60 tiny locked sections per second, which is nothing compared to a serial console running a 115kbaud doing really slow mmio writes for each byte. So for now the raw spintrylock in drm panic notifier callback should be good enough. Another benefit of making panic notifiers more like full blown consoles (that are used in panics only) would be that we get the two stage design, where first all the safe outputs are used. And then the dangerous takeover tricks are deployed (where for display drivers we also might try to intercept any in-flight display buffer flips, which if we race and misprogram fifos and watermarks can hang the memory controller on some hw). For context the actual implementation on the drm side is by Jocelyn and this patch is meant to be combined with the overall approach in v7 (v8 is a bit less flexible, which I think is the wrong direction): https://lore.kernel.org/dri-devel/20240104160301.185915-1-jfale...@redhat.com/ Note that the locking is very much not correct there, hence this separate rfc. v2: - fix authorship, this was all my typing - some typo oopsies - link to the drm panic work by Jocelyn for context v10: - Use spinlock_irqsave/restore (John Ogness) Signed-off-by: Daniel Vetter Cc: Jocelyn Falempe Cc: Andrew Morton Cc: "Peter Zijlstra (Intel)" Cc: Lukas Wunner Cc: Petr Mladek Cc: Steven Rostedt Cc: John Ogness Cc: Sergey Senozhatsky Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++ drivers/gpu/drm/drm_drv.c | 1 + include/drm/drm_mode_config.h | 10 +++ include/drm/drm_panic.h | 107 4 files changed, 122 insertions(+) create mode 100644 include/drm/drm_panic.h diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 39ef0a6addeb..89dbf0ab1ba8 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -3016,6 +3017,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall) { int i, ret; + unsigned long flags; struct drm_connector *connector; struct drm_connector_state *old_conn_state, *new_conn_state; struct drm_crtc *crtc; @@ -3099,6 +3101,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, } } + flags = drm_panic_lock(state->dev); for_each_oldne
[PATCH v10 0/9] drm/panic: Add a drm panic handler
This introduces a new drm panic handler, which displays a message when a panic occurs. So when fbcon is disabled, you can still see a kernel panic. This is one of the missing feature, when disabling VT/fbcon in the kernel: https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/ Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel. This is a proof of concept, and works with simpledrm, mgag200, ast, and imx. To test it, make sure you're using one of the supported driver, and trigger a panic: echo c > /proc/sysrq-trigger or you can enable CONFIG_DRM_PANIC_DEBUG and echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 v2: * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) * Add the panic reason to the panic message (Nerdopolis) * Add an exclamation mark (Nerdopolis) v3: * Rework the drawing functions, to write the pixels line by line and to use the drm conversion helper to support other formats. (Thomas Zimmermann) v4: * Fully support all simpledrm formats using drm conversion helpers * Rename dpanic_* to drm_panic_*, and have more coherent function name. (Thomas Zimmermann) * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) * Add foreground/background color config option * Fix the bottom lines not painted if the framebuffer height is not a multiple of the font height. * Automatically register the driver to drm_panic, if the function get_scanout_buffer() exists. (Thomas Zimmermann) * Add mgag200 support. v5: * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. (Thomas Zimmermann) * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). * Add ast support. * Add experimental imx/ipuv3 support, to test on an ARM hw. (Maxime Ripard) v6: * Fix sparse and __le32 warnings * Drop the IMX/IPUV3 experiment, it was just to show that it works also on ARM devices. v7: * Add a check to see if the 4cc format is supported by drm_panic. * Add a drm/plane helper to loop over all visible primary buffer, simplifying the get_scanout_buffer implementations * Add a generic implementation for drivers that uses drm_fb_dma. (Maxime Ripard) * Add back the IMX/IPUV3 support, and use the generic implementation. (Maxime Ripard) v8: * Directly register each plane to the panic notifier (Sima) * Replace get_scanout_buffer() with set_scanout_buffer() to simplify the locking. (Thomas Zimmermann) * Add a debugfs entry, to trigger the drm_panic without a real panic (Sima) * Fix the drm_panic Documentation, and include it in drm-kms.rst v9: * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) * Create a debugfs file for each plane in the device's debugfs directory. This allows to test for each plane of each GPU independently. v10: * Move blit and fill functions back in drm_panic (Thomas Zimmermann). * Simplify the text drawing functions. * Use kmsg_dumper instead of panic_notifier (Sima). * Use spinlock_irqsave/restore (John Ogness) Daniel Vetter (1): drm/panic: Add drm panic locking Jocelyn Falempe (8): drm/panic: Add a drm panic handler drm/panic: Add support for color format conversion drm/panic: Add debugfs entry to test without triggering panic. drm/fb_dma: Add generic get_scanout_buffer() for drm_panic drm/simpledrm: Add drm_panic support drm/mgag200: Add drm_panic support drm/imx: Add drm_panic support drm/ast: Add drm_panic support Documentation/gpu/drm-kms.rst| 12 + drivers/gpu/drm/Kconfig | 32 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/ast/ast_mode.c | 18 + drivers/gpu/drm/drm_atomic_helper.c | 4 + drivers/gpu/drm/drm_drv.c| 5 + drivers/gpu/drm/drm_fb_dma_helper.c | 47 ++ drivers/gpu/drm/drm_panic.c | 581 +++ drivers/gpu/drm/drm_plane.c | 1 + drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 11 +- drivers/gpu/drm/mgag200/mgag200_drv.h| 7 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 18 + drivers/gpu/drm/tiny/simpledrm.c | 16 + include/drm/drm_fb_dma_helper.h | 4 + include/drm/drm_mode_config.h| 10 + include/drm/drm_modeset_helper_vtables.h | 37 ++ include/drm/drm_panic.h | 159 +++ include/drm/drm_plane.h | 6 + 18 files changed, 967 insertions(+), 2 del
Re: [PATCH] vmwgfx: Create debugfs ttm_resource_manager entry only if needed
On 13/03/2024 18:57, Zack Rusin wrote: On Tue, Mar 12, 2024 at 5:36 AM Jocelyn Falempe wrote: [...] Thanks! That looks great. I can push it through drm-misc-fixes. Thanks, I think I only forget the "drm/" in the commit title, but yes you can push it with this small correction. Reviewed-by: Zack Rusin z Best regards, -- Jocelyn
Re: [PATCH 12/43] drm/mgag200: Use fbdev-shmem
Hi, Thanks, it looks good to me. Reviewed-by: Jocelyn Falempe -- Jocelyn On 12/03/2024 16:45, Thomas Zimmermann wrote: Implement fbdev emulation with fbdev-shmem. Avoids the overhead of fbdev-generic's additional shadow buffering. No functional changes. Signed-off-by: Thomas Zimmermann Cc: Dave Airlie Cc: Thomas Zimmermann Cc: Jocelyn Falempe --- drivers/gpu/drm/mgag200/mgag200_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 573dbe256aa8b..65f2ed18b31c5 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include #include #include @@ -285,7 +285,7 @@ mgag200_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) * FIXME: A 24-bit color depth does not work with 24 bpp on * G200ER. Force 32 bpp. */ - drm_fbdev_generic_setup(dev, 32); + drm_fbdev_shmem_setup(dev, 32); return 0; }
Re: [PATCH 09/43] drm/ast: Use fbdev-shmem
Hi, Thanks, it looks good to me. Reviewed-by: Jocelyn Falempe -- Jocelyn On 12/03/2024 16:45, Thomas Zimmermann wrote: Implement fbdev emulation with fbdev-shmem. Avoids the overhead of fbdev-generic's additional shadow buffering. No functional changes. Signed-off-by: Thomas Zimmermann Cc: Dave Airlie Cc: Thomas Zimmermann Cc: Jocelyn Falempe --- drivers/gpu/drm/ast/ast_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c index 90bcb1eb9cd94..4fcab4304e176 100644 --- a/drivers/gpu/drm/ast/ast_drv.c +++ b/drivers/gpu/drm/ast/ast_drv.c @@ -32,7 +32,7 @@ #include #include #include -#include +#include #include #include #include @@ -359,7 +359,7 @@ static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) return ret; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_shmem_setup(drm, 32); return 0; }
Re: [v9,5/9] drm/fb_dma: Add generic get_scanout_buffer() for drm_panic
On 12/03/2024 17:44, Sui Jingfeng wrote: Hi, I'm get attracted by your patch, so I want to comment. Please correct or ignore me if I say something wrong. On 2024/3/7 17:14, Jocelyn Falempe wrote: This was initialy done for imx6, but should work on most drivers using drm_fb_dma_helper. v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * go back to get_scanout_buffer() * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_dma_helper.c | 47 + include/drm/drm_fb_dma_helper.h | 4 +++ 2 files changed, 51 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index 3b535ad1b07c..010327069ad4 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -148,3 +149,49 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, } } EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); + +#if defined(CONFIG_DRM_PANIC) +/** + * @plane: DRM primary plane + * @drm_scanout_buffer: scanout buffer for the panic handler + * Returns: 0 or negative error code + * + * Generic get_scanout_buffer() implementation, for drivers that uses the + * drm_fb_dma_helper. + */ +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct drm_gem_dma_object *dma_obj; + struct drm_framebuffer *fb; + + fb = plane->state->fb; + /* Only support linear modifier */ + if (fb->modifier != DRM_FORMAT_MOD_LINEAR) + return -ENODEV; I think, the framebuffer format check clause here should be moved to the core layer. For example, move this check into thedrm_panic_is_format_supported() function. And update the drm_panic_is_format_supported() function to make it take 'struct drm_framebuffer *fb' as argument. Because this check is needed for all implement, not driver specific or implement specific. I'm unsure about this. I think for some drivers it might be easier to give a memory buffer different from the plane's drm_framebuffer, and do their own specific things to display it. So forcing the use of a drm_framebuffer will remove some flexibility. I know that some display controller support TILE format, but then you can try to trigger modeset to let the display controller using linear format to display. Or, you can also convert local linear buffer(with content pained) to the device specific TILED framebuffer, then feed TILED framebuffer to the display controller to scanout. This is something like reverse the process of resolve, but the convert program is running on the CPU. As panic is not performance critical, so it's possible. This maybe a bit more difficult, but the idea behind this is that the goal of this drm_panic_gem_get_scanout_buffer() function is just to get a buffer scanout-able. Other things beyond that (like format checking) can be moved to upper level(the caller of it). So you don't need to modify(update) the specific implement if one day you have some fancy idea implemented. Correct me if I'm wrong, but the tiled format are mostly hardware dependent, and I don't think we can have a generic implementation, that can cover multiple hardware. I want to add support for multi-planar format like YUV for drm_panic later, but for tiled buffer, I think it should be easier to deactivate tiling on the hardware itself. + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); + + /* Buffer should be accessible from the CPU */ + if (dma_obj->base.import_attach) + return -ENODEV; + + /* Buffer should be already mapped to CPU */ + if (!dma_obj->vaddr) + return -ENODEV; How about to call drm_gem_vmap_unlocked(dma_obj, >map); ? It is not safe to call it from panic context, because it takes locks: https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_gem.c#L1212 It may lockup the panic handler, and prevent other panic routine to run (like kdump). So it's better to call drm_gem_vmap_unlocked() when the driver prepares the buffer for the primary plane, than doing it from the panic handler. Best regards, -- Jocelyn + iosys_map_set_vaddr(>map, dma_obj->vaddr); + sb->format = fb->format; + sb->height = fb->height; + sb->width = fb->width; + sb->pitch = fb->pitches[0]; + return 0; +} +#else +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + return -ENODEV; +} +#endif +EXPORT_SYMBOL(drm_panic_gem_get_scanout_buffer); diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h index d5e036c57801..61f24c2aba2f 100644 --- a/include/drm/drm_fb_dma_helper.h +++ b/include/drm/drm_fb_dma_helper
Re: [PATCH v9 0/9] drm/panic: Add a drm panic handler
On 12/03/2024 16:16, Sui Jingfeng wrote: Hi, On 2024/3/7 17:14, Jocelyn Falempe wrote: This introduces a new drm panic handler, which displays a message when a panic occurs. So when fbcon is disabled, you can still see a kernel panic. This is one of the missing feature, when disabling VT/fbcon in the kernel: https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/ Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel. This is a proof of concept, and works with simpledrm, mgag200, ast, and imx. To test it, make sure you're using one of the supported driver, and trigger a panic: echo c > /proc/sysrq-trigger or you can enable CONFIG_DRM_PANIC_DEBUG and echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 Yes, the whole series works for me! I have tested with drm/loongson, Are you willing to add a implement for drm/loongson driver? If you do please add the fallowing code snippet info drm/loongson/lsdc_plane.c. Thanks you. Thanks for testing, and for enabling drm_panic on loongson. My plan is to have a first version of drm_panic merged before adding more drivers. You will be able to send this patch when drm_panic is merged in drm-misc-next, and I will review it. Best regards, -- Jocelyn static int lsdc_primary_plane_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *scanout) { struct drm_framebuffer *fb; if (!plane->state || !plane->state->fb) return -ENODEV; fb = plane->state->fb; if (fb->modifier != DRM_FORMAT_MOD_LINEAR) return -ENODEV; scanout->format = fb->format; scanout->width = fb->width; scanout->height = fb->height; scanout->pitch = fb->pitches[0]; return drm_gem_vmap_unlocked(fb->obj[0], >map); } hook this lsdc_primary_plane_get_scanout_buffer() up to the .get_scanout_buffer member of lsdc_primary_helper_funcs, and include the #include Thanks you in advance!
Re: [v9,3/9] drm/panic: Add a drm panic handler
On 12/03/2024 14:18, Sui Jingfeng wrote: Hi, [...] While applying you patch, there is new blank line at EOF reported, see below. This is not an issue, but I want to report this to you. Hi, Thanks, I will remove it for the next version. -- Jocelyn git am ../drm-panic-Add-a-drm-panic-handler.mbox Applying: drm/panic: Add drm panic locking Applying: drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill Applying: drm/panic: Add a drm panic handler .git/rebase-apply/patch:439: new blank line at EOF. + warning: 1 line adds whitespace errors. Applying: drm/panic: Add debugfs entry to test without triggering panic. Applying: drm/fb_dma: Add generic get_scanout_buffer() for drm_panic Applying: drm/simpledrm: Add drm_panic support Applying: drm/mgag200: Add drm_panic support Applying: drm/imx: Add drm_panic support Applying: drm/ast: Add drm_panic support
Re: [v2] drm/mgag200: Add a workaround for low-latency
On 12/03/2024 13:56, Sui Jingfeng wrote: Hi, Interesting patch! I know this patch already merged. While study this patch, I have a few questions. On 2024/2/8 17:51, Jocelyn Falempe wrote: We found a regression in v5.10 on real-time server, using the rt-kernel and the mgag200 driver. It's some really specialized workload, with <10us latency expectation on isolated core. After the v5.10, the real time tasks missed their <10us latency when something prints on the screen (fbcon or printk) The regression has been bisected to 2 commits: commit 0b34d58b6c32 ("drm/mgag200: Enable caching for SHMEM pages") commit 4862ffaec523 ("drm/mgag200: Move vmap out of commit tail") The first one changed the system memory framebuffer from Write-Combine to the default caching. Before the second commit, the mgag200 driver used to unmap the framebuffer after each frame, which implicitly does a cache flush. I don't know why it need to do a cache flush, where is the code. I'm asking because I want to study this technique. Generally speaking, X86-64 platform's default page caching is cached. And I think the cached mapping is fastest for software rendering. And the platform guaranteed the coherency for us, right? Because X86-64 platform(or CPU)'s write buffer is implemented on the top of cache? I'm means that for ARM(or other) CPU, when using Write-combine the data will has nothing to do with cache. Both regressions are fixed by this commit, which restore WC mapping for the framebuffer in system memory, and add a cache flush. So switch back to WC probably will decrease overall performance, I think. And the cache flush operation should not have a impact. Except X86-64's Write-Combine is different other platform's Write-Combine? Yes this patch is a bit weird. Usually you want your VRAM mapping to be Write-Combine. Here it also set the system memory framebuffer as Write-Combine. On most x86-64, Write Combine uses its own hardware buffer that is not in L1/L2/L3. So when it copies the framebuffer from WC system memory to VRAM, it doesn't involve the cache, and have less impact on latency for other tasks running on other CPU. Also I think the cache flush is important to flush those WC buffers, so when the next frame comes, it won't have to wait for the buffers to be copied to the slow VRAM. When running the latency tests, it's obvious that both are needed. This is how I understand it, but I may be wrong. -- Jocelyn This is only needed on x86_64, for low-latency workload, so the new kconfig DRM_MGAG200_IOBURST_WORKAROUND depends on PREEMPT_RT and X86. For more context, the whole thread can be found here [1] Signed-off-by: Jocelyn Falempe Link: https://lore.kernel.org/dri-devel/20231019135655.313759-1-jfale...@redhat.com/ # 1 Acked-by: Thomas Zimmermann
[PATCH] vmwgfx: Create debugfs ttm_resource_manager entry only if needed
The driver creates /sys/kernel/debug/dri/0/mob_ttm even when the corresponding ttm_resource_manager is not allocated. This leads to a crash when trying to read from this file. Add a check to create mob_ttm, system_mob_ttm, and gmr_ttm debug file only when the corresponding ttm_resource_manager is allocated. crash> bt PID: 3133409 TASK: 8fe4834a5000 CPU: 3COMMAND: "grep" #0 [b954506b3b20] machine_kexec at b2a6bec3 #1 [b954506b3b78] __crash_kexec at b2bb598a #2 [b954506b3c38] crash_kexec at b2bb68c1 #3 [b954506b3c50] oops_end at b2a2a9b1 #4 [b954506b3c70] no_context at b2a7e913 #5 [b954506b3cc8] __bad_area_nosemaphore at b2a7ec8c #6 [b954506b3d10] do_page_fault at b2a7f887 #7 [b954506b3d40] page_fault at b360116e [exception RIP: ttm_resource_manager_debug+0x11] RIP: c04afd11 RSP: b954506b3df0 RFLAGS: 00010246 RAX: 8fe41a6d1200 RBX: RCX: 0940 RDX: RSI: c04b4338 RDI: RBP: b954506b3e08 R8: 8fee3ffad000 R9: R10: 8fe41a76a000 R11: 0001 R12: R13: 0001 R14: 8fe5bb6f3900 R15: 8fe41a6d1200 ORIG_RAX: CS: 0010 SS: 0018 #8 [b954506b3e00] ttm_resource_manager_show at c04afde7 [ttm] #9 [b954506b3e30] seq_read at b2d8f9f3 RIP: 7f4c4eda8985 RSP: 7ffdbba9e9f8 RFLAGS: 0246 RAX: ffda RBX: 0037e000 RCX: 7f4c4eda8985 RDX: 0037e000 RSI: 7f4c41573000 RDI: 0003 RBP: 0037e000 R8: R9: 0037fe30 R10: R11: 0246 R12: 7f4c41573000 R13: 0003 R14: 7f4c41572010 R15: 0003 ORIG_RAX: CS: 0033 SS: 002b Signed-off-by: Jocelyn Falempe Fixes: af4a25bbe5e7 ("drm/vmwgfx: Add debugfs entries for various ttm resource managers") Cc: --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index d3e308fdfd5b..c7d90f96d16a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -1444,12 +1444,15 @@ static void vmw_debugfs_resource_managers_init(struct vmw_private *vmw) root, "system_ttm"); ttm_resource_manager_create_debugfs(ttm_manager_type(>bdev, TTM_PL_VRAM), root, "vram_ttm"); - ttm_resource_manager_create_debugfs(ttm_manager_type(>bdev, VMW_PL_GMR), - root, "gmr_ttm"); - ttm_resource_manager_create_debugfs(ttm_manager_type(>bdev, VMW_PL_MOB), - root, "mob_ttm"); - ttm_resource_manager_create_debugfs(ttm_manager_type(>bdev, VMW_PL_SYSTEM), - root, "system_mob_ttm"); + if (vmw->has_gmr) + ttm_resource_manager_create_debugfs(ttm_manager_type(>bdev, VMW_PL_GMR), + root, "gmr_ttm"); + if (vmw->has_mob) { + ttm_resource_manager_create_debugfs(ttm_manager_type(>bdev, VMW_PL_MOB), + root, "mob_ttm"); + ttm_resource_manager_create_debugfs(ttm_manager_type(>bdev, VMW_PL_SYSTEM), + root, "system_mob_ttm"); + } } static int vmwgfx_pm_notifier(struct notifier_block *nb, unsigned long val, base-commit: b33651a5c98dbd5a919219d8c129d0674ef74299 -- 2.44.0
Re: [PATCH v9 1/9] drm/panic: Add drm panic locking
On 07/03/2024 11:27, John Ogness wrote: On 2024-03-07, Jocelyn Falempe wrote: diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 39ef0a6addeb..c0bb91312fb2 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -3099,6 +3100,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, } } + drm_panic_lock(state->dev); for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { WARN_ON(plane->state != old_plane_state); @@ -3108,6 +3110,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, state->planes[i].state = old_plane_state; plane->state = new_plane_state; } + drm_panic_unlock(state->dev); Is there a reason irqsave/irqrestore variants are not used? Maybe this code path is too hot? This lock will be taken for each page flip, so typically at 60Hz (or maybe 144Hz for gamers). I don't know what are the performance impacts of the irqsave/irqrestore variant. By leaving interrupts enabled, there is the risk that a panic from within any interrupt handler may block the drm panic handler. The current design is that the panic handler will just use try_lock(), and if it can't take it, the panic screen will not be seen. The goal is to make sure drm_panic won't crash the machine and prevent kdump or other panic handler to run. So there is a very small race chance that the panic screen won't be seen, but that's ok. So I think in this case the drm panic handler shouldn't be blocked, as it only use try_lock(). Best regards, -- Jocelyn John Ogness
Re: [RFC] How to test panic handlers, without crashing the kernel
On 05/03/2024 18:50, Guilherme G. Piccoli wrote: On 05/03/2024 13:52, Jocelyn Falempe wrote: [...] Or maybe have two lists of panic notifiers, the safe and the destructive list. So in case of fake panic, we can only call the safe notifiers. I tried something like that: https://lore.kernel.org/lkml/20220427224924.592546-1-gpicc...@igalia.com/ There were many suggestions, a completely refactor of the idea (panic lists are not really seen as reliable things). Thanks for sharing this, so it's much more complex than what I though. Given that, I'm not really sure splitting in lists gonna fly; maybe restricting the test infrastructure to drm_panic plus some paths of panic would be enough for this debugfs interface, in principle? I mean, to unblock your work on the drm panic stuff. For drm_panic, I changed the way the debugfs is calling the drm_panic functions in the last version: https://patchwork.freedesktop.org/patch/581845/?series=122244=9 It doesn't use the panic notifier list, but create a file for each plane of each device directly. It allows to test the panic handler, not in a real panic condition, but that's still better than nothing. Cheers, Guilherme Best regards, -- Jocelyn
Re: [PATCH v9 2/9] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
Hi, On 07/03/2024 16:41, Thomas Zimmermann wrote: Am 07.03.24 um 15:08 schrieb Thomas Zimmermann: Hi Am 07.03.24 um 10:14 schrieb Jocelyn Falempe: This is needed for drm_panic, to draw the font, and fill the background color, in multiple color format. As you know, I'm not happy about this patch and the the changes do not reflect my easier s/easier/earlier review. These format helpers are supposed to serve all of DRM and not just the panic handler. I know that the current code isn't perfect, but the R1 support is a step backwards IMHO. I suggest to the drawing routines entirely within the panic-handler code and maybe try to sort out things later. IIRC that's how it was in earlier revisions of the patchset. Sorry I didn't change this patch in v8 and v9, I was focusing on locking and the overall design. I agree to move this back to drm_panic.c, so we can merge a first version of drm_panic. And when a generic format converter will be ready, I will do the patch to use it. Best regards, -- Jocelyn Best regards Thomas v5: * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. * Also add drm_fb_fill() to fill area with background color. v6: * fix __le32 conversion warning found by the kernel test bot Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_format_helper.c | 432 ++-- include/drm/drm_format_helper.h | 9 + 2 files changed, 360 insertions(+), 81 deletions(-) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index b1be458ed4dd..2d9646cefc4f 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -111,6 +111,153 @@ void drm_format_conv_state_release(struct drm_format_conv_state *state) } EXPORT_SYMBOL(drm_format_conv_state_release); +static __le16 drm_format_xrgb_to_rgb565(__le32 val32) +{ + u16 val16; + u32 pix; + + pix = le32_to_cpu(val32); + val16 = ((pix & 0x00F8) >> 8) | + ((pix & 0xFC00) >> 5) | + ((pix & 0x00F8) >> 3); + return cpu_to_le16(val16); +} + +static __le16 drm_format_xrgb_to_rgba5551(__le32 val32) +{ + u16 val16; + u32 pix; + + pix = le32_to_cpu(val32); + val16 = ((pix & 0x00f8) >> 8) | + ((pix & 0xf800) >> 5) | + ((pix & 0x00f8) >> 2) | + BIT(0); /* set alpha bit */ + return cpu_to_le16(val16); +} + +static __le16 drm_format_xrgb_to_xrgb1555(__le32 val32) +{ + u16 val16; + u32 pix; + + pix = le32_to_cpu(val32); + val16 = ((pix & 0x00f8) >> 9) | + ((pix & 0xf800) >> 6) | + ((pix & 0x00f8) >> 3); + return cpu_to_le16(val16); +} + +static __le16 drm_format_xrgb_to_argb1555(__le32 val32) +{ + u16 val16; + u32 pix; + + pix = le32_to_cpu(val32); + val16 = BIT(15) | /* set alpha bit */ + ((pix & 0x00f8) >> 9) | + ((pix & 0xf800) >> 6) | + ((pix & 0x00f8) >> 3); + return cpu_to_le16(val16); +} + +static __le32 drm_format_xrgb_to_argb(__le32 pix) +{ + u32 val32; + + val32 = le32_to_cpu(pix); + val32 |= GENMASK(31, 24); /* fill alpha bits */ + return cpu_to_le32(val32); +} + +static __le32 drm_format_xrgb_to_xbgr(__le32 pix) +{ + u32 val32; + + val32 = le32_to_cpu(pix); + val32 = ((val32 & 0x00ff) >> 16) << 0 | + ((val32 & 0xff00) >> 8) << 8 | + ((val32 & 0x00ff) >> 0) << 16 | + ((val32 & 0xff00) >> 24) << 24; + return cpu_to_le32(val32); +} + +static __le32 drm_format_xrgb_to_abgr(__le32 pix) +{ + u32 val32; + + val32 = le32_to_cpu(pix); + val32 = ((val32 & 0x00ff) >> 16) << 0 | + ((val32 & 0xff00) >> 8) << 8 | + ((val32 & 0x00ff) >> 0) << 16 | + GENMASK(31, 24); /* fill alpha bits */ + return cpu_to_le32(val32); +} + +static __le32 drm_format_xrgb_to_xrgb2101010(__le32 pix) +{ + u32 val32; + + val32 = le32_to_cpu(pix); + val32 = ((val32 & 0x00FF) << 2) | + ((val32 & 0xFF00) << 4) | + ((val32 & 0x00FF) << 6); + return cpu_to_le32(val32 | ((val32 >> 8) & 0x00300C03)); +} + +static __le32 drm_format_xrgb_to_argb2101010(__le32 pix) +{ + u32 val32; + + val32 = le32_to_cpu(pix); + val32 = ((val32 & 0x00FF) << 2) | + ((val32 & 0xFF00) << 4) | + ((val32 & 0x00FF) << 6); + val32 = GENMASK(31, 30) | /* set alpha bits */ + val32 | ((val32 >> 8) & 0x00300c03); + return cpu_to_le32(val32); +} + +/** + * drm_fb_convert_from_xrgb - convert one pixel from xrgb to the desired format
[PATCH v9 9/9] drm/ast: Add drm_panic support
Add support for the drm_panic module, which displays a message to the screen when a kernel panic occurs. v7 * Use drm_for_each_primary_visible_plane() v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/ast/ast_mode.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index a718646a66b8..49f2d8bd3377 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -700,12 +701,29 @@ static void ast_primary_plane_helper_atomic_disable(struct drm_plane *plane, ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x1, 0xdf, 0x20); } +static int ast_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct ast_plane *ast_plane = to_ast_plane(plane); + + if (plane->state && plane->state->fb && ast_plane->vaddr) { + sb->format = plane->state->fb->format; + sb->width = plane->state->fb->width; + sb->height = plane->state->fb->height; + sb->pitch = plane->state->fb->pitches[0]; + iosys_map_set_vaddr_iomem(>map, ast_plane->vaddr); + return 0; + } + return -ENODEV; +} + static const struct drm_plane_helper_funcs ast_primary_plane_helper_funcs = { DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, .atomic_check = ast_primary_plane_helper_atomic_check, .atomic_update = ast_primary_plane_helper_atomic_update, .atomic_enable = ast_primary_plane_helper_atomic_enable, .atomic_disable = ast_primary_plane_helper_atomic_disable, + .get_scanout_buffer = ast_primary_plane_helper_get_scanout_buffer, }; static const struct drm_plane_funcs ast_primary_plane_funcs = { -- 2.43.2
[PATCH v9 7/9] drm/mgag200: Add drm_panic support
Add support for the drm_panic module, which displays a message to the screen when a kernel panic occurs. v5: * Also check that the plane is visible and primary. (Thomas Zimmermann) v7: * use drm_for_each_primary_visible_plane() v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/mgag200/mgag200_drv.h | 7 ++- drivers/gpu/drm/mgag200/mgag200_mode.c | 18 ++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 765e49fd8911..58a0e62eaf18 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -366,6 +366,7 @@ struct drm_crtc_state; struct drm_display_mode; struct drm_plane; struct drm_atomic_state; +struct drm_scanout_buffer; extern const uint32_t mgag200_primary_plane_formats[]; extern const size_t mgag200_primary_plane_formats_size; @@ -379,12 +380,16 @@ void mgag200_primary_plane_helper_atomic_enable(struct drm_plane *plane, struct drm_atomic_state *state); void mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, struct drm_atomic_state *old_state); +int mgag200_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb); + #define MGAG200_PRIMARY_PLANE_HELPER_FUNCS \ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, \ .atomic_check = mgag200_primary_plane_helper_atomic_check, \ .atomic_update = mgag200_primary_plane_helper_atomic_update, \ .atomic_enable = mgag200_primary_plane_helper_atomic_enable, \ - .atomic_disable = mgag200_primary_plane_helper_atomic_disable + .atomic_disable = mgag200_primary_plane_helper_atomic_disable, \ + .get_scanout_buffer = mgag200_primary_plane_helper_get_scanout_buffer #define MGAG200_PRIMARY_PLANE_FUNCS \ .update_plane = drm_atomic_helper_update_plane, \ diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index e17cb4c5f774..8f368ecca4d4 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "mgag200_drv.h" @@ -546,6 +547,23 @@ void mgag200_primary_plane_helper_atomic_disable(struct drm_plane *plane, msleep(20); } +int mgag200_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct mga_device *mdev = to_mga_device(plane->dev); + struct iosys_map map = IOSYS_MAP_INIT_VADDR_IOMEM(mdev->vram); + + if (plane->state && plane->state->fb) { + sb->format = plane->state->fb->format; + sb->width = plane->state->fb->width; + sb->height = plane->state->fb->height; + sb->pitch = plane->state->fb->pitches[0]; + sb->map = map; + return 0; + } + return -ENODEV; +} + /* * CRTC */ -- 2.43.2
[PATCH v9 8/9] drm/imx: Add drm_panic support
Add support for the drm_panic module, which displays a user-friendly message to the screen when a kernel panic occurs. v7: * use drm_panic_gem_get_scanout_buffer() helper v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c index dade8b59feae..3e21d2a1a124 100644 --- a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c @@ -772,6 +772,12 @@ static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = { .atomic_disable = ipu_plane_atomic_disable, .atomic_update = ipu_plane_atomic_update, }; +static const struct drm_plane_helper_funcs ipu_primary_plane_helper_funcs = { + .atomic_check = ipu_plane_atomic_check, + .atomic_disable = ipu_plane_atomic_disable, + .atomic_update = ipu_plane_atomic_update, + .get_scanout_buffer = drm_panic_gem_get_scanout_buffer, +}; bool ipu_plane_atomic_update_pending(struct drm_plane *plane) { @@ -916,7 +922,10 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, ipu_plane->dma = dma; ipu_plane->dp_flow = dp; - drm_plane_helper_add(_plane->base, _plane_helper_funcs); + if (type == DRM_PLANE_TYPE_PRIMARY) + drm_plane_helper_add(_plane->base, _primary_plane_helper_funcs); + else + drm_plane_helper_add(_plane->base, _plane_helper_funcs); if (dp == IPU_DP_FLOW_SYNC_BG || dp == IPU_DP_FLOW_SYNC_FG) ret = drm_plane_create_zpos_property(_plane->base, zpos, 0, -- 2.43.2
[PATCH v9 6/9] drm/simpledrm: Add drm_panic support
Add support for the drm_panic module, which displays a user-friendly message to the screen when a kernel panic occurs. v8: * Replace get_scanout_buffer() with drm_panic_set_buffer() (Thomas Zimmermann) v9: * Revert to using get_scanout_buffer() (Sima) * move get_scanout_buffer() to plane helper functions (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/tiny/simpledrm.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 7ce1c4617675..f498665be8c4 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #define DRIVER_NAME"simpledrm" @@ -671,11 +672,26 @@ static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plan drm_dev_exit(idx); } +static int simpledrm_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb) +{ + struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev); + + sb->width = sdev->mode.hdisplay; + sb->height = sdev->mode.vdisplay; + sb->format = sdev->format; + sb->pitch = sdev->pitch; + sb->map = sdev->screen_base; + + return 0; +} + static const struct drm_plane_helper_funcs simpledrm_primary_plane_helper_funcs = { DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, .atomic_check = simpledrm_primary_plane_helper_atomic_check, .atomic_update = simpledrm_primary_plane_helper_atomic_update, .atomic_disable = simpledrm_primary_plane_helper_atomic_disable, + .get_scanout_buffer = simpledrm_primary_plane_helper_get_scanout_buffer, }; static const struct drm_plane_funcs simpledrm_primary_plane_funcs = { -- 2.43.2
[PATCH v9 3/9] drm/panic: Add a drm panic handler
This module displays a user friendly message when a kernel panic occurs. It currently doesn't contain any debug information, but that can be added later. v2 * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) * Add the panic reason to the panic message (Nerdopolis) * Add an exclamation mark (Nerdopolis) v3 * Rework the drawing functions, to write the pixels line by line and to use the drm conversion helper to support other formats. (Thomas Zimmermann) v4 * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) * Add foreground/background color config option * Fix the bottom lines not painted if the framebuffer height is not a multiple of the font height. * Automatically register the device to drm_panic, if the function get_scanout_buffer exists. (Thomas Zimmermann) v5 * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). v6 * Fix sparse warning for panic_msg and logo. v7 * Add select DRM_KMS_HELPER for the color conversion functions. v8 * Register directly each plane to the panic notifier (Sima) * Add raw_spinlock to properly handle concurrency (Sima) * Register plane instead of device, to avoid looping through plane list, and simplify code. * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) * Removed the draw_pixel_xy() API, will see later if it can be added back. v9 * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) Signed-off-by: Jocelyn Falempe --- Documentation/gpu/drm-kms.rst| 12 + drivers/gpu/drm/Kconfig | 23 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_drv.c| 4 + drivers/gpu/drm/drm_panic.c | 322 +++ drivers/gpu/drm/drm_plane.c | 1 + include/drm/drm_modeset_helper_vtables.h | 37 +++ include/drm/drm_panic.h | 52 include/drm/drm_plane.h | 5 + 9 files changed, 457 insertions(+) create mode 100644 drivers/gpu/drm/drm_panic.c diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 13d3627d8bc0..b64334661aeb 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -398,6 +398,18 @@ Plane Damage Tracking Functions Reference .. kernel-doc:: include/drm/drm_damage_helper.h :internal: +Plane Panic Feature +--- + +.. kernel-doc:: drivers/gpu/drm/drm_panic.c + :doc: overview + +Plane Panic Functions Reference +--- + +.. kernel-doc:: drivers/gpu/drm/drm_panic.c + :export: + Display Modes Function Reference diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 16029435b750..f07ca38d3f98 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -102,6 +102,29 @@ config DRM_KMS_HELPER help CRTC helpers for KMS drivers. +config DRM_PANIC + bool "Display a user-friendly message when a kernel panic occurs" + depends on DRM && !FRAMEBUFFER_CONSOLE + select DRM_KMS_HELPER + select FONT_SUPPORT + help + Enable a drm panic handler, which will display a user-friendly message + when a kernel panic occurs. It's useful when using a user-space + console instead of fbcon. + It will only work if your graphic driver supports this feature. + To support Hi-DPI Display, you can enable bigger fonts like + FONT_TER16x32 + +config DRM_PANIC_FOREGROUND_COLOR + hex "Drm panic screen foreground color, in RGB" + depends on DRM_PANIC + default 0xff + +config DRM_PANIC_BACKGROUND_COLOR + hex "Drm panic screen background color, in RGB" + depends on DRM_PANIC + default 0x00 + config DRM_DEBUG_DP_MST_TOPOLOGY_REFS bool "Enable refcount backtrace history in the DP MST helpers" depends on STACKTRACE_SUPPORT diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index a73c04d2d7a3..f9ca4f8fa6c5 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -88,6 +88,7 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += \ drm_privacy_screen.o \ drm_privacy_screen_x86.o drm-$(CONFIG_DRM_ACCEL) += ../../accel/drm_accel.o +drm-$(CONFIG_DRM_PANIC) += drm
[PATCH v9 5/9] drm/fb_dma: Add generic get_scanout_buffer() for drm_panic
This was initialy done for imx6, but should work on most drivers using drm_fb_dma_helper. v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * go back to get_scanout_buffer() * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_dma_helper.c | 47 + include/drm/drm_fb_dma_helper.h | 4 +++ 2 files changed, 51 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index 3b535ad1b07c..010327069ad4 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -148,3 +149,49 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, } } EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); + +#if defined(CONFIG_DRM_PANIC) +/** + * @plane: DRM primary plane + * @drm_scanout_buffer: scanout buffer for the panic handler + * Returns: 0 or negative error code + * + * Generic get_scanout_buffer() implementation, for drivers that uses the + * drm_fb_dma_helper. + */ +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb) +{ + struct drm_gem_dma_object *dma_obj; + struct drm_framebuffer *fb; + + fb = plane->state->fb; + /* Only support linear modifier */ + if (fb->modifier != DRM_FORMAT_MOD_LINEAR) + return -ENODEV; + + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); + + /* Buffer should be accessible from the CPU */ + if (dma_obj->base.import_attach) + return -ENODEV; + + /* Buffer should be already mapped to CPU */ + if (!dma_obj->vaddr) + return -ENODEV; + + iosys_map_set_vaddr(>map, dma_obj->vaddr); + sb->format = fb->format; + sb->height = fb->height; + sb->width = fb->width; + sb->pitch = fb->pitches[0]; + return 0; +} +#else +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb) +{ + return -ENODEV; +} +#endif +EXPORT_SYMBOL(drm_panic_gem_get_scanout_buffer); diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h index d5e036c57801..61f24c2aba2f 100644 --- a/include/drm/drm_fb_dma_helper.h +++ b/include/drm/drm_fb_dma_helper.h @@ -7,6 +7,7 @@ struct drm_device; struct drm_framebuffer; struct drm_plane_state; +struct drm_scanout_buffer; struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb, unsigned int plane); @@ -19,5 +20,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, struct drm_plane_state *old_state, struct drm_plane_state *state); +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, +struct drm_scanout_buffer *sb); + #endif -- 2.43.2
[PATCH v9 4/9] drm/panic: Add debugfs entry to test without triggering panic.
Add a debugfs file, so you can test drm_panic without freezing your machine. This is unsafe, and should be enabled only for developer or tester. To display the drm_panic screen on the device 0: echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 v9: * Create a debugfs file for each plane in the device's debugfs directory. This allows to test for each plane of each GPU independently. Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/Kconfig | 9 + drivers/gpu/drm/drm_panic.c | 38 + 2 files changed, 47 insertions(+) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index f07ca38d3f98..6e41fbd16b3d 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR depends on DRM_PANIC default 0x00 +config DRM_PANIC_DEBUG + bool "Add a debug fs entry to trigger drm_panic" + depends on DRM_PANIC && DEBUG_FS + help + Add dri/[device]/drm_panic_plane_x in the kernel debugfs, to force the + panic handler to write the panic message to this plane scanout buffer. + This is unsafe and should not be enabled on a production build. + If in doubt, say "N". + config DRM_DEBUG_DP_MST_TOPOLOGY_REFS bool "Enable refcount backtrace history in the DP MST helpers" depends on STACKTRACE_SUPPORT diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 2e2a033d1267..24dc1f1a388f 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -270,6 +270,43 @@ static int drm_panic(struct notifier_block *this, unsigned long event, return NOTIFY_OK; } + +/* + * DEBUG FS, This is currently unsafe. + * Create one file per plane, so it's possible to debug one plane at a time. + */ +#ifdef CONFIG_DRM_PANIC_DEBUG +#include + +static ssize_t debugfs_trigger_write(struct file *file, const char __user *user_buf, + size_t count, loff_t *ppos) +{ + bool run; + + if (kstrtobool_from_user(user_buf, count, ) == 0 && run) { + struct drm_plane *plane = file->private_data; + + draw_panic_plane(plane, "Test from debugfs"); + } + return count; +} + +static const struct file_operations dbg_drm_panic_ops = { + .owner = THIS_MODULE, + .write = debugfs_trigger_write, + .open = simple_open, +}; + +static void debugfs_register_plane(struct drm_plane *plane, int index) +{ + char fname[32]; + + snprintf(fname, 32, "drm_panic_plane_%d", index); + debugfs_create_file(fname, 0200, plane->dev->debugfs_root, + plane, _drm_panic_ops); +} +#endif /* CONFIG_DRM_PANIC_DEBUG */ + /** * drm_panic_register() - Initialize DRM panic for a device * @dev: the drm device on which the panic screen will be displayed. @@ -291,6 +328,7 @@ void drm_panic_register(struct drm_device *dev) >panic_notifier)) { drm_warn(dev, "Failed to register panic handler\n"); } else { + debugfs_register_plane(plane, registered_plane); registered_plane++; } } -- 2.43.2
[PATCH v9 2/9] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
This is needed for drm_panic, to draw the font, and fill the background color, in multiple color format. v5: * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. * Also add drm_fb_fill() to fill area with background color. v6: * fix __le32 conversion warning found by the kernel test bot Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_format_helper.c | 432 ++-- include/drm/drm_format_helper.h | 9 + 2 files changed, 360 insertions(+), 81 deletions(-) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index b1be458ed4dd..2d9646cefc4f 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -111,6 +111,153 @@ void drm_format_conv_state_release(struct drm_format_conv_state *state) } EXPORT_SYMBOL(drm_format_conv_state_release); +static __le16 drm_format_xrgb_to_rgb565(__le32 val32) +{ + u16 val16; + u32 pix; + + pix = le32_to_cpu(val32); + val16 = ((pix & 0x00F8) >> 8) | + ((pix & 0xFC00) >> 5) | + ((pix & 0x00F8) >> 3); + return cpu_to_le16(val16); +} + +static __le16 drm_format_xrgb_to_rgba5551(__le32 val32) +{ + u16 val16; + u32 pix; + + pix = le32_to_cpu(val32); + val16 = ((pix & 0x00f8) >> 8) | + ((pix & 0xf800) >> 5) | + ((pix & 0x00f8) >> 2) | + BIT(0); /* set alpha bit */ + return cpu_to_le16(val16); +} + +static __le16 drm_format_xrgb_to_xrgb1555(__le32 val32) +{ + u16 val16; + u32 pix; + + pix = le32_to_cpu(val32); + val16 = ((pix & 0x00f8) >> 9) | + ((pix & 0xf800) >> 6) | + ((pix & 0x00f8) >> 3); + return cpu_to_le16(val16); +} + +static __le16 drm_format_xrgb_to_argb1555(__le32 val32) +{ + u16 val16; + u32 pix; + + pix = le32_to_cpu(val32); + val16 = BIT(15) | /* set alpha bit */ + ((pix & 0x00f8) >> 9) | + ((pix & 0xf800) >> 6) | + ((pix & 0x00f8) >> 3); + return cpu_to_le16(val16); +} + +static __le32 drm_format_xrgb_to_argb(__le32 pix) +{ + u32 val32; + + val32 = le32_to_cpu(pix); + val32 |= GENMASK(31, 24); /* fill alpha bits */ + return cpu_to_le32(val32); +} + +static __le32 drm_format_xrgb_to_xbgr(__le32 pix) +{ + u32 val32; + + val32 = le32_to_cpu(pix); + val32 = ((val32 & 0x00ff) >> 16) << 0 | + ((val32 & 0xff00) >> 8) << 8 | + ((val32 & 0x00ff) >> 0) << 16 | + ((val32 & 0xff00) >> 24) << 24; + return cpu_to_le32(val32); +} + +static __le32 drm_format_xrgb_to_abgr(__le32 pix) +{ + u32 val32; + + val32 = le32_to_cpu(pix); + val32 = ((val32 & 0x00ff) >> 16) << 0 | + ((val32 & 0xff00) >> 8) << 8 | + ((val32 & 0x00ff) >> 0) << 16 | + GENMASK(31, 24); /* fill alpha bits */ + return cpu_to_le32(val32); +} + +static __le32 drm_format_xrgb_to_xrgb2101010(__le32 pix) +{ + u32 val32; + + val32 = le32_to_cpu(pix); + val32 = ((val32 & 0x00FF) << 2) | + ((val32 & 0xFF00) << 4) | + ((val32 & 0x00FF) << 6); + return cpu_to_le32(val32 | ((val32 >> 8) & 0x00300C03)); +} + +static __le32 drm_format_xrgb_to_argb2101010(__le32 pix) +{ + u32 val32; + + val32 = le32_to_cpu(pix); + val32 = ((val32 & 0x00FF) << 2) | + ((val32 & 0xFF00) << 4) | + ((val32 & 0x00FF) << 6); + val32 = GENMASK(31, 30) | /* set alpha bits */ + val32 | ((val32 >> 8) & 0x00300c03); + return cpu_to_le32(val32); +} + +/** + * drm_fb_convert_from_xrgb - convert one pixel from xrgb to the desired format + * @color: input color, in xrgb format + * @format: output format + * + * Returns: + * Color in the format specified, casted to u32. + * Or 0 if the format is unknown. + */ +u32 drm_fb_convert_from_xrgb(u32 color, u32 format) +{ + __le32 pix = cpu_to_le32(color); + + switch (format) { + case DRM_FORMAT_RGB565: + return le16_to_cpu(drm_format_xrgb_to_rgb565(pix)); + case DRM_FORMAT_RGBA5551: + return le16_to_cpu(drm_format_xrgb_to_rgba5551(pix)); + case DRM_FORMAT_XRGB1555: + return le16_to_cpu(drm_format_xrgb_to_xrgb1555(pix)); + case DRM_FORMAT_ARGB1555: + return le16_to_cpu(drm_format
[PATCH v9 1/9] drm/panic: Add drm panic locking
From: Daniel Vetter Rough sketch for the locking of drm panic printing code. The upshot of this approach is that we can pretty much entirely rely on the atomic commit flow, with the pair of raw_spin_lock/unlock providing any barriers we need, without having to create really big critical sections in code. This also avoids the need that drivers must explicitly update the panic handler state, which they might forget to do, or not do consistently, and then we blow up in the worst possible times. It is somewhat racy against a concurrent atomic update, and we might write into a buffer which the hardware will never display. But there's fundamentally no way to avoid that - if we do the panic state update explicitly after writing to the hardware, we might instead write to an old buffer that the user will barely ever see. Note that an rcu protected deference of plane->state would give us the the same guarantees, but it has the downside that we then need to protect the plane state freeing functions with call_rcu too. Which would very widely impact a lot of code and therefore doesn't seem worth the complexity compared to a raw spinlock with very tiny critical sections. Plus rcu cannot be used to protect access to peek/poke registers anyway, so we'd still need it for those cases. Peek/poke registers for vram access (or a gart pte reserved just for panic code) are also the reason I've gone with a per-device and not per-plane spinlock, since usually these things are global for the entire display. Going with per-plane locks would mean drivers for such hardware would need additional locks, which we don't want, since it deviates from the per-console takeoverlocks design. Longer term it might be useful if the panic notifiers grow a bit more structure than just the absolute bare EXPORT_SYMBOL(panic_notifier_list) - somewhat aside, why is that not EXPORT_SYMBOL_GPL ... If panic notifiers would be more like console drivers with proper register/unregister interfaces we could perhaps reuse the very fancy console lock with all it's check and takeover semantics that John Ogness is developing to fix the console_lock mess. But for the initial cut of a drm panic printing support I don't think we need that, because the critical sections are extremely small and only happen once per display refresh. So generally just 60 tiny locked sections per second, which is nothing compared to a serial console running a 115kbaud doing really slow mmio writes for each byte. So for now the raw spintrylock in drm panic notifier callback should be good enough. Another benefit of making panic notifiers more like full blown consoles (that are used in panics only) would be that we get the two stage design, where first all the safe outputs are used. And then the dangerous takeover tricks are deployed (where for display drivers we also might try to intercept any in-flight display buffer flips, which if we race and misprogram fifos and watermarks can hang the memory controller on some hw). For context the actual implementation on the drm side is by Jocelyn and this patch is meant to be combined with the overall approach in v7 (v8 is a bit less flexible, which I think is the wrong direction): https://lore.kernel.org/dri-devel/20240104160301.185915-1-jfale...@redhat.com/ Note that the locking is very much not correct there, hence this separate rfc. v2: - fix authorship, this was all my typing - some typo oopsies - link to the drm panic work by Jocelyn for context Signed-off-by: Daniel Vetter Cc: Jocelyn Falempe Cc: Andrew Morton Cc: "Peter Zijlstra (Intel)" Cc: Lukas Wunner Cc: Petr Mladek Cc: Steven Rostedt Cc: John Ogness Cc: Sergey Senozhatsky Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/drm_atomic_helper.c | 3 + drivers/gpu/drm/drm_drv.c | 1 + include/drm/drm_mode_config.h | 10 +++ include/drm/drm_panic.h | 99 + 4 files changed, 113 insertions(+) create mode 100644 include/drm/drm_panic.h diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 39ef0a6addeb..c0bb91312fb2 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -3099,6 +3100,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, } } + drm_panic_lock(state->dev); for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { WARN_ON(plane->state != old_plane_state); @@ -3108,6 +3110,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, state->planes[i].state = old_plane_state; plane->state = new_plane_state; } + drm_panic_unlock(state->dev); for_each_oldnew_pri
[PATCH v9 0/9] drm/panic: Add a drm panic handler
This introduces a new drm panic handler, which displays a message when a panic occurs. So when fbcon is disabled, you can still see a kernel panic. This is one of the missing feature, when disabling VT/fbcon in the kernel: https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/ Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel. This is a proof of concept, and works with simpledrm, mgag200, ast, and imx. To test it, make sure you're using one of the supported driver, and trigger a panic: echo c > /proc/sysrq-trigger or you can enable CONFIG_DRM_PANIC_DEBUG and echo 1 > /sys/kernel/debug/dri/0/drm_panic_plane_0 v2: * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann) * Add the panic reason to the panic message (Nerdopolis) * Add an exclamation mark (Nerdopolis) v3: * Rework the drawing functions, to write the pixels line by line and to use the drm conversion helper to support other formats. (Thomas Zimmermann) v4: * Fully support all simpledrm formats using drm conversion helpers * Rename dpanic_* to drm_panic_*, and have more coherent function name. (Thomas Zimmermann) * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann) * Remove the default y to DRM_PANIC config option (Thomas Zimmermann) * Add foreground/background color config option * Fix the bottom lines not painted if the framebuffer height is not a multiple of the font height. * Automatically register the driver to drm_panic, if the function get_scanout_buffer() exists. (Thomas Zimmermann) * Add mgag200 support. v5: * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. (Thomas Zimmermann) * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). * Add ast support. * Add experimental imx/ipuv3 support, to test on an ARM hw. (Maxime Ripard) v6: * Fix sparse and __le32 warnings * Drop the IMX/IPUV3 experiment, it was just to show that it works also on ARM devices. v7: * Add a check to see if the 4cc format is supported by drm_panic. * Add a drm/plane helper to loop over all visible primary buffer, simplifying the get_scanout_buffer implementations * Add a generic implementation for drivers that uses drm_fb_dma. (Maxime Ripard) * Add back the IMX/IPUV3 support, and use the generic implementation. (Maxime Ripard) v8: * Directly register each plane to the panic notifier (Sima) * Replace get_scanout_buffer() with set_scanout_buffer() to simplify the locking. (Thomas Zimmermann) * Add a debugfs entry, to trigger the drm_panic without a real panic (Sima) * Fix the drm_panic Documentation, and include it in drm-kms.rst v9: * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) * Create a debugfs file for each plane in the device's debugfs directory. This allows to test for each plane of each GPU independently. Daniel Vetter (1): drm/panic: Add drm panic locking Jocelyn Falempe (8): drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill drm/panic: Add a drm panic handler drm/panic: Add debugfs entry to test without triggering panic. drm/fb_dma: Add generic get_scanout_buffer() for drm_panic drm/simpledrm: Add drm_panic support drm/mgag200: Add drm_panic support drm/imx: Add drm_panic support drm/ast: Add drm_panic support Documentation/gpu/drm-kms.rst| 12 + drivers/gpu/drm/Kconfig | 32 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/ast/ast_mode.c | 18 + drivers/gpu/drm/drm_atomic_helper.c | 3 + drivers/gpu/drm/drm_drv.c| 5 + drivers/gpu/drm/drm_fb_dma_helper.c | 47 +++ drivers/gpu/drm/drm_format_helper.c | 432 ++- drivers/gpu/drm/drm_panic.c | 360 +++ drivers/gpu/drm/drm_plane.c | 1 + drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 11 +- drivers/gpu/drm/mgag200/mgag200_drv.h| 7 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 18 + drivers/gpu/drm/tiny/simpledrm.c | 16 + include/drm/drm_fb_dma_helper.h | 4 + include/drm/drm_format_helper.h | 9 + include/drm/drm_mode_config.h| 10 + include/drm/drm_modeset_helper_vtables.h | 37 ++ include/drm/drm_panic.h | 151 include/drm/drm_plane.h | 5 + 20 files changed, 1096 insertions(+), 83 deletions(-) create mode 100644 drivers/gpu/drm/drm_panic.c create mode 100644 inclu
Re: [RFC] How to test panic handlers, without crashing the kernel
On 05/03/2024 17:23, Michael Kelley wrote: From: Guilherme G. Piccoli Sent: Monday, March 4, 2024 1:43 PM On 04/03/2024 18:12, John Ogness wrote: [...] The second question is how to simulate a panic context in a non-destructive way, so we can test the panic notifiers in CI, without crashing the machine. I'm wondering if a "fake panic" can be implemented that quiesces all the other CPUs via NMI (similar to kdb) and then calls the panic notifiers. And finally releases everything back to normal. That might produce a fairly realistic panic situation and should be fairly non-destructive (depending on what the notifiers do and how long they take). Hi Jocelyn / John, one concern here is that the panic notifiers are kind of a no man's land, so we can have very simple / safe ones, while others are destructive in nature. An example of a good behaving notifier that is destructive is the Hyper-V one, that destroys an essential host-guest interface (called "vmbus connection"). What happens if we trigger this one just for testing purposes in a debugfs interface? Likely the guest would die... [+CCing Michael Kelley here since he seems interested in panic and is also expert in Hyper-V, just in case my example is bogus.] The Hyper-V example is valid. After hv_panic_vmbus_unload() is called, the VM won't be able to do any disk, network, or graphics frame buffer I/O. There's no recovery short of restarting the VM. Thanks for the confirmation. Michael [I have retired from Microsoft. I'm still occasionally contributing to Linux kernel work with email mhkli...@outlook.com.] So, maybe the problem could be split in 2: the non-notifiers portion of the panic path, and the the notifiers; maybe restricting the notifiers you'd run is a way to circumvent the risks, like if you could pass a list of the specific notifiers you aim to test, this could be interesting. Let's see what the others think and thanks for your work in the DRM panic notifier =) Or maybe have two lists of panic notifiers, the safe and the destructive list. So in case of fake panic, we can only call the safe notifiers. Cheers, Guilherme
Re: [RFC] How to test panic handlers, without crashing the kernel
On 04/03/2024 22:12, John Ogness wrote: [Added printk maintainer and kdb folks] Hi Jocelyn, On 2024-03-01, Jocelyn Falempe wrote: While writing a panic handler for drm devices [1], I needed a way to test it without crashing the machine. So from debugfs, I called atomic_notifier_call_chain(_notifier_list, ...), but it has the side effect of calling all other panic notifiers registered. So Sima suggested to move that to the generic panic code, and test all panic notifiers with a dedicated debugfs interface. I can move that code to kernel/, but before doing that, I would like to know if you think that's the right way to test the panic code. One major event that happens before the panic notifiers is panic_other_cpus_shutdown(). This can cause special situations because CPUs can be stopped while holding resources (such as raw spin locks). And these are the situations that make it so tricky to have safe and reliable notifiers. If triggered from debugfs, these situations will never occur. My concern is that the tests via debugfs will always succeed, but in the real world panic notifiers are failing/hanging/exploding. IMHO useful panic testing requires real panic'ing. Yes, but for the drm panic, it's still useful to check that the output is working (ie: make sure the color format and the framebuffer address are good). Also I've reworked the debugfs patch, so I don't have to call all panic notifiers. It's now per device, so your can trigger the drm_panic handler on a specific GPU. For my printk panic tests I trigger unknown NMIs while booting with "unknown_nmi_panic". Particularly with Qemu this is quite easy and amazingly effective at catching problems. In fact, a recent printk series [0] fixed seven issues that were found through this method of panic testing. Thanks for this tip, I used to test with "echo c > /proc/sysrq-trigger" in the guest, but that's more permissive. I'm now testing with virsh inject-nmi, and drm_panic is still working. The second question is how to simulate a panic context in a non-destructive way, so we can test the panic notifiers in CI, without crashing the machine. I'm wondering if a "fake panic" can be implemented that quiesces all the other CPUs via NMI (similar to kdb) and then calls the panic notifiers. And finally releases everything back to normal. That might produce a fairly realistic panic situation and should be fairly non-destructive (depending on what the notifiers do and how long they take). The worst case for a panic notifier, is when the panic occurs in NMI context, but I don't know how to simulate that. The goal would be to find early if a panic notifier tries to sleep, or do other things that are not allowed in a panic context. Maybe with a new boot argument "unknown_nmi_fake_panic" that triggers the fake panic instead? John Ogness [0] https://lore.kernel.org/lkml/20240207134103.1357162-1-john.ogn...@linutronix.de Best regards, -- Jocelyn
Re: [RFC] drm/panic: Add drm panic locking
Thanks for the patch. I think it misses to initialize the lock, so we need to add a raw_spin_lock_init() in the drm device initialization. Also I'm wondering if it make sense to put that under the CONFIG_DRM_PANIC flag, so that if you don't enable it, panic_lock() and panic_unlock() would be no-op. But that may not work if the driver uses this lock to protect some register access. Best regards, -- Jocelyn On 01/03/2024 11:39, Daniel Vetter wrote: Rough sketch for the locking of drm panic printing code. The upshot of this approach is that we can pretty much entirely rely on the atomic commit flow, with the pair of raw_spin_lock/unlock providing any barriers we need, without having to create really big critical sections in code. This also avoids the need that drivers must explicitly update the panic handler state, which they might forget to do, or not do consistently, and then we blow up in the worst possible times. It is somewhat racy against a concurrent atomic update, and we might write into a buffer which the hardware will never display. But there's fundamentally no way to avoid that - if we do the panic state update explicitly after writing to the hardware, we might instead write to an old buffer that the user will barely ever see. Note that an rcu protected deference of plane->state would give us the the same guarantees, but it has the downside that we then need to protect the plane state freeing functions with call_rcu too. Which would very widely impact a lot of code and therefore doesn't seem worth the complexity compared to a raw spinlock with very tiny critical sections. Plus rcu cannot be used to protect access to peek/poke registers anyway, so we'd still need it for those cases. Peek/poke registers for vram access (or a gart pte reserved just for panic code) are also the reason I've gone with a per-device and not per-plane spinlock, since usually these things are global for the entire display. Going with per-plane locks would mean drivers for such hardware would need additional locks, which we don't want, since it deviates from the per-console takeoverlocks design. Longer term it might be useful if the panic notifiers grow a bit more structure than just the absolute bare EXPORT_SYMBOL(panic_notifier_list) - somewhat aside, why is that not EXPORT_SYMBOL_GPL ... If panic notifiers would be more like console drivers with proper register/unregister interfaces we could perhaps reuse the very fancy console lock with all it's check and takeover semantics that John Ogness is developing to fix the console_lock mess. But for the initial cut of a drm panic printing support I don't think we need that, because the critical sections are extremely small and only happen once per display refresh. So generally just 60 tiny locked sections per second, which is nothing compared to a serial console running a 115kbaud doing really slow mmio writes for each byte. So for now the raw spintrylock in drm panic notifier callback should be good enough. Another benefit of making panic notifiers more like full blown consoles (that are used in panics only) would be that we get the two stage design, where first all the safe outputs are used. And then the dangerous takeover tricks are deployed (where for display drivers we also might try to intercept any in-flight display buffer flips, which if we race and misprogram fifos and watermarks can hang the memory controller on some hw). For context the actual implementation on the drm side is by Jocelyn and this patch is meant to be combined with the overall approach in v7 (v8 is a bit less flexible, which I think is the wrong direction): https://lore.kernel.org/dri-devel/20240104160301.185915-1-jfale...@redhat.com/ Note that the locking is very much not correct there, hence this separate rfc. v2: - fix authorship, this was all my typing - some typo oopsies - link to the drm panic work by Jocelyn for context Signed-off-by: Daniel Vetter Cc: Jocelyn Falempe Cc: Andrew Morton Cc: "Peter Zijlstra (Intel)" Cc: Lukas Wunner Cc: Petr Mladek Cc: Steven Rostedt Cc: John Ogness Cc: Sergey Senozhatsky Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/drm_atomic_helper.c | 3 + include/drm/drm_mode_config.h | 10 +++ include/drm/drm_panic.h | 99 + 3 files changed, 112 insertions(+) create mode 100644 include/drm/drm_panic.h diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 40c2bd3e62e8..5a908c186037 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -3086,6 +3087,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, } } + drm_panic_lock(state->dev); for_each_oldnew_plane_in_st
[RFC] How to test panic handlers, without crashing the kernel
Hi, While writing a panic handler for drm devices [1], I needed a way to test it without crashing the machine. So from debugfs, I called atomic_notifier_call_chain(_notifier_list, ...), but it has the side effect of calling all other panic notifiers registered. So Sima suggested to move that to the generic panic code, and test all panic notifiers with a dedicated debugfs interface. I can move that code to kernel/, but before doing that, I would like to know if you think that's the right way to test the panic code. The second question is how to simulate a panic context in a non-destructive way, so we can test the panic notifiers in CI, without crashing the machine. The worst case for a panic notifier, is when the panic occurs in NMI context, but I don't know how to simulate that. The goal would be to find early if a panic notifier tries to sleep, or do other things that are not allowed in a panic context. Best regards, -- Jocelyn [1] https://patchwork.freedesktop.org/patch/580183/?series=122244=8
Re: [PATCH v8 5/8] drm/simpledrm: Add drm_panic support
On 29/02/2024 12:17, Daniel Vetter wrote: On Tue, Feb 27, 2024 at 11:04:16AM +0100, Jocelyn Falempe wrote: Add support for the drm_panic module, which displays a user-friendly message to the screen when a kernel panic occurs. v8: * Replace get_scanout_buffer() with drm_panic_set_buffer() (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/tiny/simpledrm.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 7ce1c4617675..a2190995354a 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #define DRIVER_NAME "simpledrm" @@ -735,6 +736,20 @@ static const struct drm_connector_funcs simpledrm_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; +static void simpledrm_init_panic_buffer(struct drm_plane *plane) +{ + struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev); + struct drm_framebuffer fb; + + /* Fake framebuffer struct for drm_panic_set_buffer */ + fb.width = sdev->mode.hdisplay; + fb.height = sdev->mode.vdisplay; + fb.format = sdev->format; + fb.pitches[0] = sdev->pitch; + + drm_panic_set_buffer(plane->panic_scanout, , >screen_base); +} + static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = { .fb_create = drm_gem_fb_create_with_dirty, .atomic_check = drm_atomic_helper_check, @@ -945,6 +960,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv, return ERR_PTR(ret); drm_plane_helper_add(primary_plane, _primary_plane_helper_funcs); drm_plane_enable_fb_damage_clips(primary_plane); + drm_panic_register(primary_plane); Just a quick comment on this: This does not work, the driver is not ready to handle panic calls at this stage. Instead we need to automatically register all planes that support panic handling in drm_dev_register(), and we need to remove them all again in drm_dev_unregister(). Outside of these functions it is not safe to call into driver code. If you register the primary plane and didn't call drm_panic_set_buffer() yet, the panic handler will not do anything, so it should be safe. But if we revert to using the get_scanout_buffer(), this makes sense. At that point it might be simpler to only register one panic notifier per drm_device, and push the loop into the panic handler again. Cheers, Sima + simpledrm_init_panic_buffer(primary_plane); /* CRTC */ -- 2.43.0
Re: [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.
On 29/02/2024 12:21, Daniel Vetter wrote: On Tue, Feb 27, 2024 at 11:04:14AM +0100, Jocelyn Falempe wrote: Add a debugfs file, so you can test drm_panic without freezing your machine. This is unsafe, and should be enabled only for developer or tester. to display the drm_panic screen, just run: echo 1 > /sys/kernel/debug/drm_panic/trigger Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/Kconfig | 9 +++ drivers/gpu/drm/drm_panic.c | 47 + 2 files changed, 56 insertions(+) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index c17d8a8f6877..8dcea29f595c 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR depends on DRM_PANIC default 0x00 +config DRM_PANIC_DEBUG + bool "Add a debug fs entry to trigger drm_panic" + depends on DRM_PANIC && DEBUG_FS + help + Add drm_panic/trigger in the kernel debugfs, to force the panic + handler to write the panic message to the scanout buffer. This is + unsafe and should not be enabled on a production build. + If in doubt, say "N". + config DRM_DEBUG_DP_MST_TOPOLOGY_REFS bool "Enable refcount backtrace history in the DP MST helpers" depends on STACKTRACE_SUPPORT diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index c9f386476ef9..c5d3f725c5f5 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -398,3 +398,50 @@ void drm_panic_unregister(struct drm_plane *plane) } EXPORT_SYMBOL(drm_panic_unregister); + +/* + * DEBUG, This is currently unsafe. + * Also it will call all panic_notifier, since there is no way to filter and + * only call the drm_panic notifier. + */ +#ifdef CONFIG_DRM_PANIC_DEBUG +#include + +static struct dentry *debug_dir; +static struct dentry *debug_trigger; + +static ssize_t dbgfs_trigger_write(struct file *file, const char __user *user_buf, + size_t count, loff_t *ppos) +{ + bool run; + + if (kstrtobool_from_user(user_buf, count, ) == 0 && run) + atomic_notifier_call_chain(_notifier_list, 0, "Test drm panic from debugfs"); Since this is just the general panic notifier it feels very misplaced in the drm subsystem. I think moving that code into the core panic code makes a lot more sense, then we'd also have all the right people on Cc: to figure out how we can best recreate the correct calling context (like nmi context or whatever) for best case simulation of panic code. John Ogness definitely needs to see this and ack, wherever we put it. I'm not sure it makes sense to test all panic notifiers at once. So maybe I can write an atomic_notifier_call_chain_with_filter(), and filter on the callback address, so it will only call the drm_panic handlers ? -- Jocelyn -Sima + return count; +} + +static const struct file_operations dbg_drm_panic_ops = { + .owner = THIS_MODULE, + .write = dbgfs_trigger_write, +}; + +static int __init debugfs_start(void) +{ + debug_dir = debugfs_create_dir("drm_panic", NULL); + + if (IS_ERR(debug_dir)) + return PTR_ERR(debug_dir); + debug_trigger = debugfs_create_file("trigger", 0200, debug_dir, + NULL, _drm_panic_ops); + return 0; +} + +static void __exit debugfs_end(void) +{ + debugfs_remove_recursive(debug_dir); +} + +module_init(debugfs_start); +module_exit(debugfs_end); + +#endif -- 2.43.0
[PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic
This was initialy done for imx6, but should work on most drivers using drm_fb_dma_helper. v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/drm_fb_dma_helper.c | 37 + include/drm/drm_fb_dma_helper.h | 4 2 files changed, 41 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index 3b535ad1b07c..31ba71644e2b 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -148,3 +149,39 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, } } EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); + +#if defined(CONFIG_DRM_PANIC) +/** + * drm_panic_gem_set_scanout_buffer - helper around drm_panic_set_buffer() + * + * @plane: primary plane registered to drm_panic + * @fb: framebuffer attached to the plane state + * + * Update plane->panic_scanout with the new framebuffer. + */ +void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane, + struct drm_framebuffer *fb) +{ + struct drm_gem_dma_object *dma_obj; + struct iosys_map map; + + if (!plane->panic_scanout) + return; + + if (fb->modifier == DRM_FORMAT_MOD_LINEAR) { + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); + if (dma_obj && dma_obj->vaddr) { + iosys_map_set_vaddr(, dma_obj->vaddr); + drm_panic_set_buffer(plane->panic_scanout, fb, ); + return; + } + } + drm_panic_unset_buffer(plane->panic_scanout); +} +#else +void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane, + struct drm_framebuffer *fb) +{ +} +#endif +EXPORT_SYMBOL(drm_panic_gem_set_scanout_buffer); diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h index d5e036c57801..9f9ec11343cd 100644 --- a/include/drm/drm_fb_dma_helper.h +++ b/include/drm/drm_fb_dma_helper.h @@ -7,6 +7,7 @@ struct drm_device; struct drm_framebuffer; struct drm_plane_state; +struct drm_scanout_buffer; struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb, unsigned int plane); @@ -19,5 +20,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, struct drm_plane_state *old_state, struct drm_plane_state *state); +void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane, +struct drm_framebuffer *fb); + #endif -- 2.43.0
[PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.
Add a debugfs file, so you can test drm_panic without freezing your machine. This is unsafe, and should be enabled only for developer or tester. to display the drm_panic screen, just run: echo 1 > /sys/kernel/debug/drm_panic/trigger Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/Kconfig | 9 +++ drivers/gpu/drm/drm_panic.c | 47 + 2 files changed, 56 insertions(+) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index c17d8a8f6877..8dcea29f595c 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR depends on DRM_PANIC default 0x00 +config DRM_PANIC_DEBUG + bool "Add a debug fs entry to trigger drm_panic" + depends on DRM_PANIC && DEBUG_FS + help + Add drm_panic/trigger in the kernel debugfs, to force the panic + handler to write the panic message to the scanout buffer. This is + unsafe and should not be enabled on a production build. + If in doubt, say "N". + config DRM_DEBUG_DP_MST_TOPOLOGY_REFS bool "Enable refcount backtrace history in the DP MST helpers" depends on STACKTRACE_SUPPORT diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index c9f386476ef9..c5d3f725c5f5 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -398,3 +398,50 @@ void drm_panic_unregister(struct drm_plane *plane) } EXPORT_SYMBOL(drm_panic_unregister); + +/* + * DEBUG, This is currently unsafe. + * Also it will call all panic_notifier, since there is no way to filter and + * only call the drm_panic notifier. + */ +#ifdef CONFIG_DRM_PANIC_DEBUG +#include + +static struct dentry *debug_dir; +static struct dentry *debug_trigger; + +static ssize_t dbgfs_trigger_write(struct file *file, const char __user *user_buf, + size_t count, loff_t *ppos) +{ + bool run; + + if (kstrtobool_from_user(user_buf, count, ) == 0 && run) + atomic_notifier_call_chain(_notifier_list, 0, "Test drm panic from debugfs"); + return count; +} + +static const struct file_operations dbg_drm_panic_ops = { + .owner = THIS_MODULE, + .write = dbgfs_trigger_write, +}; + +static int __init debugfs_start(void) +{ + debug_dir = debugfs_create_dir("drm_panic", NULL); + + if (IS_ERR(debug_dir)) + return PTR_ERR(debug_dir); + debug_trigger = debugfs_create_file("trigger", 0200, debug_dir, + NULL, _drm_panic_ops); + return 0; +} + +static void __exit debugfs_end(void) +{ + debugfs_remove_recursive(debug_dir); +} + +module_init(debugfs_start); +module_exit(debugfs_end); + +#endif -- 2.43.0
[PATCH v8 8/8] drm/ast: Add drm_panic support
Add support for the drm_panic module, which displays a message to the screen when a kernel panic occurs. v7 * Use drm_for_each_primary_visible_plane() v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe --- drivers/gpu/drm/ast/ast_mode.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index a718646a66b8..3d6d4c71bc34 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -656,9 +657,13 @@ static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane, struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc); struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state); struct ast_vbios_mode_info *vbios_mode_info = _crtc_state->vbios_mode_info; + struct iosys_map map; ast_set_color_reg(ast, fb->format); ast_set_vbios_color_reg(ast, fb->format, vbios_mode_info); + + iosys_map_set_vaddr_iomem(, ast_plane->vaddr); + drm_panic_set_buffer(plane->panic_scanout, fb, ); } drm_atomic_helper_damage_iter_init(, old_plane_state, plane_state); @@ -736,6 +741,7 @@ static int ast_primary_plane_init(struct ast_device *ast) } drm_plane_helper_add(primary_plane, _primary_plane_helper_funcs); drm_plane_enable_fb_damage_clips(primary_plane); + drm_panic_register(primary_plane); return 0; } -- 2.43.0