"Pat Campbell" <plc@xxxxxxxxxx> writes:
>>>> On Thu, Jan 10, 2008 at 3:21 AM, in message
> <87abnehtfn.fsf@xxxxxxxxxxxxxxxxx>, Markus Armbruster <armbru@xxxxxxxxxx>
> wrote:
>> "Pat Campbell" <plc@xxxxxxxxxx> writes:
>>
>>> Attached patch adds multiple frame buffer resolution support to
>>> the PV xenfb frame buffer driver.
>>>
>>> Code is essentially the same as I sent in the previous RFC except
>>> the frame buffer size is now 800x600 if the backend does not
>>> support feature- resize, same memory footprint.
>>>
>>> Corresponding backend IOEMU patch is required for functionality but
>>> this patch is not dependent on it, preserving backwards compatibility.
[...]
>>> static int xenfb_thread(void *data)
>>> {
>>> struct xenfb_info *info = data;
>>> @@ - 217,6 +253,9 @@ static int xenfb_thread(void *data)
>>> if (info- >dirty) {
>>> info- >dirty = 0;
>>> xenfb_update_screen(info);
>>> + }
>>> + if (info- >resize_dpy) {
>>> + xenfb_resize_screen(info);
>>> }
>>
>> Hmm. If both dirty and resize_dpy are set, the update event goes out
>> before the resize event. What if the update is for the resized
>> screen? If whatever caused the update happened after the screen was
>> resized, then the update event overtakes the resize event, and its
>> coordinates will not make sense, will they?
>
> An update event for a rectangle outside the new size will be clipped and
> then discarded in the backend. Wasted time sending it but it should
> have no ill effect.
Yes. It's still ugly. Hmm, the resize makes the backend refresh the
whole screen, doesn't it? That makes the update *always* redundant.
We could suppress the event. I guess that's not quite worth the
trouble. What about a comment explaining this?
>>> wait_event_interruptible(info- >wq,
>>> kthread_should_stop() || info- >dirty);
>>> @@ - 413,6 +452,45 @@ static int xenfb_mmap(struct fb_info *fb
>>> return 0;
>>> }
>>>
>>> +static int
>>> +xenfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>>> +{
>>> + struct xenfb_info *xenfb_info;
>>> +
>>> + xenfb_info = info- >par;
>>
>> Style nitpick:
>>
>> struct xenfb_info *xenfb_info = info- >par;
>>
>>> +
>>> + if (!xenfb_info- >feature_resize) {
>>> + if (var- >xres == XENFB_WIDTH && var- >yres ==
>>> XENFB_HEIGHT
>>> + && var- >bits_per_pixel == xenfb_info-
>>> >page- >depth) {
>>> + return 0;
>>> + }
>>> + return - EINVAL;
>>> + }
>>> + if (var- >bits_per_pixel == xenfb_info- >page- >depth &&
>>> + XENFB_SCAN_LINE_LEN >= var- >xres &&
>>> + xenfb_mem_len >= (var- >xres * var- >yres *
>>> (xenfb_info- >page- >depth / 8))) {
>>> + var- >xres_virtual = var- >xres;
>>> + var- >yres_virtual = var- >yres;
>>> + return 0;
>>> + }
>>> + return - EINVAL;
>>> +}
>>> +
>>> +static int xenfb_set_par(struct fb_info *info)
>>> +{
>>> + struct xenfb_info *xenfb_info;
>>> +
>>> + xenfb_info = info- >par;
>>
>> Style nitpick:
>>
>> struct xenfb_info *xenfb_info = info- >par;
>>
>>> +
>>> + if (xenfb_info- >xres != info- >var.xres ||
>>> + xenfb_info- >yres != info- >var.yres) {
>>> + xenfb_info- >resize_dpy = 1;
>>> + xenfb_info- >xres = info- >var.xres_virtual = info-
>>> >var.xres;
>>> + xenfb_info- >yres = info- >var.yres_virtual = info-
>>> >var.yres;
>>> + }
>>> + return 0;
>>> +}
>>
>> How is the resolution change synchronized between guest user land,
>> pvfb frontend and pvfb backend?
>>
>
> Resolution change is asynchronous. Short of making fb_set_par()
> a blocking call I see no way to make this synchronized.
>
>> I guess method fb_set_par() changes the resolution instantly as far as
>> the guest is concerned. The framebuffer then contains junk, until
>> guest user land fixes it up. The notification of the backend is
>> delayed until xenfb_thread() next checks xenfb_info- >resize_dpy.
>>
>> By the way, this delay is arbitrary; we could call
>> xenfb_resize_screen() here. We route screen updates through
>> xenfb_thread() because we can't do that work from xenfb_vm_nopage().
>> Welcome side effect: also collapses multiple quick updates into one.
>> None of this is of concern for screen resize.
>
> Sending the resize event from within xenfb_thread protects against the
> possibility of the queue being full.
You're right.
> resize_dpy will not be cleared until
> the
> event is successfully inserted in the queue unlike update events which
> are silently discarded if the queue is full. Probably will never happen but
> that is why I put it there.
Update events are not discarded, they're accumulated in the dirty
rectangle.
>> Anyway, the backend continues to interpret the framebuffer according
>> to the old resolution until it receives the notification. The
>> notification also makes it redraw the whole screen. There's no
>> guarantee that guest user land finished fixing up the framebuffer
>> contents by then. If it hasn't, the redraw draws junk. This junk can
>> then remain on the screen indefinitely, because no further screen
>> update requests need to arrive from the frontend. Correct?
>>
>
> Yes, although in testing I have not seen artifacts of the old resolution
> on a resized screen. A simple VNC client refresh by the user would
> correct that :)
I guess (hope?) guest user land saves us there by triggering suitable
update events.
[...]
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home