| Message ID | 20250124105625.822459-11-jouni.hogander@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PSR DSB support | expand |
On Fri, Jan 24, 2025 at 12:56:21PM +0200, Jouni Högander wrote: > We have different approach on how flip is considered being complete. We are > waiting for vblank on DSB and generate interrupt when it happens and this > interrupt is considered as indication of completion -> we definitely do not > want to skip vblank wait. > > Also not skipping scanline wait shouldn't cause any problems if we are in > DEEP_SLEEP PIPEDSL register is returning 0 -> evasion does nothing and if > we are not in DEEP_SLEEP evasion works same way as without PSR. > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dsb.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c > index 89d3496bcbdbd..bb77ded8bf726 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsb.c > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c > @@ -168,13 +168,12 @@ static u32 dsb_chicken(struct intel_atomic_state *state, > struct intel_crtc *crtc) > { IIRC bspec suggests that we should always set DSB_SKIP_WAITS_EN, so we should probably have a comment here explaining our reason for omitting it. Otherwise I fear that someone is going to be blindly reading the spec and then try to add the bit back. > if (pre_commit_is_vrr_active(state, crtc)) > - return DSB_SKIP_WAITS_EN | > - DSB_CTRL_WAIT_SAFE_WINDOW | > + return DSB_CTRL_WAIT_SAFE_WINDOW | > DSB_CTRL_NO_WAIT_VBLANK | > DSB_INST_WAIT_SAFE_WINDOW | > DSB_INST_NO_WAIT_VBLANK; > else > - return DSB_SKIP_WAITS_EN; > + return 0; > } > > static bool assert_dsb_has_room(struct intel_dsb *dsb) > -- > 2.43.0
On Fri, 2025-01-24 at 13:46 +0200, Ville Syrjälä wrote: > On Fri, Jan 24, 2025 at 12:56:21PM +0200, Jouni Högander wrote: > > We have different approach on how flip is considered being > > complete. We are > > waiting for vblank on DSB and generate interrupt when it happens > > and this > > interrupt is considered as indication of completion -> we > > definitely do not > > want to skip vblank wait. > > > > Also not skipping scanline wait shouldn't cause any problems if we > > are in > > DEEP_SLEEP PIPEDSL register is returning 0 -> evasion does nothing > > and if > > we are not in DEEP_SLEEP evasion works same way as without PSR. > > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dsb.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c > > b/drivers/gpu/drm/i915/display/intel_dsb.c > > index 89d3496bcbdbd..bb77ded8bf726 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dsb.c > > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c > > @@ -168,13 +168,12 @@ static u32 dsb_chicken(struct > > intel_atomic_state *state, > > struct intel_crtc *crtc) > > { > > IIRC bspec suggests that we should always set DSB_SKIP_WAITS_EN, > so we should probably have a comment here explaining our reason > for omitting it. Otherwise I fear that someone is going to be > blindly reading the spec and then try to add the bit back. Ok, I will add that. BR, Joui Högander > > > if (pre_commit_is_vrr_active(state, crtc)) > > - return DSB_SKIP_WAITS_EN | > > - DSB_CTRL_WAIT_SAFE_WINDOW | > > + return DSB_CTRL_WAIT_SAFE_WINDOW | > > DSB_CTRL_NO_WAIT_VBLANK | > > DSB_INST_WAIT_SAFE_WINDOW | > > DSB_INST_NO_WAIT_VBLANK; > > else > > - return DSB_SKIP_WAITS_EN; > > + return 0; > > } > > > > static bool assert_dsb_has_room(struct intel_dsb *dsb) > > -- > > 2.43.0 >
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c index 89d3496bcbdbd..bb77ded8bf726 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.c +++ b/drivers/gpu/drm/i915/display/intel_dsb.c @@ -168,13 +168,12 @@ static u32 dsb_chicken(struct intel_atomic_state *state, struct intel_crtc *crtc) { if (pre_commit_is_vrr_active(state, crtc)) - return DSB_SKIP_WAITS_EN | - DSB_CTRL_WAIT_SAFE_WINDOW | + return DSB_CTRL_WAIT_SAFE_WINDOW | DSB_CTRL_NO_WAIT_VBLANK | DSB_INST_WAIT_SAFE_WINDOW | DSB_INST_NO_WAIT_VBLANK; else - return DSB_SKIP_WAITS_EN; + return 0; } static bool assert_dsb_has_room(struct intel_dsb *dsb)
We have different approach on how flip is considered being complete. We are waiting for vblank on DSB and generate interrupt when it happens and this interrupt is considered as indication of completion -> we definitely do not want to skip vblank wait. Also not skipping scanline wait shouldn't cause any problems if we are in DEEP_SLEEP PIPEDSL register is returning 0 -> evasion does nothing and if we are not in DEEP_SLEEP evasion works same way as without PSR. Signed-off-by: Jouni Högander <jouni.hogander@intel.com> --- drivers/gpu/drm/i915/display/intel_dsb.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)