Re: [PATCH 2/2] drm/i915: Refactor panel fitting on the LVDS.

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Sat, 17 Jul 2010 13:38:44 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Move the common routines into separate functions to not only increase
> readability, but also throwaway surplus code.
> 
> In doing so, we review the calculation of the aspect preserving
> scaling and avoid the use of fixed-point until we need to calculate
> the accurate scale factor.
> 
> 1 files changed, 128 insertions(+), 194 deletions(-)

Nice cleanups, that function was getting too big...  misc nitpicks
below.

> +	/* keep the border be even */
> +	if (border & 1)
> +		border++;

May as well fix the comment here or combine it with the above.  There
are a few other comments like this elsewhere too.

While you're in there it would also be cool to write a small CRTC mode
verification function that checks the htotal/vtotal/etc against the
constraints documented in the manual (e.g. sync before end, active
divisible by 2 in ganged mode, etc).  If you're feeling adventurous
anyway. :)

Other than that:

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center