Re: [PATCH 4/5] drm/amd/display: add DC drm_panic support

[Harry Wentland <harry.wentland@amd.com>]

On 2024-11-05 09:06, Alex Deucher wrote:
> From: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Add support for the drm_panic module, which displays a pretty user
> friendly message on the screen when a Linux kernel panic occurs.
> 
> It doesn't work yet on laptop panels, maybe due to PSR.
> 
> Adapted from Jocelyn's original patch to add DC drm_panic
> support.
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: Lu Yao <yaolu@kylinos.cn>
> Cc: Jocelyn Falempe <jfalempe@redhat.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 103 +++++++++++++++++-
>  1 file changed, 102 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 495e3cd70426..5ba64e7ad3f3 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -26,6 +26,7 @@
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_blend.h>
> +#include "drm/drm_framebuffer.h"
>  #include <drm/drm_gem_atomic_helper.h>
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
> @@ -1421,6 +1422,93 @@ static void amdgpu_dm_plane_atomic_async_update(struct drm_plane *plane,
>  	amdgpu_dm_plane_handle_cursor_update(plane, old_state);
>  }
>  
> +static void amdgpu_dm_plane_panic_flush(struct drm_plane *plane)
> +{
> +	struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane->state);
> +	struct drm_framebuffer *fb = plane->state->fb;
> +	struct dc_plane_state *dc_plane_state;
> +	struct dc *dc;
> +	int i;
> +
> +	if (!dm_plane_state || !dm_plane_state->dc_state)
> +		return;
> +
> +	dc_plane_state = dm_plane_state->dc_state;
> +	dc = dc_plane_state->ctx->dc;
> +	if (!dc || !dc->current_state)
> +		return;
> +
> +	for (i = 0; i < dc->res_pool->pipe_count; i++) {
> +		struct pipe_ctx *pipe_ctx = &dc->current_state->res_ctx.pipe_ctx[i];
> +		struct hubp *hubp;
> +		struct mem_input *mi;
> +
> +		if (!pipe_ctx)
> +			continue;
> +
> +		switch (dc->ctx->dce_version) {

The HW-specific code should live in DC. Can we add a new
DC interface function (in dc.h, or dc_plane.h) to disable
tiling and call that from here? That function should then
do the switch.

Instead of a case statement it's probably easier to check
against > / < DCE_VERSION_MAX.

Harry

> +#if defined(CONFIG_DRM_AMD_DC_SI)
> +		case DCE_VERSION_6_0:
> +		case DCE_VERSION_6_1:
> +		case DCE_VERSION_6_4:
> +#endif
> +		case DCE_VERSION_8_0:
> +		case DCE_VERSION_8_1:
> +		case DCE_VERSION_8_3:
> +		case DCE_VERSION_10_0:
> +		case DCE_VERSION_11_0:
> +		case DCE_VERSION_11_2:
> +		case DCE_VERSION_11_22:
> +		case DCE_VERSION_12_0:
> +		case DCE_VERSION_12_1:
> +			mi = pipe_ctx->plane_res.mi;
> +			if (!mi)
> +				continue;
> +			/* if framebuffer is tiled, disable tiling */
> +			if (fb->modifier && mi->funcs->mem_input_clear_tiling)
> +				mi->funcs->mem_input_clear_tiling(mi);
> +
> +			/* force page flip to see the new content of the framebuffer */
> +			mi->funcs->mem_input_program_surface_flip_and_addr(mi,
> +									   &dc_plane_state->address,
> +									   true);
> +			break;
> +		case DCN_VERSION_1_0:
> +		case DCN_VERSION_1_01:
> +		case DCN_VERSION_2_0:
> +		case DCN_VERSION_2_01:
> +		case DCN_VERSION_2_1:
> +		case DCN_VERSION_3_0:
> +		case DCN_VERSION_3_01:
> +		case DCN_VERSION_3_02:
> +		case DCN_VERSION_3_03:
> +		case DCN_VERSION_3_1:
> +		case DCN_VERSION_3_14:
> +		case DCN_VERSION_3_16:
> +		case DCN_VERSION_3_15:
> +		case DCN_VERSION_3_2:
> +		case DCN_VERSION_3_21:
> +		case DCN_VERSION_3_5:
> +		case DCN_VERSION_3_51:
> +		case DCN_VERSION_4_01:
> +			hubp = pipe_ctx->plane_res.hubp;
> +			if (!hubp)
> +				continue;
> +			/* if framebuffer is tiled, disable tiling */
> +			if (fb->modifier && hubp->funcs->hubp_clear_tiling)
> +				hubp->funcs->hubp_clear_tiling(hubp);
> +
> +			/* force page flip to see the new content of the framebuffer */
> +			hubp->funcs->hubp_program_surface_flip_and_addr(hubp,
> +									&dc_plane_state->address,
> +									true);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
>  static const struct drm_plane_helper_funcs dm_plane_helper_funcs = {
>  	.prepare_fb = amdgpu_dm_plane_helper_prepare_fb,
>  	.cleanup_fb = amdgpu_dm_plane_helper_cleanup_fb,
> @@ -1429,6 +1517,16 @@ static const struct drm_plane_helper_funcs dm_plane_helper_funcs = {
>  	.atomic_async_update = amdgpu_dm_plane_atomic_async_update
>  };
>  
> +static const struct drm_plane_helper_funcs dm_primary_plane_helper_funcs = {
> +	.prepare_fb = amdgpu_dm_plane_helper_prepare_fb,
> +	.cleanup_fb = amdgpu_dm_plane_helper_cleanup_fb,
> +	.atomic_check = amdgpu_dm_plane_atomic_check,
> +	.atomic_async_check = amdgpu_dm_plane_atomic_async_check,
> +	.atomic_async_update = amdgpu_dm_plane_atomic_async_update,
> +	.get_scanout_buffer = amdgpu_display_get_scanout_buffer,
> +	.panic_flush = amdgpu_dm_plane_panic_flush,
> +};
> +
>  static void amdgpu_dm_plane_drm_plane_reset(struct drm_plane *plane)
>  {
>  	struct dm_plane_state *amdgpu_state = NULL;
> @@ -1855,7 +1953,10 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
>  	    plane->type != DRM_PLANE_TYPE_CURSOR)
>  		drm_plane_enable_fb_damage_clips(plane);
>  
> -	drm_plane_helper_add(plane, &dm_plane_helper_funcs);
> +	if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> +		drm_plane_helper_add(plane, &dm_primary_plane_helper_funcs);
> +	else
> +		drm_plane_helper_add(plane, &dm_plane_helper_funcs);
>  
>  #ifdef AMD_PRIVATE_COLOR
>  	dm_atomic_plane_attach_color_mgmt_properties(dm, plane);