drm/amd/display: add FB_DAMAGE_CLIPS support

Message ID 20221115202413.283685-1-hamza.mahfooz@amd.com
State New
Headers
Series drm/amd/display: add FB_DAMAGE_CLIPS support |

Commit Message

Hamza Mahfooz Nov. 15, 2022, 8:24 p.m. UTC
  Currently, userspace doesn't have a way to communicate selective updates
to displays. So, enable support for FB_DAMAGE_CLIPS for DCN ASICs newer
than DCN301, convert DRM damage clips to dc dirty rectangles and fill
them into dirty_rects in fill_dc_dirty_rects().

Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 91 +++++++++++--------
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  4 +
 2 files changed, 58 insertions(+), 37 deletions(-)
  

Comments

Leo Li Nov. 17, 2022, 5:31 p.m. UTC | #1
On 11/15/22 15:24, Hamza Mahfooz wrote:
> Currently, userspace doesn't have a way to communicate selective updates
> to displays. So, enable support for FB_DAMAGE_CLIPS for DCN ASICs newer
> than DCN301, convert DRM damage clips to dc dirty rectangles and fill
> them into dirty_rects in fill_dc_dirty_rects().
> 
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 91 +++++++++++--------
>   .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  4 +
>   2 files changed, 58 insertions(+), 37 deletions(-)
> 
> 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 185d09c760ba..18b710ba802d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4842,6 +4842,31 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
>   	return 0;
>   }
>   
> +static inline void fill_dc_dirty_rect(struct drm_plane *plane,
> +				      struct rect *dirty_rect, int32_t x,
> +				      int32_t y, int32_t width, int32_t height,
> +				      int *i, bool ffu)
> +{
> +	WARN_ON(*i >= DC_MAX_DIRTY_RECTS);

Hi Hamza,

Since dc_flip_addrs->dirty_rects has a fixed length of 
DC_MAX_DIRTY_RECTS per pipe (a restriction by DMUB FW), I think it makes 
more sense to fallback to a full-frame-update (FFU) if num_clips > 
DC_MAX_DIRTY_RECTS. An alternative would be to reject the atomic commit, 
but that sounds heavy handed.


> +
> +	dirty_rect->x = x;
> +	dirty_rect->y = y;
> +	dirty_rect->width = width;
> +	dirty_rect->height = height;
> +
> +	if (ffu)
> +		drm_dbg(plane->dev,
> +			"[PLANE:%d] PSR FFU dirty rect size (%d, %d)\n",
> +			plane->base.id, width, height);
> +	else
> +		drm_dbg(plane->dev,
> +			"[PLANE:%d] PSR SU dirty rect at (%d, %d) size (%d, %d)",
> +			plane->base.id, x, y, width, height);
> +
> +	(*i)++;
> +}
> +
> +
>   /**
>    * fill_dc_dirty_rects() - Fill DC dirty regions for PSR selective updates
>    *
> @@ -4862,10 +4887,6 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
>    * addition, certain use cases - such as cursor and multi-plane overlay (MPO) -
>    * implicitly provide damage clips without any client support via the plane
>    * bounds.
> - *
> - * Today, amdgpu_dm only supports the MPO and cursor usecase.
> - *
> - * TODO: Also enable for FB_DAMAGE_CLIPS
>    */
>   static void fill_dc_dirty_rects(struct drm_plane *plane,
>   				struct drm_plane_state *old_plane_state,
> @@ -4876,12 +4897,11 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
>   	struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(crtc_state);
>   	struct rect *dirty_rects = flip_addrs->dirty_rects;
>   	uint32_t num_clips;
> +	struct drm_mode_rect *clips;
>   	bool bb_changed;
>   	bool fb_changed;
>   	uint32_t i = 0;
>   
> -	flip_addrs->dirty_rect_count = 0;
> -
>   	/*
>   	 * Cursor plane has it's own dirty rect update interface. See
>   	 * dcn10_dmub_update_cursor_data and dmub_cmd_update_cursor_info_data
> @@ -4894,15 +4914,11 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
>   	 * requested, and there is a plane update, do FFU.
>   	 */
>   	if (!dm_crtc_state->mpo_requested) {
> -		dirty_rects[0].x = 0;
> -		dirty_rects[0].y = 0;
> -		dirty_rects[0].width = dm_crtc_state->base.mode.crtc_hdisplay;
> -		dirty_rects[0].height = dm_crtc_state->base.mode.crtc_vdisplay;
> -		flip_addrs->dirty_rect_count = 1;
> -		DRM_DEBUG_DRIVER("[PLANE:%d] PSR FFU dirty rect size (%d, %d)\n",
> -				 new_plane_state->plane->base.id,
> -				 dm_crtc_state->base.mode.crtc_hdisplay,
> -				 dm_crtc_state->base.mode.crtc_vdisplay);
> +		fill_dc_dirty_rect(new_plane_state->plane, &dirty_rects[0], 0,
> +				   0, dm_crtc_state->base.mode.crtc_hdisplay,
> +				   dm_crtc_state->base.mode.crtc_vdisplay, &i,
> +				   true);
> +		flip_addrs->dirty_rect_count = i;
>   		return;
>   	}

Previously, we always do FFU on plane updates if no MPO has been 
requested. Now, since we want to look at user-provided DRM damage clips, 
this bit needs a bit of a rework.

In short, if there are valid clips for this plane 
(drm_plane_get_damage_clips_count() > 0), they should be added to 
dc_dirty_rects. Otherwise, fallback to old FFU logic.

With MPO, the damage clips are more interesting, since the entire 
plane's bounding box can be moved. I wonder if that is reflected in 
DRM's damage clips. Do you know if a plane bb change will be reflected 
in drm_plane_get_damage_clips()?

Thanks,
Leo
>   
> @@ -4914,6 +4930,7 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
>   	 * rects.
>   	 */
>   	num_clips = drm_plane_get_damage_clips_count(new_plane_state);
> +	clips = drm_plane_get_damage_clips(new_plane_state);
>   	fb_changed = old_plane_state->fb->base.id !=
>   		     new_plane_state->fb->base.id;
>   	bb_changed = (old_plane_state->crtc_x != new_plane_state->crtc_x ||
> @@ -4921,33 +4938,33 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
>   		      old_plane_state->crtc_w != new_plane_state->crtc_w ||
>   		      old_plane_state->crtc_h != new_plane_state->crtc_h);
>   
> -	DRM_DEBUG_DRIVER("[PLANE:%d] PSR bb_changed:%d fb_changed:%d num_clips:%d\n",
> -			 new_plane_state->plane->base.id,
> -			 bb_changed, fb_changed, num_clips);
> +	drm_dbg(plane->dev,
> +		"[PLANE:%d] PSR bb_changed:%d fb_changed:%d num_clips:%d\n",
> +		new_plane_state->plane->base.id,
> +		bb_changed, fb_changed, num_clips);
>   
> -	if (num_clips || fb_changed || bb_changed) {
> -		dirty_rects[i].x = new_plane_state->crtc_x;
> -		dirty_rects[i].y = new_plane_state->crtc_y;
> -		dirty_rects[i].width = new_plane_state->crtc_w;
> -		dirty_rects[i].height = new_plane_state->crtc_h;
> -		DRM_DEBUG_DRIVER("[PLANE:%d] PSR SU dirty rect at (%d, %d) size (%d, %d)\n",
> -				 new_plane_state->plane->base.id,
> -				 dirty_rects[i].x, dirty_rects[i].y,
> -				 dirty_rects[i].width, dirty_rects[i].height);
> -		i += 1;
> +	if (num_clips) {
> +		for (; i < num_clips; i++, clips++) {
> +			fill_dc_dirty_rect(new_plane_state->plane,
> +					   &dirty_rects[i], clips->x1,
> +					   clips->y1, clips->x2 - clips->x1,
> +					   clips->y2 - clips->y1, &i, false);
> +		}
> +	} else if (fb_changed || bb_changed) {
> +		fill_dc_dirty_rect(new_plane_state->plane, &dirty_rects[i],
> +				   new_plane_state->crtc_x,
> +				   new_plane_state->crtc_y,
> +				   new_plane_state->crtc_w,
> +				   new_plane_state->crtc_h, &i, false);
>   	}
>   
>   	/* Add old plane bounding-box if plane is moved or resized */
>   	if (bb_changed) {
> -		dirty_rects[i].x = old_plane_state->crtc_x;
> -		dirty_rects[i].y = old_plane_state->crtc_y;
> -		dirty_rects[i].width = old_plane_state->crtc_w;
> -		dirty_rects[i].height = old_plane_state->crtc_h;
> -		DRM_DEBUG_DRIVER("[PLANE:%d] PSR SU dirty rect at (%d, %d) size (%d, %d)\n",
> -				old_plane_state->plane->base.id,
> -				dirty_rects[i].x, dirty_rects[i].y,
> -				dirty_rects[i].width, dirty_rects[i].height);
> -		i += 1;
> +		fill_dc_dirty_rect(new_plane_state->plane, &dirty_rects[i],
> +				   old_plane_state->crtc_x,
> +				   old_plane_state->crtc_y,
> +				   old_plane_state->crtc_w,
> +				   old_plane_state->crtc_h, &i, false);
>   	}
>   
>   	flip_addrs->dirty_rect_count = i;
> 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 e6854f7270a6..3c50b3ff7954 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
> @@ -1600,6 +1600,10 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
>   		drm_plane_create_rotation_property(plane, DRM_MODE_ROTATE_0,
>   						   supported_rotations);
>   
> +	if (dm->adev->ip_versions[DCE_HWIP][0] > IP_VERSION(3, 0, 1) &&
> +	    plane->type != DRM_PLANE_TYPE_CURSOR)
> +		drm_plane_enable_fb_damage_clips(plane);
> +
>   	drm_plane_helper_add(plane, &dm_plane_helper_funcs);
>   
>   #ifdef CONFIG_DRM_AMD_DC_HDR
  
Hamza Mahfooz Nov. 18, 2022, 3:07 p.m. UTC | #2
Hey Leo,

On 11/17/22 12:31, Leo Li wrote:
> 
> 
> On 11/15/22 15:24, Hamza Mahfooz wrote:
>> Currently, userspace doesn't have a way to communicate selective updates
>> to displays. So, enable support for FB_DAMAGE_CLIPS for DCN ASICs newer
>> than DCN301, convert DRM damage clips to dc dirty rectangles and fill
>> them into dirty_rects in fill_dc_dirty_rects().
>>
>> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
>> ---
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 91 +++++++++++--------
>>   .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  4 +
>>   2 files changed, 58 insertions(+), 37 deletions(-)
>>
>> 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 185d09c760ba..18b710ba802d 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -4842,6 +4842,31 @@ static int fill_dc_plane_attributes(struct 
>> amdgpu_device *adev,
>>       return 0;
>>   }
>> +static inline void fill_dc_dirty_rect(struct drm_plane *plane,
>> +                      struct rect *dirty_rect, int32_t x,
>> +                      int32_t y, int32_t width, int32_t height,
>> +                      int *i, bool ffu)
>> +{
>> +    WARN_ON(*i >= DC_MAX_DIRTY_RECTS);
> 
> Hi Hamza,
> 
> Since dc_flip_addrs->dirty_rects has a fixed length of 
> DC_MAX_DIRTY_RECTS per pipe (a restriction by DMUB FW), I think it makes 
> more sense to fallback to a full-frame-update (FFU) if num_clips > 
> DC_MAX_DIRTY_RECTS. An alternative would be to reject the atomic commit, 
> but that sounds heavy handed.
> 
> 
>> +
>> +    dirty_rect->x = x;
>> +    dirty_rect->y = y;
>> +    dirty_rect->width = width;
>> +    dirty_rect->height = height;
>> +
>> +    if (ffu)
>> +        drm_dbg(plane->dev,
>> +            "[PLANE:%d] PSR FFU dirty rect size (%d, %d)\n",
>> +            plane->base.id, width, height);
>> +    else
>> +        drm_dbg(plane->dev,
>> +            "[PLANE:%d] PSR SU dirty rect at (%d, %d) size (%d, %d)",
>> +            plane->base.id, x, y, width, height);
>> +
>> +    (*i)++;
>> +}
>> +
>> +
>>   /**
>>    * fill_dc_dirty_rects() - Fill DC dirty regions for PSR selective 
>> updates
>>    *
>> @@ -4862,10 +4887,6 @@ static int fill_dc_plane_attributes(struct 
>> amdgpu_device *adev,
>>    * addition, certain use cases - such as cursor and multi-plane 
>> overlay (MPO) -
>>    * implicitly provide damage clips without any client support via 
>> the plane
>>    * bounds.
>> - *
>> - * Today, amdgpu_dm only supports the MPO and cursor usecase.
>> - *
>> - * TODO: Also enable for FB_DAMAGE_CLIPS
>>    */
>>   static void fill_dc_dirty_rects(struct drm_plane *plane,
>>                   struct drm_plane_state *old_plane_state,
>> @@ -4876,12 +4897,11 @@ static void fill_dc_dirty_rects(struct 
>> drm_plane *plane,
>>       struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(crtc_state);
>>       struct rect *dirty_rects = flip_addrs->dirty_rects;
>>       uint32_t num_clips;
>> +    struct drm_mode_rect *clips;
>>       bool bb_changed;
>>       bool fb_changed;
>>       uint32_t i = 0;
>> -    flip_addrs->dirty_rect_count = 0;
>> -
>>       /*
>>        * Cursor plane has it's own dirty rect update interface. See
>>        * dcn10_dmub_update_cursor_data and 
>> dmub_cmd_update_cursor_info_data
>> @@ -4894,15 +4914,11 @@ static void fill_dc_dirty_rects(struct 
>> drm_plane *plane,
>>        * requested, and there is a plane update, do FFU.
>>        */
>>       if (!dm_crtc_state->mpo_requested) {
>> -        dirty_rects[0].x = 0;
>> -        dirty_rects[0].y = 0;
>> -        dirty_rects[0].width = dm_crtc_state->base.mode.crtc_hdisplay;
>> -        dirty_rects[0].height = dm_crtc_state->base.mode.crtc_vdisplay;
>> -        flip_addrs->dirty_rect_count = 1;
>> -        DRM_DEBUG_DRIVER("[PLANE:%d] PSR FFU dirty rect size (%d, 
>> %d)\n",
>> -                 new_plane_state->plane->base.id,
>> -                 dm_crtc_state->base.mode.crtc_hdisplay,
>> -                 dm_crtc_state->base.mode.crtc_vdisplay);
>> +        fill_dc_dirty_rect(new_plane_state->plane, &dirty_rects[0], 0,
>> +                   0, dm_crtc_state->base.mode.crtc_hdisplay,
>> +                   dm_crtc_state->base.mode.crtc_vdisplay, &i,
>> +                   true);
>> +        flip_addrs->dirty_rect_count = i;
>>           return;
>>       }
> 
> Previously, we always do FFU on plane updates if no MPO has been 
> requested. Now, since we want to look at user-provided DRM damage clips, 
> this bit needs a bit of a rework.
> 
> In short, if there are valid clips for this plane 
> (drm_plane_get_damage_clips_count() > 0), they should be added to 
> dc_dirty_rects. Otherwise, fallback to old FFU logic.
> 
> With MPO, the damage clips are more interesting, since the entire 
> plane's bounding box can be moved. I wonder if that is reflected in 
> DRM's damage clips. Do you know if a plane bb change will be reflected 
> in drm_plane_get_damage_clips()?

 From what I've seen, plane bb changes are not counted as damage clips.

> 
> Thanks,
> Leo
>> @@ -4914,6 +4930,7 @@ static void fill_dc_dirty_rects(struct drm_plane 
>> *plane,
>>        * rects.
>>        */
>>       num_clips = drm_plane_get_damage_clips_count(new_plane_state);
>> +    clips = drm_plane_get_damage_clips(new_plane_state);
>>       fb_changed = old_plane_state->fb->base.id !=
>>                new_plane_state->fb->base.id;
>>       bb_changed = (old_plane_state->crtc_x != new_plane_state->crtc_x ||
>> @@ -4921,33 +4938,33 @@ static void fill_dc_dirty_rects(struct 
>> drm_plane *plane,
>>                 old_plane_state->crtc_w != new_plane_state->crtc_w ||
>>                 old_plane_state->crtc_h != new_plane_state->crtc_h);
>> -    DRM_DEBUG_DRIVER("[PLANE:%d] PSR bb_changed:%d fb_changed:%d 
>> num_clips:%d\n",
>> -             new_plane_state->plane->base.id,
>> -             bb_changed, fb_changed, num_clips);
>> +    drm_dbg(plane->dev,
>> +        "[PLANE:%d] PSR bb_changed:%d fb_changed:%d num_clips:%d\n",
>> +        new_plane_state->plane->base.id,
>> +        bb_changed, fb_changed, num_clips);
>> -    if (num_clips || fb_changed || bb_changed) {
>> -        dirty_rects[i].x = new_plane_state->crtc_x;
>> -        dirty_rects[i].y = new_plane_state->crtc_y;
>> -        dirty_rects[i].width = new_plane_state->crtc_w;
>> -        dirty_rects[i].height = new_plane_state->crtc_h;
>> -        DRM_DEBUG_DRIVER("[PLANE:%d] PSR SU dirty rect at (%d, %d) 
>> size (%d, %d)\n",
>> -                 new_plane_state->plane->base.id,
>> -                 dirty_rects[i].x, dirty_rects[i].y,
>> -                 dirty_rects[i].width, dirty_rects[i].height);
>> -        i += 1;
>> +    if (num_clips) {
>> +        for (; i < num_clips; i++, clips++) {
>> +            fill_dc_dirty_rect(new_plane_state->plane,
>> +                       &dirty_rects[i], clips->x1,
>> +                       clips->y1, clips->x2 - clips->x1,
>> +                       clips->y2 - clips->y1, &i, false);
>> +        }
>> +    } else if (fb_changed || bb_changed) {
>> +        fill_dc_dirty_rect(new_plane_state->plane, &dirty_rects[i],
>> +                   new_plane_state->crtc_x,
>> +                   new_plane_state->crtc_y,
>> +                   new_plane_state->crtc_w,
>> +                   new_plane_state->crtc_h, &i, false);
>>       }
>>       /* Add old plane bounding-box if plane is moved or resized */
>>       if (bb_changed) {
>> -        dirty_rects[i].x = old_plane_state->crtc_x;
>> -        dirty_rects[i].y = old_plane_state->crtc_y;
>> -        dirty_rects[i].width = old_plane_state->crtc_w;
>> -        dirty_rects[i].height = old_plane_state->crtc_h;
>> -        DRM_DEBUG_DRIVER("[PLANE:%d] PSR SU dirty rect at (%d, %d) 
>> size (%d, %d)\n",
>> -                old_plane_state->plane->base.id,
>> -                dirty_rects[i].x, dirty_rects[i].y,
>> -                dirty_rects[i].width, dirty_rects[i].height);
>> -        i += 1;
>> +        fill_dc_dirty_rect(new_plane_state->plane, &dirty_rects[i],
>> +                   old_plane_state->crtc_x,
>> +                   old_plane_state->crtc_y,
>> +                   old_plane_state->crtc_w,
>> +                   old_plane_state->crtc_h, &i, false);
>>       }
>>       flip_addrs->dirty_rect_count = i;
>> 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 e6854f7270a6..3c50b3ff7954 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
>> @@ -1600,6 +1600,10 @@ int amdgpu_dm_plane_init(struct 
>> amdgpu_display_manager *dm,
>>           drm_plane_create_rotation_property(plane, DRM_MODE_ROTATE_0,
>>                              supported_rotations);
>> +    if (dm->adev->ip_versions[DCE_HWIP][0] > IP_VERSION(3, 0, 1) &&
>> +        plane->type != DRM_PLANE_TYPE_CURSOR)
>> +        drm_plane_enable_fb_damage_clips(plane);
>> +
>>       drm_plane_helper_add(plane, &dm_plane_helper_funcs);
>>   #ifdef CONFIG_DRM_AMD_DC_HDR
  

Patch

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 185d09c760ba..18b710ba802d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4842,6 +4842,31 @@  static int fill_dc_plane_attributes(struct amdgpu_device *adev,
 	return 0;
 }
 
+static inline void fill_dc_dirty_rect(struct drm_plane *plane,
+				      struct rect *dirty_rect, int32_t x,
+				      int32_t y, int32_t width, int32_t height,
+				      int *i, bool ffu)
+{
+	WARN_ON(*i >= DC_MAX_DIRTY_RECTS);
+
+	dirty_rect->x = x;
+	dirty_rect->y = y;
+	dirty_rect->width = width;
+	dirty_rect->height = height;
+
+	if (ffu)
+		drm_dbg(plane->dev,
+			"[PLANE:%d] PSR FFU dirty rect size (%d, %d)\n",
+			plane->base.id, width, height);
+	else
+		drm_dbg(plane->dev,
+			"[PLANE:%d] PSR SU dirty rect at (%d, %d) size (%d, %d)",
+			plane->base.id, x, y, width, height);
+
+	(*i)++;
+}
+
+
 /**
  * fill_dc_dirty_rects() - Fill DC dirty regions for PSR selective updates
  *
@@ -4862,10 +4887,6 @@  static int fill_dc_plane_attributes(struct amdgpu_device *adev,
  * addition, certain use cases - such as cursor and multi-plane overlay (MPO) -
  * implicitly provide damage clips without any client support via the plane
  * bounds.
- *
- * Today, amdgpu_dm only supports the MPO and cursor usecase.
- *
- * TODO: Also enable for FB_DAMAGE_CLIPS
  */
 static void fill_dc_dirty_rects(struct drm_plane *plane,
 				struct drm_plane_state *old_plane_state,
@@ -4876,12 +4897,11 @@  static void fill_dc_dirty_rects(struct drm_plane *plane,
 	struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(crtc_state);
 	struct rect *dirty_rects = flip_addrs->dirty_rects;
 	uint32_t num_clips;
+	struct drm_mode_rect *clips;
 	bool bb_changed;
 	bool fb_changed;
 	uint32_t i = 0;
 
-	flip_addrs->dirty_rect_count = 0;
-
 	/*
 	 * Cursor plane has it's own dirty rect update interface. See
 	 * dcn10_dmub_update_cursor_data and dmub_cmd_update_cursor_info_data
@@ -4894,15 +4914,11 @@  static void fill_dc_dirty_rects(struct drm_plane *plane,
 	 * requested, and there is a plane update, do FFU.
 	 */
 	if (!dm_crtc_state->mpo_requested) {
-		dirty_rects[0].x = 0;
-		dirty_rects[0].y = 0;
-		dirty_rects[0].width = dm_crtc_state->base.mode.crtc_hdisplay;
-		dirty_rects[0].height = dm_crtc_state->base.mode.crtc_vdisplay;
-		flip_addrs->dirty_rect_count = 1;
-		DRM_DEBUG_DRIVER("[PLANE:%d] PSR FFU dirty rect size (%d, %d)\n",
-				 new_plane_state->plane->base.id,
-				 dm_crtc_state->base.mode.crtc_hdisplay,
-				 dm_crtc_state->base.mode.crtc_vdisplay);
+		fill_dc_dirty_rect(new_plane_state->plane, &dirty_rects[0], 0,
+				   0, dm_crtc_state->base.mode.crtc_hdisplay,
+				   dm_crtc_state->base.mode.crtc_vdisplay, &i,
+				   true);
+		flip_addrs->dirty_rect_count = i;
 		return;
 	}
 
@@ -4914,6 +4930,7 @@  static void fill_dc_dirty_rects(struct drm_plane *plane,
 	 * rects.
 	 */
 	num_clips = drm_plane_get_damage_clips_count(new_plane_state);
+	clips = drm_plane_get_damage_clips(new_plane_state);
 	fb_changed = old_plane_state->fb->base.id !=
 		     new_plane_state->fb->base.id;
 	bb_changed = (old_plane_state->crtc_x != new_plane_state->crtc_x ||
@@ -4921,33 +4938,33 @@  static void fill_dc_dirty_rects(struct drm_plane *plane,
 		      old_plane_state->crtc_w != new_plane_state->crtc_w ||
 		      old_plane_state->crtc_h != new_plane_state->crtc_h);
 
-	DRM_DEBUG_DRIVER("[PLANE:%d] PSR bb_changed:%d fb_changed:%d num_clips:%d\n",
-			 new_plane_state->plane->base.id,
-			 bb_changed, fb_changed, num_clips);
+	drm_dbg(plane->dev,
+		"[PLANE:%d] PSR bb_changed:%d fb_changed:%d num_clips:%d\n",
+		new_plane_state->plane->base.id,
+		bb_changed, fb_changed, num_clips);
 
-	if (num_clips || fb_changed || bb_changed) {
-		dirty_rects[i].x = new_plane_state->crtc_x;
-		dirty_rects[i].y = new_plane_state->crtc_y;
-		dirty_rects[i].width = new_plane_state->crtc_w;
-		dirty_rects[i].height = new_plane_state->crtc_h;
-		DRM_DEBUG_DRIVER("[PLANE:%d] PSR SU dirty rect at (%d, %d) size (%d, %d)\n",
-				 new_plane_state->plane->base.id,
-				 dirty_rects[i].x, dirty_rects[i].y,
-				 dirty_rects[i].width, dirty_rects[i].height);
-		i += 1;
+	if (num_clips) {
+		for (; i < num_clips; i++, clips++) {
+			fill_dc_dirty_rect(new_plane_state->plane,
+					   &dirty_rects[i], clips->x1,
+					   clips->y1, clips->x2 - clips->x1,
+					   clips->y2 - clips->y1, &i, false);
+		}
+	} else if (fb_changed || bb_changed) {
+		fill_dc_dirty_rect(new_plane_state->plane, &dirty_rects[i],
+				   new_plane_state->crtc_x,
+				   new_plane_state->crtc_y,
+				   new_plane_state->crtc_w,
+				   new_plane_state->crtc_h, &i, false);
 	}
 
 	/* Add old plane bounding-box if plane is moved or resized */
 	if (bb_changed) {
-		dirty_rects[i].x = old_plane_state->crtc_x;
-		dirty_rects[i].y = old_plane_state->crtc_y;
-		dirty_rects[i].width = old_plane_state->crtc_w;
-		dirty_rects[i].height = old_plane_state->crtc_h;
-		DRM_DEBUG_DRIVER("[PLANE:%d] PSR SU dirty rect at (%d, %d) size (%d, %d)\n",
-				old_plane_state->plane->base.id,
-				dirty_rects[i].x, dirty_rects[i].y,
-				dirty_rects[i].width, dirty_rects[i].height);
-		i += 1;
+		fill_dc_dirty_rect(new_plane_state->plane, &dirty_rects[i],
+				   old_plane_state->crtc_x,
+				   old_plane_state->crtc_y,
+				   old_plane_state->crtc_w,
+				   old_plane_state->crtc_h, &i, false);
 	}
 
 	flip_addrs->dirty_rect_count = i;
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 e6854f7270a6..3c50b3ff7954 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
@@ -1600,6 +1600,10 @@  int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
 		drm_plane_create_rotation_property(plane, DRM_MODE_ROTATE_0,
 						   supported_rotations);
 
+	if (dm->adev->ip_versions[DCE_HWIP][0] > IP_VERSION(3, 0, 1) &&
+	    plane->type != DRM_PLANE_TYPE_CURSOR)
+		drm_plane_enable_fb_damage_clips(plane);
+
 	drm_plane_helper_add(plane, &dm_plane_helper_funcs);
 
 #ifdef CONFIG_DRM_AMD_DC_HDR