Re: [PATCH v2] drm/amd/display: add FB_DAMAGE_CLIPS support

2022-12-01 Thread Leo Li




On 11/18/22 16:51, 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 


Thanks for the patch, it LGTM.

Reviewed-by: Leo Li 

It would be good to add an IGT case to cover combinations of MPO & 
damage clip commits. Perhaps something that covers the usecase of moving 
a MPO video, while desktop UI updates. We'd expect the SU region to 
cover both the MPO plane + UI damage clips, or fallback to FFU.


Thanks,
Leo

---
v2: fallback to FFU if we run into too many dirty rectangles, consider
 dirty rectangles in non MPO case and always add a dirty rectangle
 for the new plane if there are bb changes in the MPO case.
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 130 +++---
  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |   4 +
  2 files changed, 88 insertions(+), 46 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 4eb8201a2608..7af94a2c6237 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,35 @@ 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)
+{
+   if (*i > DC_MAX_DIRTY_RECTS)
+   return;
+
+   if (*i == DC_MAX_DIRTY_RECTS)
+   goto out;
+
+   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);
+
+out:
+   (*i)++;
+}
+
  /**
   * fill_dc_dirty_rects() - Fill DC dirty regions for PSR selective updates
   *
@@ -4862,10 +4891,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 +4901,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
@@ -4889,20 +4913,20 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
if (plane->type == DRM_PLANE_TYPE_CURSOR)
return;
  
-	/*

-* Today, we only consider MPO use-case for PSR SU. If MPO not
-* requested, and there is a plane update, do FFU.
-*/
+   num_clips = drm_plane_get_damage_clips_count(new_plane_state);
+   clips = drm_plane_get_damage_clips(new_plane_state);
+
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);
+   if (!num_clips || num_clips > DC_MAX_DIRTY_RECTS)
+   goto ffu;
+
+   for (; flip_addrs->dirty_rect_count < num_clips; clips++)
+   fill_dc_dirty_rect(new_plane_state->plane,
+  &dirty_rects[i], clips->x1,
+  clips->y1, clip

[PATCH v2] drm/amd/display: add FB_DAMAGE_CLIPS support

2022-11-18 Thread Hamza Mahfooz
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 
---
v2: fallback to FFU if we run into too many dirty rectangles, consider
dirty rectangles in non MPO case and always add a dirty rectangle
for the new plane if there are bb changes in the MPO case.
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 130 +++---
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |   4 +
 2 files changed, 88 insertions(+), 46 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 4eb8201a2608..7af94a2c6237 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,35 @@ 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)
+{
+   if (*i > DC_MAX_DIRTY_RECTS)
+   return;
+
+   if (*i == DC_MAX_DIRTY_RECTS)
+   goto out;
+
+   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);
+
+out:
+   (*i)++;
+}
+
 /**
  * fill_dc_dirty_rects() - Fill DC dirty regions for PSR selective updates
  *
@@ -4862,10 +4891,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 +4901,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
@@ -4889,20 +4913,20 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
if (plane->type == DRM_PLANE_TYPE_CURSOR)
return;
 
-   /*
-* Today, we only consider MPO use-case for PSR SU. If MPO not
-* requested, and there is a plane update, do FFU.
-*/
+   num_clips = drm_plane_get_damage_clips_count(new_plane_state);
+   clips = drm_plane_get_damage_clips(new_plane_state);
+
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);
+   if (!num_clips || num_clips > DC_MAX_DIRTY_RECTS)
+   goto ffu;
+
+   for (; flip_addrs->dirty_rect_count < num_clips; clips++)
+   fill_dc_dirty_rect(new_plane_state->plane,
+  &dirty_rects[i], clips->x1,
+  clips->y1, clips->x2 - clips->x1,
+  clips->y2 - clips->y1,
+  &flip_addrs->dirty_rect_count,
+  false);
return;
}
 
@@ -4913,7 +4937,6 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
 * If plane is moved or resized, also add old bounding box to dirty