Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/display: Program PSR2 selective fetch registers

2020-09-24 Thread Souza, Jose 
On Thu, 2020-09-24 at 14:27 +0100, Mun, Gwan-gyeong wrote:
> On Wed, 2020-09-23 at 10:09 -0700, Souza, Jose wrote:
> > On Wed, 2020-09-23 at 10:10 -0700, José Roberto de Souza wrote:
> > > On Wed, 2020-09-23 at 14:02 +0100, Mun, Gwan-gyeong wrote:
> > > > On Thu, 2020-09-17 at 18:02 -0700, José Roberto de Souza wrote:
> > > > > Another step towards PSR2 selective fetch, here programming
> > > > > plane
> > > > > selective fetch registers and MAN_TRK_CTL enabling selective
> > > > > fetch
> > > > > but
> > > > > for now it is fetching the whole area of the planes.
> > > > > The damaged area calculation will come as next and final step.
> > > > > 
> > > > > v2:
> > > > > - removed warn on when no plane is visible in state
> > > > > - removed calculations using plane damaged area in
> > > > > intel_psr2_program_plane_sel_fetch()
> > > > > 
> > > > > v3:
> > > > > - do not shift 16 positions the plane dst coordinates, only src
> > > > > is
> > > > > shifted
> > > > > 
> > > > > BSpec: 55229
> > > > > Cc: Gwan-gyeong Mun <
> > > > > gwan-gyeong@intel.com
> > > > > 
> > > > > 
> > > > > 
> > > > > Cc: Ville Syrjälä <
> > > > > ville.syrj...@linux.intel.com
> > > > > 
> > > > > 
> > > > > 
> > > > > Signed-off-by: José Roberto de Souza <
> > > > > jose.so...@intel.com
> > > > > 
> > > > > 
> > > > > 
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_display.c |  10 +-
> > > > >  drivers/gpu/drm/i915/display/intel_psr.c | 120
> > > > > ++-
> > > > >  drivers/gpu/drm/i915/display/intel_psr.h |  10 +-
> > > > >  drivers/gpu/drm/i915/display/intel_sprite.c  |   3 +
> > > > >  4 files changed, 134 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index 5a9d933e425a..96bc515497c1 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct
> > > > > intel_plane *plane,
> > > > >   if (INTEL_GEN(dev_priv) >= 9)
> > > > >   skl_write_cursor_wm(plane, crtc_state);
> > > > >  
> > > > > + if (!needs_modeset(crtc_state))
> > > > > + intel_psr2_program_plane_sel_fetch(plane,
> > > > > crtc_state,
> > > > > plane_state, 0);
> > > > > +
> > > > >   if (plane->cursor.base != base ||
> > > > >   plane->cursor.size != fbc_ctl ||
> > > > >   plane->cursor.cntl != cntl) {
> > > > > @@ -12823,8 +12826,11 @@ static int
> > > > > intel_crtc_atomic_check(struct
> > > > > intel_atomic_state *state,
> > > > >  
> > > > >   }
> > > > >  
> > > > > - if (!mode_changed)
> > > > > - intel_psr2_sel_fetch_update(state, crtc);
> > > > > + if (!mode_changed) {
> > > > > + ret = intel_psr2_sel_fetch_update(state, crtc);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > + }
> > > > >  
> > > > >   return 0;
> > > > >  }
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > index 02f74b0ddec1..deb0523f9f29 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > @@ -1166,6 +1166,41 @@ static void
> > > > > psr_force_hw_tracking_exit(struct
> > > > > drm_i915_private *dev_priv)
> > > > >   intel_psr_exit(dev_priv);
> > > > >  }
> > > > >  
> > > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane
> > > > > *plane,
> > > > > + const struct
> > > > > intel_crtc_state
> > > > > *crtc_state,
> > > > > + const struct
> > > > > intel_plane_state
> > > > > *plane_state,
> > > > > + int color_plane)
> > > > > +{
> > > > > + struct drm_i915_private *dev_priv = to_i915(plane-
> > > > > > base.dev);
> > > > > 
> > > > > + enum pipe pipe = plane->pipe;
> > > > > + u32 val;
> > > > > +
> > > > > + if (!crtc_state->enable_psr2_sel_fetch)
> > > > > + return;
> > > > > +
> > > > > + /*
> > > > > +  * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0
> > > > > for
> > > > > gen11+, so
> > > > > +  * plane_state->ctl is the right value
> > > > > +  */
> > > > > + val = plane_state ? plane_state->ctl : 0;
> > > > 
> > > > IMHO, skl_plane_ctl() might set other ctl bits, it would be
> > > > better to
> > > > have separated ctl bit value for "selective fetch ctl".
> > > 
> > > Like said all other bits are spares so can be set without issues
> > > but okay will a "plane_state->ctl & PLANE_SEL_FETCH_CTL_ENABLE".
> > 
> > Please take a look to the answer of your other comment bellow, with
> > the change above can I have your rv-b?
> 
> The purpose and composition of bits of Register_SEL_FETCH_PLANE_CTL is
> different

Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/display: Program PSR2 selective fetch registers

2020-09-24 Thread Mun, Gwan-gyeong 
On Wed, 2020-09-23 at 10:09 -0700, Souza, Jose wrote:
> On Wed, 2020-09-23 at 10:10 -0700, José Roberto de Souza wrote:
> > On Wed, 2020-09-23 at 14:02 +0100, Mun, Gwan-gyeong wrote:
> > > On Thu, 2020-09-17 at 18:02 -0700, José Roberto de Souza wrote:
> > > > Another step towards PSR2 selective fetch, here programming
> > > > plane
> > > > selective fetch registers and MAN_TRK_CTL enabling selective
> > > > fetch
> > > > but
> > > > for now it is fetching the whole area of the planes.
> > > > The damaged area calculation will come as next and final step.
> > > > 
> > > > v2:
> > > > - removed warn on when no plane is visible in state
> > > > - removed calculations using plane damaged area in
> > > > intel_psr2_program_plane_sel_fetch()
> > > > 
> > > > v3:
> > > > - do not shift 16 positions the plane dst coordinates, only src
> > > > is
> > > > shifted
> > > > 
> > > > BSpec: 55229
> > > > Cc: Gwan-gyeong Mun <
> > > > gwan-gyeong@intel.com
> > > > 
> > > > 
> > > > Cc: Ville Syrjälä <
> > > > ville.syrj...@linux.intel.com
> > > > 
> > > > 
> > > > Signed-off-by: José Roberto de Souza <
> > > > jose.so...@intel.com
> > > > 
> > > > 
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_display.c |  10 +-
> > > >  drivers/gpu/drm/i915/display/intel_psr.c | 120
> > > > ++-
> > > >  drivers/gpu/drm/i915/display/intel_psr.h |  10 +-
> > > >  drivers/gpu/drm/i915/display/intel_sprite.c  |   3 +
> > > >  4 files changed, 134 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 5a9d933e425a..96bc515497c1 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct
> > > > intel_plane *plane,
> > > > if (INTEL_GEN(dev_priv) >= 9)
> > > > skl_write_cursor_wm(plane, crtc_state);
> > > >  
> > > > +   if (!needs_modeset(crtc_state))
> > > > +   intel_psr2_program_plane_sel_fetch(plane,
> > > > crtc_state,
> > > > plane_state, 0);
> > > > +
> > > > if (plane->cursor.base != base ||
> > > > plane->cursor.size != fbc_ctl ||
> > > > plane->cursor.cntl != cntl) {
> > > > @@ -12823,8 +12826,11 @@ static int
> > > > intel_crtc_atomic_check(struct
> > > > intel_atomic_state *state,
> > > >  
> > > > }
> > > >  
> > > > -   if (!mode_changed)
> > > > -   intel_psr2_sel_fetch_update(state, crtc);
> > > > +   if (!mode_changed) {
> > > > +   ret = intel_psr2_sel_fetch_update(state, crtc);
> > > > +   if (ret)
> > > > +   return ret;
> > > > +   }
> > > >  
> > > > return 0;
> > > >  }
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 02f74b0ddec1..deb0523f9f29 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -1166,6 +1166,41 @@ static void
> > > > psr_force_hw_tracking_exit(struct
> > > > drm_i915_private *dev_priv)
> > > > intel_psr_exit(dev_priv);
> > > >  }
> > > >  
> > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane
> > > > *plane,
> > > > +   const struct
> > > > intel_crtc_state
> > > > *crtc_state,
> > > > +   const struct
> > > > intel_plane_state
> > > > *plane_state,
> > > > +   int color_plane)
> > > > +{
> > > > +   struct drm_i915_private *dev_priv = to_i915(plane-
> > > > >base.dev);
> > > > +   enum pipe pipe = plane->pipe;
> > > > +   u32 val;
> > > > +
> > > > +   if (!crtc_state->enable_psr2_sel_fetch)
> > > > +   return;
> > > > +
> > > > +   /*
> > > > +* skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0
> > > > for
> > > > gen11+, so
> > > > +* plane_state->ctl is the right value
> > > > +*/
> > > > +   val = plane_state ? plane_state->ctl : 0;
> > > 
> > > IMHO, skl_plane_ctl() might set other ctl bits, it would be
> > > better to
> > > have separated ctl bit value for "selective fetch ctl".
> > 
> > Like said all other bits are spares so can be set without issues
> > but okay will a "plane_state->ctl & PLANE_SEL_FETCH_CTL_ENABLE".
> 
> Please take a look to the answer of your other comment bellow, with
> the change above can I have your rv-b?

The purpose and composition of bits of Register_SEL_FETCH_PLANE_CTL is
different from Register_PLANE_CTL.
And the Spares bits of Register_SEL_FETCH_PLANE_CTL might be used for
other purpose.
(And we current don't know the side effect of setting of Spares bit of
Register_SEL_FETCH_PLANE_CTL )
therefor I recommend "Read and Modify" to SEL_FETCH_PLANE_CTL.
> 
> > > > +

Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/display: Program PSR2 selective fetch registers

2020-09-23 Thread Souza, Jose 
On Wed, 2020-09-23 at 10:10 -0700, José Roberto de Souza wrote:
> On Wed, 2020-09-23 at 14:02 +0100, Mun, Gwan-gyeong wrote:
> > On Thu, 2020-09-17 at 18:02 -0700, José Roberto de Souza wrote:
> > > Another step towards PSR2 selective fetch, here programming plane
> > > selective fetch registers and MAN_TRK_CTL enabling selective fetch
> > > but
> > > for now it is fetching the whole area of the planes.
> > > The damaged area calculation will come as next and final step.
> > > 
> > > v2:
> > > - removed warn on when no plane is visible in state
> > > - removed calculations using plane damaged area in
> > > intel_psr2_program_plane_sel_fetch()
> > > 
> > > v3:
> > > - do not shift 16 positions the plane dst coordinates, only src is
> > > shifted
> > > 
> > > BSpec: 55229
> > > Cc: Gwan-gyeong Mun <
> > > gwan-gyeong@intel.com
> > > 
> > > 
> > > Cc: Ville Syrjälä <
> > > ville.syrj...@linux.intel.com
> > > 
> > > 
> > > Signed-off-by: José Roberto de Souza <
> > > jose.so...@intel.com
> > > 
> > > 
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c |  10 +-
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 120
> > > ++-
> > >  drivers/gpu/drm/i915/display/intel_psr.h |  10 +-
> > >  drivers/gpu/drm/i915/display/intel_sprite.c  |   3 +
> > >  4 files changed, 134 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 5a9d933e425a..96bc515497c1 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct
> > > intel_plane *plane,
> > >   if (INTEL_GEN(dev_priv) >= 9)
> > >   skl_write_cursor_wm(plane, crtc_state);
> > >  
> > > + if (!needs_modeset(crtc_state))
> > > + intel_psr2_program_plane_sel_fetch(plane, crtc_state,
> > > plane_state, 0);
> > > +
> > >   if (plane->cursor.base != base ||
> > >   plane->cursor.size != fbc_ctl ||
> > >   plane->cursor.cntl != cntl) {
> > > @@ -12823,8 +12826,11 @@ static int intel_crtc_atomic_check(struct
> > > intel_atomic_state *state,
> > >  
> > >   }
> > >  
> > > - if (!mode_changed)
> > > - intel_psr2_sel_fetch_update(state, crtc);
> > > + if (!mode_changed) {
> > > + ret = intel_psr2_sel_fetch_update(state, crtc);
> > > + if (ret)
> > > + return ret;
> > > + }
> > >  
> > >   return 0;
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 02f74b0ddec1..deb0523f9f29 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1166,6 +1166,41 @@ static void psr_force_hw_tracking_exit(struct
> > > drm_i915_private *dev_priv)
> > >   intel_psr_exit(dev_priv);
> > >  }
> > >  
> > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> > > + const struct intel_crtc_state
> > > *crtc_state,
> > > + const struct intel_plane_state
> > > *plane_state,
> > > + int color_plane)
> > > +{
> > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > > + enum pipe pipe = plane->pipe;
> > > + u32 val;
> > > +
> > > + if (!crtc_state->enable_psr2_sel_fetch)
> > > + return;
> > > +
> > > + /*
> > > +  * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for
> > > gen11+, so
> > > +  * plane_state->ctl is the right value
> > > +  */
> > > + val = plane_state ? plane_state->ctl : 0;
> > 
> > IMHO, skl_plane_ctl() might set other ctl bits, it would be better to
> > have separated ctl bit value for "selective fetch ctl".
> 
> Like said all other bits are spares so can be set without issues but okay 
> will a "plane_state->ctl & PLANE_SEL_FETCH_CTL_ENABLE".

Please take a look to the answer of your other comment bellow, with the change 
above can I have your rv-b?

> 
> > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane-
> > > > id), val);
> > > 
> > > + if (!val || plane->id == PLANE_CURSOR)
> > > + return;
> > > +
> > > + val = plane_state->uapi.dst.y1 << 16 | plane_state-
> > > > uapi.dst.x1;
> > > 
> > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane-
> > > > id), val);
> > > 
> > > +
> > > + val = plane_state->color_plane[color_plane].y << 16;
> > > + val |= plane_state->color_plane[color_plane].x;
> > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane-
> > > > id),
> > > 
> > > +   val);
> > > +
> > > + /* Sizes are 0 based */
> > > + val = ((drm_rect_height(_state->uapi.src) >> 16) - 1) <<
> > > 16;
> > > + val |= (drm_rect_width(_state->uapi.src) >> 16) - 1;
> > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane-
> > > > id), val);
> > > 
> > > +}
> > > +
> > >  void

Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/display: Program PSR2 selective fetch registers

