| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a221ee69-0afd-ea36-1ca1-c5540da0091b@amd.com>
Date: Thu, 1 Dec 2022 11:12:16 -0500
From: Leo Li <sunpeng.li@....com>
To: Hamza Mahfooz <hamza.mahfooz@....com>,
amd-gfx@...ts.freedesktop.org
Cc: Harry Wentland <harry.wentland@....com>,
Rodrigo Siqueira <Rodrigo.Siqueira@....com>,
Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>,
"Pan, Xinhui" <Xinhui.Pan@....com>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
Nicholas Kazlauskas <nicholas.kazlauskas@....com>,
Aurabindo Pillai <aurabindo.pillai@....com>,
Roman Li <roman.li@....com>, Fangzhi Zuo <Jerry.Zuo@....com>,
Yifan Zhang <yifan1.zhang@....com>,
Maíra Canal <maira.canal@....br>,
Joaquín Ignacio Aramendía
<samsagax@...il.com>,
Mario Limonciello <mario.limonciello@....com>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/amd/display: add FB_DAMAGE_CLIPS support
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 <hamza.mahfooz@....com>
Thanks for the patch, it LGTM.
Reviewed-by: Leo Li <sunpeng.li@....com>
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, 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
> * rects.
> */
> - num_clips = drm_plane_get_damage_clips_count(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,36 +4944,51 @@ 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);
> -
> - 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;
> - }
> + 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);
>
> - /* 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],
> + 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 */
> + 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);
> + }
> +
> + if (num_clips) {
> + for (; i < 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, &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);
> + }
> +
> + if (i > DC_MAX_DIRTY_RECTS)
> + goto ffu;
>
> flip_addrs->dirty_rect_count = i;
> + return;
> +
> +ffu:
> + 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,
> + &flip_addrs->dirty_rect_count, true);
> }
>
> static void update_stream_scaling_settings(const struct drm_display_mode *mode,
> 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
Powered by blists - more mailing lists