XMir now on vmwgfx

[Thomas Hellstrom]

On 12/20/2013 08:59 AM, Christopher James Halse Rogers wrote:
> On Fri, 2013-12-20 at 08:45 +0100, Thomas Hellstrom wrote:
>> Hi,
>>
>> Thanks for the reply.
>>
>> On 12/20/2013 03:25 AM, Daniel van Vugt wrote:
>>> Thomas,
>>>
>>> Excellent work, thanks.
>>>
>>> The two people best placed to answer your questions are now on
>>> vacation, but I shall try;
>>>
>>> 1) There is no explicit message that DRM mastership has been dropped.
>>> Mir will just block in the page flip while the (Mir) server is no
>>> longer consuming buffers:
>>>
https://urldefense.proofpoint.com/v1/url?u=https://github.com/RAOF/xserver/blob/vladmir-upstreaming/hw/xfree86/xmir/xmir-window.c%23L149&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0A&m=OZU2uhMnuCLeFGemU2b1KMBMAOfsNVVUYP8oK4asWXQ%3D%0A&s=b5554ffe56b763b91618a403405d0a7f02c321e648c7095e828fba712ec8a6c5
>>>
>>> But that happens after the drop of mastership so there's still a
>>> potential race between the server dropping it, and the DDX rendering.
>>> Worst case is that the DDX could render 2 frames before filling the
>>> queue and then no more will be requested until the server resumes. Is
>>> it more than 2 extraneous frames you're seeing? Or is two still a
>>> problem for the DDX? Do we need something like this?...
>>>    mir_drm_lock();
>>>    /* DDX code in which mastership won't be dropped */
>>>    mir_drm_unlock();
>>>
>>
>> Actually, it's more an issue of DRM security: Is a client allowed to use
>> DRM after its master has dropped? The vmwgfx kernel module blocks any
>> such attempts (but I'm not sure that is the right solution).
>
> All the hardware DRM drivers support rendering while not-master; master
> is only required for modesetting, IIRC (which XMir doesn't do). Mir will
> switch to handing out rendernodes at some point, as our clients don't
> need flink, and you can always render to a rendernode; they don't *have*
> associated masters.

Yes, rendernodes is a different thing, but the question is whether the
hardware DRM drivers *support* it or whether that's a security bug;
rendering as an authenticated client is OK, but when the master that
authenticated has dropped (or died) and there is another master, I'd say
that's a security violation, but that's not a Mir problem but a DRM problem.

>
>
>>  In the XMir
>> case, that should be OK, as long as XMir itself is allowed to block like
>> that, and I guess if it would otherwise block in the Mir pageflip that
>> should be OK. Bare X would typically not block since it's
>> single-threaded and that would stop it from becoming master again.
>> Blocking might however be an issue for X clients with critical timing
>> like Xv video players, that would need to start dropping frames.
>>
>> I think, if anything, one should consider using enterVT / leaveVT as a
>> notification that master has been reclaimed / is about to be dropped.
>> Drivers can then take appropriate action (drop Xv frames, software
>> rendering, block DRI, much of what's done under bare X).
>
> We could indeed hook up that notification if drivers want it.

Yes, I think that could be useful.

>
>
>>
>>> 2) The pixel format of the XMir surface appears to be fixed at
>>> mir_pixel_format_xrgb_8888 right now. See:
>>>
https://urldefense.proofpoint.com/v1/url?u=https://github.com/RAOF/xserver/blob/vladmir-upstreaming/hw/xfree86/xmir/xmir-output.c%23L311&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0A&m=OZU2uhMnuCLeFGemU2b1KMBMAOfsNVVUYP8oK4asWXQ%3D%0A&s=be4e70174e7d611985b4e17c6d63f0b1ce1ecfab9c62c9fb660ce4b3bc482f9f
>>>
>>
>> Hmm, yes, I was more referring to what might happen in the future and
>> the semantics of the XMir interface.
>> If there is a way for the DDX to obtain the format of the copy buffer,
>> the DDX could run in whatever color-depth it prefers and just do a
>> format-conversion blit when copying to Mir.
>>
>
> I intend (at some point ☺) to choose the Mir format based upon the X
> visual format; xrgb8888 is just enough to cover all the standard
> usecases we cared about!
>
> Would that be sufficient, or would you additionally want to access to
> the buffer format?

For vmwgfx, we can always obtain the buffer format from the kernel,
since we're using typed surfaces as shareable objects, but thinking of
a generic solution I think there should be an xmir function to get the
format. Perhaps just a convenience function that uses the drawable depth?

Another thing that springs to my mind is that it would be good to be
able to cache the copy buffer handle to avoid two kernel calls for each
xmir_copy_to_mir() window. We can't do that right now because we can't
guarantee that an fd is not reused for a new copy buffer if the old one
is destroyed.

Thanks,
Thomas