[Spice-devel] [PATCH] [xf86-video-qxl] In XSpice mode, enable the ability to specify the various ram buffer sizes.

[Jeremy White]

On 08/22/2014 06:55 AM, Marc-André Lureau wrote:
> Hi Jeremy
>
> I wish we could have dynamic allocation for Xspice, avoiding those parameters, but this could be improved later.

Yes; I looked into that, and am willing to do that work.  For example, I 
considered simply replacing qxl_mem.c with an Xspice specific version 
using malloc. The code is a bit hard to parse thoroughly, but I have the 
recollection that the io port messaging mechanism relies on ram being 
contiguous, so it seemed like that approach would not readily work.

If that's wrong, I'd happily be tapped with the clue bat.

>> +    # The amount of frame buffer ram, in megabytes, to allocate
>> +    # defaults to 16
>> +    #Option "FrameBufferSize" "16"
>> +
>> +    # The amount of surface buffer ram, in megabytes, to allocate
>> +    # Must be larger than FrameBufferSize
>
> Is there a runtime check for that?

No, and I learned late yesterday that isn't true in the case of dfps 
mode (we don't use surface ram for dfps mode).  In fact, the memory 
balance of these parameters is unclear; I'm not sure if we have any 
simple rules.

What I'll do is change these comments to simply document my 
understanding of what they do, and remove any suggestion of rules. 
[Arguably, if we knew exactly how to compute one buffer relative to 
another, we should just do that, instead of allowing configuration]

>>       for (i = 0; i < modes->n_modes; i++) {
>>           fb = qxl_modes[i].y_res * qxl_modes[i].stride;
>> -        if (maxfb < fb) {
>> -            maxfb = fb;
>> +        if (fb > qxl->surface0_size)
>> +        {
>> +            modes->n_modes--; i--;
>> +            continue;
>
> Isn't this going to check the same qxl_modes[i] until i == modes->n_modes?

Yep, that's a bug.  Thanks for catching that.

Cheers,

Jeremy