From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
To: "José Roberto de Souza" <jose.souza@intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915/display: Match PSR2 selective fetch sequences with specification
Date: Mon, 20 Sep 2021 21:12:00 +0300 [thread overview]
Message-ID: <d3fa328b-8600-0185-49dd-b0da87ac1b93@intel.com> (raw)
In-Reply-To: <20210917205241.231527-3-jose.souza@intel.com>
Looks good to me.
Tested-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
On 9/17/21 11:52 PM, José Roberto de Souza wrote:
> We were not completely following the selective fetch programming
> sequence, here some things we were doing wrong:
> - not programming plane selective fetch a PSR2_MAN_TRK_CTL registers
> when doing a modeset
> - programming PSR2_MAN_TRK_CTL out of vblank
>
> With this changes the last remainig underrun found in Alderlake-P is
> fixed.
>
> Bspec: 55229
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cursor.c | 4 ++-
> drivers/gpu/drm/i915/display/intel_display.c | 12 +++----
> drivers/gpu/drm/i915/display/intel_psr.c | 33 ++++++++++++++-----
> drivers/gpu/drm/i915/display/intel_psr.h | 2 ++
> .../drm/i915/display/skl_universal_plane.c | 4 +--
> 5 files changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> index c7618fef01439..901ad3a4c8c3b 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -536,8 +536,10 @@ static void i9xx_update_cursor(struct intel_plane *plane,
> if (DISPLAY_VER(dev_priv) >= 9)
> skl_write_cursor_wm(plane, crtc_state);
>
> - if (!intel_crtc_needs_modeset(crtc_state))
> + if (plane_state)
> intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, 0);
> + else
> + intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
>
> if (plane->cursor.base != base ||
> plane->cursor.size != fbc_ctl ||
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f6c0c595f6313..224bf622a1c1a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6812,11 +6812,9 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
>
> }
>
> - if (!mode_changed) {
> - ret = intel_psr2_sel_fetch_update(state, crtc);
> - if (ret)
> - return ret;
> - }
> + ret = intel_psr2_sel_fetch_update(state, crtc);
> + if (ret)
> + return ret;
>
> return 0;
> }
> @@ -9709,10 +9707,10 @@ static void commit_pipe_pre_planes(struct intel_atomic_state *state,
>
> if (new_crtc_state->update_pipe)
> intel_pipe_fastset(old_crtc_state, new_crtc_state);
> -
> - intel_psr2_program_trans_man_trk_ctl(new_crtc_state);
> }
>
> + intel_psr2_program_trans_man_trk_ctl(new_crtc_state);
> +
> if (dev_priv->display.atomic_update_watermarks)
> dev_priv->display.atomic_update_watermarks(state, crtc);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index bd13325782f11..101a23bdd7b5b 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -561,15 +561,16 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
> val |= EDP_PSR2_SU_SDP_SCANLINE;
>
> if (intel_dp->psr.psr2_sel_fetch_enabled) {
> + u32 tmp;
> +
> /* Wa_1408330847 */
> if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0))
> intel_de_rmw(dev_priv, CHICKEN_PAR1_1,
> DIS_RAM_BYPASS_PSR2_MAN_TRACK,
> DIS_RAM_BYPASS_PSR2_MAN_TRACK);
>
> - intel_de_write(dev_priv,
> - PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
> - PSR2_MAN_TRK_CTL_ENABLE);
> + tmp = intel_de_read(dev_priv, PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder));
> + drm_WARN_ON(&dev_priv->drm, !(tmp & PSR2_MAN_TRK_CTL_ENABLE));
> } else if (HAS_PSR2_SEL_FETCH(dev_priv)) {
> intel_de_write(dev_priv,
> PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder), 0);
> @@ -1450,6 +1451,18 @@ static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
> intel_de_write(dev_priv, CURSURFLIVE(intel_dp->psr.pipe), 0);
> }
>
> +void intel_psr2_disable_plane_sel_fetch(struct intel_plane *plane,
> + const struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> + enum pipe pipe = plane->pipe;
> +
> + if (!crtc_state->enable_psr2_sel_fetch)
> + return;
> +
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), 0);
> +}
> +
> void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> const struct intel_crtc_state *crtc_state,
> const struct intel_plane_state *plane_state,
> @@ -1464,11 +1477,11 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> if (!crtc_state->enable_psr2_sel_fetch)
> return;
>
> - val = plane_state ? plane_state->ctl : 0;
> - val &= plane->id == PLANE_CURSOR ? val : PLANE_SEL_FETCH_CTL_ENABLE;
> - intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), val);
> - if (!val || plane->id == PLANE_CURSOR)
> + if (plane->id == PLANE_CURSOR) {
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id),
> + plane_state->ctl);
> return;
> + }
>
> clip = &plane_state->psr2_sel_fetch_area;
>
> @@ -1487,14 +1500,16 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> val = (drm_rect_height(clip) - 1) << 16;
> val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1;
> intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane->id), val);
> +
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id),
> + PLANE_SEL_FETCH_CTL_ENABLE);
> }
>
> void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
>
> - if (!HAS_PSR2_SEL_FETCH(dev_priv) ||
> - !crtc_state->enable_psr2_sel_fetch)
> + if (!crtc_state->enable_psr2_sel_fetch)
> return;
>
> intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(crtc_state->cpu_transcoder),
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
> index 641521b101c82..e502964697c62 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -51,6 +51,8 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> const struct intel_crtc_state *crtc_state,
> const struct intel_plane_state *plane_state,
> int color_plane);
> +void intel_psr2_disable_plane_sel_fetch(struct intel_plane *plane,
> + const struct intel_crtc_state *crtc_state);
> void intel_psr_pause(struct intel_dp *intel_dp);
> void intel_psr_resume(struct intel_dp *intel_dp);
>
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 724e7b04f3b63..247155f2a5538 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -656,6 +656,7 @@ skl_disable_plane(struct intel_plane *plane,
>
> skl_write_plane_wm(plane, crtc_state);
>
> + intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
> intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0);
>
> @@ -1096,8 +1097,7 @@ skl_program_plane(struct intel_plane *plane,
> (plane_state->view.color_plane[1].y << 16) |
> plane_state->view.color_plane[1].x);
>
> - if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi))
> - intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane);
> + intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane);
>
> /*
> * Enable the scaler before the plane so that we don't
>
next prev parent reply other threads:[~2021-09-20 18:12 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-17 20:52 [Intel-gfx] [PATCH 1/3] drm/i915/display/dmc: Set DC_STATE_DEBUG_MASK_CORES after firmware load José Roberto de Souza
2021-09-17 20:52 ` [Intel-gfx] [PATCH 2/3] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe José Roberto de Souza
2021-09-23 13:27 ` Gwan-gyeong Mun
2021-09-23 17:25 ` Souza, Jose
2021-09-17 20:52 ` [Intel-gfx] [PATCH 3/3] drm/i915/display: Match PSR2 selective fetch sequences with specification José Roberto de Souza
2021-09-20 18:12 ` Gwan-gyeong Mun [this message]
2021-09-17 21:05 ` [Intel-gfx] [PATCH 1/3] drm/i915/display/dmc: Set DC_STATE_DEBUG_MASK_CORES after firmware load Imre Deak
2021-09-20 19:06 ` Imre Deak
2021-09-17 21:14 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] " Patchwork
2021-09-17 21:37 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-09-17 22:50 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d3fa328b-8600-0185-49dd-b0da87ac1b93@intel.com \
--to=gwan-gyeong.mun@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jose.souza@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox