Bug 13505 - need better panel fitting control
Summary: need better panel fitting control
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/intel (show other bugs)
Version: 7.3 (2007.09)
Hardware: x86 (IA32) Linux (All)
: medium enhancement
Assignee: Jesse Barnes
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords: NEEDINFO
: 10956 14530 (view as bug list)
Depends on:
Blocks: 15000
  Show dependency treegraph
 
Reported: 2007-12-03 15:25 UTC by Aaron
Modified: 2008-03-29 04:51 UTC (History)
6 users (show)

See Also:
i915 platform:
i915 features:


Attachments
disable panel fitter (1.50 KB, text/x-diff)
2007-12-04 17:32 UTC, Jesse Barnes
no flags Details
patch to configure scaler to keep aspect ratio (576 bytes, patch)
2008-02-25 09:11 UTC, Ondrej Caletka
no flags Details | Splinter Review
Expose panel fitting modes (13.22 KB, patch)
2008-03-11 17:46 UTC, Jesse Barnes
no flags Details | Splinter Review
Updated panel fitting patch (18.73 KB, patch)
2008-03-13 16:16 UTC, Jesse Barnes
no flags Details | Splinter Review
Updated panel fitting patch (18.48 KB, patch)
2008-03-13 18:06 UTC, Jesse Barnes
no flags Details | Splinter Review

Description Aaron 2007-12-03 15:25:18 UTC
I have a laptop with intel 945 graphics card. with i810 driver version <2.0.0 when I changed resolution for example to 800x600 (I have a wide-screen laptop with maximum resolution 1280x800), the screen appeared small in the centrer of the monitor, and the rest remained black. Now with newer drivers (renamed to intel drivers) always my whole screen gets covered, even when I use a non-wide resolution, what results in a stretched screen that doesn't look well. I have the corresponding option enabled in bios, still it doesn't work with the newer drivers.
Comment 1 Gordon Jin 2007-12-03 21:54:46 UTC
Keith, is this related to your screen scaling property work?
Comment 2 Jesse Barnes 2007-12-04 17:31:55 UTC
Aaron, does this patch prevent the stretching for you?  If so, then it's just a panel fitter issue.  We enable it unconditionally right now, but it would be easy to change that...
Comment 3 Jesse Barnes 2007-12-04 17:32:13 UTC
Created attachment 12951 [details]
disable panel fitter
Comment 4 Aaron 2007-12-05 07:17:15 UTC
It almost works.
The screen (not stretched) appears in left top corner of my monitor, but the rest of the monitor is not blank. From what I see, rightmost pixels get copied in the area that should be black (they move when I move windows and so on). Something like that also applies to the bottommost pixels.
Comment 5 Jesse Barnes 2007-12-11 17:14:25 UTC
Ok, thanks for testing, I think we can fix this up.  I'll post another patch when I get a chance.
Comment 6 Jesse Barnes 2008-01-03 12:03:40 UTC
The current driver doesn't expose panel fitting options to the user.  It would be good if it did, but we'll have to add some randr properties and additional modesetting code for smaller-than-native, non-scaled panel modes so that the mode is centered with black borders.
Comment 7 Jesse Barnes 2008-02-19 10:25:54 UTC
*** Bug 14530 has been marked as a duplicate of this bug. ***
Comment 8 Gordon Jin 2008-02-24 19:24:44 UTC
*** Bug 10956 has been marked as a duplicate of this bug. ***
Comment 9 Ondrej Caletka 2008-02-25 09:11:05 UTC
Created attachment 14560 [details] [review]
patch to configure scaler to keep aspect ratio

I have written an ugly patch, which sets the scaler to use pilarbox, so any narrower resolution than native is fitted on full screen using black bars on left and right side. It is just a quick workaround.
Comment 10 Aaron 2008-02-25 13:43:56 UTC
No, it doesn't work for me - when I change resolution, then my whole screen gets black excluding mouse cursor, right side copping of mouse still occurs.
Comment 11 Jesse Barnes 2008-03-11 17:46:33 UTC
Created attachment 15044 [details] [review]
Expose panel fitting modes

