[Intel-gfx] [PATCH 1/3] drm/i915: Nuke mode.vrefresh usage

[Intel-gfx] [PATCH 1/3] drm/i915: Nuke mode.vrefresh usage

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

mode.vrefresh is rounded to the nearest integer. You don't want to use
it anywhere that requires precision. Also I want to nuke it.
vtotal*vrefresh == 1000*clock/htotal, so let's use the latter.

Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_audio.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 36aaee8536f1..f5686e50833f 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -524,14 +524,12 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder
 	unsigned int link_clks_available, link_clks_required;
 	unsigned int tu_data, tu_line, link_clks_active;
 	unsigned int hblank_rise, hblank_early_prog;
-	unsigned int h_active, h_total, hblank_delta, pixel_clk, v_total;
-	unsigned int fec_coeff, refresh_rate, cdclk, vdsc_bpp;
+	unsigned int h_active, h_total, hblank_delta, pixel_clk;
+	unsigned int fec_coeff, cdclk, vdsc_bpp;
 
 	h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
 	h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
-	v_total = crtc_state->hw.adjusted_mode.crtc_vtotal;
 	pixel_clk = crtc_state->hw.adjusted_mode.crtc_clock;
-	refresh_rate = crtc_state->hw.adjusted_mode.vrefresh;
 	vdsc_bpp = crtc_state->dsc.compressed_bpp;
 	cdclk = i915->cdclk.hw.cdclk;
 	/* fec= 0.972261, using rounding multiplier of 1000000 */
@@ -549,9 +547,7 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder
 	link_clks_available = ((((h_total - h_active) *
 			       ((crtc_state->port_clock * ROUNDING_FACTOR) /
 				pixel_clk)) / ROUNDING_FACTOR) - 28);
-
-	link_clks_required = DIV_ROUND_UP(192000, (refresh_rate *
-					  v_total)) * ((48 /
+	link_clks_required = DIV_ROUND_UP(192000, (1000 * pixel_clk / h_total)) * ((48 /
 					  crtc_state->lane_count) + 2);
 
 	if (link_clks_available > link_clks_required)
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/3] drm/i915: Rename variables to be consistent with bspec

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Since the code seems insistent on using the variable names from the
bspec formulat, let's be consistent and use those names for all
the things. For some reason 'link_clk' and 'lanes' were left out
in the code until now.

Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_audio.c | 30 ++++++++++++----------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index f5686e50833f..00f7a3cf9a04 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -526,6 +526,7 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder
 	unsigned int hblank_rise, hblank_early_prog;
 	unsigned int h_active, h_total, hblank_delta, pixel_clk;
 	unsigned int fec_coeff, cdclk, vdsc_bpp;
+	unsigned int link_clk, lanes;
 
 	h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
 	h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
@@ -534,40 +535,40 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder
 	cdclk = i915->cdclk.hw.cdclk;
 	/* fec= 0.972261, using rounding multiplier of 1000000 */
 	fec_coeff = 972261;
+	link_clk = crtc_state->port_clock;
+	lanes = crtc_state->lane_count;
 
 	drm_dbg_kms(&i915->drm, "h_active = %u link_clk = %u :"
 		    "lanes = %u vdsc_bpp = %u cdclk = %u\n",
-		    h_active, crtc_state->port_clock, crtc_state->lane_count,
-		    vdsc_bpp, cdclk);
+		    h_active, link_clk, lanes, vdsc_bpp, cdclk);
 
