Re: [Intel-gfx] [PATCH 1/2] drm/i915/psr: Use full update In case of area calculation fails

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Fri, 2022-05-06 at 15:29 +0000, Souza, Jose wrote:
> On Fri, 2022-05-06 at 08:48 +0300, Jouni Högander wrote:
> > Currently we have some corner cases where area calculation
> > fails.  For
> > these sel fetch are calculation ends up having update area as y1 =
> > 0,
> > y2 = 4. Instead of these values safer option is full update.
> 
> Aren't you able to reproduce this scenarios with IGT? So why not
> probably fix the calculations?

There were some discussion with Ville Syrjälä that the proper fix for
this would be to move psr update area calculation into where other
calculations for planes are done. Currently we don't have e.g. proper
offset information available here. I have this in my tasklist, but been
busy with other tracks.

I'm also concerned generally on the first loop possibly ending up with
y1=-1,y2=-1 values due to other reasons as well. So using that
full_update prevents this posibility completely.

If I forget how I originally found this problem with bigfb I think this
backup using full update if something goes wrong is generally a good
idea. Currently it's just using y1=0,y2=4.

> 
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Tested-by: Mark Pearson <markpearson@lenovo.com>
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 06db407e2749..8c099d24de86 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1770,6 +1770,9 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> >  clip_area_update(&pipe_clip, &damaged_area);
> >  }
> > 
> > +if (pipe_clip.y1 == -1)
> > +full_update = true;
> > +
> >  if (full_update)
> >  goto skip_sel_fetch_set_loop;
> >