On Fri, 2022-05-06 at 18:40 +0000, Souza, Jose wrote: > 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. Added some of this into commit message in new version, please check. > > > 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. I left the warn out. There is some case during boot-up at least in Fedora35. I.e. This warning would be there always. How about if I just file own bug for this in gitlab and assign it to myself? > > > > > +if (pipe_clip.y1 == -1) > > > > +full_update = true; > > > > + > > > > if (full_update) > > > > goto skip_sel_fetch_set_loop; > > > >