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@xxxxxxxxx>
> > ---
> > 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
>
