| Message ID | 20190306064728.8234-6-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/9] drm/i915/psr: Remove PSR2 FIXME | expand |
On Tue, 2019-03-05 at 22:47 -0800, José Roberto de Souza wrote: > When PSR2 is active aka after the number of frames programmed in > PSR2_CTL 'Frames Before SU Entry' hardware stops to generate CRC > interruptions causing IGT tests to fail due timeout. s/interruptions/interrupts I suppose there are no bugzilla references, since we do not check CRCs in kms_psr and PSR2 is not enabled in kms_frontbuffer_tracking > > This same behavior don't happen with PSR1, as soon as pipe CRC is > enabled it blocks PSR1 activation The hardware also deactivates PSR1, not just block future PSR1 activation. Right? > so CRC calculation continues to > happens normaly. > > This patch also set mode_changed as true when PSR is available to > force atomic check functions to compute new PSR state, otherwise PSR2 > would not be disabled. > > v4: Only setting mode_changed if has_psr is set(Dhinakaran) > > v3: Reusing intel_crtc_crc_prepare() and crc_enabled, only setting > mode_changed if it can do PSR. > > v2: Changed commit description to describe that PSR2 inhibit CRC > calculations. > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/intel_pipe_crc.c | 1 + > drivers/gpu/drm/i915/intel_psr.c | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c > b/drivers/gpu/drm/i915/intel_pipe_crc.c > index af64597c5c6e..c17f02b88453 100644 > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c > @@ -307,6 +307,7 @@ intel_crtc_crc_setup_workarounds(struct > intel_crtc *crtc, bool enable) > goto put_state; > } > > + pipe_config->base.mode_changed = pipe_config->has_psr; > pipe_config->crc_enabled = enable; > > if (IS_HASWELL(dev_priv) && crtc->pipe == PIPE_A) { > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index 2d9f64c362e2..73453d89a841 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -572,6 +572,9 @@ static bool intel_psr2_config_valid(struct > intel_dp *intel_dp, > return false; > } > > + if (crtc_state->crc_enabled) > + return false; No DRM_DEBUG_KMS()? with that added Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > + > return true; > } >
On Thu, 2019-03-07 at 12:47 -0800, Dhinakaran Pandiyan wrote: > On Tue, 2019-03-05 at 22:47 -0800, José Roberto de Souza wrote: > > When PSR2 is active aka after the number of frames programmed in > > PSR2_CTL 'Frames Before SU Entry' hardware stops to generate CRC > > interruptions causing IGT tests to fail due timeout. > s/interruptions/interrupts > > I suppose there are no bugzilla references, since we do not check > CRCs > in kms_psr and PSR2 is not enabled in kms_frontbuffer_tracking Exacly, I have added simple CRC tests in kms_psr to test the expected behavior with PSR1 and PSR2 enabled, will send it after get PSR2 enabled by default. > > This same behavior don't happen with PSR1, as soon as pipe CRC is > > enabled it blocks PSR1 activation > The hardware also deactivates PSR1, not just block future PSR1 > activation. Right? It do not exit PSR1 when writing PIPE CRC registers that is why 'drm/i915: Force PSR exit when getting pipe CRC' is needed. > > > so CRC calculation continues to > > happens normaly. > > > > This patch also set mode_changed as true when PSR is available to > > force atomic check functions to compute new PSR state, otherwise > > PSR2 > > would not be disabled. > > > > v4: Only setting mode_changed if has_psr is set(Dhinakaran) > > > > v3: Reusing intel_crtc_crc_prepare() and crc_enabled, only setting > > mode_changed if it can do PSR. > > > > v2: Changed commit description to describe that PSR2 inhibit CRC > > calculations. > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/intel_pipe_crc.c | 1 + > > drivers/gpu/drm/i915/intel_psr.c | 3 +++ > > 2 files changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c > > b/drivers/gpu/drm/i915/intel_pipe_crc.c > > index af64597c5c6e..c17f02b88453 100644 > > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c > > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c > > @@ -307,6 +307,7 @@ intel_crtc_crc_setup_workarounds(struct > > intel_crtc *crtc, bool enable) > > goto put_state; > > } > > > > + pipe_config->base.mode_changed = pipe_config->has_psr; > > pipe_config->crc_enabled = enable; > > > > if (IS_HASWELL(dev_priv) && crtc->pipe == PIPE_A) { > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > b/drivers/gpu/drm/i915/intel_psr.c > > index 2d9f64c362e2..73453d89a841 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -572,6 +572,9 @@ static bool intel_psr2_config_valid(struct > > intel_dp *intel_dp, > > return false; > > } > > > > + if (crtc_state->crc_enabled) > > + return false; > No DRM_DEBUG_KMS()? Done: DRM_DEBUG_KMS("PSR2 not enabled because it would inhibit pipe CRC calculation\n"); > > with that added > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > + > > return true; > > } > >
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c index af64597c5c6e..c17f02b88453 100644 --- a/drivers/gpu/drm/i915/intel_pipe_crc.c +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c @@ -307,6 +307,7 @@ intel_crtc_crc_setup_workarounds(struct intel_crtc *crtc, bool enable) goto put_state; } + pipe_config->base.mode_changed = pipe_config->has_psr; pipe_config->crc_enabled = enable; if (IS_HASWELL(dev_priv) && crtc->pipe == PIPE_A) { diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 2d9f64c362e2..73453d89a841 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -572,6 +572,9 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; } + if (crtc_state->crc_enabled) + return false; + return true; }