2020-09-23 Thread Souza, Jose 
On Wed, 2020-09-23 at 14:02 +0100, Mun, Gwan-gyeong wrote:
> On Thu, 2020-09-17 at 18:02 -0700, José Roberto de Souza wrote:
> > Another step towards PSR2 selective fetch, here programming plane
> > selective fetch registers and MAN_TRK_CTL enabling selective fetch
> > but
> > for now it is fetching the whole area of the planes.
> > The damaged area calculation will come as next and final step.
> > 
> > v2:
> > - removed warn on when no plane is visible in state
> > - removed calculations using plane damaged area in
> > intel_psr2_program_plane_sel_fetch()
> > 
> > v3:
> > - do not shift 16 positions the plane dst coordinates, only src is
> > shifted
> > 
> > BSpec: 55229
> > Cc: Gwan-gyeong Mun <
> > gwan-gyeong@intel.com
> > >
> > Cc: Ville Syrjälä <
> > ville.syrj...@linux.intel.com
> > >
> > Signed-off-by: José Roberto de Souza <
> > jose.so...@intel.com
> > >
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c |  10 +-
> >  drivers/gpu/drm/i915/display/intel_psr.c | 120
> > ++-
> >  drivers/gpu/drm/i915/display/intel_psr.h |  10 +-
> >  drivers/gpu/drm/i915/display/intel_sprite.c  |   3 +
> >  4 files changed, 134 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 5a9d933e425a..96bc515497c1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct
> > intel_plane *plane,
> > if (INTEL_GEN(dev_priv) >= 9)
> > skl_write_cursor_wm(plane, crtc_state);
> >  
> > +   if (!needs_modeset(crtc_state))
> > +   intel_psr2_program_plane_sel_fetch(plane, crtc_state,
> > plane_state, 0);
> > +
> > if (plane->cursor.base != base ||
> > plane->cursor.size != fbc_ctl ||
> > plane->cursor.cntl != cntl) {
> > @@ -12823,8 +12826,11 @@ static int intel_crtc_atomic_check(struct
> > intel_atomic_state *state,
> >  
> > }
> >  
> > -   if (!mode_changed)
> > -   intel_psr2_sel_fetch_update(state, crtc);
> > +   if (!mode_changed) {
> > +   ret = intel_psr2_sel_fetch_update(state, crtc);
> > +   if (ret)
> > +   return ret;
> > +   }
> >  
> > return 0;
> >  }
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 02f74b0ddec1..deb0523f9f29 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1166,6 +1166,41 @@ static void psr_force_hw_tracking_exit(struct
> > drm_i915_private *dev_priv)
> > intel_psr_exit(dev_priv);
> >  }
> >  
> > +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> > +   const struct intel_crtc_state
> > *crtc_state,
> > +   const struct intel_plane_state
> > *plane_state,
> > +   int color_plane)
> > +{
> > +   struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > +   enum pipe pipe = plane->pipe;
> > +   u32 val;
> > +
> > +   if (!crtc_state->enable_psr2_sel_fetch)
> > +   return;
> > +
> > +   /*
> > +* skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for
> > gen11+, so
> > +* plane_state->ctl is the right value
> > +*/
> > +   val = plane_state ? plane_state->ctl : 0;
> 
> IMHO, skl_plane_ctl() might set other ctl bits, it would be better to
> have separated ctl bit value for "selective fetch ctl".

Like said all other bits are spares so can be set without issues but okay will 
a "plane_state->ctl & PLANE_SEL_FETCH_CTL_ENABLE".

> > +   intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane-
> > > id), val);
> > 
> > +   if (!val || plane->id == PLANE_CURSOR)
> > +   return;
> > +
> > +   val = plane_state->uapi.dst.y1 << 16 | plane_state-
> > > uapi.dst.x1;
> > 
> > +   intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane-
> > > id), val);
> > 
> > +
> > +   val = plane_state->color_plane[color_plane].y << 16;
> > +   val |= plane_state->color_plane[color_plane].x;
> > +   intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane-
> > > id),
> > 
> > + val);
> > +
> > +   /* Sizes are 0 based */
> > +   val = ((drm_rect_height(_state->uapi.src) >> 16) - 1) <<
> > 16;
> > +   val |= (drm_rect_width(_state->uapi.src) >> 16) - 1;
> > +   intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane-
> > > id), val);
> > 
> > +}
> > +
> >  void intel_psr2_program_trans_man_trk_ctl(const struct
> > intel_crtc_state *crtc_state)
> >  {
> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > @@ -1180,16 +1215,91 @@ void
> > intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state
> > *crtc_st
> >crtc_state->psr2_man_track_ctl);
> >  }
> >  
> > -void

Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/display: Program PSR2 selective fetch registers

