On Mon, 2019-03-04 at 11:00 -0800, Dhinakaran Pandiyan wrote:
> On Mon, 2019-03-04 at 10:40 -0800, Souza, Jose wrote:
> > On Mon, 2019-03-04 at 10:31 -0800, Dhinakaran Pandiyan wrote:
> > > On Fri, 2019-03-01 at 17:34 -0800, José Roberto de Souza wrote:
> > > > Increase the idle frames to activate PSR1 to avoid CRC
> > > > timeouts,
> > > > as
> > > > soon as pipe CRC is enabled it will avoid PSR1 to activate but
> > > > if
> > > > PSR1 is activate before that, hardware goes to lower power
> > > > states
> > > > that inhibits CRC calculations causing CRC timeout errors in
> > > > IGT
> > > > tests.
>
> Okay, so you are solving the case where PSR is already active when we
> want CRCs. How does modifying the idle frame count help if PSR is
> already active?
Exactly this sporadicts timeout should be caused by this:
- IGT test modify frontbuffer or do a flip
- IGT execution is preempted and it takes more than X idle frames
- PSR is activated
- IGT test start CRC capture
- CRC timeout
So increasing the idle frames should reduce the scenario above.
>
> The commit also says PSR does not become active if CRCs were already
> enabled (idle count should not matter here). In that case, just
> disabling and re-enabling should be good, isn't it? Or perhaps, just
> doing a psr_flush() would do the same job?
Good suggestion, psr_flush() should do the same job I will test and
change to that.
>
> > > > 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/i915_drv.h | 1 +
> > > > drivers/gpu/drm/i915/intel_psr.c | 17 +++++++++++++++--
> > > > 2 files changed, 16 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > > b/drivers/gpu/drm/i915/i915_drv.h
> > > > index 453af7438e67..e336f758e481 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -521,6 +521,7 @@ struct i915_psr {
> > > > bool sink_not_reliable;
> > > > bool irq_aux_error;
> > > > u16 su_x_granularity;
> > > > + bool crc_enabled;
> > > > };
> > > >
> > > > enum intel_pch {
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index d3e3996551c6..b237d96db277 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -452,6 +452,16 @@ static void hsw_activate_psr1(struct
> > > > intel_dp
> > > > *intel_dp)
> > > > * frames, we'll go with 9 frames for now
> > > > */
> > > > idle_frames = max(idle_frames, dev_priv-
> > > > >psr.sink_sync_latency
> > > > + 1);
> > > > +
> > > > + /*
> > > > + * Increase the idle frames to active PSR1 to avoid CRC
> > > > timeouts, as
> > > > + * soon as pipe CRC is enabled it will avoid PSR1 to
> > > > activate
> > > > but if
> > > > + * PSR1 is activate before that, hardware goes to lower
> > > > power
> > > > states
> > > > + * that inhibits CRC calculations.
> > > > + */
> > > > + if (dev_priv->psr.crc_enabled)
> > > > + idle_frames = 0xf;
> > >
> > > My understanding was only the enable bit can be changed when PSR
> > > is
> > > enabled. Does this work?
> >
> > That is true, the additional changes in intel_psr_update() will
> > disable
> > and then enable PSR with this new idle_frames.
> >
> > > > +
> > > > val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
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
