| Message ID | 20230127082702.3460571-1-jouni.hogander@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/psr: Split sel fetch plane configuration into arm and noarm | expand |
Hi Jouni, On Fri, 2023-01-27 at 10:27 +0200, Jouni Högander wrote: > SEL_FETCH_CTL registers are armed immediately when plane is disabled. > SEL_FETCH_* instances of plane configuration are used when doing > selective update and normal plane register instances for full updates. > Currently all SEL_FETCH_* registers are written as a part of noarm > plane configuration. If noarm and arm plane configuration are not > happening within same vblank we may end up having plane as a part of > selective update before it's PLANE_SURF register is written. > > Fix this by splitting plane selective fetch configuration into arm and > noarm versions and call them accordingly. Write SEL_FETCH_CTL in arm > version. > > v2: > - drop color_plane parameter from arm part > - dev_priv -> i915 in arm part > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: José Roberto de Souza <jose.souza@intel.com> > Cc: Mika Kahola <mika.kahola@intel.com> > Cc: Vinod Govindapillai <vinod.govindapillai@intel.com> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > Cc: Luca Coelho <luciano.coelho@intel.com> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > --- > drivers/gpu/drm/i915/display/intel_cursor.c | 3 +- > drivers/gpu/drm/i915/display/intel_psr.c | 28 +++++++++++++------ > drivers/gpu/drm/i915/display/intel_psr.h | 6 +++- > .../drm/i915/display/skl_universal_plane.c | 4 ++- > 4 files changed, 30 insertions(+), 11 deletions(-) Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com> BR Vinod > > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c > b/drivers/gpu/drm/i915/display/intel_cursor.c > index d190fa0d393b..ae9f0b6c92db 100644 > --- a/drivers/gpu/drm/i915/display/intel_cursor.c > +++ b/drivers/gpu/drm/i915/display/intel_cursor.c > @@ -532,7 +532,8 @@ static void i9xx_cursor_update_arm(struct intel_plane *plane, > skl_write_cursor_wm(plane, crtc_state); > > if (plane_state) > - intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, 0); > + intel_psr2_program_plane_sel_fetch_arm(plane, crtc_state, > + plane_state); > else > intel_psr2_disable_plane_sel_fetch(plane, crtc_state); > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index 7a72e15e6836..a3f4451eb66d 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1559,7 +1559,25 @@ void intel_psr2_disable_plane_sel_fetch(struct intel_plane *plane, > 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, > +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state) > +{ > + struct drm_i915_private *i915 = to_i915(plane->base.dev); > + enum pipe pipe = plane->pipe; > + > + if (!crtc_state->enable_psr2_sel_fetch) > + return; > + > + if (plane->id == PLANE_CURSOR) > + intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, plane->id), > + plane_state->ctl); > + else > + intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, plane->id), > + PLANE_SEL_FETCH_CTL_ENABLE); > +} > + > +void intel_psr2_program_plane_sel_fetch_noarm(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state, > int color_plane) > @@ -1573,11 +1591,8 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > if (!crtc_state->enable_psr2_sel_fetch) > return; > > - if (plane->id == PLANE_CURSOR) { > - intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), > - plane_state->ctl); > + if (plane->id == PLANE_CURSOR) > return; > - } > > clip = &plane_state->psr2_sel_fetch_area; > > @@ -1605,9 +1620,6 @@ 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) > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h > index 2ac3a46cccc5..c87ae2e6ee6c 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.h > +++ b/drivers/gpu/drm/i915/display/intel_psr.h > @@ -46,10 +46,14 @@ bool intel_psr_enabled(struct intel_dp *intel_dp); > int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > struct intel_crtc *crtc); > void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state); > -void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > +void intel_psr2_program_plane_sel_fetch_noarm(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state, > int color_plane); > +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state); > + > 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); > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c > b/drivers/gpu/drm/i915/display/skl_universal_plane.c > index 9b172a1e90de..296da4bc297c 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -1260,7 +1260,7 @@ icl_plane_update_noarm(struct intel_plane *plane, > if (plane_state->force_black) > icl_plane_csc_load_black(plane); > > - intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane); > + intel_psr2_program_plane_sel_fetch_noarm(plane, crtc_state, plane_state, color_plane); > } > > static void > @@ -1287,6 +1287,8 @@ icl_plane_update_arm(struct intel_plane *plane, > if (plane_state->scaler_id >= 0) > skl_program_plane_scaler(plane, crtc_state, plane_state); > > + intel_psr2_program_plane_sel_fetch_arm(plane, crtc_state, plane_state); > + > /* > * The control register self-arms if the plane was previously > * disabled. Try to make the plane enable atomic by writing
On Fri, 2023-01-27 at 10:27 +0200, Jouni Högander wrote: > SEL_FETCH_CTL registers are armed immediately when plane is disabled. > SEL_FETCH_* instances of plane configuration are used when doing > selective update and normal plane register instances for full updates. > Currently all SEL_FETCH_* registers are written as a part of noarm > plane configuration. If noarm and arm plane configuration are not > happening within same vblank we may end up having plane as a part of > selective update before it's PLANE_SURF register is written. > > Fix this by splitting plane selective fetch configuration into arm and > noarm versions and call them accordingly. Write SEL_FETCH_CTL in arm > version. > > v2: > - drop color_plane parameter from arm part > - dev_priv -> i915 in arm part > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: José Roberto de Souza <jose.souza@intel.com> > Cc: Mika Kahola <mika.kahola@intel.com> > Cc: Vinod Govindapillai <vinod.govindapillai@intel.com> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > Cc: Luca Coelho <luciano.coelho@intel.com> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > --- Looks good to me: Reviewed-by: Luca Coelho <luciano.coelho@intel.com> -- Cheers, Luca.
On Fri, 2023-01-27 at 10:27 +0200, Jouni Högander wrote: > SEL_FETCH_CTL registers are armed immediately when plane is disabled. > SEL_FETCH_* instances of plane configuration are used when doing > selective update and normal plane register instances for full updates. > Currently all SEL_FETCH_* registers are written as a part of noarm > plane configuration. If noarm and arm plane configuration are not > happening within same vblank we may end up having plane as a part of > selective update before it's PLANE_SURF register is written. > > Fix this by splitting plane selective fetch configuration into arm and > noarm versions and call them accordingly. Write SEL_FETCH_CTL in arm > version. Does this helps to revert the set of SFF and CFF at the same time? > > v2: > - drop color_plane parameter from arm part > - dev_priv -> i915 in arm part > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: José Roberto de Souza <jose.souza@intel.com> > Cc: Mika Kahola <mika.kahola@intel.com> > Cc: Vinod Govindapillai <vinod.govindapillai@intel.com> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > Cc: Luca Coelho <luciano.coelho@intel.com> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > --- > drivers/gpu/drm/i915/display/intel_cursor.c | 3 +- > drivers/gpu/drm/i915/display/intel_psr.c | 28 +++++++++++++------ > drivers/gpu/drm/i915/display/intel_psr.h | 6 +++- > .../drm/i915/display/skl_universal_plane.c | 4 ++- > 4 files changed, 30 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c > index d190fa0d393b..ae9f0b6c92db 100644 > --- a/drivers/gpu/drm/i915/display/intel_cursor.c > +++ b/drivers/gpu/drm/i915/display/intel_cursor.c > @@ -532,7 +532,8 @@ static void i9xx_cursor_update_arm(struct intel_plane *plane, > skl_write_cursor_wm(plane, crtc_state); > > if (plane_state) > - intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, 0); > + intel_psr2_program_plane_sel_fetch_arm(plane, crtc_state, > + plane_state); > else > intel_psr2_disable_plane_sel_fetch(plane, crtc_state); Missing rename intel_psr2_disable_plane_sel_fetch() to intel_psr2_disable_plane_sel_fetch_arm(). With this LGTM. Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index 7a72e15e6836..a3f4451eb66d 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1559,7 +1559,25 @@ void intel_psr2_disable_plane_sel_fetch(struct intel_plane *plane, > 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, > +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state) > +{ > + struct drm_i915_private *i915 = to_i915(plane->base.dev); > + enum pipe pipe = plane->pipe; > + > + if (!crtc_state->enable_psr2_sel_fetch) > + return; > + > + if (plane->id == PLANE_CURSOR) > + intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, plane->id), > + plane_state->ctl); > + else > + intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, plane->id), > + PLANE_SEL_FETCH_CTL_ENABLE); > +} > + > +void intel_psr2_program_plane_sel_fetch_noarm(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state, > int color_plane) > @@ -1573,11 +1591,8 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > if (!crtc_state->enable_psr2_sel_fetch) > return; > > - if (plane->id == PLANE_CURSOR) { > - intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), > - plane_state->ctl); > + if (plane->id == PLANE_CURSOR) > return; > - } > > clip = &plane_state->psr2_sel_fetch_area; > > @@ -1605,9 +1620,6 @@ 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) > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h > index 2ac3a46cccc5..c87ae2e6ee6c 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.h > +++ b/drivers/gpu/drm/i915/display/intel_psr.h > @@ -46,10 +46,14 @@ bool intel_psr_enabled(struct intel_dp *intel_dp); > int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > struct intel_crtc *crtc); > void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state); > -void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > +void intel_psr2_program_plane_sel_fetch_noarm(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state, > int color_plane); > +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state); > + > 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); > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c > index 9b172a1e90de..296da4bc297c 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -1260,7 +1260,7 @@ icl_plane_update_noarm(struct intel_plane *plane, > if (plane_state->force_black) > icl_plane_csc_load_black(plane); > > - intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane); > + intel_psr2_program_plane_sel_fetch_noarm(plane, crtc_state, plane_state, color_plane); > } > > static void > @@ -1287,6 +1287,8 @@ icl_plane_update_arm(struct intel_plane *plane, > if (plane_state->scaler_id >= 0) > skl_program_plane_scaler(plane, crtc_state, plane_state); > > + intel_psr2_program_plane_sel_fetch_arm(plane, crtc_state, plane_state); > + > /* > * The control register self-arms if the plane was previously > * disabled. Try to make the plane enable atomic by writing
On Fri, 2023-01-27 at 14:00 +0000, Souza, Jose wrote: > On Fri, 2023-01-27 at 10:27 +0200, Jouni Högander wrote: > > SEL_FETCH_CTL registers are armed immediately when plane is > > disabled. > > SEL_FETCH_* instances of plane configuration are used when doing > > selective update and normal plane register instances for full > > updates. > > Currently all SEL_FETCH_* registers are written as a part of noarm > > plane configuration. If noarm and arm plane configuration are not > > happening within same vblank we may end up having plane as a part > > of > > selective update before it's PLANE_SURF register is written. > > > > Fix this by splitting plane selective fetch configuration into arm > > and > > noarm versions and call them accordingly. Write SEL_FETCH_CTL in > > arm > > version. > > Does this helps to revert the set of SFF and CFF at the same time? No, this one is a separate issue. > > > > > v2: > > - drop color_plane parameter from arm part > > - dev_priv -> i915 in arm part > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: José Roberto de Souza <jose.souza@intel.com> > > Cc: Mika Kahola <mika.kahola@intel.com> > > Cc: Vinod Govindapillai <vinod.govindapillai@intel.com> > > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > Cc: Luca Coelho <luciano.coelho@intel.com> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_cursor.c | 3 +- > > drivers/gpu/drm/i915/display/intel_psr.c | 28 +++++++++++++-- > > ---- > > drivers/gpu/drm/i915/display/intel_psr.h | 6 +++- > > .../drm/i915/display/skl_universal_plane.c | 4 ++- > > 4 files changed, 30 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c > > b/drivers/gpu/drm/i915/display/intel_cursor.c > > index d190fa0d393b..ae9f0b6c92db 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cursor.c > > +++ b/drivers/gpu/drm/i915/display/intel_cursor.c > > @@ -532,7 +532,8 @@ static void i9xx_cursor_update_arm(struct > > intel_plane *plane, > > skl_write_cursor_wm(plane, crtc_state); > > > > if (plane_state) > > - intel_psr2_program_plane_sel_fetch(plane, > > crtc_state, plane_state, 0); > > + intel_psr2_program_plane_sel_fetch_arm(plane, > > crtc_state, > > + > > plane_state); > > else > > intel_psr2_disable_plane_sel_fetch(plane, > > crtc_state); > > Missing rename intel_psr2_disable_plane_sel_fetch() to > intel_psr2_disable_plane_sel_fetch_arm(). Yes, this makes sense. I will update the patch. Thank you for the review. > > With this LGTM. > Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index 7a72e15e6836..a3f4451eb66d 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -1559,7 +1559,25 @@ void > > intel_psr2_disable_plane_sel_fetch(struct intel_plane *plane, > > 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, > > +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane > > *plane, > > + const struct > > intel_crtc_state *crtc_state, > > + const struct > > intel_plane_state *plane_state) > > +{ > > + struct drm_i915_private *i915 = to_i915(plane->base.dev); > > + enum pipe pipe = plane->pipe; > > + > > + if (!crtc_state->enable_psr2_sel_fetch) > > + return; > > + > > + if (plane->id == PLANE_CURSOR) > > + intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, > > plane->id), > > + plane_state->ctl); > > + else > > + intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, > > plane->id), > > + PLANE_SEL_FETCH_CTL_ENABLE); > > +} > > + > > +void intel_psr2_program_plane_sel_fetch_noarm(struct intel_plane > > *plane, > > const struct > > intel_crtc_state *crtc_state, > > const struct > > intel_plane_state *plane_state, > > int color_plane) > > @@ -1573,11 +1591,8 @@ void > > intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > > if (!crtc_state->enable_psr2_sel_fetch) > > return; > > > > - if (plane->id == PLANE_CURSOR) { > > - intel_de_write_fw(dev_priv, > > PLANE_SEL_FETCH_CTL(pipe, plane->id), > > - plane_state->ctl); > > + if (plane->id == PLANE_CURSOR) > > return; > > - } > > > > clip = &plane_state->psr2_sel_fetch_area; > > > > @@ -1605,9 +1620,6 @@ 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) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h > > b/drivers/gpu/drm/i915/display/intel_psr.h > > index 2ac3a46cccc5..c87ae2e6ee6c 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.h > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h > > @@ -46,10 +46,14 @@ bool intel_psr_enabled(struct intel_dp > > *intel_dp); > > int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > > struct intel_crtc *crtc); > > void intel_psr2_program_trans_man_trk_ctl(const struct > > intel_crtc_state *crtc_state); > > -void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > > +void intel_psr2_program_plane_sel_fetch_noarm(struct intel_plane > > *plane, > > const struct > > intel_crtc_state *crtc_state, > > const struct > > intel_plane_state *plane_state, > > int color_plane); > > +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane > > *plane, > > + const struct > > intel_crtc_state *crtc_state, > > + const struct > > intel_plane_state *plane_state); > > + > > 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); > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > index 9b172a1e90de..296da4bc297c 100644 > > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > @@ -1260,7 +1260,7 @@ icl_plane_update_noarm(struct intel_plane > > *plane, > > if (plane_state->force_black) > > icl_plane_csc_load_black(plane); > > > > - intel_psr2_program_plane_sel_fetch(plane, crtc_state, > > plane_state, color_plane); > > + intel_psr2_program_plane_sel_fetch_noarm(plane, crtc_state, > > plane_state, color_plane); > > } > > > > static void > > @@ -1287,6 +1287,8 @@ icl_plane_update_arm(struct intel_plane > > *plane, > > if (plane_state->scaler_id >= 0) > > skl_program_plane_scaler(plane, crtc_state, > > plane_state); > > > > + intel_psr2_program_plane_sel_fetch_arm(plane, crtc_state, > > plane_state); > > + > > /* > > * The control register self-arms if the plane was > > previously > > * disabled. Try to make the plane enable atomic by writing > BR, Jouni Högander
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index d190fa0d393b..ae9f0b6c92db 100644 --- a/drivers/gpu/drm/i915/display/intel_cursor.c +++ b/drivers/gpu/drm/i915/display/intel_cursor.c @@ -532,7 +532,8 @@ static void i9xx_cursor_update_arm(struct intel_plane *plane, skl_write_cursor_wm(plane, crtc_state); if (plane_state) - intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, 0); + intel_psr2_program_plane_sel_fetch_arm(plane, crtc_state, + plane_state); else intel_psr2_disable_plane_sel_fetch(plane, crtc_state); diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 7a72e15e6836..a3f4451eb66d 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1559,7 +1559,25 @@ void intel_psr2_disable_plane_sel_fetch(struct intel_plane *plane, 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, +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state) +{ + struct drm_i915_private *i915 = to_i915(plane->base.dev); + enum pipe pipe = plane->pipe; + + if (!crtc_state->enable_psr2_sel_fetch) + return; + + if (plane->id == PLANE_CURSOR) + intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, plane->id), + plane_state->ctl); + else + intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, plane->id), + PLANE_SEL_FETCH_CTL_ENABLE); +} + +void intel_psr2_program_plane_sel_fetch_noarm(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state, int color_plane) @@ -1573,11 +1591,8 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, if (!crtc_state->enable_psr2_sel_fetch) return; - if (plane->id == PLANE_CURSOR) { - intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), - plane_state->ctl); + if (plane->id == PLANE_CURSOR) return; - } clip = &plane_state->psr2_sel_fetch_area; @@ -1605,9 +1620,6 @@ 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) diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h index 2ac3a46cccc5..c87ae2e6ee6c 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.h +++ b/drivers/gpu/drm/i915/display/intel_psr.h @@ -46,10 +46,14 @@ bool intel_psr_enabled(struct intel_dp *intel_dp); int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, struct intel_crtc *crtc); void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state); -void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, +void intel_psr2_program_plane_sel_fetch_noarm(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state, int color_plane); +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state); + 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); diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index 9b172a1e90de..296da4bc297c 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -1260,7 +1260,7 @@ icl_plane_update_noarm(struct intel_plane *plane, if (plane_state->force_black) icl_plane_csc_load_black(plane); - intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane); + intel_psr2_program_plane_sel_fetch_noarm(plane, crtc_state, plane_state, color_plane); } static void @@ -1287,6 +1287,8 @@ icl_plane_update_arm(struct intel_plane *plane, if (plane_state->scaler_id >= 0) skl_program_plane_scaler(plane, crtc_state, plane_state); + intel_psr2_program_plane_sel_fetch_arm(plane, crtc_state, plane_state); + /* * The control register self-arms if the plane was previously * disabled. Try to make the plane enable atomic by writing
SEL_FETCH_CTL registers are armed immediately when plane is disabled. SEL_FETCH_* instances of plane configuration are used when doing selective update and normal plane register instances for full updates. Currently all SEL_FETCH_* registers are written as a part of noarm plane configuration. If noarm and arm plane configuration are not happening within same vblank we may end up having plane as a part of selective update before it's PLANE_SURF register is written. Fix this by splitting plane selective fetch configuration into arm and noarm versions and call them accordingly. Write SEL_FETCH_CTL in arm version. v2: - drop color_plane parameter from arm part - dev_priv -> i915 in arm part Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: José Roberto de Souza <jose.souza@intel.com> Cc: Mika Kahola <mika.kahola@intel.com> Cc: Vinod Govindapillai <vinod.govindapillai@intel.com> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Cc: Luca Coelho <luciano.coelho@intel.com> Signed-off-by: Jouni Högander <jouni.hogander@intel.com> --- drivers/gpu/drm/i915/display/intel_cursor.c | 3 +- drivers/gpu/drm/i915/display/intel_psr.c | 28 +++++++++++++------ drivers/gpu/drm/i915/display/intel_psr.h | 6 +++- .../drm/i915/display/skl_universal_plane.c | 4 ++- 4 files changed, 30 insertions(+), 11 deletions(-)