Re: [PATCH 2/2] drm/i915: Implement PSF GV point support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 31, 2021 at 09:48:45AM +0300, Stanislav Lisovskiy wrote:
> PSF GV points are an additional factor that can limit the
> bandwidth available to display, separate from the traditional
> QGV points.  Whereas traditional QGV points represent possible
> memory clock frequencies, PSF GV points reflect possible
> frequencies of the memory fabric.
> 
> Switching between PSF GV points has the advantage of incurring
> almost no memory access block time and thus does not need to be
> accounted for in watermark calculations.
> 
> This patch adds support for those on top of regular QGV points.
> Those are supposed to be used simultaneously, i.e we are always
> at some QGV and some PSF GV point, based on the current video
> mode requirements.
> Bspec: 64631, 53998
> 
> v2: Seems that initial assumption made during ml conversation
>     was wrong, PCode rejects any masks containing points beyond
>     the ones returned, so even though BSpec says we have around
>     8 points theoretically, we can mask/unmask only those which
>     are returned, trying to manipulate those beyond causes a
>     failure from PCode. So switched back to generating mask
>     from 1 - num_qgv_points, where num_qgv_points is the actual
>     amount of points, advertised by PCode.
> 
> v3: - Extended restricted qgv point mask to 0xf, as we have now
>       3:2 bits for PSF GV points(Matt Roper)
>     - Replaced val2 with NULL from PCode request, since its not being
>       used(Matt Roper)
>     - Replaced %d to 0x%x for better readability(thanks for spotting)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>

Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>

Sorry for the delay with the review.  I overlooked this version when you
initially sent it.


Matt

> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 113 +++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_drv.h         |   7 ++
>  drivers/gpu/drm/i915/i915_reg.h         |   6 +-
>  drivers/gpu/drm/i915/intel_dram.c       |   1 +
>  4 files changed, 122 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index bfb398f0432e..7345746d3a67 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -17,9 +17,15 @@ struct intel_qgv_point {
>  	u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
>  };
>  
> +struct intel_psf_gv_point {
> +	u8 clk; /* clock in multiples of 16.6666 MHz */
> +};
> +
>  struct intel_qgv_info {
>  	struct intel_qgv_point points[I915_NUM_QGV_POINTS];
> +	struct intel_psf_gv_point psf_points[I915_NUM_PSF_GV_POINTS];
>  	u8 num_points;
> +	u8 num_psf_points;
>  	u8 t_bl;
>  };
>  
> @@ -49,6 +55,28 @@ static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
>  	return 0;
>  }
>  
> +static int adls_pcode_read_psf_gv_point_info(struct drm_i915_private *dev_priv,
> +					    struct intel_psf_gv_point *points)
> +{
> +	u32 val = 0;
> +	int ret;
> +	int i;
> +
> +	ret = sandybridge_pcode_read(dev_priv,
> +				     ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> +				     ADL_PCODE_MEM_SS_READ_PSF_GV_INFO,
> +				     &val, NULL);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < I915_NUM_PSF_GV_POINTS; i++) {
> +		points[i].clk = val & 0xff;
> +		val >>= 8;
> +	}
> +
> +	return 0;
> +}
> +
>  int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
>  				  u32 points_mask)
>  {
> @@ -62,7 +90,7 @@ int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
>  				1);
>  
>  	if (ret < 0) {
> -		drm_err(&dev_priv->drm, "Failed to disable qgv points (%d)\n", ret);
> +		drm_err(&dev_priv->drm, "Failed to disable qgv points (%d) points: 0x%x\n", ret, points_mask);
>  		return ret;
>  	}
>  
> @@ -76,6 +104,7 @@ static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
>  	int i, ret;
>  
>  	qi->num_points = dram_info->num_qgv_points;
> +	qi->num_psf_points = dram_info->num_psf_gv_points;
>  
>  	if (DISPLAY_VER(dev_priv) == 12)
>  		switch (dram_info->type) {
> @@ -109,6 +138,19 @@ static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
>  			    sp->t_rcd, sp->t_rc);
>  	}
>  
> +	if (qi->num_psf_points > 0) {
> +		ret = adls_pcode_read_psf_gv_point_info(dev_priv, qi->psf_points);
> +		if (ret) {
> +			drm_err(&dev_priv->drm, "Failed to read PSF point data; PSF points will not be considered in bandwidth calculations.\n");
> +			qi->num_psf_points = 0;
> +		}
> +
> +		for (i = 0; i < qi->num_psf_points; i++)
> +			drm_dbg_kms(&dev_priv->drm,
> +				    "PSF GV %d: CLK=%d \n",
> +				    i, qi->psf_points[i].clk);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -118,6 +160,16 @@ static int icl_calc_bw(int dclk, int num, int den)
>  	return DIV_ROUND_CLOSEST(num * dclk * 100, den * 6);
>  }
>  
> +static int adl_calc_psf_bw(int clk)
> +{
> +	/*
> +	 * clk is multiples of 16.666MHz (100/6)
> +	 * According to BSpec PSF GV bandwidth is
> +	 * calculated as BW = 64 * clk * 16.666Mhz
> +	 */
> +	return DIV_ROUND_CLOSEST(64 * clk * 100, 6);
> +}
> +
>  static int icl_sagv_max_dclk(const struct intel_qgv_info *qi)
>  {
>  	u16 dclk = 0;
> @@ -194,6 +246,7 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
>  		bi->num_planes = (ipqdepth - clpchgroup) / clpchgroup + 1;
>  
>  		bi->num_qgv_points = qi.num_points;
> +		bi->num_psf_gv_points = qi.num_psf_points;
>  
>  		for (j = 0; j < qi.num_points; j++) {
>  			const struct intel_qgv_point *sp = &qi.points[j];
> @@ -217,6 +270,16 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
>  				    i, j, bi->num_planes, bi->deratedbw[j]);
>  		}
>  
> +		for (j = 0; j < qi.num_psf_points; j++) {
> +			const struct intel_psf_gv_point *sp = &qi.psf_points[j];
> +
> +			bi->psf_bw[j] = adl_calc_psf_bw(sp->clk);
> +
> +			drm_dbg_kms(&dev_priv->drm,
> +				    "BW%d / PSF GV %d: num_planes=%d bw=%u\n",
> +				    i, j, bi->num_planes, bi->psf_bw[j]);
> +		}
> +
>  		if (bi->num_planes == 1)
>  			break;
>  	}
> @@ -262,6 +325,15 @@ static unsigned int icl_max_bw(struct drm_i915_private *dev_priv,
>  	return 0;
>  }
>  
> +static unsigned int adl_psf_bw(struct drm_i915_private *dev_priv,
> +			       int psf_gv_point)
> +{
> +	const struct intel_bw_info *bi =
> +			&dev_priv->max_bw[0];
> +
> +	return bi->psf_bw[psf_gv_point];
> +}
> +
>  void intel_bw_init_hw(struct drm_i915_private *dev_priv)
>  {
>  	if (!HAS_DISPLAY(dev_priv))
> @@ -534,12 +606,24 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
>  	u32 allowed_points = 0;
>  	unsigned int max_bw_point = 0, max_bw = 0;
>  	unsigned int num_qgv_points = dev_priv->max_bw[0].num_qgv_points;
> -	u32 mask = (1 << num_qgv_points) - 1;
> +	unsigned int num_psf_gv_points = dev_priv->max_bw[0].num_psf_gv_points;
> +	u32 mask = 0;
>  
>  	/* FIXME earlier gens need some checks too */
>  	if (DISPLAY_VER(dev_priv) < 11)
>  		return 0;
>  
> +	/*
> +	 * We can _not_ use the whole ADLS_QGV_PT_MASK here, as PCode rejects
> +	 * it with failure if we try masking any unadvertised points.
> +	 * So need to operate only with those returned from PCode.
> +	 */
> +	if (num_qgv_points > 0)
> +		mask |= REG_GENMASK(num_qgv_points - 1, 0);
> +
> +	if (num_psf_gv_points > 0)
> +		mask |= REG_GENMASK(num_psf_gv_points - 1, 0) << ADLS_PSF_PT_SHIFT;
> +
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
>  		unsigned int old_data_rate =
> @@ -602,23 +686,44 @@ int intel_bw_atomic_check(struct intel_atomic_state *state)
>  			max_bw = max_data_rate;
>  		}
>  		if (max_data_rate >= data_rate)
> -			allowed_points |= BIT(i);
> +			allowed_points |= REG_FIELD_PREP(ADLS_QGV_PT_MASK, BIT(i));
> +
>  		drm_dbg_kms(&dev_priv->drm, "QGV point %d: max bw %d required %d\n",
>  			    i, max_data_rate, data_rate);
>  	}
>  
> +	for (i = 0; i < num_psf_gv_points; i++) {
> +		unsigned int max_data_rate = adl_psf_bw(dev_priv, i);
> +
> +		if (max_data_rate >= data_rate)
> +			allowed_points |= REG_FIELD_PREP(ADLS_PSF_PT_MASK, BIT(i));
> +
> +		drm_dbg_kms(&dev_priv->drm, "PSF GV point %d: max bw %d"
> +			    " required %d\n",
> +			    i, max_data_rate, data_rate);
> +	}
> +
>  	/*
>  	 * BSpec states that we always should have at least one allowed point
>  	 * left, so if we couldn't - simply reject the configuration for obvious
>  	 * reasons.
>  	 */
> -	if (allowed_points == 0) {
> +	if ((allowed_points & ADLS_QGV_PT_MASK) == 0) {
>  		drm_dbg_kms(&dev_priv->drm, "No QGV points provide sufficient memory"
>  			    " bandwidth %d for display configuration(%d active planes).\n",
>  			    data_rate, num_active_planes);
>  		return -EINVAL;
>  	}
>  
> +	if (num_psf_gv_points > 0) {
> +		if ((allowed_points & ADLS_PSF_PT_MASK) == 0) {
> +			drm_dbg_kms(&dev_priv->drm, "No PSF GV points provide sufficient memory"
> +				    " bandwidth %d for display configuration(%d active planes).\n",
> +				    data_rate, num_active_planes);
> +			return -EINVAL;
> +		}
> +	}
> +
>  	/*
>  	 * Leave only single point with highest bandwidth, if
>  	 * we can't enable SAGV due to the increased memory latency it may
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0f6d27da69ac..8321833292cf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -591,6 +591,9 @@ i915_fence_timeout(const struct drm_i915_private *i915)
>  
>  #define HAS_HW_SAGV_WM(i915) (DISPLAY_VER(i915) >= 13 && !IS_DGFX(i915))
>  
> +/* Amount of PSF GV points, BSpec precisely defines this */
> +#define I915_NUM_PSF_GV_POINTS 3
> +
>  struct ddi_vbt_port_info {
>  	/* Non-NULL if port present. */
>  	struct intel_bios_encoder_data *devdata;
> @@ -1103,12 +1106,16 @@ struct drm_i915_private {
>  			INTEL_DRAM_LPDDR5,
>  		} type;
>  		u8 num_qgv_points;
> +		u8 num_psf_gv_points;
>  	} dram_info;
>  
>  	struct intel_bw_info {
>  		/* for each QGV point */
>  		unsigned int deratedbw[I915_NUM_QGV_POINTS];
> +		/* for each PSF GV point */
> +		unsigned int psf_bw[I915_NUM_PSF_GV_POINTS];
>  		u8 num_qgv_points;
> +		u8 num_psf_gv_points;
>  		u8 num_planes;
>  	} max_bw[6];
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0037e3d4049a..32299e60eb26 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9347,9 +9347,13 @@ enum {
>  #define   ICL_PCODE_MEM_SUBSYSYSTEM_INFO	0xd
>  #define     ICL_PCODE_MEM_SS_READ_GLOBAL_INFO	(0x0 << 8)
>  #define     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point)	(((point) << 16) | (0x1 << 8))
> +#define     ADL_PCODE_MEM_SS_READ_PSF_GV_INFO	((0) | (0x2 << 8))
>  #define   ICL_PCODE_SAGV_DE_MEM_SS_CONFIG	0xe
>  #define     ICL_PCODE_POINTS_RESTRICTED		0x0
> -#define     ICL_PCODE_POINTS_RESTRICTED_MASK	0x3
> +#define     ICL_PCODE_POINTS_RESTRICTED_MASK	0xf
> +#define   ADLS_PSF_PT_SHIFT			8
> +#define   ADLS_QGV_PT_MASK			REG_GENMASK(7, 0)
> +#define   ADLS_PSF_PT_MASK			REG_GENMASK(10, 8)
>  #define   GEN6_PCODE_READ_D_COMP		0x10
>  #define   GEN6_PCODE_WRITE_D_COMP		0x11
>  #define   ICL_PCODE_EXIT_TCCOLD			0x12
> diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
> index 1e53c017c30d..53ccd106785f 100644
> --- a/drivers/gpu/drm/i915/intel_dram.c
> +++ b/drivers/gpu/drm/i915/intel_dram.c
> @@ -468,6 +468,7 @@ static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv)
>  
>  	dram_info->num_channels = (val & 0xf0) >> 4;
>  	dram_info->num_qgv_points = (val & 0xf00) >> 8;
> +	dram_info->num_psf_gv_points = (val & 0x3000) >> 12;
>  
>  	return 0;
>  }
> -- 
> 2.24.1.485.gad05a3d8e5
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux