| Message ID | 1354195773-4022-3-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 29 Nov 2012 11:29:33 -0200, Paulo Zanoni <przanoni@gmail.com> wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > The previous code was making the bps value 5% higher than what the > spec says, which was enough to make certain VGA modes require 3 lanes > instead of 2, which makes us reject these modes on Haswell since it > only has 2 FDI lanes. For previous gens this was not much of a > problem, since they had 4 lanes, and requiring more lanes than the > needed is ok, as long as you have all the lanes. > > Notice that this might improve the case where we use pipes B and C on > Ivy Bridge since both pipes only have 4 lanes to share (see > ironlake_check_fdi_lanes). > > Cc: Adam Jackson <ajax@redhat.com> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > As it is, this one will make the list of supported modes on Haswell VGA bigger, > so we could skip 3.8 and send this through 3.9, so we have plently of time to > get confident this won't break older platforms. > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 8d86a39..1825ae7 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5231,12 +5231,10 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc) > int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp) > { > /* > - * Account for spread spectrum to avoid > - * oversubscribing the link. Max center spread > - * is 2.5%; use 5% for safety's sake. > + * The spec says: > + * Number of lanes >= INT(dot clock * bytes per pixel / ls_clk) > */ > - u32 bps = target_clock * bpp * 21 / 20; > - return bps / (link_bw * 8) + 1; > + return DIV_ROUND_UP(target_clock * bpp, link_bw * 8); Can you split this into two patches, one for using DIV_ROUND_UP and for removingthe oversubscription, as the DIV_ROUND_UP looks to be a separate issue worth testing. (Handling the case where bps % (link_bw * 8) == 0.) -Chris
On 11/29/12 8:29 AM, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > The previous code was making the bps value 5% higher than what the > spec says, which was enough to make certain VGA modes require 3 lanes > instead of 2, which makes us reject these modes on Haswell since it > only has 2 FDI lanes. For previous gens this was not much of a > problem, since they had 4 lanes, and requiring more lanes than the > needed is ok, as long as you have all the lanes. > > Notice that this might improve the case where we use pipes B and C on > Ivy Bridge since both pipes only have 4 lanes to share (see > ironlake_check_fdi_lanes). Fine with me. I'm not entirely sure the SS check I had there was necessary; I do remember the docs saying to account for it, but I'm not sure the check we had there was correct. Reviewed-by: Adam Jackson <ajax@redhat.com> - ajax
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8d86a39..1825ae7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5231,12 +5231,10 @@ static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc) int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp) { /* - * Account for spread spectrum to avoid - * oversubscribing the link. Max center spread - * is 2.5%; use 5% for safety's sake. + * The spec says: + * Number of lanes >= INT(dot clock * bytes per pixel / ls_clk) */ - u32 bps = target_clock * bpp * 21 / 20; - return bps / (link_bw * 8) + 1; + return DIV_ROUND_UP(target_clock * bpp, link_bw * 8); } static void ironlake_set_m_n(struct drm_crtc *crtc, @@ -5296,6 +5294,8 @@ static void ironlake_set_m_n(struct drm_crtc *crtc, lane = ironlake_get_lanes_required(target_clock, link_bw, intel_crtc->bpp); + DRM_DEBUG_KMS("Using %d FDI lanes on pipe %c\n", lane, + pipe_name(intel_crtc->pipe)); intel_crtc->fdi_lanes = lane; if (pixel_multiplier > 1)