| Message ID | 1312208395-13855-2-git-send-email-apw@canonical.com |
|---|---|
| State | New |
| Headers | show |
Applied to Oneiric master-next. Thanks, Leann On Mon, 2011-08-01 at 15:19 +0100, Andy Whitcroft wrote: > From: Adam Jackson <ajax@redhat.com> > > 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@redhat.com> > Tested-By: Daniel Manrique <daniel.manrique@canonical.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38851 > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: stable@kernel.org > Signed-off-by: Keith Packard <keithp@keithp.com> > > BugLink: http://bugs.launchpad.net/bugs/753994 > Signed-off-by: Andy Whitcroft <apw@canonical.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; > -- > 1.7.4.1 > >
On Mon, 2011-08-01 at 15:19 +0100, Andy Whitcroft wrote: > From: Adam Jackson <ajax@redhat.com> > > 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@redhat.com> > Tested-By: Daniel Manrique <daniel.manrique@canonical.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38851 > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: stable@kernel.org > Signed-off-by: Keith Packard <keithp@keithp.com> > > BugLink: http://bugs.launchpad.net/bugs/753994 > Signed-off-by: Andy Whitcroft <apw@canonical.com> For Natty, Acked-by: Leann Ogasawara <leann.ogasawara@canonical.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; > -- > 1.7.4.1 > >
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;