[PATCH xserver] present: fix msc offset calculation in window mode

[Roman Gilg]

You are right that this would prevent possible future backends
supporting window mode and with real crtcs to return consistent msc
values. But this wouldn't work anyway currently since
present_wnmd_get_ust_msc_ptr does not provide the crtc it expects to
the backend, but only the window.

I think now that this should change instead of the patch I sent in
here (add the crtc argument back to the call). This way Xwayland could
check the expected crtc and if it is not the fake one currently in its
xwl_window struct it would just return an error (and then
present_wnmd_window_to_crtc_msc would use the window_priv->msc value
instead).

On Fri, Apr 20, 2018 at 6:11 PM, Michel Dänzer <michel at daenzer.net> wrote:
> On 2018-04-20 05:10 PM, Roman Gilg wrote:
>> Instead of getting the current msc value from the window, which might be
>> different to old one directly take the last saved msc value saved in
>> the window_priv struct.
>>
>> Signed-off-by: Roman Gilg <subdiff at gmail.com>
>> ---
>>  present/present_wnmd.c | 7 +------
>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/present/present_wnmd.c b/present/present_wnmd.c
>> index 72bda2f..80ffb01 100644
>> --- a/present/present_wnmd.c
>> +++ b/present/present_wnmd.c
>> @@ -518,8 +518,6 @@ present_wnmd_window_to_crtc_msc(WindowPtr window, RRCrtcPtr crtc, uint64_t windo
>>      present_window_priv_ptr window_priv = present_get_window_priv(window, TRUE);
>>
>>      if (crtc != window_priv->crtc) {
>> -        uint64_t old_ust, old_msc;
>> -
>>          if (window_priv->crtc == PresentCrtcNeverSet) {
>>              window_priv->msc_offset = 0;
>>          } else {
>> @@ -527,10 +525,7 @@ present_wnmd_window_to_crtc_msc(WindowPtr window, RRCrtcPtr crtc, uint64_t windo
>>               * we'll just use whatever previous MSC we'd seen from this CRTC
>>               */
>>
>> -            if (present_wnmd_get_ust_msc(window->drawable.pScreen, window, &old_ust, &old_msc) != Success)
>> -                old_msc = window_priv->msc;
>> -
>> -            window_priv->msc_offset += new_msc - old_msc;
>> +            window_priv->msc_offset += new_msc - window_priv->msc;
>
> This is working around an issue in xwl_present_get_ust_msc (it doesn't
> return consistent MSC values for a window which has been unmapped and
> mapped again). This change might do the wrong thing with another backend
> which has real CRTCs and returns consistent MSC values for them.
>
> That said, maybe this is the best that can be done to address the
> immediate issue, but it might be good to at least add a comment saying
> this is a kludge which should be revisited after 1.20.
>
>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer