On Mon, Dec 01, 2025 at 03:24:56PM +0200, Jouni Högander wrote:
> According to bspec selective fetch is not supported with async flips and
> instructing full frame update on async flip.
>
> v3:
> - rebase
> - fix old_crtc_state->pipe_srcsz_early_tpt
> - fix using intel_atomic_get_new_crtc_state
> v2:
> - check also crtc_state->async_flip_planes in
> psr2_sel_fetch_plane_state_supported
>
> Bspec: 55229
> Signed-off-by: Jouni Högander <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 72 ++++++++++++++----------
> 1 file changed, 41 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 15ef3b6caad6..53cf292247d7 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2728,13 +2728,20 @@ intel_psr2_sel_fetch_et_alignment(struct
> intel_atomic_state *state,
> * Plane scaling and rotation is not supported by selective fetch and both
> * properties can change without a modeset, so need to be check at every
> * atomic commit.
> + *
> + * If plane was having async flip previously we can't use selective
> + * fetch as we don't know if the flip is completed.
> */
> -static bool psr2_sel_fetch_plane_state_supported(const struct
> intel_plane_state *plane_state)
> +static bool psr2_sel_fetch_plane_state_supported(const struct
> intel_crtc_state *old_crtc_state,
> + const struct intel_plane_state
> *plane_state)
> {
> + struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
> +
> if (plane_state->uapi.dst.y1 < 0 ||
> plane_state->uapi.dst.x1 < 0 ||
> plane_state->scaler_id >= 0 ||
> - plane_state->hw.rotation != DRM_MODE_ROTATE_0)
> + plane_state->hw.rotation != DRM_MODE_ROTATE_0 ||
> + old_crtc_state->async_flip_planes & plane->id)
Why are you looking at the old crtc state? There should be nothing of
interest to us there.
> return false;
>
> return true;
> @@ -2749,7 +2756,8 @@ static bool psr2_sel_fetch_plane_state_supported(const
> struct intel_plane_state
> */
> static bool psr2_sel_fetch_pipe_state_supported(const struct
> intel_crtc_state *crtc_state)
> {
> - if (crtc_state->scaler_state.scaler_id >= 0)
> + if (crtc_state->scaler_state.scaler_id >= 0 ||
> + crtc_state->uapi.async_flip)
I think just checking crtc_state->async_flip_planes!=0 here should be
sufficient. The rest of the patch seems unnecessary.
On a related note, someone should add a new igt that does async flips
while eg. the cursor is enabled and overlapping the plane doing the
async flips. That's basically how I noticed the problem in the first
place (with Xorg), so would be good to have an igt to make sure we
don't break this in the future.
> return false;
>
> return true;
> @@ -2808,24 +2816,25 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> struct intel_crtc *crtc)
> {
> struct intel_display *display = to_intel_display(state);
> - struct intel_crtc_state *crtc_state =
> intel_atomic_get_new_crtc_state(state, crtc);
> + struct intel_crtc_state *new_crtc_state =
> intel_atomic_get_new_crtc_state(state, crtc);
> + struct intel_crtc_state *old_crtc_state =
> intel_atomic_get_old_crtc_state(state, crtc);
> struct intel_plane_state *new_plane_state, *old_plane_state;
> struct intel_plane *plane;
> bool full_update = false, cursor_in_su_area = false;
> int i, ret;
>
> - if (!crtc_state->enable_psr2_sel_fetch)
> + if (!new_crtc_state->enable_psr2_sel_fetch)
> return 0;
>
> - if (!psr2_sel_fetch_pipe_state_supported(crtc_state)) {
> + if (!psr2_sel_fetch_pipe_state_supported(new_crtc_state)) {
> full_update = true;
> goto skip_sel_fetch_set_loop;
> }
>
> - crtc_state->psr2_su_area.x1 = 0;
> - crtc_state->psr2_su_area.y1 = -1;
> - crtc_state->psr2_su_area.x2 = drm_rect_width(&crtc_state->pipe_src);
> - crtc_state->psr2_su_area.y2 = -1;
> + new_crtc_state->psr2_su_area.x1 = 0;
> + new_crtc_state->psr2_su_area.y1 = -1;
> + new_crtc_state->psr2_su_area.x2 =
> drm_rect_width(&new_crtc_state->pipe_src);
> + new_crtc_state->psr2_su_area.y2 = -1;
>
> /*
> * Calculate minimal selective fetch area of each plane and calculate
> @@ -2838,14 +2847,14 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> struct drm_rect src, damaged_area = { .x1 = 0, .y1 = -1,
> .x2 = INT_MAX };
>
> - if (new_plane_state->hw.crtc != crtc_state->uapi.crtc)
> + if (new_plane_state->hw.crtc != new_crtc_state->uapi.crtc)
> continue;
>
> if (!new_plane_state->uapi.visible &&
> !old_plane_state->uapi.visible)
> continue;
>
> - if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
> + if (!psr2_sel_fetch_plane_state_supported(old_crtc_state,
> new_plane_state)) {
> full_update = true;
> break;
> }
> @@ -2861,23 +2870,23 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> if (old_plane_state->uapi.visible) {
> damaged_area.y1 = old_plane_state->uapi.dst.y1;
> damaged_area.y2 = old_plane_state->uapi.dst.y2;
> - clip_area_update(&crtc_state->psr2_su_area,
> &damaged_area,
> - &crtc_state->pipe_src);
> + clip_area_update(&new_crtc_state->psr2_su_area,
> &damaged_area,
> + &new_crtc_state->pipe_src);
> }
>
> if (new_plane_state->uapi.visible) {
> damaged_area.y1 = new_plane_state->uapi.dst.y1;
> damaged_area.y2 = new_plane_state->uapi.dst.y2;
> - clip_area_update(&crtc_state->psr2_su_area,
> &damaged_area,
> - &crtc_state->pipe_src);
> + clip_area_update(&new_crtc_state->psr2_su_area,
> &damaged_area,
> + &new_crtc_state->pipe_src);
> }
> continue;
> } else if (new_plane_state->uapi.alpha !=
> old_plane_state->uapi.alpha) {
> /* If alpha changed mark the whole plane area as
> damaged */
> damaged_area.y1 = new_plane_state->uapi.dst.y1;
> damaged_area.y2 = new_plane_state->uapi.dst.y2;
> - clip_area_update(&crtc_state->psr2_su_area,
> &damaged_area,
> - &crtc_state->pipe_src);
> + clip_area_update(&new_crtc_state->psr2_su_area,
> &damaged_area,
> + &new_crtc_state->pipe_src);
> continue;
> }
>
> @@ -2893,7 +2902,8 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> damaged_area.x1 += new_plane_state->uapi.dst.x1 - src.x1;
> damaged_area.x2 += new_plane_state->uapi.dst.x1 - src.x1;
>
> - clip_area_update(&crtc_state->psr2_su_area, &damaged_area,
> &crtc_state->pipe_src);
> + clip_area_update(&new_crtc_state->psr2_su_area, &damaged_area,
> + &new_crtc_state->pipe_src);
> }
>
> /*
> @@ -2902,7 +2912,7 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> * should identify cases where this happens and fix the area
> * calculation for those.
> */
> - if (crtc_state->psr2_su_area.y1 == -1) {
> + if (new_crtc_state->psr2_su_area.y1 == -1) {
> drm_info_once(display->drm,
> "Selective fetch area calculation failed in pipe
> %c\n",
> pipe_name(crtc->pipe));
> @@ -2912,7 +2922,7 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> if (full_update)
> goto skip_sel_fetch_set_loop;
>
> - intel_psr_apply_su_area_workarounds(crtc_state);
> + intel_psr_apply_su_area_workarounds(new_crtc_state);
>
> ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
> if (ret)
> @@ -2926,7 +2936,7 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> */
> intel_psr2_sel_fetch_et_alignment(state, crtc, &cursor_in_su_area);
>
> - intel_psr2_sel_fetch_pipe_alignment(crtc_state);
> + intel_psr2_sel_fetch_pipe_alignment(new_crtc_state);
>
> /*
> * Now that we have the pipe damaged area check if it intersect with
> @@ -2937,11 +2947,11 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> struct drm_rect *sel_fetch_area, inter;
> struct intel_plane *linked =
> new_plane_state->planar_linked_plane;
>
> - if (new_plane_state->hw.crtc != crtc_state->uapi.crtc ||
> + if (new_plane_state->hw.crtc != new_crtc_state->uapi.crtc ||
> !new_plane_state->uapi.visible)
> continue;
>
> - inter = crtc_state->psr2_su_area;
> + inter = new_crtc_state->psr2_su_area;
> sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst)) {
> sel_fetch_area->y1 = -1;
> @@ -2951,12 +2961,12 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> * disable it
> */
> if
> (drm_rect_height(&old_plane_state->psr2_sel_fetch_area) > 0)
> - crtc_state->update_planes |= BIT(plane->id);
> + new_crtc_state->update_planes |= BIT(plane->id);
>
> continue;
> }
>
> - if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
> + if (!psr2_sel_fetch_plane_state_supported(old_crtc_state,
> new_plane_state)) {
> full_update = true;
> break;
> }
> @@ -2964,7 +2974,7 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
> sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
> - crtc_state->update_planes |= BIT(plane->id);
> + new_crtc_state->update_planes |= BIT(plane->id);
>
> /*
> * Sel_fetch_area is calculated for UV plane. Use
> @@ -2981,14 +2991,14 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
> linked_sel_fetch_area =
> &linked_new_plane_state->psr2_sel_fetch_area;
> linked_sel_fetch_area->y1 = sel_fetch_area->y1;
> linked_sel_fetch_area->y2 = sel_fetch_area->y2;
> - crtc_state->update_planes |= BIT(linked->id);
> + new_crtc_state->update_planes |= BIT(linked->id);
> }
> }
>
> skip_sel_fetch_set_loop:
> - psr2_man_trk_ctl_calc(crtc_state, full_update);
> - crtc_state->pipe_srcsz_early_tpt =
> - psr2_pipe_srcsz_early_tpt_calc(crtc_state, full_update);
> + psr2_man_trk_ctl_calc(new_crtc_state, full_update);
> + new_crtc_state->pipe_srcsz_early_tpt =
> + psr2_pipe_srcsz_early_tpt_calc(new_crtc_state, full_update);
> return 0;
> }
>
> --
> 2.43.0
--
Ville Syrjälä
Intel
