From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jesse Barnes <jbarnes@virtuousgeek.org>
Subject: Re: [PATCH 2/2] drm/i915: Refactor panel fitting on the
	LVDS.
Date: Sat, 17 Jul 2010 18:15:46 -0700
Message-ID: <20100717181546.2d52b7b2@jbarnes-x201>
References: <1279370324-12761-1-git-send-email-chris@chris-wilson.co.uk>
	<1279370324-12761-2-git-send-email-chris@chris-wilson.co.uk>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from cpoproxy2-pub.bluehost.com (cpoproxy2-pub.bluehost.com
	[67.222.39.38])
	by gabe.freedesktop.org (Postfix) with SMTP id 76B129E7BF
	for <intel-gfx@lists.freedesktop.org>;
	Sat, 17 Jul 2010 18:15:54 -0700 (PDT)
In-Reply-To: <1279370324-12761-2-git-send-email-chris@chris-wilson.co.uk>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.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