Here's a test patch for this feature.  It still needs some work (for example 800x600 doesn't work on my T61 with "centered" mode) and testing on i915, but it already works on 965 with many modes.  See the man page additions in the patch for instructions on the various new LVDS properties to control this feature.
Comment 12 WuNian 2008-03-12 01:14:08 UTC
Hi, Jesse,
I tested the patch on our T61(max resolution 1280x800), tested all FITTING mode and combined with rotate, different resolution, glxgears, VT switch, video play, no bug found.

On our T61, the patch works with 800x600 and center mode.

On 945GM(HP 500 max resolution 1280x800), when the mode is 1024x768, if set the mode as center, the display will align in left instead on center. The basic function is incorrect, so I do not continue another test.

Because Eric has some modification to i830_lvds.c yesterday, so attach your patch to current tip failed. Then I attached your patch on commit ecdb5963... 


Comment 13 Jesse Barnes 2008-03-12 16:48:20 UTC
Thanks for testing, Nian.  I found some bugs that were causing some of the problems you saw (LVDS reg should have border enabled for one), but I'm still struggling with programming the right ratios for full aspect modes on i915.  Maybe I'll have another test patch by Fri.
Comment 14 Jesse Barnes 2008-03-13 16:16:12 UTC
Created attachment 15100 [details] [review]
Updated panel fitting patch

Here's an updated patch that works on my 915:
  - centered, non scaled modes with black border
  - scaled, aspect preserving modes with black border
  - full screen scaled modes

I didn't change the 965 code, so it should still work too.  Please give it a try.
Comment 15 Jesse Barnes 2008-03-13 18:06:53 UTC
Created attachment 15101 [details] [review]
Updated panel fitting patch

This one fixes a bug relative to the last patch where mode changes may not have happened correctly when going from a pillared, centered or letterbox mode to a normal or full screen mode.
Comment 16 Michael Fu 2008-03-13 19:14:42 UTC
Aaron, can you have a try of Jesse's patch? thanks.
Comment 17 Aaron 2008-03-13 23:05:28 UTC
I can try this today. Only one question - which version of the driver I should patch?
Comment 18 Michael Fu 2008-03-14 06:19:33 UTC
(In reply to comment #17)
> I can try this today. Only one question - which version of the driver I should
> patch?
> 

you can just pull the git..
Comment 19 Aaron 2008-03-14 06:38:15 UTC
Probably I can, but it is much easier for me to patch a gentoo package. But I assume that your answer is - the newest one.
Comment 20 Aaron 2008-03-14 07:29:47 UTC
I've ended using the git version anyway. It works, at least the standard mode, as I can't figure out how to change it (even after reading the manual=).
Comment 21 Jesse Barnes 2008-03-14 09:47:16 UTC
Oh sorry, the man page assumes some familiarity with how to change properties.  If you have a recent enough 'xrandr' command installed, you can use it:
  xrandr --prop # show available outputs, properties, valid values, etc.
  xrandr --output LVDS --set PANEL_FITTING center # set PANEL_FITTING property to 'center'

We should probably expand the man page to include a few examples like that.
Comment 22 Aaron 2008-03-14 09:54:58 UTC
It works as it should.
In fact I've tried this command, but using capitalised CENTER as it was in the manual and it doesn't work then.
Comment 23 Jesse Barnes 2008-03-14 09:56:26 UTC
Ah ok, thanks for testing.  I'll fix the man page.
Comment 24 Bruno 2008-03-14 15:46:35 UTC
I did test the patch from comment #15 as well (against git revision 69fbc17441d0f894d17b058e65ae22300cd2a54c), the initial results were fine (1400x1050, then 1280x1024 center, full, full_aspect).

Then I selected mode 640x350 and other small modes offered by xrandr and got "nice" display:
Display starts black with lots of dark red, green and blue dots, the whole "picture" slowly changing to white and then back to black with a few surviving black green dots.
Switching back to text-mode console or switching modes does not recover display (only reboot helps).

This broken display does not happen without this patch.
Comment 25 Bruno 2008-03-14 15:55:37 UTC
Ups, forgotten to tell what system I have:
  Fujitsu Siemens S7020 with i915:

00:02.0 VGA compatible controller [0300]: Intel Corporation Mobile 915GM/GMS/910GML Express Graphics Controller [8086:2592] (rev 04)
00:02.1 Display controller [0380]: Intel Corporation Mobile 915GM/GMS/910GML Express Graphics Controller [8086:2792] (rev 04)

Flat panel: 1400x1050


Note 1:
  The "center/stretch" key of the keyboard has no effect (screen flickers
  once) when Xorg is started from text-console, not tested from framebuffer
Note 2:
  make dist on current git does not include i810_ring.h and i830_ring.h which
  are needed when compiling from the generated tarball
Note 3:
  display breakage does at least happen for full_aspect layout but not center
  with the small mode 640x350.
Comment 26 WuNian 2008-03-17 01:17:11 UTC
On my 945GM(HP 500, max resolution 1280x800), the patch works fine. No fault found.
Comment 27 Jesse Barnes 2008-03-17 15:28:31 UTC
Hi Bruno, thanks for testing.  I saw you had a few observations:

> Note 1:
>   The "center/stretch" key of the keyboard has no effect (screen flickers
>   once) when Xorg is started from text-console, not tested from framebuffer

Yeah, this would need to be wired up somehow.  On recent Linux kernels, events like this are delivered as regular key presses, so it's up to users or distros to configure applications to respond to them appropriately (in this case running an xrandr command or equivalent).

> Note 2:
>   make dist on current git does not include i810_ring.h and i830_ring.h
>   which are needed when compiling from the generated tarball

Can you file a new bug for that?

> Note 3:
>   display breakage does at least happen for full_aspect layout but not
>   center with the small mode 640x350.

I just confirmed this on my i915 when adding a custom 640x350 mode.  It only seems to break on full_aspect though, both center and full were fine.  All the pieces seem to be in place though, so I'll treat this as a new and separate bug with panel fitting.
Comment 28 Julien Cristau 2008-03-18 04:15:00 UTC
> --- Comment #27 from Jesse Barnes <jbarnes@virtuousgeek.org>  2008-03-17 15:28:31 PST ---
> > Note 2:
> >   make dist on current git does not include i810_ring.h and i830_ring.h
> >   which are needed when compiling from the generated tarball
> 
> Can you file a new bug for that?
> 
I just fixed that in b1b173d03b3acd300c3b0f0ceffeddf1a8137839.
Thanks, Bruno!

Cheers,
Julien
Comment 29 Bruno 2008-03-18 14:07:36 UTC
(In reply to comment #27)
> > Note 1:
> >   The "center/stretch" key of the keyboard has no effect (screen flickers
> >   once) when Xorg is started from text-console, not tested from
> >   framebuffer
> 
> Yeah, this would need to be wired up somehow.  On recent Linux kernels,
> events like this are delivered as regular key presses, so it's up to
> users or distros to configure applications to respond to them appropriately
> (in this case running an xrandr command or equivalent).

Unfortunately Fujitsu-Siemens does not implement ACPI-video but does it in a proprietary way where kernel does not yet pass key presses through (or prevent ACPI/VBIOS from attempting to operate on configuration).
I still need to (re)check how the modesetting/fitting interoperates with the ACPI/VBIOS response to the center/stretch key when kernel was started with framebuffer instead of text mode.
See also bug #13844 for this.

> > Note 2:
> >   make dist on current git does not include i810_ring.h and i830_ring.h
> >   which are needed when compiling from the generated tarball
> 
> Can you file a new bug for that?
Already fixed, see comment #28


Would it be possible to add a fourth fitting mode like full_aspect but that would only scale with a integer factor? (e.g. 640x480 => 1280x960 centered but 800x600 would remain 800x600 as 1600x1200 does not fit into 1400x1050)

Another possible issue I plan on checking tomorrow when I have more time is if resolution (dpi) is still correct in the different fitting modes.
Comment 30 Jesse Barnes 2008-03-18 15:31:02 UTC
> Unfortunately Fujitsu-Siemens does not implement ACPI-video but does it in
> a proprietary way where kernel does not yet pass key presses through (or
> prevent ACPI/VBIOS from attempting to operate on configuration).
> I still need to (re)check how the modesetting/fitting interoperates with
> the ACPI/VBIOS response to the center/stretch key when kernel was started
> with framebuffer instead of text mode.
> See also bug #13844 for this.

Hm, ok, looks like we still have some work to do in that area.  We should add more support to the 2.x driver for handling events like these...

> > > Note 2:
> > >   make dist on current git does not include i810_ring.h and i830_ring.h
> > >   which are needed when compiling from the generated tarball
> >
> > Can you file a new bug for that?
>
> Already fixed, see comment #28

Yep, saw that.  Thanks Julien!

> Would it be possible to add a fourth fitting mode like full_aspect but that
> would only scale with a integer factor? (e.g. 640x480 => 1280x960 centered
> but 800x600 would remain 800x600 as 1600x1200 does not fit into 1400x1050)

You think this would make the scaled images look better?  Or is there some other reason you want a mode like this?

> Another possible issue I plan on checking tomorrow when I have more time is
> if resolution (dpi) is still correct in the different fitting modes.

I'm not doing anything special to update it, so it may be wrong.  If so, it would be good to fix...
Comment 31 Bruno 2008-03-19 12:14:59 UTC
(In reply to comment #30)
> > Unfortunately Fujitsu-Siemens does not implement ACPI-video but does it in
> > a proprietary way where kernel does not yet pass key presses through (or
> > prevent ACPI/VBIOS from attempting to operate on configuration).
> > I still need to (re)check how the modesetting/fitting interoperates with
> > the ACPI/VBIOS response to the center/stretch key when kernel was started
> > with framebuffer instead of text mode.
> > See also bug #13844 for this.
> 
> Hm, ok, looks like we still have some work to do in that area.  We should
> add more support to the 2.x driver for handling events like these...

After some basic testing from kernel framebuffer, with the intel driver then prefering the fb mode (1280x1024) to the one detected in BIOS (1400x1050) I get the bad results I expected:
- starts with X surface in top-left corner at 1280x1024
- choosing a different mode in fitting-center centers the new mode in the
  top-left 1280x1024 rectangle (logical) but here the Center/Stretch key
  does apply changes (in stetch mode it wraps pixel-lines at 1400 instead of
  selected mode, in center mode it centers the 1280x1024 rectangle which
  contains the X mode at its top-left corner, the rest being filled with
  previous framebuffer content - mouse area properly cropped)

I can't try in full_aspect mode as it breaks display as in bug #15099 even at not so small modes.

> > Would it be possible to add a fourth fitting mode like full_aspect but
> > that would only scale with a integer factor? (e.g. 640x480 => 1280x960
> > centered but 800x600 would remain 800x600 as 1600x1200 does not fit into
> > 1400x1050)
> 
> You think this would make the scaled images look better?  Or is there some
> other reason you want a mode like this?

The idea here is to use available display space without distortion. Some images with high contrast (like default X background) look like moiré when stretched as done by full_aspect.
For fonts the same effect is to be exected.
As such it's a fair mid-way between full_aspect and center.
(Not sure how the intel graphics can do the stretching here - display one original pixel as 2x2 pixels or still do some distortion; as an example the nvidia proprietary drivers does this in 1px => 2x2 px)

> > Another possible issue I plan on checking tomorrow when I have more time
> > is if resolution (dpi) is still correct in the different fitting modes.
> 
> I'm not doing anything special to update it, so it may be wrong.  If so, it
> would be good to fix...

In full mode the DPI remains unchanged but display size is reported as changed (xdpyinfo output). The opposite would be correct here.

For center fitting the result is correct with non-changing DPI :)

========== generic DPI request ==========
Note: there are multiple bugs around regarding DPI... (migh need own bug or be a dupe of an existing one)

According to xdpyinfo this setting is per-screen (same can be deduced from xrandr --fbmm option).
As long as multiple outputs don't display overlapping areas of the framebuffer their DPI should be independent (if the output distinction is visible to X applications like e.g. via xinerama), in case of overlapping areas the DPI for the "primary" output of both should be used.

No idea if this can be implemented in a reasonable effort as windows can overlap both outputs... (applications would certainly need review in this case)
Comment 32 Michael Fu 2008-03-24 00:05:42 UTC
(In reply to comment #31)
> (In reply to comment #30)
> > > Unfortunately Fujitsu-Siemens does not implement ACPI-video but does it in
> > > a proprietary way where kernel does not yet pass key presses through (or
> > > prevent ACPI/VBIOS from attempting to operate on configuration).
> > > I still need to (re)check how the modesetting/fitting interoperates with
> > > the ACPI/VBIOS response to the center/stretch key when kernel was started
> > > with framebuffer instead of text mode.
> > > See also bug #13844 for this.
> > 
> > Hm, ok, looks like we still have some work to do in that area.  We should
> > add more support to the 2.x driver for handling events like these...
> 
> After some basic testing from kernel framebuffer, with the intel driver then
> prefering the fb mode (1280x1024) to the one detected in BIOS (1400x1050) I get
> the bad results I expected:
> - starts with X surface in top-left corner at 1280x1024
> - choosing a different mode in fitting-center centers the new mode in the
>   top-left 1280x1024 rectangle (logical) but here the Center/Stretch key
>   does apply changes (in stetch mode it wraps pixel-lines at 1400 instead of
>   selected mode, in center mode it centers the 1280x1024 rectangle which
>   contains the X mode at its top-left corner, the rest being filled with
>   previous framebuffer content - mouse area properly cropped)
> 
> I can't try in full_aspect mode as it breaks display as in bug #15099 even at
> not so small modes.
> 
Bruno, To keep the bug clear and focused on its summary line, let's focused on the panel fitting topic here. I think you may need a special acpi module to use the proprietary hotkey feature on your laptop. :)
Comment 33 Krzysztof Lubański 2008-03-24 13:16:18 UTC
(In reply to comment #15)
> Created an attachment (id=15101) [details]
> Updated panel fitting patch

Hello,

I have just patched the driver (from git) and loaded it for a 945GM (Toshiba Satellite M105-S3074):

00:02.0 VGA compatible controller [0300]: Intel Corporation Mobile 945GM/GMS, 943/940GML Express Integrated Graphics Controller [8086:27a2] (rev 03)
00:02.1 Display controller [0380]: Intel Corporation Mobile 945GM/GMS/GME, 943/940GML Express Integrated Graphics Controller [8086:27a6] (rev 03)

Panel fitting settings work, but it seems that bit depth setting in 2D gets incorrect.

After patching, when in 24bpp some desktop bitmaps look like in 16bpp (color gradients are not smooth), and when I switch to 16bpp in xorg.conf - they look smooth, as if in 24bpp. With a clean git, all is good - smooth gradients in 24bpp.

In 3D, however, there is nothing wrong after applying the patch - GL applications in 24/32bpp look as they should look.
Comment 34 Jesse Barnes 2008-03-26 14:56:32 UTC
Done.  Pushed the pfit branch into master today.
Comment 35 Jesse Barnes 2008-03-26 15:00:35 UTC
Oh, forgot about your report, Krzysztof.  Can you file a new bug for it?  There may be something wrong with the dither setting for your machine (look for PANEL_8TO6_DITHER_ENABLE if you want to try turning it off).
Comment 36 Krzysztof Lubański 2008-03-29 04:51:35 UTC
(In reply to comment #35)
> Oh, forgot about your report, Krzysztof.  Can you file a new bug for it?  There
> may be something wrong with the dither setting for your machine (look for
> PANEL_8TO6_DITHER_ENABLE if you want to try turning it off).
> 

Ok, I'm filing a new bug in a minute. Setting PANEL_8TO6_DITHER_ENABLE to 0 didn't help, unfortunately.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.