diff mbox series

[10/29] drm/i915/dp: Specify the FEC overhead as an increment vs. a remainder

Message ID 20231024010925.3949910-11-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Improve BW management on MST links | expand

Commit Message

Imre Deak Oct. 24, 2023, 1:09 a.m. UTC
A follow-up patch will add up all the overheads on a DP link, where it
makes more sense to specify each overhead factor in terms of the added
overhead amount vs. the reciprocal remainder (of usable BW remaining
after deducting the overhead). Prepare for that here, keeping the
existing behavior.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Ville Syrjala Oct. 25, 2023, 3:27 p.m. UTC | #1
On Tue, Oct 24, 2023 at 04:09:06AM +0300, Imre Deak wrote:
> A follow-up patch will add up all the overheads on a DP link, where it
> makes more sense to specify each overhead factor in terms of the added
> overhead amount vs. the reciprocal remainder (of usable BW remaining
> after deducting the overhead). Prepare for that here, keeping the
> existing behavior.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 2048649b420b2..0c0f026fb3161 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -85,8 +85,8 @@
>  #define DP_DSC_MAX_ENC_THROUGHPUT_0		340000
>  #define DP_DSC_MAX_ENC_THROUGHPUT_1		400000
>  
> -/* DP DSC FEC Overhead factor = 1/(0.972261) */
> -#define DP_DSC_FEC_OVERHEAD_FACTOR		972261

Does anyone know where this magic number comes from?

AFAICS we should have 250 LL + 5 FEC_PARITY_PH + 1 CD_ADJ, which
gives us the 256/250 = 2.4% number. In addition there's the
extra parity marker symbol insterted every 128 FEC blocks,
which makes the total overhead 2.4015625%, which is still
not that magic number.

> +/* DP DSC FEC Overhead factor = 1/(0.972261) = 1.028530 ppm */
> +#define DP_DSC_FEC_OVERHEAD_FACTOR		1028530
>  
>  /* Compliance test status bits  */
>  #define INTEL_DP_RESOLUTION_SHIFT_MASK	0
> @@ -680,8 +680,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  
>  u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
>  {
> -	return div_u64(mul_u32_u32(mode_clock, 1000000U),
> -		       DP_DSC_FEC_OVERHEAD_FACTOR);
> +	return div_u64(mul_u32_u32(mode_clock, DP_DSC_FEC_OVERHEAD_FACTOR),
> +		       1000000U);
>  }
>  
>  static int
> -- 
> 2.39.2
Imre Deak Oct. 25, 2023, 3:37 p.m. UTC | #2
On Wed, Oct 25, 2023 at 06:27:58PM +0300, Ville Syrjälä wrote:
> On Tue, Oct 24, 2023 at 04:09:06AM +0300, Imre Deak wrote:
> > A follow-up patch will add up all the overheads on a DP link, where it
> > makes more sense to specify each overhead factor in terms of the added
> > overhead amount vs. the reciprocal remainder (of usable BW remaining
> > after deducting the overhead). Prepare for that here, keeping the
> > existing behavior.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 2048649b420b2..0c0f026fb3161 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -85,8 +85,8 @@
> >  #define DP_DSC_MAX_ENC_THROUGHPUT_0		340000
> >  #define DP_DSC_MAX_ENC_THROUGHPUT_1		400000
> >  
> > -/* DP DSC FEC Overhead factor = 1/(0.972261) */
> > -#define DP_DSC_FEC_OVERHEAD_FACTOR		972261
> 
> Does anyone know where this magic number comes from?
> 
> AFAICS we should have 250 LL + 5 FEC_PARITY_PH + 1 CD_ADJ, which
> gives us the 256/250 = 2.4% number. In addition there's the
> extra parity marker symbol insterted every 128 FEC blocks,
> which makes the total overhead 2.4015625%, which is still
> not that magic number.

IIRC it's mentioned in the bpsec audio programming sequence, but the
rational for it isn't detailed. I suppose it could be the 2.4% FEC
overhead + a fixed overhead for DSC EOC symbols; but the latter is not
really fixed, it depends on the video mode/slice count etc. as I
commented on later.

> 
> > +/* DP DSC FEC Overhead factor = 1/(0.972261) = 1.028530 ppm */
> > +#define DP_DSC_FEC_OVERHEAD_FACTOR		1028530
> >  
> >  /* Compliance test status bits  */
> >  #define INTEL_DP_RESOLUTION_SHIFT_MASK	0
> > @@ -680,8 +680,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> >  
> >  u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
> >  {
> > -	return div_u64(mul_u32_u32(mode_clock, 1000000U),
> > -		       DP_DSC_FEC_OVERHEAD_FACTOR);
> > +	return div_u64(mul_u32_u32(mode_clock, DP_DSC_FEC_OVERHEAD_FACTOR),
> > +		       1000000U);
> >  }
> >  
> >  static int
> > -- 
> > 2.39.2
> 
> -- 
> Ville Syrjälä
> Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 2048649b420b2..0c0f026fb3161 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -85,8 +85,8 @@ 
 #define DP_DSC_MAX_ENC_THROUGHPUT_0		340000
 #define DP_DSC_MAX_ENC_THROUGHPUT_1		400000
 
-/* DP DSC FEC Overhead factor = 1/(0.972261) */
-#define DP_DSC_FEC_OVERHEAD_FACTOR		972261
+/* DP DSC FEC Overhead factor = 1/(0.972261) = 1.028530 ppm */
+#define DP_DSC_FEC_OVERHEAD_FACTOR		1028530
 
 /* Compliance test status bits  */
 #define INTEL_DP_RESOLUTION_SHIFT_MASK	0
@@ -680,8 +680,8 @@  int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
 
 u32 intel_dp_mode_to_fec_clock(u32 mode_clock)
 {
-	return div_u64(mul_u32_u32(mode_clock, 1000000U),
-		       DP_DSC_FEC_OVERHEAD_FACTOR);
+	return div_u64(mul_u32_u32(mode_clock, DP_DSC_FEC_OVERHEAD_FACTOR),
+		       1000000U);
 }
 
 static int