Re: [video-qxl 2/2] Default to only one head, not 4.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 01/23/2013 11:39 PM, Jeremy White wrote:
BTW reading patch 1 again, I wonder what exactly it tries to solve, since
currently when using spice with qemu (no experience with x-spice) you can
already set modes > 1024x768 without problems.

Alright, I've spent my day relearning this code, and feel I have a
better grasp.

The issue is primarily this code here:
  http://cgit.freedesktop.org/xorg/driver/xf86-video-qxl/tree/src/qxl_driver.c#n2325

That sets the current virtual x/y to 1024x768, and calls Xorg's code to
process the initial configuration.  One of the things that code does is
read the configured modelines in the supplied xorg.conf file, and finds
a mode that fits within that virtual size.  However, having had the
virtual display set to 1024, that code will apply that as a maximum, and
will pick a modeline from the spiceqxl.xorg.conf file that is within
1024x768.

You *can* use xrandr to change to a larger resolution later on, but you
cannot specify a default starting resolution in the spiceqxl.xorg.conf
file.  As far as I can tell, virt-viewer usage never uses an xorg.conf
file, so you guys never see this problem.  You've always got
gnome-settings-daemon or someone else resizing displays for you (which
does, in fact work).

In my humble opinion, the current code attempts to set a default mode of
1024x768 in a crude and incorrect fashion.

This led to Marc-André's patch of last July (c1b537fc,
  Return a preferred mode matching the current mode), which
I think in turn triggers bug 894421.

Agreed.

The good news is that I ever so humbly <grin> think my patch 1 fixes all
of these issues.

Agreed too, I actually send a patch yesterday which borrows part of
your patch to fix bug 894421.

Essentially, it sets the M_T_PREFERRED flag on a 1024x768 mode in a
non invasive way.  In my testing, it works correctly for my case, and
for the cases I tried with virt viewer and fc18. I was able to reproduce
the behavior Marc-André reported last summer,
and confirm that my patch seems to also correct for it.

Good!

I do have some mysteries.  fc18 doesn't seem to restore screen
resolution after reboot, which surprises me.  (This is true both with
and without my patch).

This is normal behavior. The screen resolution "restoring" is done
by gnome-settings-daemon (or something similar if running another desktop
environment) and gnome-settings-manager will only restore the last resolution
set through gnome-display-settings. So if you've changed the size by simply
resizing the window this won't get restored since it never got stored in
the gnome-settings. So you end up with the default 1024x768, unless you've
actually used gnome-display-settings before, and then you end up with whatever
you picked the last time you ran gnome-display-settings.

This is all not very pretty, but basically the agent and the gnome-settings
stuff are running independent of each other, and integrating them is tricky
because we don't want to dependent on a certain desktop environment.

<snip>

I also could not reproduce bug 894421,

I can and I'm going to try your patch now instead of my more minimal fix.

> although without my patch I see a variation of it,
where default window sizes are often perversely small.

Yes I just hit that too, but only once. I'm happy to hear that your patch
fixes this too :)

I am resubmitting my patch 1 as a new patch, which partially reverts
c1b537fc; I would appreciate it if others could review that code.

I've reviewed it and it looks good, but I'm a bit unfamiliar with some
of the parts of xorg involved. I'm going to do some testing with your
patch now on both RHEL-6.4 and F-18 guests and then I'll reply to
the mail where you send the new version of the patch.

Regards,

Hans
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]