Re: [PATCH 3/3] drm/i915: fix FDI lane calculation

[Chris Wilson <chris@chris-wilson.co.uk>]

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

-- 
Chris Wilson, Intel Open Source Technology Centre