On Tue, 24 Oct 2023, Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> 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 <stanislav.lisovskiy@xxxxxxxxx>
> ---
> 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
