On Thu, 2019-02-28 at 17:57 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-02-28 at 15:07 -0800, Souza, Jose wrote:
> > On Thu, 2019-02-28 at 18:58 +0200, Ville Syrjälä wrote:
> > > On Wed, Feb 27, 2019 at 05:32:58PM -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.
> > > >
> > > > Oddly that don't happen when PSR1 active, so here it switches
> > > > from
> > > > PSR2 to PSR1 while user is requesting pipe CRC.
> > > >
> > > > Force setting mode_changed as true is necessary to atomic
> > > > checks
> > > > functions compute new PSR state, that is why it was added to
> > > > intel_crtc_crc_prepare().
> > > >
> > > > v3: Reusing intel_crtc_crc_prepare() and crc_enabled
> > > >
> > > > v2: Changed commit description to describe that PSR2 inhibit
> > > > CRC
> > > > calculations.
> > > >
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> > > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > > > ---
> > > > 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 f6d0b2aaffe2..e7ac24c33650 100644
> > > > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> > > > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> > > > @@ -308,6 +308,7 @@ intel_crtc_crc_prepare(struct
> > > > drm_i915_private
> > > > *dev_priv, struct drm_crtc *crtc,
> > > > goto put_state;
> > > > }
> > > >
> > > > + pipe_config->base.mode_changed = pipe_config-
> > > > >crc_enabled !=
> > > > enable;
> > >
> > > Do we really want to set that unconditionally?
> >
> > Without it atomic helpers will return ealier as there is no state
> > changes, I was wondering if the IPS was being applied in the bellow
> > connectors_changed is not set.
> > Anyways to triggers the code paths to disable PSR2 it is needed,
> > and
> > with fastboot enable by default in gen9+ it will do a fastset so
> > not
> > much drawbacks.
> What about pre gen9 platforms that do not have PSR? Running through
> state checks and acquiring several locks on platforms that will never
> need it is wasteful.
>
> Why not make it conditional upon?
> if (HAS_PSR()) or even CAN_PSR() for that matter.
Okay, done.
>
>
>
> > > > pipe_config->crc_enabled = enable;
> > > >
> > > > if (IS_HASWELL(dev_priv) && intel_crtc->pipe == PIPE_A)
> > > > {
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index 6175b1d2e0c8..f7730b8b2ec0 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;
> > > > }
> > > >
> > > > --
> > > > 2.21.0
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
