diff mbox

[4/7] drm/i915: Check display_bpc against max_fdi_bpp after display_bpc is set

Message ID 1344918891-6283-5-git-send-email-keithp@keithp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Keith Packard Aug. 14, 2012, 4:34 a.m. UTC 
display_bpc might not have been set before comparing with the
requested mode, so wait until afterwards before comparing with the
supported fdi bandwidth. Not a significant change as any case that
mattered would have worked; this just makes the debug messages look nicer.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_display.c |   39 ++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 16 deletions(-)

Comments

Lespiau, Damien Aug. 17, 2012, 2:58 p.m. UTC | #1 
On Tue, Aug 14, 2012 at 5:34 AM, Keith Packard <keithp@keithp.com> wrote:
> @@ -3845,8 +3836,20 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
>
>         display_bpc = min(display_bpc, bpc);
>
> -       DRM_DEBUG_KMS("setting pipe bpc to %d (max display bpc %d)\n",
> -                     bpc, display_bpc);
> +       display_bpc = 6;

It seems that you are overriding display_bpc unconditionally here,
some left over from debugging?

> +       if (display_bpc * 3 > max_fdi_bpp) {
> +               if (max_fdi_bpp < 24)
> +                       display_bpc = 6;
> +               else if (max_fdi_bpp < 30)
> +                       display_bpc = 8;
> +               else if (max_fdi_bpp < 36)
> +                       display_bpc = 10;
> +               DRM_DEBUG_KMS("Dithering FDI to %dbpc\n", display_bpc);
> +       }
> +
> +       DRM_DEBUG_KMS("setting pipe bpc to %d (max display bpc %d) (max_fdi_bpp %d)\n",
> +                     bpc, display_bpc, max_fdi_bpp);
>
>         *pipe_bpp = display_bpc * 3;

"setting pipe bpc to %d", bpc and  *pipe_bpp = display_bpc, looks like
a bogus debug message to me.

> @@ -4763,9 +4765,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>                  * Compute the available FDI bandwidth, use that
>                  * to compute the maximum supported BPP
>                  */
> -               fdi_bw = link_bw * max_lane * 19 / 20;
> -               max_fdi_bpp = fdi_bw / target_clock;
> -               DRM_DEBUG_KMS("max lane %d yields max fdi bpp %d\n", max_lane, max_fdi_bpp);
> +               fdi_bw = (link_bw * 8) * max_lane;
> +               pps = target_clock * 21 / 20;
> +
> +               max_fdi_bpp = fdi_bw / pps;
> +               DRM_DEBUG_KMS("link_bw %d max_lane %d fdi_bw %u pps %u max_fdi_bpp %d\n",
> +                             link_bw, max_lane, fdi_bw, pps, max_fdi_bpp);
>         }

While I understood the first computation of max_fdi_bpp in patch 2 of
this series, I have to confess you lost me there. Would you mind
clarifying this?
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 95248bd..b099a17 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3804,15 +3804,6 @@  static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 		display_bpc = 6;
 	}
 
-	if (display_bpc * 3 > max_fdi_bpp) {
-		if (max_fdi_bpp < 24)
-			display_bpc = 6;
-		else if (max_fdi_bpp < 30)
-			display_bpc = 8;
-		else if (max_fdi_bpp < 36)
-			display_bpc = 10;
-		DRM_DEBUG_KMS("Dithering FDI to %dbpc\n", display_bpc);
-	}
 	/*
 	 * We could just drive the pipe at the highest bpc all the time and
 	 * enable dithering as needed, but that costs bandwidth.  So choose
@@ -3845,8 +3836,20 @@  static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 
 	display_bpc = min(display_bpc, bpc);
 
-	DRM_DEBUG_KMS("setting pipe bpc to %d (max display bpc %d)\n",
-		      bpc, display_bpc);
+	display_bpc = 6;
+
+	if (display_bpc * 3 > max_fdi_bpp) {
+		if (max_fdi_bpp < 24)
+			display_bpc = 6;
+		else if (max_fdi_bpp < 30)
+			display_bpc = 8;
+		else if (max_fdi_bpp < 36)
+			display_bpc = 10;
+		DRM_DEBUG_KMS("Dithering FDI to %dbpc\n", display_bpc);
+	}
+
+	DRM_DEBUG_KMS("setting pipe bpc to %d (max display bpc %d) (max_fdi_bpp %d)\n",
+		      bpc, display_bpc, max_fdi_bpp);
 
 	*pipe_bpp = display_bpc * 3;
 
@@ -4737,8 +4740,7 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		max_fdi_bpp = 0;
 		max_lane = lane;
 	} else {
-		u32	fdi_bw;
-
+		u32 fdi_bw, pps;
 		/* [e]DP over FDI requires target mode clock
 		   instead of link clock */
 		if (is_dp)
@@ -4763,9 +4765,12 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		 * Compute the available FDI bandwidth, use that
 		 * to compute the maximum supported BPP
 		 */
-		fdi_bw = link_bw * max_lane * 19 / 20;
-		max_fdi_bpp = fdi_bw / target_clock;
-		DRM_DEBUG_KMS("max lane %d yields max fdi bpp %d\n", max_lane, max_fdi_bpp);
+		fdi_bw = (link_bw * 8) * max_lane;
+		pps = target_clock * 21 / 20;
+
+		max_fdi_bpp = fdi_bw / pps;
+		DRM_DEBUG_KMS("link_bw %d max_lane %d fdi_bw %u pps %u max_fdi_bpp %d\n",
+			      link_bw, max_lane, fdi_bw, pps, max_fdi_bpp);
 	}
 
 	/* [e]DP over FDI requires target mode clock instead of link clock. */
@@ -4809,6 +4814,8 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		 */
 		u32 bps = target_clock * pipe_bpp * 21 / 20;
 		lane = bps / (link_bw * 8) + 1;
+		DRM_DEBUG_KMS("target_clock %u pipe_bpp %u bps %u link_bw %u lane %u\n",
+			      target_clock, pipe_bpp, bps, link_bw, lane);
 		if (lane > max_lane) {
 			DRM_ERROR("Not enough lanes available for mode! (want %d have %d)\n",
 				  lane, max_lane);