Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats

["Souza, Jose" <jose.souza@intel.com>]

On Wed, 2021-09-22 at 15:08 +0300, Gwan-gyeong Mun wrote:
> 
> On 9/22/21 11:28 AM, Manna, Animesh wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of José
> > > Roberto de Souza
> > > Sent: Tuesday, September 21, 2021 6:11 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: Mun, Gwan-gyeong <gwan-gyeong.mun@intel.com>; Souza, Jose
> > > <jose.souza@intel.com>
> > > Subject: [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when
> > > handling biplanar formats
> > > 
> > > From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > 
> > > We are still missing the PSR2 selective fetch handling of biplanar formats but
> > > until proper handle is added we can workaround it by doing full frames fetch
> > > when state has biplanar formats.
> > > 
> > > We need the second check because an update in a regular format could
> > > intersect with a biplanar plane that was not initialy part of the atomic commit.
> > > 
> > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_psr.c | 11 ++++++++++-
> > >   1 file changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 8ceb22c5a1a6b..e6a4c27975d8c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1601,9 +1601,13 @@ int intel_psr2_sel_fetch_update(struct
> > > intel_atomic_state *state,
> > >    * TODO: Not clear how to handle planes with negative
> > > position,
> > >    * also planes are not updated if they have a negative X
> > >    * position so for now doing a full update in this cases
> > > + *
> > > + * TODO: We are missing biplanar formats handling, until it is
> > > + * implemented it will send full frame updates.
> > >    */
> > >   if (new_plane_state->uapi.dst.y1 < 0 ||
> > > -    new_plane_state->uapi.dst.x1 < 0) {
> > > +    new_plane_state->uapi.dst.x1 < 0 ||
> > > +    new_plane_state->hw.fb->format->is_yuv) {
> > >   full_update = true;
> > >   break;
> > >   }
> > > @@ -1682,6 +1686,11 @@ int intel_psr2_sel_fetch_update(struct
> > > intel_atomic_state *state,
> > >   if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
> > >   continue;
> > > 
> > 
> > Code comment can be added here why we need this check again in same function.
> > Enabling full frame update is fine for me for planar format but not sure we need the 2nd check.
> > 
> That's right, we don't need to set this code here because we set 
> full_update above when " new_plane_state->hw.fb->format->is_yuv" is true.

We need it, the reason is on the commit description.

> 
> I will update this in the next version.
> 
> Thanks, Animesh.
> 
> > Regards,
> > Animesh
> > 
> > > +if (new_plane_state->hw.fb->format->is_yuv) {
> > > +full_update = true;
> > > +break;
> > > +}
> > > +
> > >   sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> > >   sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
> > >   sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
> > > --
> > > 2.33.0
> >