On Thu, 2021-10-21 at 13:10 +0300, Jouni Högander wrote:
> Biplanar formats are using two planes (Y and UV). This patch adds handling
> of Y selective fetch area by utilizing existing linked plane mechanism.
> Also UV plane Y offset configuration is modified according to Bspec.
>
> Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 30 +++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 49c2dfbd4055..469bf95178f3 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1467,10 +1467,19 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> val |= plane_state->uapi.dst.x1;
> intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane->id), val);
>
> - /* TODO: consider auxiliary surfaces */
> - x = plane_state->uapi.src.x1 >> 16;
> - y = (plane_state->uapi.src.y1 >> 16) + clip->y1;
> + x = plane_state->view.color_plane[color_plane].x;
> +
> + /*
> + * From Bspec: UV surface Start Y Position = half of Y plane Y
> + * start position.
> + */
> + if (!color_plane)
> + y = plane_state->view.color_plane[color_plane].y + clip->y1;
> + else
> + y = plane_state->view.color_plane[color_plane].y + clip->y1 / 2;
Matches bspec but I'm not an expert in colorimetry not sure if it is missing handling for other non-RGB format.
Let ship with it and fix on top but would be nice to ask around if we are missing cases for other formats.
> +
> val = y << 16 | x;
> +
> intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane->id),
> val);
>
> @@ -1700,6 +1709,7 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state,
> new_plane_state, i) {
> struct drm_rect *sel_fetch_area, inter;
> + struct intel_plane *linked = new_plane_state->planar_linked_plane;
>
> if (new_plane_state->uapi.crtc != crtc_state->uapi.crtc ||
> !new_plane_state->uapi.visible)
> @@ -1718,6 +1728,20 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> 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);
> +
> + /*
> + * Sel_fetch_area is calculated for UV plane. Use
> + * same area for Y plane as well.
> + */
> + if (linked) {
> + struct intel_plane_state *linked_new_plane_state =
> + intel_atomic_get_new_plane_state(state, linked);
> + struct drm_rect *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;
Okay this is needed because the slave plane has visible = false.
Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> + }
> }
>
> skip_sel_fetch_set_loop:
