RandR bugs

[Alan Hourihane]

On Thu, 2005-09-29 at 13:16 +0200, Thomas Winischhofer wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Alan Hourihane wrote:
> > On Thu, 2005-09-29 at 12:40 +0200, Thomas Winischhofer wrote:
> >>>>3) This is a more conceptual issue:
> >>>>
> >>>>The sis driver allows switching output devices during server-runtime.
> >>>>However, not all modes are supported for all available output devices.
> >>>>Under normal circumstances, the driver's mode validation takes care of this.
> >>>>
> >>>>But:
> >>>>
> >>>>RandR does not allow setting a configuration, be it display size, be it
> >>>>rotation/reflection, without (re-)setting the display mode. This is
> >>>>suboptimal.
> >>>>
> >>>>For example, if I have a 1280x800 virtual screen and a current display
> >>>>mode of 1024x768 (eg because I switched from LCD to TV, and TV does not
> >>>>support 1280x800), setting the RandR config just in order to change the
> >>>>rotation will cause RandR to try to switch the display mode to 1280x800
> >>>>(because that is the desired screen size). This will fail (due to the
> >>>>driver validating the display mode).
> >>>>
> >>>>At this point everything goes havoc due to the bugs mentioned in 1) and
> >>>>2) above.
> >>>>
> >>>>However, even if the failures of xf86SwitchMode() are handled properly,
> >>>>the whole RandR request will fail.
> >>>>
> >>>>So, I propose the following to solve this: RandR should only touch the
> >>>>display mode if it is too large for the desired screen size. Otherwise
> >>>>the display mode should be left untouched.
> >>>>
> >>>>Comments?
> >>>
> >>>
> >>>Sounds reasonable. Have you got a patch ? That's much easier to comment
> >>>on. It might be reasonable to open a bug on bugzilla for this so
> >>>reasonable patches can be made and reviewed.
> >>>
> >>>Also, I'm not sure RRFunc is needed at all. It seems to me that
> >>>xf86SwitchMode() is good enough to let the driver handle the necessary
> >>>switch.
> >>>
> >>>The only thing that is missing is a call that the driver can make to
> >>>obtain the current rotation mode (i.e. randrp->rotation). Removing the
> >>>need completely for RRFunc.
> >>
> >>
> >>I somewhat like the idea of the DriverFunc (as it is called in HEAD).
> >>And it's already used for Egbert's method to eventually run the server
> >>without being root.
> > 
> > 
> > Mmm, I've not heard about that.
> 
> 
> See the fbdev and sisusb drivers and xf86Init.c for this. Both drivers
> allow running the server as non-root.

I'll take a look. Thanks for the pointer.

> >>And using the driver's switchmode function does not work around the
> >>issue neither. Basically, I don't see what stuff like screen size or
> >>rotation or reflection have to do with the display mode. It is not
> >>RandR's business to mess with the display mode, IMHO.
> > 
> > 
> > RandR doesn't mess with the display mode at all. It just validates the
> > calling data against the current modepool.
> >
> > We already use xf86SwitchMode(), as RandR allows us to switch to
> > different resolutions. 
> 
> 
> What is "resolution" in that case? This sounds very much like "display
> resolution", ie "display mode".

Right.

> But I question just that: Is it really the "*Resize* and *Rotate*"
> extension's job to set the display mode/rate?

It's resize and rotate with respect to the drivers validated modes from
it's own modepool. 

> It completely ignores that eventually the user wants a different display
> mode than screen size...

I'm not sure what you mean here, but when you do 'xrandr -q' it lists
valid display modes to switch to. You can use 'xrandr -s ...' to switch
to a different mode which is pretty much the same as using CTRL+ALT and
+/- to switch, but 'xrandr -s ..' let's you jump straight to the mode in
question.

In this respect the call to DriverFunc is probably never needed, and
SwitchMode does it's thing.

> Semantically, it's no different to handle
> > rotation in that call as well. Given that the driver doesn't have to
> > implement DriverFunc at all, and it could conceivably just deal with
> > things in it's own SwitchMode, I've got no problems with DriverFunc
> > staying around.
> 
> If the rotation parameter is accessible somewhere from switchmode, I'll
> be find with that as well. I, by no means, insist of having this
> "DriverFunc" (although it could come handy for binary compatibility,
> because eventual further driver hooks don't need an entry in pScrn or
> anywhere else. Just a new "op".)
> 
> I tried to think of a way to work around the issue mention above. Now,
> what is the intended way to handle the user-provided "rate" in case
> randr would not touch the display mode according to my proposal...?
> 
> That is a question that touches the whole idea of RandR.

The drivers own validated modepool is key to the current implementation
in X.Org. If you add the rights modes with the correct refresh rates to
the modepool and it's validated for the monitor then it will appear in
the 'xrandr -q' output as an available mode to switch to.

So, rate, is determined from the modepool and advertised via 'xrandr
-q'. And when the user asks for the specified rate - randr can deal with
it properly as it's in the modepool.

Alan.