Re: [PATCH v6 5/8] drm/i915/display: Add drm_panic support

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 01 Apr 2025, Jocelyn Falempe <jfalempe@redhat.com> wrote:
> This adds drm_panic support for a wide range of Intel GPU. I've
> tested it only on 4 laptops, Haswell (with 128MB of eDRAM),
> Comet Lake, Raptor Lake, and Lunar Lake.
> For hardware using DPT, it's not possible to disable tiling, as you
> will need to reconfigure the way the GPU is accessing the
> framebuffer.
>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
>
> v4:
>  * Add support for Xe driver.
>  
> v6:
>  * Use struct intel_display instead of drm_i915_private for intel_atomic_plane.c 
>
>  .../gpu/drm/i915/display/intel_atomic_plane.c | 79 ++++++++++++++++++-
>  1 file changed, 78 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 7276179df878..eebf20fafaeb 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -33,13 +33,16 @@
>  
>  #include <linux/dma-fence-chain.h>
>  #include <linux/dma-resv.h>
> +#include <linux/iosys-map.h>
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_blend.h>
>  #include <drm/drm_damage_helper.h>
> +#include <drm/drm_cache.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_gem.h>
>  #include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_panic.h>
>  
>  #include "gem/i915_gem_object.h"
>  #include "i915_config.h"
> @@ -47,6 +50,7 @@
>  #include "i915_vma.h"
>  #include "i9xx_plane_regs.h"
>  #include "intel_atomic_plane.h"
> +#include "intel_bo.h"
>  #include "intel_cdclk.h"
>  #include "intel_cursor.h"
>  #include "intel_display_rps.h"
> @@ -54,6 +58,7 @@
>  #include "intel_display_types.h"
>  #include "intel_fb.h"
>  #include "intel_fb_pin.h"
> +#include "intel_fbdev.h"
>  #include "skl_scaler.h"
>  #include "skl_universal_plane.h"
>  #include "skl_watermark.h"
> @@ -1251,14 +1256,86 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>  	intel_plane_unpin_fb(old_plane_state);
>  }
>  
> +/* Only used by drm_panic get_scanout_buffer() and panic_flush(), so it is
> + * protected by the drm panic spinlock
> + */
> +static struct iosys_map panic_map;
> +
> +static void intel_panic_flush(struct drm_plane *plane)
> +{
> +	struct intel_plane_state *plane_state = to_intel_plane_state(plane->state);
> +	struct intel_plane *iplane = to_intel_plane(plane);
> +	struct intel_display *display = to_intel_display(iplane);
> +	struct drm_framebuffer *fb = plane_state->hw.fb;
> +
> +	/* Force a cache flush, otherwise the new pixels won't show up */
> +	drm_clflush_virt_range(panic_map.vaddr, fb->height * fb->pitches[0]);
> +
> +	/* Don't disable tiling if it's the fbdev framebuffer.*/
> +	if (to_intel_framebuffer(fb) == intel_fbdev_framebuffer(display->fbdev.fbdev)) {
> +		return;
> +

Mismatched {}


> +	if (fb->modifier && iplane->disable_tiling)
> +		iplane->disable_tiling(iplane);
> +}
> +
> +static int intel_get_scanout_buffer(struct drm_plane *plane,
> +				    struct drm_scanout_buffer *sb)
> +{
> +	struct intel_plane_state *plane_state;
> +	struct drm_gem_object *obj;
> +	struct drm_framebuffer *fb;
> +	struct intel_display *display = to_intel_display(plane->dev);
> +
> +	if (!plane->state || !plane->state->fb || !plane->state->visible)
> +		return -ENODEV;
> +
> +	plane_state = to_intel_plane_state(plane->state);
> +	fb = plane_state->hw.fb;
> +	obj = intel_fb_bo(fb);
> +	if (!obj)
> +		return -ENODEV;
> +
> +	iosys_map_clear(&panic_map);
> +	if (to_intel_framebuffer(fb) == intel_fbdev_framebuffer(display->fbdev.fbdev)) {
> +		intel_fbdev_get_map(display->fbdev.fbdev, &panic_map);
> +	} else {
> +		/* Can't disable tiling if DPT is in use */
> +		if (intel_fb_uses_dpt(fb))
> +			return -EOPNOTSUPP;
> +
> +		intel_bo_panic_map(obj, &panic_map);
> +	}
> +	if (iosys_map_is_null(&panic_map))
> +		return -ENOMEM;
> +
> +	sb->map[0] = panic_map;
> +	sb->width = fb->width;
> +	sb->height = fb->height;
> +	sb->format = fb->format;
> +	sb->pitch[0] = fb->pitches[0];
> +
> +	return 0;
> +}
> +
>  static const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
>  	.prepare_fb = intel_prepare_plane_fb,
>  	.cleanup_fb = intel_cleanup_plane_fb,
>  };
>  
> +static const struct drm_plane_helper_funcs intel_primary_plane_helper_funcs = {
> +	.prepare_fb = intel_prepare_plane_fb,
> +	.cleanup_fb = intel_cleanup_plane_fb,
> +	.get_scanout_buffer = intel_get_scanout_buffer,
> +	.panic_flush = intel_panic_flush,
> +};
> +
>  void intel_plane_helper_add(struct intel_plane *plane)
>  {
> -	drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs);
> +	if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
> +		drm_plane_helper_add(&plane->base, &intel_primary_plane_helper_funcs);
> +	else
> +		drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs);
>  }
>  
>  void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state,

-- 
Jani Nikula, Intel