2020-09-23 Thread Mun, Gwan-gyeong 
On Thu, 2020-09-17 at 18:02 -0700, José Roberto de Souza wrote:
> Another step towards PSR2 selective fetch, here programming plane
> selective fetch registers and MAN_TRK_CTL enabling selective fetch
> but
> for now it is fetching the whole area of the planes.
> The damaged area calculation will come as next and final step.
> 
> v2:
> - removed warn on when no plane is visible in state
> - removed calculations using plane damaged area in
> intel_psr2_program_plane_sel_fetch()
> 
> v3:
> - do not shift 16 positions the plane dst coordinates, only src is
> shifted
> 
> BSpec: 55229
> Cc: Gwan-gyeong Mun 
> Cc: Ville Syrjälä 
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  10 +-
>  drivers/gpu/drm/i915/display/intel_psr.c | 120
> ++-
>  drivers/gpu/drm/i915/display/intel_psr.h |  10 +-
>  drivers/gpu/drm/i915/display/intel_sprite.c  |   3 +
>  4 files changed, 134 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 5a9d933e425a..96bc515497c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct
> intel_plane *plane,
>   if (INTEL_GEN(dev_priv) >= 9)
>   skl_write_cursor_wm(plane, crtc_state);
>  
> + if (!needs_modeset(crtc_state))
> + intel_psr2_program_plane_sel_fetch(plane, crtc_state,
> plane_state, 0);
> +
>   if (plane->cursor.base != base ||
>   plane->cursor.size != fbc_ctl ||
>   plane->cursor.cntl != cntl) {
> @@ -12823,8 +12826,11 @@ static int intel_crtc_atomic_check(struct
> intel_atomic_state *state,
>  
>   }
>  
> - if (!mode_changed)
> - intel_psr2_sel_fetch_update(state, crtc);
> + if (!mode_changed) {
> + ret = intel_psr2_sel_fetch_update(state, crtc);
> + if (ret)
> + return ret;
> + }
>  
>   return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 02f74b0ddec1..deb0523f9f29 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1166,6 +1166,41 @@ static void psr_force_hw_tracking_exit(struct
> drm_i915_private *dev_priv)
>   intel_psr_exit(dev_priv);
>  }
>  
> +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> + const struct intel_crtc_state
> *crtc_state,
> + const struct intel_plane_state
> *plane_state,
> + int color_plane)
> +{
> + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> + enum pipe pipe = plane->pipe;
> + u32 val;
> +
> + if (!crtc_state->enable_psr2_sel_fetch)
> + return;
> +
> + /*
> +  * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for
> gen11+, so
> +  * plane_state->ctl is the right value
> +  */
> + val = plane_state ? plane_state->ctl : 0;
IMHO, skl_plane_ctl() might set other ctl bits, it would be better to
have separated ctl bit value for "selective fetch ctl".
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane-
> >id), val);
> + if (!val || plane->id == PLANE_CURSOR)
> + return;
> +
> + val = plane_state->uapi.dst.y1 << 16 | plane_state-
> >uapi.dst.x1;
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane-
> >id), val);
> +
> + val = plane_state->color_plane[color_plane].y << 16;
> + val |= plane_state->color_plane[color_plane].x;
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane-
> >id),
> +   val);
> +
> + /* Sizes are 0 based */
> + val = ((drm_rect_height(_state->uapi.src) >> 16) - 1) <<
> 16;
> + val |= (drm_rect_width(_state->uapi.src) >> 16) - 1;
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane-
> >id), val);
> +}
> +
>  void intel_psr2_program_trans_man_trk_ctl(const struct
> intel_crtc_state *crtc_state)
>  {
>   struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> @@ -1180,16 +1215,91 @@ void
> intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state
> *crtc_st
>  crtc_state->psr2_man_track_ctl);
>  }
>  
> -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> -  struct intel_crtc *crtc)
> +static void psr2_man_trk_ctl_calc(struct intel_crtc_state
> *crtc_state,
> +   struct drm_rect *clip, bool
> full_update)
> +{
> + u32 val = PSR2_MAN_TRK_CTL_ENABLE;
> +
> + if (full_update) {
> + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> + goto exit;
> + }
> +
> + if (clip->y1 == -1)
> + goto exit;
> +
> + val |=

[Intel-gfx] [PATCH v3 3/3] drm/i915/display: Program PSR2 selective fetch registers

2020-09-17 Thread José Roberto de Souza 
Another step towards PSR2 selective fetch, here programming plane
selective fetch registers and MAN_TRK_CTL enabling selective fetch but
for now it is fetching the whole area of the planes.
The damaged area calculation will come as next and final step.

v2:
- removed warn on when no plane is visible in state
- removed calculations using plane damaged area in
intel_psr2_program_plane_sel_fetch()

v3:
- do not shift 16 positions the plane dst coordinates, only src is shifted

BSpec: 55229
Cc: Gwan-gyeong Mun 
Cc: Ville Syrjälä 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/display/intel_display.c |  10 +-
 drivers/gpu/drm/i915/display/intel_psr.c | 120 ++-
 drivers/gpu/drm/i915/display/intel_psr.h |  10 +-
 drivers/gpu/drm/i915/display/intel_sprite.c  |   3 +
 4 files changed, 134 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 5a9d933e425a..96bc515497c1 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct intel_plane 
*plane,
if (INTEL_GEN(dev_priv) >= 9)
skl_write_cursor_wm(plane, crtc_state);
 
+   if (!needs_modeset(crtc_state))
+   intel_psr2_program_plane_sel_fetch(plane, crtc_state, 
plane_state, 0);
+
if (plane->cursor.base != base ||
plane->cursor.size != fbc_ctl ||
plane->cursor.cntl != cntl) {
@@ -12823,8 +12826,11 @@ static int intel_crtc_atomic_check(struct 
intel_atomic_state *state,
 
}
 
-   if (!mode_changed)
-   intel_psr2_sel_fetch_update(state, crtc);
+   if (!mode_changed) {
+   ret = intel_psr2_sel_fetch_update(state, crtc);
+   if (ret)
+   return ret;
+   }
 
return 0;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c 
b/drivers/gpu/drm/i915/display/intel_psr.c
index 02f74b0ddec1..deb0523f9f29 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1166,6 +1166,41 @@ static void psr_force_hw_tracking_exit(struct 
drm_i915_private *dev_priv)
intel_psr_exit(dev_priv);
 }
 
+void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
+   const struct intel_crtc_state 
*crtc_state,
+   const struct intel_plane_state 
*plane_state,
+   int color_plane)
+{
+   struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+   enum pipe pipe = plane->pipe;
+   u32 val;
+
+   if (!crtc_state->enable_psr2_sel_fetch)
+   return;
+
+   /*
+* skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for gen11+, so
+* plane_state->ctl is the right value
+*/
+   val = plane_state ? plane_state->ctl : 0;
+   intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), val);
+   if (!val || plane->id == PLANE_CURSOR)
+   return;
+
+   val = plane_state->uapi.dst.y1 << 16 | plane_state->uapi.dst.x1;
+   intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane->id), val);
+
+   val = plane_state->color_plane[color_plane].y << 16;
+   val |= plane_state->color_plane[color_plane].x;
+   intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane->id),
+ val);
+
+   /* Sizes are 0 based */
+   val = ((drm_rect_height(_state->uapi.src) >> 16) - 1) << 16;
+   val |= (drm_rect_width(_state->uapi.src) >> 16) - 1;
+   intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane->id), val);
+}
+
 void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state 
*crtc_state)
 {
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -1180,16 +1215,91 @@ void intel_psr2_program_trans_man_trk_ctl(const struct 
intel_crtc_state *crtc_st
   crtc_state->psr2_man_track_ctl);
 }
 
-void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
-struct intel_crtc *crtc)
+static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
+ struct drm_rect *clip, bool full_update)
+{
+   u32 val = PSR2_MAN_TRK_CTL_ENABLE;
+
+   if (full_update) {
+   val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
+   goto exit;
+   }
+
+   if (clip->y1 == -1)
+   goto exit;
+
+   val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
+   val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
+   val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip->y2, 4) + 
1);
+exit:
+   crtc_state->psr2_man_track_ctl = val;
+}
+
+static void clip_area_update(struct drm_rect *overlap_damage_area,
+struct