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@xxxxxxxxx> > > > Cc: Mika Kahola <mika.kahola@xxxxxxxxx> > > > Tested-by: Mark Pearson <markpearson@xxxxxxxxxx> > > > Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx> > > > --- > > > 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; > > > >
