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

[Souza, Jose]

On Fri, 2022-05-06 at 18:28 +0000, Hogander, Jouni wrote:
> 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.

Okay so please add some of that to the commit description.

> 
> 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 at intel.com>
> > > Cc: Mika Kahola <mika.kahola at intel.com>
> > > Tested-by: Mark Pearson <markpearson at lenovo.com>
> > > Signed-off-by: Jouni Högander <jouni.hogander at 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);
> > >  }
> > > 

Add a TODO and a "drm_DEBUG_ONCE()"(check drm_WARN_ONCE) here so we get a warning about this at least once and this is not forgot.

> > > +if (pipe_clip.y1 == -1)
> > > +full_update = true;
> > > +
> > >  if (full_update)
> > >  goto skip_sel_fetch_set_loop;
> > > 
>