[Intel-gfx] [PATCH] drm/i915/pch: Fix integer math bugs in panel fitting

[Christopher James Halse Rogers]

On Wed, 2011-07-13 at 16:32 -0400, Adam Jackson wrote:
> Consider a 1600x900 panel, upscaling a 1360x768 mode, full-aspect.  The
> old math would give you:
> 
>     scaled_width  = 1600 * 768;         /* 1228800 */
>     scaled_height = 1360 * 900;         /* 1224000 */
>     if (scaled_width > scaled_height) { /* pillarbox, and true */
>         width  = 1224000 / 768;         /* int(1593.75) = 1593 */
>         x      = (1600 - 1593 + 1) / 2; /* 4 */
>         y      = 0;
>         height = 768;
>     } /* ... */
> 
> This is broken.  The total width of scanout would then be 1593 + 4 + 4,
> or 1601, which is wider than the panel itself.  The hardware very
> dutifully implements this, and you end up with a black 45° diagonal from
> the top-left corner to the bottom edge of the screen.  It's a cool
> effect and all, but not what you wanted.  Similar things happen for the
> letterbox case.
> 
> The problem is that you have an integer number of pixels, which means
> it's usually impossible to upscale equally on both axes.  1360/768 is
> 1.7708, 1600/900 is 1.7777.  Since we're constrained on the one axis,
> the other one wants to come out as an even number of pixels (the panel
> is almost certainly even on both axes, and the x/y offsets will be
> applied on both sides).  In the math above, if 'width' comes out even,
> rounding down is correct; if it's odd, you'd rather round up.  So just
> increment width/height in those cases.
> 
> Tested on a Lenovo T500 (Ironlake).
> 
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_panel.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index a06ff07..05f500c 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -83,11 +83,15 @@ intel_pch_panel_fitting(struct drm_device *dev,
>  			u32 scaled_height = mode->hdisplay * adjusted_mode->vdisplay;
>  			if (scaled_width > scaled_height) { /* pillar */
>  				width = scaled_height / mode->vdisplay;
> +				if (width & 1)
> +				    	width++;
>  				x = (adjusted_mode->hdisplay - width + 1) / 2;
>  				y = 0;
>  				height = adjusted_mode->vdisplay;
>  			} else if (scaled_width < scaled_height) { /* letter */
>  				height = scaled_width / mode->hdisplay;
> +				if (height & 1)
> +				    height++;
>  				y = (adjusted_mode->vdisplay - height + 1) / 2;
>  				x = 0;
>  				width = adjusted_mode->hdisplay;

We've seen this in Ubuntu in the form of
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/753994 so we can
add a

Tested-By: Daniel Manrique <daniel.manrique at canonical.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20110720/8c11f22c/attachment.sig>