Re: [PATCH v10 6/6] drm/i915/display: force qgv check after the hw state readout

["Govindapillai, Vinod" <vinod.govindapillai@intel.com>]

On Fri, 2024-04-19 at 20:19 +0300, Ville Syrjälä wrote:
> On Fri, Apr 05, 2024 at 02:35:33PM +0300, Vinod Govindapillai wrote:
> > The current intel_bw_atomic_check do not check the possbility
> > of a sagv configuration change after the hw state readout.
> > Hence cannot update the sagv configuration until some other
> > relevant changes like data rates, number of planes etc. happen.
> > Introduce a flag to force qgv check in such cases.
> 
> The correct fix would be to make sure the readout actually
> works, and thus the software state reflects the state of
> the hardare accurately.

Okay! Does that mean, we need to update the qgv points and update to pcode right after the HW
readout?

Right now, on bw_init we force disable sagv, so that we have a known sagv state. Then on
	intel_modeset_readout_hw_state() we update the bw_state parameters. SAGV is still disabled. So on nextmodesetifthebw_stateparametersaresameastheoldm,thenwewillnotbecalculatingtheQGVpoints.Ithinksuchascenariohappensonlyonceaftertheboot.SothatsthereasonIaddedthisflagtoforcecalculateQGV.


BR
Vinod

> 
> > 
> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_bw.c | 8 ++++++--
> >  drivers/gpu/drm/i915/display/intel_bw.h | 6 ++++++
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> > index 6fb228a1a28f..1b190be745a0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -755,6 +755,7 @@ void intel_bw_crtc_update(struct intel_bw_state *bw_state,
> >                 intel_bw_crtc_data_rate(crtc_state);
> >         bw_state->num_active_planes[crtc->pipe] =
> >                 intel_bw_crtc_num_active_planes(crtc_state);
> > +       bw_state->force_check_qgv = true;
> >  
> >         drm_dbg_kms(&i915->drm, "pipe %c data rate %u num active planes %u\n",
> >                     pipe_name(crtc->pipe),
> > @@ -1339,8 +1340,9 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
> >         new_bw_state = intel_atomic_get_new_bw_state(state);
> >  
> >         if (new_bw_state &&
> > -           intel_can_enable_sagv(i915, old_bw_state) !=
> > -           intel_can_enable_sagv(i915, new_bw_state))
> > +           (intel_can_enable_sagv(i915, old_bw_state) !=
> > +            intel_can_enable_sagv(i915, new_bw_state) ||
> > +            new_bw_state->force_check_qgv))
> >                 changed = true;
> >  
> >         /*
> > @@ -1354,6 +1356,8 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
> >         if (ret)
> >                 return ret;
> >  
> > +       new_bw_state->force_check_qgv = false;
> > +
> >         return 0;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
> > index fa1e924ec961..161813cca473 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.h
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> > @@ -47,6 +47,12 @@ struct intel_bw_state {
> >          */
> >         u16 qgv_points_mask;
> >  
> > +       /*
> > +        * Flag to force the QGV comparison in atomic check right after the
> > +        * hw state readout
> > +        */
> > +       bool force_check_qgv;
> > +
> >         int min_cdclk[I915_MAX_PIPES];
> >         unsigned int data_rate[I915_MAX_PIPES];
> >         u8 num_active_planes[I915_MAX_PIPES];
> > -- 
> > 2.34.1
>