[PATCH] drm/amdgpu: make damage clips support configurable

[Mario Limonciello]

On 2/8/2024 16:11, Hamza Mahfooz wrote:
> We have observed that there are quite a number of PSR-SU panels on the
> market that are unable to keep up with what user space throws at them,
> resulting in hangs and random black screens. So, make damage clips
> support configurable and disable it by default for PSR-SU displays.
> 
> Cc: Mario Limonciello <mario.limonciello at amd.com>
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz at amd.com>

I think this with this patch in place you could also revert

571c2fa26aa6 ("drm/amd/display: Disable PSR-SU on Parade 0803 TCON again")

One minor nit below otherwise LGTM.

Reviewed-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h               |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c           | 13 +++++++++++++
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  7 +++++++
>   3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 312dfaec7b4a..1291b8eb9dff 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -198,6 +198,7 @@ extern uint amdgpu_dc_debug_mask;
>   extern uint amdgpu_dc_visual_confirm;
>   extern uint amdgpu_dm_abm_level;
>   extern int amdgpu_backlight;
> +extern int amdgpu_damage_clips;
>   extern struct amdgpu_mgpu_info mgpu_info;
>   extern int amdgpu_ras_enable;
>   extern uint amdgpu_ras_mask;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 161ecf9b4174..2b75aeb8280f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -211,6 +211,7 @@ int amdgpu_seamless = -1; /* auto */
>   uint amdgpu_debug_mask;
>   int amdgpu_agp = -1; /* auto */
>   int amdgpu_wbrf = -1;
> +int amdgpu_damage_clips = -1; /* auto */
>   
>   static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
>   
> @@ -859,6 +860,18 @@ int amdgpu_backlight = -1;
>   MODULE_PARM_DESC(backlight, "Backlight control (0 = pwm, 1 = aux, -1 auto (default))");
>   module_param_named(backlight, amdgpu_backlight, bint, 0444);
>   
> +/**
> + * DOC: damageclips (int)
> + * Enable or disable damage clips support. If damage clips support is disabled,
> + * we will force full frame updates, irrespective of what user space sends to
> + * us.
> + *
> + * Defaults to -1.

I think it would be better if this documentation explained what the 
values are (IE what -1 really means).

> + */
> +MODULE_PARM_DESC(damageclips,
> +		 "Damage clips support (0 = disable, 1 = enable, -1 auto (default))");
> +module_param_named(damageclips, amdgpu_damage_clips, int, 0444);
> +
>   /**
>    * DOC: tmz (int)
>    * Trusted Memory Zone (TMZ) is a method to protect data being written
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index b3a5e730be24..fbe2aa40c21a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5253,6 +5253,7 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
>   				struct drm_plane_state *new_plane_state,
>   				struct drm_crtc_state *crtc_state,
>   				struct dc_flip_addrs *flip_addrs,
> +				bool is_psr_su,
>   				bool *dirty_regions_changed)
>   {
>   	struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(crtc_state);
> @@ -5277,6 +5278,10 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
>   	num_clips = drm_plane_get_damage_clips_count(new_plane_state);
>   	clips = drm_plane_get_damage_clips(new_plane_state);
>   
> +	if (num_clips && (!amdgpu_damage_clips || (amdgpu_damage_clips < 0 &&
> +						   is_psr_su)))
> +		goto ffu;
> +
>   	if (!dm_crtc_state->mpo_requested) {
>   		if (!num_clips || num_clips > DC_MAX_DIRTY_RECTS)
>   			goto ffu;
> @@ -8411,6 +8416,8 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>   			fill_dc_dirty_rects(plane, old_plane_state,
>   					    new_plane_state, new_crtc_state,
>   					    &bundle->flip_addrs[planes_count],
> +					    acrtc_state->stream->link->psr_settings.psr_version ==
> +					    DC_PSR_VERSION_SU_1,
>   					    &dirty_rects_changed);
>   
>   			/*