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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
---
  .../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

--
Hamza




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux