On Tue, Aug 14, 2012 at 5:34 AM, Keith Packard <keithp at keithp.com> wrote:
@@ -3728,7 +3728,8 @@ static inline bool intel_panel_use_ssc(struct
drm_i915_private *dev_priv)
*/
static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
unsigned int *pipe_bpp,
- struct drm_display_mode *mode)
+ struct drm_display_mode *mode,
+ int max_fdi_bpp)
There's some kernel-doc for this function, maybe add a @max_fdi_bpp there?
> @@ -3800,6 +3801,15 @@ 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);
> + }
This chunk is being moved around in a later patch in the series,
merging the two patches in one looks like a good idea?
> + /* If the other FDI link is using too many lanes, we can't have
> + * any
> + */
> + if (other_intel_crtc->fdi_lanes > 2)
> + return 0;
> +
> + /* When both are running, we only get 2 lanes at most
> + */
> + return 2;
> +}
I guess this does not cover the case of pipe B using 3 lanes meaning
pipe C can use 1?
> @@ -4672,7 +4731,18 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> according to current link config */
> if (is_cpu_edp) {
> intel_edp_link_config(edp_encoder, &lane, &link_bw);
> + max_fdi_bpp = 0;
> + max_lane = lane;
> } else {
> + u32 fdi_bw;
> +
> + /* [e]DP over FDI requires target mode clock
> + instead of link clock */
> + if (is_dp)
> + target_clock = mode->clock;
> + else
> + target_clock = adjusted_mode->clock;
> +
This duplicates the code just that is just a few lines away, instead
how about moving the logic to set target_clock up in front of this
whole if()?
> /* FDI is a binary signal running at ~2.7GHz, encoding
> * each output octet as 10 bits. The actual frequency
> * is stored as a divider into a 100MHz clock, and the
> @@ -4681,6 +4751,18 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> * is:
> */
> link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
> +
> + max_lane = 4;
> + if (IS_IVYBRIDGE(dev))
> + max_lane = ivb_fdi_max_lanes(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);
> }
This chunk is also reworked in a later commit in this series.
--
Damien
