On 2024-03-15 13:09, sunpeng...@amd.com wrote:
> From: Leo Li
>
> [Why]
>
> Compositors have different ways of assigning surfaces to DRM planes for
> render offloading. It may decide between various strategies: overlay,
> underlay, or a mix of both
>
> One way for compositors to implement the underlay strategy is to assign
> a higher zpos to the DRM_PRIMARY plane than the DRM_OVERLAY planes,
> effectively turning the DRM_OVERLAY plane into an underlay plane.
>
> Today, amdgpu attaches an immutable zpos of 0 to the DRM_PRIMARY plane.
> This however, is an arbitrary restriction. DCN pipes are general
> purpose, and can be arranged in any z-order. To support compositors
> using this allocation scheme, we can set a non-zero immutable zpos for
> the PRIMARY, allowing the placement of OVERLAYS (mutable zpos range
> 0-254) beneath the PRIMARY.
>
> [How]
>
> Assign a zpos = #no of OVERLAY planes to the PRIMARY plane. Then, clean
> up any assumptions in the driver of PRIMARY plane having the lowest
> zpos.
>
> Signed-off-by: Leo Li
With the typo mentioned below fixes this is
Reviewed-by: Harry Wentland
Before merging we should run a full promotion test (especially the IGT
tests) on it as this could break things in subtle ways.
Harry
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 96 ++-
> .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 17 +++-
> 2 files changed, 104 insertions(+), 9 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 09ab330aed17..01b00f587701 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -80,6 +80,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -369,6 +370,20 @@ static inline void reverse_planes_order(struct
> dc_surface_update *array_of_surfa
> swap(array_of_surface_update[i], array_of_surface_update[j]);
> }
>
> +/*
> + * DC will program planes with their z-order determined by their ordering
> + * in the dc_surface_updates array. This comparator is used to sort them
> + * by descending zpos.
> + */
> +static int dm_plane_layer_index_cmp(const void *a, const void *b)
> +{
> + const struct dc_surface_update *sa = (struct dc_surface_update *)a;
> + const struct dc_surface_update *sb = (struct dc_surface_update *)b;
> +
> + /* Sort by descending dc_plane layer_index (i.e. normalized_zpos) */
> + return sb->surface->layer_index - sa->surface->layer_index;
> +}
> +
> /**
> * update_planes_and_stream_adapter() - Send planes to be updated in DC
> *
> @@ -393,7 +408,8 @@ static inline bool
> update_planes_and_stream_adapter(struct dc *dc,
> struct dc_stream_update
> *stream_update,
> struct dc_surface_update
> *array_of_surface_update)
> {
> - reverse_planes_order(array_of_surface_update, planes_count);
> + sort(array_of_surface_update, planes_count,
> + sizeof(*array_of_surface_update), dm_plane_layer_index_cmp, NULL);
>
> /*
>* Previous frame finished and HW is ready for optimization.
> @@ -9363,6 +9379,8 @@ static void amdgpu_dm_atomic_commit_tail(struct
> drm_atomic_state *state)
> for (j = 0; j < status->plane_count; j++)
> dummy_updates[j].surface = status->plane_states[0];
>
> + sort(dummy_updates, status->plane_count,
> + sizeof(*dummy_updates), dm_plane_layer_index_cmp, NULL);
>
> mutex_lock(>dc_lock);
> dc_update_planes_and_stream(dm->dc,
> @@ -10097,6 +10115,17 @@ static bool should_reset_plane(struct
> drm_atomic_state *state,
> if (new_crtc_state->color_mgmt_changed)
> return true;
>
> + /*
> + * On zpos change, planes need to be reordered by removing and re-adding
> + * them one by one to the dc state, in order of descending zpos.
> + *
> + * TODO: We can likely skip bandwidth validation if the only thing that
> + * changed about the plane was it'z z-ordering.
> + */
> + if (new_crtc_state->zpos_changed) {
> + return true;
> + }
> +
> if (drm_atomic_crtc_needs_modeset(new_crtc_state))
> return true;
>
> @@ -10509,6 +10538,65 @@ dm_get_plane_scale(struct drm_plane_state
> *plane_state,
> *out_plane_scale_h = plane_state->crtc_h * 1000 / plane_src_h;
> }
>
> +/*
> + * The normalized_zpos value cannot be used by this iterator directly. It's
> only
> + * calculated for enabled planes, potentially causing normalized_zpos
> collisions
> + * between enabled/disabled planes in the atomic state. We need a unique
> value
> + * so that the iterator will not generate the same object twice, or loop
> + * indefinitely.
> + */
> +static inline struct __drm_planes_state
