[PATCH xserver] WIP : Add support for variable refresh rate in modesetting DDX

[Michel Dänzer]

Thanks for your patch.

These days, we're using GitLab merge requests for submitting and
reviewing xserver patches:
https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests

Anyway, while we're here, some early feedback:

First of all, it would be better to port xf86-video-amdgpu commits
2d18b37159ed "Check last flip window instead of screen root before
flipping" & ef8fbe33b7d9 "Wrap change/delete window property request
handlers" as separate commits for the modesetting driver as well.


On 2020-06-10 2:05 p.m., pichika.uday.kiran at intel.com wrote:
> 
> Kernel Changes to support this feature is under development.

This is a bit confusing as is, since the amdgpu kernel driver already
has everything needed. Presumably you're referring to the i915 driver?
I'm not sure that really needs to be mentioned here.


> What is Tested ?
> Verified with DOTA2, Xonotic gaming apps, private glxapp runs in
> Fullscreen mode. When these apps are running in fullscreen mode,
> able to call the set_vrr on all the CRTCs with VRR_ENABLED property.

This could be simplified to something like

 Tested with DOTA2, Xonotic and custom GLX apps.


> @@ -3023,6 +3100,9 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_r
>      }
>      /* work out the possible clones later */
>      output->possible_clones = 0;
> +    props = drmModeObjectGetProperties(drmmode->fd,
> +                                       drmmode_output->output_id,
> +                                       DRM_MODE_OBJECT_CONNECTOR);
>  
>      if (ms->atomic_modeset) {
>          if (!drmmode_prop_info_copy(drmmode_output->props_connector,
> @@ -3048,6 +3128,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_r
>              RRPostPendingProperties(output->randr_output);
>          }
>      }
> +    ms->is_connector_vrr_capable = drmmode_connector_check_vrr_capable(drmmode->fd, props, "VRR_CAPABLE");
>  
>      return 1;
>  

	drmModeFreeObjectProperties(props);

is needed at the end of drmmode_output_init, or it leaks the resources
referenced by props.


> @@ -295,11 +311,17 @@ ms_present_check_flip(RRCrtcPtr crtc,
>      ScreenPtr screen = window->drawable.pScreen;
>      ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
>      modesettingPtr ms = modesettingPTR(scrn);
> +    Bool ret;
>  
>      if (ms->drmmode.sprites_visible > 0)
>          return FALSE;
>  
> -    return ms_present_check_unflip(crtc, window, pixmap, sync_flip, reason);
> +    if(ret = ms_present_check_unflip(crtc, window, pixmap, sync_flip, reason))
> +    {
> +        ms->flip_window = window;
> +    }
> +
> +    return ret;

This doesn't match the style of the existing code very well. I'd write
it as:

    if (!ms_present_check_unflip(...))
        return FALSE;

    ms->flip_window = window;
    return TRUE;
}


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer