On Tue, Aug 14, 2012 at 5:34 AM, Keith Packard <keithp at 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? -- Damien