qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: Alon Levy <alevy@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] qxl: change rom so that 4096 < size < 8192
Date: Tue, 15 Jan 2013 16:19:38 +0100	[thread overview]
Message-ID: <50F5738A.7080904@redhat.com> (raw)
In-Reply-To: <20130115133423.GH21732@garlic.redhat.com>

  Hi,

>>>>  #define QXL_MODE_EX(x_res, y_res)                 \
>>>>      QXL_MODE_16_32(x_res, y_res, 0),              \
>>>> -    QXL_MODE_16_32(y_res, x_res, 1),              \
>>>> -    QXL_MODE_16_32(x_res, y_res, 2),              \
>>>> -    QXL_MODE_16_32(y_res, x_res, 3)
>>>> +    QXL_MODE_16_32(x_res, y_res, 1)
>>>
>>> Why do you leave orientation = 1 in?  Just to keep the size above 4K?
>>> Shouldn't we just hardcode the rom size to 8k instead?  Then assert that
>>> everything fits into 8k?  Or even better add a compile time check?
>>>
>>> While being at it it might be a good idea to move the mode table to a
>>> fixed, large enougth offset (say 0x4096), so it doesn't move around
>>> again in case we extend QXLRom one more time.
>>
>> This solution is breaking backward compatibility like Yonit pointed
>> out. The fact that I can't produce a user that would break because of
>> this doesn't prove there is no such user. I suggest we go back to the
>> original patch I posted, breaking it into two like you requested. What
>> do you say?
> 
> Ping?

I think I've answered this one already.

It is still not clear to me how this orientation thing is supposed to
work and what exactly we loose when we drop those modes from the list.

I havn't found a way to activate a portrait mode (orientation == 1,3) in
the windows  guest.

Orientation isn't used anywhere in qxl (other than rom initialization),
so in case the guest activates a mode with orientation != 0 spice server
(and thus spice client) isn't notified about that.  So spice client
wouldn't rotate the display according to the guests orientation.  Hmm?

And finally only prehistorical 0.4.x spice guests (which use SET_MODE)
can pick modes with orientation != 0.  The QXLSurfaceCreate struct has
no orientation field ...

cheers,
  Gerd

  reply	other threads:[~2013-01-15 15:19 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-06 15:41 [Qemu-devel] [PATCH] qxl: use correct rom size for revision < 4 Alon Levy
2012-12-12 11:46 ` Gerd Hoffmann
2012-12-13 10:40   ` Alon Levy
2012-12-13 14:30     ` Yonit Halperin
2012-12-13 14:43       ` Gerd Hoffmann
2012-12-23 20:33         ` Alon Levy
2013-01-03  7:55           ` Gerd Hoffmann
2012-12-13 11:36   ` [Qemu-devel] [PATCH 1/2] qxl: stop using non revision 4 rom fields " Alon Levy
2012-12-13 11:36     ` [Qemu-devel] [PATCH 2/2] qxl: change rom so that 4096 < size < 8192 Alon Levy
2012-12-13 12:05       ` Gerd Hoffmann
2012-12-23 20:31         ` Alon Levy
2013-01-15 13:34           ` Alon Levy
2013-01-15 15:19             ` Gerd Hoffmann [this message]
2013-01-16 17:59               ` [Qemu-devel] [PATCH v2 0/2] fix two revision related errors Alon Levy
2013-01-16 17:59                 ` [Qemu-devel] [PATCH v2 1/2] qxl: stop using non revision 4 rom fields for revision < 4 Alon Levy
2013-01-16 17:59                 ` [Qemu-devel] [PATCH v2 2/2] qxl: change rom size to 8192 Alon Levy
2013-01-17 13:02                 ` [Qemu-devel] [PATCH v2 0/2] fix two revision related errors Gerd Hoffmann
2013-01-17 13:28                   ` Alon Levy
2013-01-17 13:44                     ` Gerd Hoffmann
2013-01-20 16:30                   ` Alon Levy
2013-01-21  6:26                     ` Gerd Hoffmann
2013-01-21 12:47                   ` Alon Levy
2013-01-21 12:48                   ` [Qemu-devel] [PATCH v3 1/2] qxl: stop using non revision 4 rom fields for revision < 4 Alon Levy
2013-01-21 12:48                     ` [Qemu-devel] [PATCH v3 2/2] qxl: change rom size to 8192 Alon Levy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50F5738A.7080904@redhat.com \
    --to=kraxel@redhat.com \
    --cc=alevy@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).