On Tue, 24 Oct 2023, Stanislav Lisovskiy <[email protected]> wrote: > In some customer cases, machine can start up with all > GV points restricted. However we don't ever read those > from hw and initial driver qgv_points_mask is initialized > as 0, which would make driver think that all points are unrestricted, > so we never update them with proper value, unless > some demanding scenario is requested or we have to toggle SAGV > and we have to restrict some of those. > Lets fix that by initializing all points as restricted, > then on first modeset, that will make sure driver will naturally > calculate, which of those need to be relaxed and do correspondent update. > > Signed-off-by: Stanislav Lisovskiy <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_bw.c | 7 ++++--- > drivers/gpu/drm/i915/display/intel_bw.h | 1 + > drivers/gpu/drm/i915/display/intel_modeset_setup.c | 13 +++++++++++++ > 3 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c > b/drivers/gpu/drm/i915/display/intel_bw.c > index bef96db62c80..fbfa01f38db8 100644 > --- a/drivers/gpu/drm/i915/display/intel_bw.c > +++ b/drivers/gpu/drm/i915/display/intel_bw.c > @@ -119,7 +119,7 @@ static int adls_pcode_read_psf_gv_point_info(struct > drm_i915_private *dev_priv, > return 0; > } > > -static u16 icl_qgv_points_mask(struct drm_i915_private *i915) > +u16 icl_qgv_points_mask(struct drm_i915_private *i915) > { > unsigned int num_psf_gv_points = > i915->display.bw.max[0].num_psf_gv_points; > unsigned int num_qgv_points = i915->display.bw.max[0].num_qgv_points; > @@ -1277,9 +1277,10 @@ int intel_bw_atomic_check(struct intel_atomic_state > *state) > > /* > * If none of our inputs (data rates, number of active > - * planes, SAGV yes/no) changed then nothing to do here. > + * planes, SAGV yes/no) changed then nothing to do here, > + * except if mask turns out to be in wrong state initially. > */ > - if (!changed) > + if (!changed && (new_bw_state->qgv_points_mask != > icl_qgv_points_mask(i915))) > return 0; > > ret = intel_bw_check_qgv_points(i915, old_bw_state, new_bw_state); > diff --git a/drivers/gpu/drm/i915/display/intel_bw.h > b/drivers/gpu/drm/i915/display/intel_bw.h > index 59cb4fc5db76..0a706ec79ce3 100644 > --- a/drivers/gpu/drm/i915/display/intel_bw.h > +++ b/drivers/gpu/drm/i915/display/intel_bw.h > @@ -70,6 +70,7 @@ void intel_bw_crtc_update(struct intel_bw_state *bw_state, > const struct intel_crtc_state *crtc_state); > int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv, > u32 points_mask); > +u16 icl_qgv_points_mask(struct drm_i915_private *i915); > int intel_bw_calc_min_cdclk(struct intel_atomic_state *state, > bool *need_cdclk_calc); > int intel_bw_min_cdclk(struct drm_i915_private *i915, > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c > b/drivers/gpu/drm/i915/display/intel_modeset_setup.c > index b8f43efb0ab5..230090c6e994 100644 > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c > @@ -871,6 +871,19 @@ static void intel_modeset_readout_hw_state(struct > drm_i915_private *i915) > intel_pmdemand_update_port_clock(i915, pmdemand_state, pipe, > crtc_state->port_clock); > > + /* > + * In some customer cases, machine can start up with all > + * GV points restricted. However we don't ever read those > + * from hw and qgv_points_mask initialized as 0, would > + * make driver think that all points are unrestricted, > + * so we never update them with proper value, unless > + * some demanding scenario is requested and we have to > + * restrict some of those. Lets fix that by initializing > + * all points as restricted, then on first modeset, driver > + * will naturally calculate, which of those need to be > + * relaxed and do correspondent update. > + */ > + bw_state->qgv_points_mask = icl_qgv_points_mask(i915);
Sad trombone for having to expose highly specific functions and stuff from intel_bw.c. Can't the below call handle it? BR, Jani. > intel_bw_crtc_update(bw_state, crtc_state); > } -- Jani Nikula, Intel