-	if (WARN_ON(!crtc_state->port_clock || !crtc_state->lane_count ||
-		    !crtc_state->dsc.compressed_bpp || !i915->cdclk.hw.cdclk))
+	if (WARN_ON(!link_clk || !lanes || !vdsc_bpp || !cdclk))
 		return 0;
 
 	link_clks_available = ((((h_total - h_active) *
-			       ((crtc_state->port_clock * ROUNDING_FACTOR) /
+			       ((link_clk * ROUNDING_FACTOR) /
 				pixel_clk)) / ROUNDING_FACTOR) - 28);
 	link_clks_required = DIV_ROUND_UP(192000, (1000 * pixel_clk / h_total)) * ((48 /
-					  crtc_state->lane_count) + 2);
+					  lanes) + 2);
 
 	if (link_clks_available > link_clks_required)
 		hblank_delta = 32;
 	else
 		hblank_delta = DIV_ROUND_UP(((((5 * ROUNDING_FACTOR) /
-					    crtc_state->port_clock) + ((5 *
+					    link_clk) + ((5 *
 					    ROUNDING_FACTOR) /
 					    cdclk)) * pixel_clk),
 					    ROUNDING_FACTOR);
 
-	tu_data = (pixel_clk * vdsc_bpp * 8) / ((crtc_state->port_clock *
-		   crtc_state->lane_count * fec_coeff) / 1000000);
-	tu_line = (((h_active * crtc_state->port_clock * fec_coeff) /
+	tu_data = (pixel_clk * vdsc_bpp * 8) / ((link_clk *
+		   lanes * fec_coeff) / 1000000);
+	tu_line = (((h_active * link_clk * fec_coeff) /
 		   1000000) / (64 * pixel_clk));
 	link_clks_active  = (tu_line - 1) * 64 + tu_data;
 
 	hblank_rise = ((link_clks_active + 6 * DIV_ROUND_UP(link_clks_active,
 			250) + 4) * ((pixel_clk * ROUNDING_FACTOR) /
-			crtc_state->port_clock)) / ROUNDING_FACTOR;
+			link_clk)) / ROUNDING_FACTOR;
 
 	hblank_early_prog = h_active - hblank_rise + hblank_delta;
 
@@ -577,16 +578,19 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder
 static unsigned int get_sample_room_req_config(const struct intel_crtc_state *crtc_state)
 {
 	unsigned int h_active, h_total, pixel_clk;
+	unsigned int link_clk, lanes;
 	unsigned int samples_room;
 
 	h_active = crtc_state->hw.adjusted_mode.hdisplay;
 	h_total = crtc_state->hw.adjusted_mode.htotal;
 	pixel_clk = crtc_state->hw.adjusted_mode.clock;
+	link_clk = crtc_state->port_clock;
+	lanes = crtc_state->lane_count;
 
-	samples_room = ((((h_total - h_active) * ((crtc_state->port_clock *
+	samples_room = ((((h_total - h_active) * ((link_clk *
 			ROUNDING_FACTOR) / pixel_clk)) /
 			ROUNDING_FACTOR) - 12) / ((48 /
-			crtc_state->lane_count) + 2);
+			lanes) + 2);
 
 	return samples_room;
 }
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/3] drm/i915: Streamline the artihmetic

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

All these ROUNDIND_FACTORs and whatnot are making this thing hard to
read. Get rid of them. And let's massage some of the fractions to
give us less questionable intermediate results and perhaps less
divisions.

Also looks like a good helping of 64bit math stuff is needed to
avoid some of overflows present in the current code. There
might still be a few overflows, namely when calculating
link_clks_available/samples_room (would require a huge hblank
though), and potentially when calculating hblank_rise (not sure
how large link_clks_active can get).

It looks like we're still not calculating exactly what the spec says
since we truncate tu_data and tu_line early. But I'm too lazy to
figure out if we could avoid that.

Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_audio.c | 54 ++++++++--------------
 1 file changed, 19 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 00f7a3cf9a04..05cab508c626 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -517,16 +517,16 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
 /* Add a factor to take care of rounding and truncations */
 #define ROUNDING_FACTOR 10000
 
-static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder,
-						   const struct intel_crtc_state *crtc_state)
+static unsigned int calc_hblank_early_prog(struct intel_encoder *encoder,
+					   const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	unsigned int link_clks_available, link_clks_required;
 	unsigned int tu_data, tu_line, link_clks_active;
-	unsigned int hblank_rise, hblank_early_prog;
 	unsigned int h_active, h_total, hblank_delta, pixel_clk;
 	unsigned int fec_coeff, cdclk, vdsc_bpp;
 	unsigned int link_clk, lanes;
+	unsigned int hblank_rise;
 
 	h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
 	h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
@@ -542,44 +542,33 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder
 		    "lanes = %u vdsc_bpp = %u cdclk = %u\n",
 		    h_active, link_clk, lanes, vdsc_bpp, cdclk);
 
-	if (WARN_ON(!link_clk || !lanes || !vdsc_bpp || !cdclk))
+	if (WARN_ON(!link_clk || !pixel_clk || !lanes || !vdsc_bpp || !cdclk))
 		return 0;
 
-	link_clks_available = ((((h_total - h_active) *
-			       ((link_clk * ROUNDING_FACTOR) /
-				pixel_clk)) / ROUNDING_FACTOR) - 28);
-	link_clks_required = DIV_ROUND_UP(192000, (1000 * pixel_clk / h_total)) * ((48 /
-					  lanes) + 2);
+	link_clks_available = (h_total - h_active) * link_clk / pixel_clk - 28;
+	link_clks_required = DIV_ROUND_UP(192000 * h_total, 1000 * pixel_clk) * (48 / lanes + 2);
 
 	if (link_clks_available > link_clks_required)
 		hblank_delta = 32;
 	else
-		hblank_delta = DIV_ROUND_UP(((((5 * ROUNDING_FACTOR) /
-					    link_clk) + ((5 *
-					    ROUNDING_FACTOR) /
-					    cdclk)) * pixel_clk),
-					    ROUNDING_FACTOR);
+		hblank_delta = DIV64_U64_ROUND_UP(mul_u32_u32(5 * link_clk + 5 * cdclk, pixel_clk),
+						  mul_u32_u32(link_clk, cdclk));
 
-	tu_data = (pixel_clk * vdsc_bpp * 8) / ((link_clk *
-		   lanes * fec_coeff) / 1000000);
-	tu_line = (((h_active * link_clk * fec_coeff) /
-		   1000000) / (64 * pixel_clk));
+	tu_data = div64_u64(mul_u32_u32(pixel_clk * vdsc_bpp * 8, 1000000),
+			    mul_u32_u32(link_clk * lanes, fec_coeff));
+	tu_line = div64_u64(h_active * mul_u32_u32(link_clk, fec_coeff),
+			    mul_u32_u32(64 * pixel_clk, 1000000));
 	link_clks_active  = (tu_line - 1) * 64 + tu_data;
 
-	hblank_rise = ((link_clks_active + 6 * DIV_ROUND_UP(link_clks_active,
-			250) + 4) * ((pixel_clk * ROUNDING_FACTOR) /
-			link_clk)) / ROUNDING_FACTOR;
+	hblank_rise = (link_clks_active + 6 * DIV_ROUND_UP(link_clks_active, 250) + 4) * pixel_clk / link_clk;
 
-	hblank_early_prog = h_active - hblank_rise + hblank_delta;
-
-	return hblank_early_prog;
+	return h_active - hblank_rise + hblank_delta;
 }
 
-static unsigned int get_sample_room_req_config(const struct intel_crtc_state *crtc_state)
+static unsigned int calc_samples_room(const struct intel_crtc_state *crtc_state)
 {
 	unsigned int h_active, h_total, pixel_clk;
 	unsigned int link_clk, lanes;
-	unsigned int samples_room;
 
 	h_active = crtc_state->hw.adjusted_mode.hdisplay;
 	h_total = crtc_state->hw.adjusted_mode.htotal;
@@ -587,12 +576,8 @@ static unsigned int get_sample_room_req_config(const struct intel_crtc_state *cr
 	link_clk = crtc_state->port_clock;
 	lanes = crtc_state->lane_count;
 
-	samples_room = ((((h_total - h_active) * ((link_clk *
-			ROUNDING_FACTOR) / pixel_clk)) /
-			ROUNDING_FACTOR) - 12) / ((48 /
-			lanes) + 2);
-
-	return samples_room;
+	return ((h_total - h_active) * link_clk - 12 * pixel_clk) /
+		(pixel_clk * (48 / lanes + 2));
 }
 
 static void enable_audio_dsc_wa(struct intel_encoder *encoder,
@@ -618,8 +603,7 @@ static void enable_audio_dsc_wa(struct intel_encoder *encoder,
 	    (crtc_state->hw.adjusted_mode.hdisplay >= 3840 &&
 	    crtc_state->hw.adjusted_mode.vdisplay >= 2160)) {
 		/* Get hblank early enable value required */
-		hblank_early_prog = get_hblank_early_enable_config(encoder,
-								   crtc_state);
+		hblank_early_prog = calc_hblank_early_prog(encoder, crtc_state);
 		if (hblank_early_prog < 32) {
 			val &= ~HBLANK_START_COUNT_MASK(pipe);
 			val |= HBLANK_START_COUNT(pipe, HBLANK_START_COUNT_32);
@@ -635,7 +619,7 @@ static void enable_audio_dsc_wa(struct intel_encoder *encoder,
 		}
 
 		/* Get samples room value required */
-		samples_room = get_sample_room_req_config(crtc_state);
+		samples_room = calc_samples_room(crtc_state);
 		if (samples_room < 3) {
 			val &= ~NUMBER_SAMPLES_PER_LINE_MASK(pipe);
 			val |= NUMBER_SAMPLES_PER_LINE(pipe, samples_room);
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Nuke mode.vrefresh usage

[Chris Wilson]

Quoting Ville Syrjala (2020-04-29 19:54:55)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> mode.vrefresh is rounded to the nearest integer. You don't want to use
> it anywhere that requires precision. Also I want to nuke it.
> vtotal*vrefresh == 1000*clock/htotal, so let's use the latter.
> 
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index 36aaee8536f1..f5686e50833f 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -524,14 +524,12 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder
>         unsigned int link_clks_available, link_clks_required;
>         unsigned int tu_data, tu_line, link_clks_active;
>         unsigned int hblank_rise, hblank_early_prog;
> -       unsigned int h_active, h_total, hblank_delta, pixel_clk, v_total;
> -       unsigned int fec_coeff, refresh_rate, cdclk, vdsc_bpp;
> +       unsigned int h_active, h_total, hblank_delta, pixel_clk;
> +       unsigned int fec_coeff, cdclk, vdsc_bpp;
>  
>         h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
>         h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
> -       v_total = crtc_state->hw.adjusted_mode.crtc_vtotal;
>         pixel_clk = crtc_state->hw.adjusted_mode.crtc_clock;
> -       refresh_rate = crtc_state->hw.adjusted_mode.vrefresh;
>         vdsc_bpp = crtc_state->dsc.compressed_bpp;
>         cdclk = i915->cdclk.hw.cdclk;
>         /* fec= 0.972261, using rounding multiplier of 1000000 */
> @@ -549,9 +547,7 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder
>         link_clks_available = ((((h_total - h_active) *
>                                ((crtc_state->port_clock * ROUNDING_FACTOR) /
>                                 pixel_clk)) / ROUNDING_FACTOR) - 28);
> -
> -       link_clks_required = DIV_ROUND_UP(192000, (refresh_rate *
> -                                         v_total)) * ((48 /
> +       link_clks_required = DIV_ROUND_UP(192000, (1000 * pixel_clk / h_total)) * ((48 /

You don't need to keep the (abuse).

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Streamline the artihmetic

[Chris Wilson]

Quoting Ville Syrjala (2020-04-29 19:54:57)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All these ROUNDIND_FACTORs and whatnot are making this thing hard to
> read. Get rid of them. And let's massage some of the fractions to
> give us less questionable intermediate results and perhaps less
> divisions.
> 
> Also looks like a good helping of 64bit math stuff is needed to
> avoid some of overflows present in the current code. There
> might still be a few overflows, namely when calculating
> link_clks_available/samples_room (would require a huge hblank
> though), and potentially when calculating hblank_rise (not sure
> how large link_clks_active can get).
> 
> It looks like we're still not calculating exactly what the spec says
> since we truncate tu_data and tu_line early. But I'm too lazy to
> figure out if we could avoid that.
> 
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c | 54 ++++++++--------------
>  1 file changed, 19 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index 00f7a3cf9a04..05cab508c626 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -517,16 +517,16 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
>  /* Add a factor to take care of rounding and truncations */
>  #define ROUNDING_FACTOR 10000
>  
> -static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder,
> -                                                  const struct intel_crtc_state *crtc_state)
> +static unsigned int calc_hblank_early_prog(struct intel_encoder *encoder,
> +                                          const struct intel_crtc_state *crtc_state)
>  {
>         struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>         unsigned int link_clks_available, link_clks_required;
>         unsigned int tu_data, tu_line, link_clks_active;
> -       unsigned int hblank_rise, hblank_early_prog;
>         unsigned int h_active, h_total, hblank_delta, pixel_clk;
>         unsigned int fec_coeff, cdclk, vdsc_bpp;
>         unsigned int link_clk, lanes;
> +       unsigned int hblank_rise;
>  
>         h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
>         h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
> @@ -542,44 +542,33 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder
>                     "lanes = %u vdsc_bpp = %u cdclk = %u\n",
>                     h_active, link_clk, lanes, vdsc_bpp, cdclk);
>  
> -       if (WARN_ON(!link_clk || !lanes || !vdsc_bpp || !cdclk))
> +       if (WARN_ON(!link_clk || !pixel_clk || !lanes || !vdsc_bpp || !cdclk))
>                 return 0;
>  
> -       link_clks_available = ((((h_total - h_active) *
> -                              ((link_clk * ROUNDING_FACTOR) /
> -                               pixel_clk)) / ROUNDING_FACTOR) - 28);
> -       link_clks_required = DIV_ROUND_UP(192000, (1000 * pixel_clk / h_total)) * ((48 /
> -                                         lanes) + 2);
> +       link_clks_available = (h_total - h_active) * link_clk / pixel_clk - 28;
> +       link_clks_required = DIV_ROUND_UP(192000 * h_total, 1000 * pixel_clk) * (48 / lanes + 2);

That's a relief.

>  
>         if (link_clks_available > link_clks_required)
>                 hblank_delta = 32;
>         else
> -               hblank_delta = DIV_ROUND_UP(((((5 * ROUNDING_FACTOR) /
> -                                           link_clk) + ((5 *
> -                                           ROUNDING_FACTOR) /
> -                                           cdclk)) * pixel_clk),
> -                                           ROUNDING_FACTOR);
> +               hblank_delta = DIV64_U64_ROUND_UP(mul_u32_u32(5 * link_clk + 5 * cdclk, pixel_clk),
> +                                                 mul_u32_u32(link_clk, cdclk));

5 * mul_u32_u32(link_clk, cdclk, pixel_clk)

>  
> -       tu_data = (pixel_clk * vdsc_bpp * 8) / ((link_clk *
> -                  lanes * fec_coeff) / 1000000);
> -       tu_line = (((h_active * link_clk * fec_coeff) /
> -                  1000000) / (64 * pixel_clk));
> +       tu_data = div64_u64(mul_u32_u32(pixel_clk * vdsc_bpp * 8, 1000000),
> +                           mul_u32_u32(link_clk * lanes, fec_coeff));

That 1000000 is fec_scale.

> +       tu_line = div64_u64(h_active * mul_u32_u32(link_clk, fec_coeff),
> +                           mul_u32_u32(64 * pixel_clk, 1000000));
>         link_clks_active  = (tu_line - 1) * 64 + tu_data;

Transformations look correct.

> -       hblank_rise = ((link_clks_active + 6 * DIV_ROUND_UP(link_clks_active,
> -                       250) + 4) * ((pixel_clk * ROUNDING_FACTOR) /
> -                       link_clk)) / ROUNDING_FACTOR;
> +       hblank_rise = (link_clks_active + 6 * DIV_ROUND_UP(link_clks_active, 250) + 4) * pixel_clk / link_clk;
>  
> -       hblank_early_prog = h_active - hblank_rise + hblank_delta;
> -
> -       return hblank_early_prog;
> +       return h_active - hblank_rise + hblank_delta;

ROUNDING_FACTOR doesn't seem to do any rounding.

>  }
>  
> -static unsigned int get_sample_room_req_config(const struct intel_crtc_state *crtc_state)
> +static unsigned int calc_samples_room(const struct intel_crtc_state *crtc_state)
>  {
>         unsigned int h_active, h_total, pixel_clk;
>         unsigned int link_clk, lanes;
> -       unsigned int samples_room;
>  
>         h_active = crtc_state->hw.adjusted_mode.hdisplay;
>         h_total = crtc_state->hw.adjusted_mode.htotal;
> @@ -587,12 +576,8 @@ static unsigned int get_sample_room_req_config(const struct intel_crtc_state *cr
>         link_clk = crtc_state->port_clock;
>         lanes = crtc_state->lane_count;
>  
> -       samples_room = ((((h_total - h_active) * ((link_clk *
> -                       ROUNDING_FACTOR) / pixel_clk)) /
> -                       ROUNDING_FACTOR) - 12) / ((48 /
> -                       lanes) + 2);
> -
> -       return samples_room;
> +       return ((h_total - h_active) * link_clk - 12 * pixel_clk) /
> +               (pixel_clk * (48 / lanes + 2));

Expansion of fractions looks fine.

The maths looks to be the same, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Don't ask me about the meaning of it though.

One question might be if there are known inputs/outputs we can build
into a unit test.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915: Nuke mode.vrefresh usage

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Nuke mode.vrefresh usage
URL   : https://patchwork.freedesktop.org/series/76741/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
c72890c16d9b drm/i915: Nuke mode.vrefresh usage
e45bfe3703e2 drm/i915: Rename variables to be consistent with bspec
01ba9553032f drm/i915: Streamline the artihmetic
-:97: WARNING:LONG_LINE: line over 100 characters
#97: FILE: drivers/gpu/drm/i915/display/intel_audio.c:563:
+	hblank_rise = (link_clks_active + 6 * DIV_ROUND_UP(link_clks_active, 250) + 4) * pixel_clk / link_clk;

total: 0 errors, 1 warnings, 0 checks, 107 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Nuke mode.vrefresh usage

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Nuke mode.vrefresh usage
URL   : https://patchwork.freedesktop.org/series/76741/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8394 -> Patchwork_17520
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/index.html

Known issues
------------

  Here are the changes found in Patchwork_17520 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6700k2:      [PASS][1] -> [INCOMPLETE][2] ([i915#151])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/fi-skl-6700k2/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/fi-skl-6700k2/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@gt_mocs:
    - fi-bwr-2160:        [PASS][3] -> [INCOMPLETE][4] ([i915#489])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/fi-bwr-2160/igt@i915_selftest@live@gt_mocs.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/fi-bwr-2160/igt@i915_selftest@live@gt_mocs.html

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [FAIL][5] ([i915#62]) -> [SKIP][6] ([fdo#109271])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#151]: https://gitlab.freedesktop.org/drm/intel/issues/151
  [i915#489]: https://gitlab.freedesktop.org/drm/intel/issues/489
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62


Participating hosts (48 -> 41)
------------------------------

  Missing    (7): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-kbl-7500u fi-ctg-p8600 fi-kbl-7560u fi-byt-clapper 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8394 -> Patchwork_17520

  CI-20190529: 20190529
  CI_DRM_8394: 532afb6da86a1c6ae10469908814f7a4f46afd88 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5615: 7605cfd9463a6778ebb7ebae294a97c5779a6c7f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17520: 01ba9553032fb839cd1d4e82d1d0e96469968b7c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

01ba9553032f drm/i915: Streamline the artihmetic
e45bfe3703e2 drm/i915: Rename variables to be consistent with bspec
c72890c16d9b drm/i915: Nuke mode.vrefresh usage

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915: Nuke mode.vrefresh usage

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Nuke mode.vrefresh usage
URL   : https://patchwork.freedesktop.org/series/76741/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8394_full -> Patchwork_17520_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_17520_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_whisper@basic-contexts-all:
    - shard-glk:          [PASS][1] -> [INCOMPLETE][2] ([i915#58] / [k.org#198133])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-glk8/igt@gem_exec_whisper@basic-contexts-all.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-glk8/igt@gem_exec_whisper@basic-contexts-all.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [PASS][3] -> [FAIL][4] ([i915#454])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-iclb1/igt@i915_pm_dc@dc6-psr.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-iclb2/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-skl:          [PASS][5] -> [INCOMPLETE][6] ([i915#300])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-skl7/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-skl4/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([i915#180]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-apl2/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-apl4/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
    - shard-kbl:          [PASS][9] -> [DMESG-WARN][10] ([i915#180])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-kbl7/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-kbl7/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [PASS][11] -> [FAIL][12] ([i915#72])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-glk5/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-glk6/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
    - shard-hsw:          [PASS][13] -> [FAIL][14] ([i915#57])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-hsw7/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-hsw8/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html

  * igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled:
    - shard-glk:          [PASS][15] -> [FAIL][16] ([i915#177] / [i915#52] / [i915#54])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-glk8/igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-glk2/igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-blt:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([i915#49])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-skl10/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-blt.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-skl8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([i915#1188])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-skl8/igt@kms_hdr@bpc-switch-suspend.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-skl5/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#108145] / [i915#265]) +3 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109441]) +3 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-iclb6/igt@kms_psr@psr2_primary_mmap_cpu.html

  
#### Possible fixes ####

  * igt@gem_exec_params@invalid-bsd-ring:
    - shard-iclb:         [SKIP][25] ([fdo#109276]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-iclb5/igt@gem_exec_params@invalid-bsd-ring.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-iclb4/igt@gem_exec_params@invalid-bsd-ring.html

  * igt@i915_pm_dc@dc5-psr:
    - shard-skl:          [INCOMPLETE][27] ([i915#198]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-skl8/igt@i915_pm_dc@dc5-psr.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-skl5/igt@i915_pm_dc@dc5-psr.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen:
    - shard-apl:          [FAIL][29] ([i915#54] / [i915#95]) -> [PASS][30] +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-apl8/igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-apl2/igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-skl:          [INCOMPLETE][31] ([i915#300]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-skl9/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-skl10/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][33] ([i915#180]) -> [PASS][34] +4 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][35] ([i915#96]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-hsw1/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-hsw4/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic:
    - shard-glk:          [FAIL][37] ([i915#1566]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-glk5/igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-glk6/igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic.html

  * igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled:
    - shard-glk:          [FAIL][39] ([i915#52] / [i915#54]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-glk2/igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-glk5/igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][41] ([i915#180]) -> [PASS][42] +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [FAIL][43] ([i915#173]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-iclb1/igt@kms_psr@no_drrs.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-iclb2/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [SKIP][45] ([fdo#109441]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-iclb1/igt@kms_psr@psr2_sprite_plane_move.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html

  * {igt@perf@blocking-parameterized}:
    - shard-iclb:         [FAIL][47] ([i915#1542]) -> [PASS][48] +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-iclb8/igt@perf@blocking-parameterized.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-iclb5/igt@perf@blocking-parameterized.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc6-psr:
    - shard-tglb:         [FAIL][49] ([i915#454]) -> [SKIP][50] ([i915#468])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-tglb6/igt@i915_pm_dc@dc6-psr.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-tglb2/igt@i915_pm_dc@dc6-psr.html
    - shard-skl:          [FAIL][51] ([i915#454]) -> [INCOMPLETE][52] ([i915#198])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-skl10/igt@i915_pm_dc@dc6-psr.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-skl10/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rpm@dpms-non-lpsp:
    - shard-snb:          [INCOMPLETE][53] ([i915#82]) -> [SKIP][54] ([fdo#109271])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-snb1/igt@i915_pm_rpm@dpms-non-lpsp.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-snb6/igt@i915_pm_rpm@dpms-non-lpsp.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-apl:          [DMESG-FAIL][55] ([i915#180] / [i915#95]) -> [FAIL][56] ([i915#95])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-apl1/igt@kms_fbcon_fbt@fbc-suspend.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-apl6/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-apl:          [FAIL][57] ([fdo#108145] / [i915#265]) -> [FAIL][58] ([fdo#108145] / [i915#265] / [i915#95])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-apl8/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-apl3/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-kbl:          [FAIL][59] ([fdo#108145] / [i915#265]) -> [FAIL][60] ([fdo#108145] / [i915#265] / [i915#93] / [i915#95])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-kbl7/igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-kbl3/igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][61] ([fdo#109642] / [fdo#111068]) -> [FAIL][62] ([i915#608])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8394/shard-iclb5/igt@kms_psr2_su@page_flip.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/shard-iclb2/igt@kms_psr2_su@page_flip.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1566]: https://gitlab.freedesktop.org/drm/intel/issues/1566
  [i915#173]: https://gitlab.freedesktop.org/drm/intel/issues/173
  [i915#177]: https://gitlab.freedesktop.org/drm/intel/issues/177
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#300]: https://gitlab.freedesktop.org/drm/intel/issues/300
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#57]: https://gitlab.freedesktop.org/drm/intel/issues/57
  [i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
  [i915#608]: https://gitlab.freedesktop.org/drm/intel/issues/608
  [i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
  [i915#96]: https://gitlab.freedesktop.org/drm/intel/issues/96
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8394 -> Patchwork_17520

  CI-20190529: 20190529
  CI_DRM_8394: 532afb6da86a1c6ae10469908814f7a4f46afd88 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5615: 7605cfd9463a6778ebb7ebae294a97c5779a6c7f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17520: 01ba9553032fb839cd1d4e82d1d0e96469968b7c @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17520/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Nuke mode.vrefresh usage

[Shankar, Uma]

> -----Original Message-----
> From: Ville Syrjala <ville.syrjala@linux.intel.com>
> Sent: Thursday, April 30, 2020 12:25 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>
> Subject: [PATCH 1/3] drm/i915: Nuke mode.vrefresh usage
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> mode.vrefresh is rounded to the nearest integer. You don't want to use it anywhere
> that requires precision. Also I want to nuke it.
> vtotal*vrefresh == 1000*clock/htotal, so let's use the latter.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> b/drivers/gpu/drm/i915/display/intel_audio.c
> index 36aaee8536f1..f5686e50833f 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -524,14 +524,12 @@ static unsigned int get_hblank_early_enable_config(struct
> intel_encoder *encoder
>  	unsigned int link_clks_available, link_clks_required;
>  	unsigned int tu_data, tu_line, link_clks_active;
>  	unsigned int hblank_rise, hblank_early_prog;
> -	unsigned int h_active, h_total, hblank_delta, pixel_clk, v_total;
> -	unsigned int fec_coeff, refresh_rate, cdclk, vdsc_bpp;
> +	unsigned int h_active, h_total, hblank_delta, pixel_clk;
> +	unsigned int fec_coeff, cdclk, vdsc_bpp;
> 
>  	h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
>  	h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
> -	v_total = crtc_state->hw.adjusted_mode.crtc_vtotal;
>  	pixel_clk = crtc_state->hw.adjusted_mode.crtc_clock;
> -	refresh_rate = crtc_state->hw.adjusted_mode.vrefresh;
>  	vdsc_bpp = crtc_state->dsc.compressed_bpp;
>  	cdclk = i915->cdclk.hw.cdclk;
>  	/* fec= 0.972261, using rounding multiplier of 1000000 */ @@ -549,9
> +547,7 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder
> *encoder
>  	link_clks_available = ((((h_total - h_active) *
>  			       ((crtc_state->port_clock * ROUNDING_FACTOR) /
>  				pixel_clk)) / ROUNDING_FACTOR) - 28);
> -
> -	link_clks_required = DIV_ROUND_UP(192000, (refresh_rate *
> -					  v_total)) * ((48 /
> +	link_clks_required = DIV_ROUND_UP(192000, (1000 * pixel_clk /
> +h_total)) * ((48 /
>  					  crtc_state->lane_count) + 2);
> 
>  	if (link_clks_available > link_clks_required)
> --
> 2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/3] drm/i915: Rename variables to be consistent with bspec

[Shankar, Uma]

> -----Original Message-----
> From: Ville Syrjala <ville.syrjala@linux.intel.com>
> Sent: Thursday, April 30, 2020 12:25 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>
> Subject: [PATCH 2/3] drm/i915: Rename variables to be consistent with bspec
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since the code seems insistent on using the variable names from the bspec formulat,
> let's be consistent and use those names for all the things. For some reason 'link_clk'
> and 'lanes' were left out in the code until now.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c | 30 ++++++++++++----------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> b/drivers/gpu/drm/i915/display/intel_audio.c
> index f5686e50833f..00f7a3cf9a04 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -526,6 +526,7 @@ static unsigned int get_hblank_early_enable_config(struct
> intel_encoder *encoder
>  	unsigned int hblank_rise, hblank_early_prog;
>  	unsigned int h_active, h_total, hblank_delta, pixel_clk;
>  	unsigned int fec_coeff, cdclk, vdsc_bpp;
> +	unsigned int link_clk, lanes;
> 
>  	h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
>  	h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
> @@ -534,40 +535,40 @@ static unsigned int get_hblank_early_enable_config(struct
> intel_encoder *encoder
>  	cdclk = i915->cdclk.hw.cdclk;
>  	/* fec= 0.972261, using rounding multiplier of 1000000 */
>  	fec_coeff = 972261;
> +	link_clk = crtc_state->port_clock;
> +	lanes = crtc_state->lane_count;
> 
>  	drm_dbg_kms(&i915->drm, "h_active = %u link_clk = %u :"
>  		    "lanes = %u vdsc_bpp = %u cdclk = %u\n",
> -		    h_active, crtc_state->port_clock, crtc_state->lane_count,
> -		    vdsc_bpp, cdclk);
> +		    h_active, link_clk, lanes, vdsc_bpp, cdclk);
> 
> -	if (WARN_ON(!crtc_state->port_clock || !crtc_state->lane_count ||
> -		    !crtc_state->dsc.compressed_bpp || !i915->cdclk.hw.cdclk))
> +	if (WARN_ON(!link_clk || !lanes || !vdsc_bpp || !cdclk))
>  		return 0;
> 
>  	link_clks_available = ((((h_total - h_active) *
> -			       ((crtc_state->port_clock * ROUNDING_FACTOR) /
> +			       ((link_clk * ROUNDING_FACTOR) /
>  				pixel_clk)) / ROUNDING_FACTOR) - 28);
>  	link_clks_required = DIV_ROUND_UP(192000, (1000 * pixel_clk / h_total)) *
> ((48 /
> -					  crtc_state->lane_count) + 2);
> +					  lanes) + 2);
> 
>  	if (link_clks_available > link_clks_required)
>  		hblank_delta = 32;
>  	else
>  		hblank_delta = DIV_ROUND_UP(((((5 * ROUNDING_FACTOR) /
> -					    crtc_state->port_clock) + ((5 *
> +					    link_clk) + ((5 *
>  					    ROUNDING_FACTOR) /
>  					    cdclk)) * pixel_clk),
>  					    ROUNDING_FACTOR);
> 
> -	tu_data = (pixel_clk * vdsc_bpp * 8) / ((crtc_state->port_clock *
> -		   crtc_state->lane_count * fec_coeff) / 1000000);
> -	tu_line = (((h_active * crtc_state->port_clock * fec_coeff) /
> +	tu_data = (pixel_clk * vdsc_bpp * 8) / ((link_clk *
> +		   lanes * fec_coeff) / 1000000);
> +	tu_line = (((h_active * link_clk * fec_coeff) /
>  		   1000000) / (64 * pixel_clk));
>  	link_clks_active  = (tu_line - 1) * 64 + tu_data;
> 
>  	hblank_rise = ((link_clks_active + 6 * DIV_ROUND_UP(link_clks_active,
>  			250) + 4) * ((pixel_clk * ROUNDING_FACTOR) /
> -			crtc_state->port_clock)) / ROUNDING_FACTOR;
> +			link_clk)) / ROUNDING_FACTOR;
> 
>  	hblank_early_prog = h_active - hblank_rise + hblank_delta;
> 
> @@ -577,16 +578,19 @@ static unsigned int get_hblank_early_enable_config(struct
> intel_encoder *encoder  static unsigned int get_sample_room_req_config(const
> struct intel_crtc_state *crtc_state)  {
>  	unsigned int h_active, h_total, pixel_clk;
> +	unsigned int link_clk, lanes;
>  	unsigned int samples_room;
> 
>  	h_active = crtc_state->hw.adjusted_mode.hdisplay;
>  	h_total = crtc_state->hw.adjusted_mode.htotal;
>  	pixel_clk = crtc_state->hw.adjusted_mode.clock;
> +	link_clk = crtc_state->port_clock;
> +	lanes = crtc_state->lane_count;
> 
> -	samples_room = ((((h_total - h_active) * ((crtc_state->port_clock *
> +	samples_room = ((((h_total - h_active) * ((link_clk *
>  			ROUNDING_FACTOR) / pixel_clk)) /
>  			ROUNDING_FACTOR) - 12) / ((48 /
> -			crtc_state->lane_count) + 2);
> +			lanes) + 2);
> 
>  	return samples_room;
>  }
> --
> 2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Streamline the artihmetic

[Ville Syrjälä]

On Wed, Apr 29, 2020 at 08:11:04PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2020-04-29 19:54:57)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > All these ROUNDIND_FACTORs and whatnot are making this thing hard to
> > read. Get rid of them. And let's massage some of the fractions to
> > give us less questionable intermediate results and perhaps less
> > divisions.
> > 
> > Also looks like a good helping of 64bit math stuff is needed to
> > avoid some of overflows present in the current code. There
> > might still be a few overflows, namely when calculating
> > link_clks_available/samples_room (would require a huge hblank
> > though), and potentially when calculating hblank_rise (not sure
> > how large link_clks_active can get).
> > 
> > It looks like we're still not calculating exactly what the spec says
> > since we truncate tu_data and tu_line early. But I'm too lazy to
> > figure out if we could avoid that.
> > 
> > Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> > Cc: Uma Shankar <uma.shankar@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_audio.c | 54 ++++++++--------------
> >  1 file changed, 19 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> > index 00f7a3cf9a04..05cab508c626 100644
> > --- a/drivers/gpu/drm/i915/display/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> > @@ -517,16 +517,16 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
> >  /* Add a factor to take care of rounding and truncations */
> >  #define ROUNDING_FACTOR 10000
> >  
> > -static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder,
> > -                                                  const struct intel_crtc_state *crtc_state)
> > +static unsigned int calc_hblank_early_prog(struct intel_encoder *encoder,
> > +                                          const struct intel_crtc_state *crtc_state)
> >  {
> >         struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >         unsigned int link_clks_available, link_clks_required;
> >         unsigned int tu_data, tu_line, link_clks_active;
> > -       unsigned int hblank_rise, hblank_early_prog;
> >         unsigned int h_active, h_total, hblank_delta, pixel_clk;
> >         unsigned int fec_coeff, cdclk, vdsc_bpp;
> >         unsigned int link_clk, lanes;
> > +       unsigned int hblank_rise;
> >  
> >         h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
> >         h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
> > @@ -542,44 +542,33 @@ static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder
> >                     "lanes = %u vdsc_bpp = %u cdclk = %u\n",
> >                     h_active, link_clk, lanes, vdsc_bpp, cdclk);
> >  
> > -       if (WARN_ON(!link_clk || !lanes || !vdsc_bpp || !cdclk))
> > +       if (WARN_ON(!link_clk || !pixel_clk || !lanes || !vdsc_bpp || !cdclk))
> >                 return 0;
> >  
> > -       link_clks_available = ((((h_total - h_active) *
> > -                              ((link_clk * ROUNDING_FACTOR) /
> > -                               pixel_clk)) / ROUNDING_FACTOR) - 28);
> > -       link_clks_required = DIV_ROUND_UP(192000, (1000 * pixel_clk / h_total)) * ((48 /
> > -                                         lanes) + 2);
> > +       link_clks_available = (h_total - h_active) * link_clk / pixel_clk - 28;
> > +       link_clks_required = DIV_ROUND_UP(192000 * h_total, 1000 * pixel_clk) * (48 / lanes + 2);
> 
> That's a relief.
> 
> >  
> >         if (link_clks_available > link_clks_required)
> >                 hblank_delta = 32;
> >         else
> > -               hblank_delta = DIV_ROUND_UP(((((5 * ROUNDING_FACTOR) /
> > -                                           link_clk) + ((5 *
> > -                                           ROUNDING_FACTOR) /
> > -                                           cdclk)) * pixel_clk),
> > -                                           ROUNDING_FACTOR);
> > +               hblank_delta = DIV64_U64_ROUND_UP(mul_u32_u32(5 * link_clk + 5 * cdclk, pixel_clk),
> > +                                                 mul_u32_u32(link_clk, cdclk));
> 
> 5 * mul_u32_u32(link_clk, cdclk, pixel_clk)

I presume you meant
5 * mul_u32_u32(link_clk + cdclk, pixel_clk)

mul_u32_u32(5 * (link_clk + cdclk), pixel_clk) 
would seem to be the cheaper option by avoiding
the 64bit multiply.

> 
> >  
> > -       tu_data = (pixel_clk * vdsc_bpp * 8) / ((link_clk *
> > -                  lanes * fec_coeff) / 1000000);
> > -       tu_line = (((h_active * link_clk * fec_coeff) /
> > -                  1000000) / (64 * pixel_clk));
> > +       tu_data = div64_u64(mul_u32_u32(pixel_clk * vdsc_bpp * 8, 1000000),
> > +                           mul_u32_u32(link_clk * lanes, fec_coeff));
> 
> That 1000000 is fec_scale.
> 
> > +       tu_line = div64_u64(h_active * mul_u32_u32(link_clk, fec_coeff),
> > +                           mul_u32_u32(64 * pixel_clk, 1000000));
> >         link_clks_active  = (tu_line - 1) * 64 + tu_data;
> 
> Transformations look correct.
> 
> > -       hblank_rise = ((link_clks_active + 6 * DIV_ROUND_UP(link_clks_active,
> > -                       250) + 4) * ((pixel_clk * ROUNDING_FACTOR) /
> > -                       link_clk)) / ROUNDING_FACTOR;
> > +       hblank_rise = (link_clks_active + 6 * DIV_ROUND_UP(link_clks_active, 250) + 4) * pixel_clk / link_clk;
> >  
> > -       hblank_early_prog = h_active - hblank_rise + hblank_delta;
> > -
> > -       return hblank_early_prog;
> > +       return h_active - hblank_rise + hblank_delta;
> 
> ROUNDING_FACTOR doesn't seem to do any rounding.
> 
> >  }
> >  
> > -static unsigned int get_sample_room_req_config(const struct intel_crtc_state *crtc_state)
> > +static unsigned int calc_samples_room(const struct intel_crtc_state *crtc_state)
> >  {
> >         unsigned int h_active, h_total, pixel_clk;
> >         unsigned int link_clk, lanes;
> > -       unsigned int samples_room;
> >  
> >         h_active = crtc_state->hw.adjusted_mode.hdisplay;
> >         h_total = crtc_state->hw.adjusted_mode.htotal;
> > @@ -587,12 +576,8 @@ static unsigned int get_sample_room_req_config(const struct intel_crtc_state *cr
> >         link_clk = crtc_state->port_clock;
> >         lanes = crtc_state->lane_count;
> >  
> > -       samples_room = ((((h_total - h_active) * ((link_clk *
> > -                       ROUNDING_FACTOR) / pixel_clk)) /
> > -                       ROUNDING_FACTOR) - 12) / ((48 /
> > -                       lanes) + 2);
> > -
> > -       return samples_room;
> > +       return ((h_total - h_active) * link_clk - 12 * pixel_clk) /
> > +               (pixel_clk * (48 / lanes + 2));
> 
> Expansion of fractions looks fine.
> 
> The maths looks to be the same, so
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Don't ask me about the meaning of it though.
> 
> One question might be if there are known inputs/outputs we can build
> into a unit test.

Yeah, that might be a good idea.

Also it looks like we could precompute all this and stuff
the result into the crtc state to get the state checker 
involved. Would also get rid of that ugly cdcdlk.hw usage.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Streamline the artihmetic

[Shankar, Uma]

> -----Original Message-----
> From: Ville Syrjala <ville.syrjala@linux.intel.com>
> Sent: Thursday, April 30, 2020 12:25 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>
> Subject: [PATCH 3/3] drm/i915: Streamline the artihmetic
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All these ROUNDIND_FACTORs and whatnot are making this thing hard to read. Get

Nit pick: Typo in ROUNDING

> rid of them. And let's massage some of the fractions to give us less questionable
> intermediate results and perhaps less divisions.
> 
> Also looks like a good helping of 64bit math stuff is needed to avoid some of
> overflows present in the current code. There might still be a few overflows, namely
> when calculating link_clks_available/samples_room (would require a huge hblank
> though), and potentially when calculating hblank_rise (not sure how large
> link_clks_active can get).
> 
> It looks like we're still not calculating exactly what the spec says since we truncate
> tu_data and tu_line early. But I'm too lazy to figure out if we could avoid that.
> 
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c | 54 ++++++++--------------
>  1 file changed, 19 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> b/drivers/gpu/drm/i915/display/intel_audio.c
> index 00f7a3cf9a04..05cab508c626 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -517,16 +517,16 @@ static void hsw_audio_codec_disable(struct intel_encoder
> *encoder,
>  /* Add a factor to take care of rounding and truncations */  #define
> ROUNDING_FACTOR 10000

We should drop this declaration as well, as its of no use now.
 
Overall this looks much better for sure, thanks Ville.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> -static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder,
> -						   const struct intel_crtc_state
> *crtc_state)
> +static unsigned int calc_hblank_early_prog(struct intel_encoder *encoder,
> +					   const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	unsigned int link_clks_available, link_clks_required;
>  	unsigned int tu_data, tu_line, link_clks_active;
> -	unsigned int hblank_rise, hblank_early_prog;
>  	unsigned int h_active, h_total, hblank_delta, pixel_clk;
>  	unsigned int fec_coeff, cdclk, vdsc_bpp;
>  	unsigned int link_clk, lanes;
> +	unsigned int hblank_rise;
> 
>  	h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
>  	h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
> @@ -542,44 +542,33 @@ static unsigned int get_hblank_early_enable_config(struct
> intel_encoder *encoder
>  		    "lanes = %u vdsc_bpp = %u cdclk = %u\n",
>  		    h_active, link_clk, lanes, vdsc_bpp, cdclk);
> 
> -	if (WARN_ON(!link_clk || !lanes || !vdsc_bpp || !cdclk))
> +	if (WARN_ON(!link_clk || !pixel_clk || !lanes || !vdsc_bpp || !cdclk))
>  		return 0;
> 
> -	link_clks_available = ((((h_total - h_active) *
> -			       ((link_clk * ROUNDING_FACTOR) /
> -				pixel_clk)) / ROUNDING_FACTOR) - 28);
> -	link_clks_required = DIV_ROUND_UP(192000, (1000 * pixel_clk / h_total)) *
> ((48 /
> -					  lanes) + 2);
> +	link_clks_available = (h_total - h_active) * link_clk / pixel_clk - 28;
> +	link_clks_required = DIV_ROUND_UP(192000 * h_total, 1000 * pixel_clk)
> +* (48 / lanes + 2);
> 
>  	if (link_clks_available > link_clks_required)
>  		hblank_delta = 32;
>  	else
> -		hblank_delta = DIV_ROUND_UP(((((5 * ROUNDING_FACTOR) /
> -					    link_clk) + ((5 *
> -					    ROUNDING_FACTOR) /
> -					    cdclk)) * pixel_clk),
> -					    ROUNDING_FACTOR);
> +		hblank_delta = DIV64_U64_ROUND_UP(mul_u32_u32(5 * link_clk +
> 5 * cdclk, pixel_clk),
> +						  mul_u32_u32(link_clk, cdclk));
> 
> -	tu_data = (pixel_clk * vdsc_bpp * 8) / ((link_clk *
> -		   lanes * fec_coeff) / 1000000);
> -	tu_line = (((h_active * link_clk * fec_coeff) /
> -		   1000000) / (64 * pixel_clk));
> +	tu_data = div64_u64(mul_u32_u32(pixel_clk * vdsc_bpp * 8, 1000000),
> +			    mul_u32_u32(link_clk * lanes, fec_coeff));
> +	tu_line = div64_u64(h_active * mul_u32_u32(link_clk, fec_coeff),
> +			    mul_u32_u32(64 * pixel_clk, 1000000));
>  	link_clks_active  = (tu_line - 1) * 64 + tu_data;
> 
> -	hblank_rise = ((link_clks_active + 6 * DIV_ROUND_UP(link_clks_active,
> -			250) + 4) * ((pixel_clk * ROUNDING_FACTOR) /
> -			link_clk)) / ROUNDING_FACTOR;
> +	hblank_rise = (link_clks_active + 6 * DIV_ROUND_UP(link_clks_active,
> +250) + 4) * pixel_clk / link_clk;
> 
> -	hblank_early_prog = h_active - hblank_rise + hblank_delta;
> -
> -	return hblank_early_prog;
> +	return h_active - hblank_rise + hblank_delta;
>  }
> 
> -static unsigned int get_sample_room_req_config(const struct intel_crtc_state
> *crtc_state)
> +static unsigned int calc_samples_room(const struct intel_crtc_state
> +*crtc_state)
>  {
>  	unsigned int h_active, h_total, pixel_clk;
>  	unsigned int link_clk, lanes;
> -	unsigned int samples_room;
> 
>  	h_active = crtc_state->hw.adjusted_mode.hdisplay;
>  	h_total = crtc_state->hw.adjusted_mode.htotal;
> @@ -587,12 +576,8 @@ static unsigned int get_sample_room_req_config(const
> struct intel_crtc_state *cr
>  	link_clk = crtc_state->port_clock;
>  	lanes = crtc_state->lane_count;
> 
> -	samples_room = ((((h_total - h_active) * ((link_clk *
> -			ROUNDING_FACTOR) / pixel_clk)) /
> -			ROUNDING_FACTOR) - 12) / ((48 /
> -			lanes) + 2);
> -
> -	return samples_room;
> +	return ((h_total - h_active) * link_clk - 12 * pixel_clk) /
> +		(pixel_clk * (48 / lanes + 2));
>  }
> 
>  static void enable_audio_dsc_wa(struct intel_encoder *encoder, @@ -618,8 +603,7
> @@ static void enable_audio_dsc_wa(struct intel_encoder *encoder,
>  	    (crtc_state->hw.adjusted_mode.hdisplay >= 3840 &&
>  	    crtc_state->hw.adjusted_mode.vdisplay >= 2160)) {
>  		/* Get hblank early enable value required */
> -		hblank_early_prog = get_hblank_early_enable_config(encoder,
> -								   crtc_state);
> +		hblank_early_prog = calc_hblank_early_prog(encoder, crtc_state);
>  		if (hblank_early_prog < 32) {
>  			val &= ~HBLANK_START_COUNT_MASK(pipe);
>  			val |= HBLANK_START_COUNT(pipe,
> HBLANK_START_COUNT_32); @@ -635,7 +619,7 @@ static void
> enable_audio_dsc_wa(struct intel_encoder *encoder,
>  		}
> 
>  		/* Get samples room value required */
> -		samples_room = get_sample_room_req_config(crtc_state);
> +		samples_room = calc_samples_room(crtc_state);
>  		if (samples_room < 3) {
>  			val &= ~NUMBER_SAMPLES_PER_LINE_MASK(pipe);
>  			val |= NUMBER_SAMPLES_PER_LINE(pipe, samples_room);
> --
> 2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx