[Mesa-dev] glx: Report the current refresh rate for GetMscRateOML

Submitted by Chris Wilson on May 16, 2014, 1:06 p.m.

Details

Message ID 1400245575-4813-1-git-send-email-chris@chris-wilson.co.uk
State New
Headers show

Not browsing as part of any series.

Commit Message

Chris Wilson May 16, 2014, 1:06 p.m.
OML_sync_control:
    The graphics MSC value is incremented once for each screen refresh.
    For a non-interlaced display, this means that the graphics MSC value
    is incremented for each frame. For an interlaced display, it means
    that it will be incremented for each field. For a multi-monitor
    system, the monitor used to determine MSC is screen 0 of <display>.

    glXGetMscRateOML returns the rate at which the MSC will be incremented
    for the display associated with <hdc>. The rate is expressed in Hertz
    as <numerator> / <denominator>. If the MSC rate in Hertz is an
    integer, then <denominator> will be 1 and <numerator> will be
    the MSC rate.

The current implementation using the XVidMode extension returns the
refresh rate of the initial mode X was started at. As an improvement,
use RandR to query the current mode of the primary output (or first
enabled CRTC) and use that as the basis for the refresh rate calculation.

We fallback to the current XVidMode method if XRandR is not available.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: TheoH <Theo0x48@gmail.com>
Cc: Jamey Sharp <jamey@minilop.net>
---
 configure.ac        |   7 +++
 src/glx/Makefile.am |   8 ++-
 src/glx/glxcmds.c   | 160 ++++++++++++++++++++++++++++++++++++++++------------
 3 files changed, 136 insertions(+), 39 deletions(-)

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index bf543c2..39e9909 100644
--- a/configure.ac
+++ b/configure.ac
@@ -880,6 +880,12 @@  xyesno)
         dri_modules="$dri_modules xcb-dri3 xcb-present xcb-sync xshmfence >= $XSHMFENCE_REQUIRED"
     fi
 
+    # add Xrandr if available
+    PKG_CHECK_MODULES([XRANDR], [xrandr], HAVE_XRANDR=yes, HAVEXRANDR=no)
+    if test "$HAVE_XRANDR" = yes ; then
+        dri_modules="$dri_modules xrandr"
+    fi
+
     # add xf86vidmode if available
     PKG_CHECK_MODULES([XF86VIDMODE], [xxf86vm], HAVE_XF86VIDMODE=yes, HAVE_XF86VIDMODE=no)
     if test "$HAVE_XF86VIDMODE" = yes ; then
@@ -903,6 +909,7 @@  fi
 
 # This is outside the case (above) so that it is invoked even for non-GLX
 # builds.
+AM_CONDITIONAL(HAVE_XRANDR, test "x$HAVE_XRANDR" = xyes)
 AM_CONDITIONAL(HAVE_XF86VIDMODE, test "x$HAVE_XF86VIDMODE" = xyes)
 
 GLESv1_CM_LIB_DEPS="$LIBDRM_LIBS -lm $PTHREAD_LIBS $DLOPEN_LIBS"
diff --git a/src/glx/Makefile.am b/src/glx/Makefile.am
index 482d952..41b008c 100644
--- a/src/glx/Makefile.am
+++ b/src/glx/Makefile.am
@@ -26,8 +26,12 @@  endif
 
 SUBDIRS=. tests
 
+EXTRA_DEFINES =
 if HAVE_XF86VIDMODE
-EXTRA_DEFINES_XF86VIDMODE = -DXF86VIDMODE
+EXTRA_DEFINES += -DXF86VIDMODE
+endif
+if HAVE_XRANDR
+EXTRA_DEFINES += -DXRANDR
 endif
 
 AM_CFLAGS = \
@@ -40,7 +44,7 @@  AM_CFLAGS = \
 	-I$(top_builddir)/src/mapi/glapi \
 	$(VISIBILITY_CFLAGS) \
 	$(SHARED_GLAPI_CFLAGS) \
-	$(EXTRA_DEFINES_XF86VIDMODE) \
+	$(EXTRA_DEFINES) \
 	-D_REENTRANT \
 	-DDEFAULT_DRIVER_DIR=\"$(DRI_DRIVER_SEARCH_DIR)\" \
 	$(DEFINES) \
diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index 7984715..7fd7648 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -45,6 +45,9 @@ 
 #include "apple_glx.h"
 #else
 #include <sys/time.h>
+#ifdef XRANDR
+#include <X11/extensions/Xrandr.h>
+#endif
 #ifdef XF86VIDMODE
 #include <X11/extensions/xf86vmode.h>
 #endif
@@ -2082,63 +2085,146 @@  __glXGetSyncValuesOML(Display * dpy, GLXDrawable drawable,
    return False;
 }
 
-#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
-_X_HIDDEN GLboolean
-__glxGetMscRate(struct glx_screen *psc,
-		int32_t * numerator, int32_t * denominator)
+static GLboolean compute_refresh_rate_from_mode(long n, long d, unsigned flags,
+						int32_t *numerator,
+						int32_t *denominator)
 {
-#ifdef XF86VIDMODE
-   XF86VidModeModeLine mode_line;
-   int dot_clock;
    int i;
 
-   if (XF86VidModeQueryVersion(psc->dpy, &i, &i) &&
-       XF86VidModeGetModeLine(psc->dpy, psc->scr, &dot_clock, &mode_line)) {
-      unsigned n = dot_clock * 1000;
-      unsigned d = mode_line.vtotal * mode_line.htotal;
-
 # define V_INTERLACE 0x010
 # define V_DBLSCAN   0x020
 
-      if (mode_line.flags & V_INTERLACE)
-         n *= 2;
-      else if (mode_line.flags & V_DBLSCAN)
-         d *= 2;
+   if (flags & V_INTERLACE)
+      n *= 2;
+   else if (flags & V_DBLSCAN)
+      d *= 2;
+
+   /* The OML_sync_control spec requires that if the refresh rate is a
+    * whole number, that the returned numerator be equal to the refresh
+    * rate and the denominator be 1.
+    */
+
+   if (n % d == 0) {
+      n /= d;
+      d = 1;
+   }
+   else {
+      static const unsigned f[] = { 13, 11, 7, 5, 3, 2, 0 };
 
-      /* The OML_sync_control spec requires that if the refresh rate is a
-       * whole number, that the returned numerator be equal to the refresh
-       * rate and the denominator be 1.
+      /* This is a poor man's way to reduce a fraction.  It's far from
+       * perfect, but it will work well enough for this situation.
        */
 
-      if (n % d == 0) {
-         n /= d;
-         d = 1;
+      for (i = 0; f[i] != 0; i++) {
+	 while (n % f[i] == 0 && d % f[i] == 0) {
+	    d /= f[i];
+	    n /= f[i];
+	 }
       }
-      else {
-         static const unsigned f[] = { 13, 11, 7, 5, 3, 2, 0 };
+   }
 
-         /* This is a poor man's way to reduce a fraction.  It's far from
-          * perfect, but it will work well enough for this situation.
-          */
+   *numerator = n;
+   *denominator = d;
+   return True;
+}
 
-         for (i = 0; f[i] != 0; i++) {
-            while (n % f[i] == 0 && d % f[i] == 0) {
-               d /= f[i];
-               n /= f[i];
-            }
-         }
+static GLboolean RRGetMscRate(struct glx_screen *psc, int32_t *numerator, int32_t *denominator)
+{
+   GLboolean ret = False;
+#ifdef XRANDR
+   Window root = RootWindow(psc->dpy, psc->scr);
+   XRRScreenResources *res;
+   int rr_event, rr_error;
+   RROutput primary;
+   RRMode mode = 0;
+   int n;
+
+   if (!XRRQueryExtension(psc->dpy, &rr_event, &rr_error))
+      return ret;
+
+   res = XRRGetScreenResourcesCurrent(psc->dpy, root);
+   if (res == NULL)
+      return ret;
+
+   /* Use the primary output if specified, otherwise
+    * use the mode on the first enabled crtc.
+    */
+   primary = XRRGetOutputPrimary(psc->dpy, root);
+   if (primary) {
+      XRROutputInfo *output;
+
+      output = XRRGetOutputInfo(psc->dpy, res, primary);
+      if (output != NULL) {
+	 if (output->crtc) {
+	    XRRCrtcInfo *crtc;
+
+	    crtc = XRRGetCrtcInfo(psc->dpy, res, output->crtc);
+	    if (crtc) {
+	       mode = crtc->mode;
+	       XRRFreeCrtcInfo(crtc);
+	    }
+	 }
+	 XRRFreeOutputInfo(output);
       }
+   }
 
-      *numerator = n;
-      *denominator = d;
+   for (n = 0; mode == 0 && n < res->ncrtc; n++) {
+      XRRCrtcInfo *crtc;
 
-      return True;
+      crtc = XRRGetCrtcInfo(psc->dpy, res, res->crtcs[n]);
+      if (crtc) {
+	 mode = crtc->mode;
+	 XRRFreeCrtcInfo(crtc);
+      }
    }
-   else
+
+   for (n = 0; n < res->nmode; n++) {
+      if (res->modes[n].id == mode) {
+	 ret = compute_refresh_rate_from_mode(res->modes[n].dotClock,
+					      res->modes[n].hTotal*res->modes[n].vTotal,
+					      res->modes[n].modeFlags,
+					      numerator, denominator);
+	 break;
+      }
+   }
+
+   XRRFreeScreenResources(res);
+#endif
+   return ret;
+}
+
+static GLboolean VMGetMscRate(struct glx_screen *psc, int32_t *numerator, int32_t *denominator)
+{
+#ifdef XF86VIDMODE
+   XF86VidModeModeLine mode_line;
+   int dot_clock;
+   int i;
+
+   if (XF86VidModeQueryVersion(psc->dpy, &i, &i) &&
+       XF86VidModeGetModeLine(psc->dpy, psc->scr, &dot_clock, &mode_line))
+      return compute_refresh_rate_from_mode(dot_clock * 1000,
+					    mode_line.vtotal * mode_line.htotal,
+					    mode_line.flags,
+					    numerator,
+					    denominator);
 #endif
 
    return False;
 }
+
+#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
+_X_HIDDEN GLboolean
+__glxGetMscRate(struct glx_screen *psc,
+		int32_t * numerator, int32_t * denominator)
+{
+   if (RRGetMscRate(psc, numerator, denominator))
+	   return True;
+
+   if (VMGetMscRate(psc, numerator, denominator))
+	   return True;
+
+   return False;
+}
 #endif
 
 /**

Comments

This seems like an excellent plan.

`git diff -w` makes it easy to see that this patch doesn't change Mesa's
behavior if the RandR headers aren't available, so that's great.

The RandR side looks plausible, but I'm not familiar enough with the
extension to review it in detail.

I'm a little uncomfortable with relying on the modeline-flags to be
defined the same way in each case. Can I suggest adding "interlaced" and
"doublescan" arguments to compute_refresh_rate_from_mode, and doing the
bit masking in RR/VMGetMscRate instead? Preferably, using symbols
defined in each extension's protocol headers, rather than the
locally-defined macros.

I guess all the new static helper functions should be wrapped in
  #if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
as they're unused otherwise and would generate warnings.

If neither RandR nor XVidMode is available, I expect the compiler will
warn that compute_refresh_rate_from_mode is unused. If it's not too much
trouble I'd suggest adding an appropriate attribute((unused))
declaration, although the only other example of that I can find off-hand
in the Mesa tree is in src/gtest as "GTEST_ATTRIBUTE_UNUSED_", so that
might be blazing new ground.

I'm in favor of applying this, with the above details taken care of.

Jamey

On Fri, May 16, 2014 at 02:06:15PM +0100, Chris Wilson wrote:
> OML_sync_control:
>     The graphics MSC value is incremented once for each screen refresh.
>     For a non-interlaced display, this means that the graphics MSC value
>     is incremented for each frame. For an interlaced display, it means
>     that it will be incremented for each field. For a multi-monitor
>     system, the monitor used to determine MSC is screen 0 of <display>.
> 
>     glXGetMscRateOML returns the rate at which the MSC will be incremented
>     for the display associated with <hdc>. The rate is expressed in Hertz
>     as <numerator> / <denominator>. If the MSC rate in Hertz is an
>     integer, then <denominator> will be 1 and <numerator> will be
>     the MSC rate.
> 
> The current implementation using the XVidMode extension returns the
> refresh rate of the initial mode X was started at. As an improvement,
> use RandR to query the current mode of the primary output (or first
> enabled CRTC) and use that as the basis for the refresh rate calculation.
> 
> We fallback to the current XVidMode method if XRandR is not available.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: TheoH <Theo0x48@gmail.com>
> Cc: Jamey Sharp <jamey@minilop.net>
> ---
>  configure.ac        |   7 +++
>  src/glx/Makefile.am |   8 ++-
>  src/glx/glxcmds.c   | 160 ++++++++++++++++++++++++++++++++++++++++------------
>  3 files changed, 136 insertions(+), 39 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index bf543c2..39e9909 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -880,6 +880,12 @@ xyesno)
>          dri_modules="$dri_modules xcb-dri3 xcb-present xcb-sync xshmfence >= $XSHMFENCE_REQUIRED"
>      fi
>  
> +    # add Xrandr if available
> +    PKG_CHECK_MODULES([XRANDR], [xrandr], HAVE_XRANDR=yes, HAVEXRANDR=no)
> +    if test "$HAVE_XRANDR" = yes ; then
> +        dri_modules="$dri_modules xrandr"
> +    fi
> +
>      # add xf86vidmode if available
>      PKG_CHECK_MODULES([XF86VIDMODE], [xxf86vm], HAVE_XF86VIDMODE=yes, HAVE_XF86VIDMODE=no)
>      if test "$HAVE_XF86VIDMODE" = yes ; then
> @@ -903,6 +909,7 @@ fi
>  
>  # This is outside the case (above) so that it is invoked even for non-GLX
>  # builds.
> +AM_CONDITIONAL(HAVE_XRANDR, test "x$HAVE_XRANDR" = xyes)
>  AM_CONDITIONAL(HAVE_XF86VIDMODE, test "x$HAVE_XF86VIDMODE" = xyes)
>  
>  GLESv1_CM_LIB_DEPS="$LIBDRM_LIBS -lm $PTHREAD_LIBS $DLOPEN_LIBS"
> diff --git a/src/glx/Makefile.am b/src/glx/Makefile.am
> index 482d952..41b008c 100644
> --- a/src/glx/Makefile.am
> +++ b/src/glx/Makefile.am
> @@ -26,8 +26,12 @@ endif
>  
>  SUBDIRS=. tests
>  
> +EXTRA_DEFINES =
>  if HAVE_XF86VIDMODE
> -EXTRA_DEFINES_XF86VIDMODE = -DXF86VIDMODE
> +EXTRA_DEFINES += -DXF86VIDMODE
> +endif
> +if HAVE_XRANDR
> +EXTRA_DEFINES += -DXRANDR
>  endif
>  
>  AM_CFLAGS = \
> @@ -40,7 +44,7 @@ AM_CFLAGS = \
>  	-I$(top_builddir)/src/mapi/glapi \
>  	$(VISIBILITY_CFLAGS) \
>  	$(SHARED_GLAPI_CFLAGS) \
> -	$(EXTRA_DEFINES_XF86VIDMODE) \
> +	$(EXTRA_DEFINES) \
>  	-D_REENTRANT \
>  	-DDEFAULT_DRIVER_DIR=\"$(DRI_DRIVER_SEARCH_DIR)\" \
>  	$(DEFINES) \
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index 7984715..7fd7648 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -45,6 +45,9 @@
>  #include "apple_glx.h"
>  #else
>  #include <sys/time.h>
> +#ifdef XRANDR
> +#include <X11/extensions/Xrandr.h>
> +#endif
>  #ifdef XF86VIDMODE
>  #include <X11/extensions/xf86vmode.h>
>  #endif
> @@ -2082,63 +2085,146 @@ __glXGetSyncValuesOML(Display * dpy, GLXDrawable drawable,
>     return False;
>  }
>  
> -#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
> -_X_HIDDEN GLboolean
> -__glxGetMscRate(struct glx_screen *psc,
> -		int32_t * numerator, int32_t * denominator)
> +static GLboolean compute_refresh_rate_from_mode(long n, long d, unsigned flags,
> +						int32_t *numerator,
> +						int32_t *denominator)
>  {
> -#ifdef XF86VIDMODE
> -   XF86VidModeModeLine mode_line;
> -   int dot_clock;
>     int i;
>  
> -   if (XF86VidModeQueryVersion(psc->dpy, &i, &i) &&
> -       XF86VidModeGetModeLine(psc->dpy, psc->scr, &dot_clock, &mode_line)) {
> -      unsigned n = dot_clock * 1000;
> -      unsigned d = mode_line.vtotal * mode_line.htotal;
> -
>  # define V_INTERLACE 0x010
>  # define V_DBLSCAN   0x020
>  
> -      if (mode_line.flags & V_INTERLACE)
> -         n *= 2;
> -      else if (mode_line.flags & V_DBLSCAN)
> -         d *= 2;
> +   if (flags & V_INTERLACE)
> +      n *= 2;
> +   else if (flags & V_DBLSCAN)
> +      d *= 2;
> +
> +   /* The OML_sync_control spec requires that if the refresh rate is a
> +    * whole number, that the returned numerator be equal to the refresh
> +    * rate and the denominator be 1.
> +    */
> +
> +   if (n % d == 0) {
> +      n /= d;
> +      d = 1;
> +   }
> +   else {
> +      static const unsigned f[] = { 13, 11, 7, 5, 3, 2, 0 };
>  
> -      /* The OML_sync_control spec requires that if the refresh rate is a
> -       * whole number, that the returned numerator be equal to the refresh
> -       * rate and the denominator be 1.
> +      /* This is a poor man's way to reduce a fraction.  It's far from
> +       * perfect, but it will work well enough for this situation.
>         */
>  
> -      if (n % d == 0) {
> -         n /= d;
> -         d = 1;
> +      for (i = 0; f[i] != 0; i++) {
> +	 while (n % f[i] == 0 && d % f[i] == 0) {
> +	    d /= f[i];
> +	    n /= f[i];
> +	 }
>        }
> -      else {
> -         static const unsigned f[] = { 13, 11, 7, 5, 3, 2, 0 };
> +   }
>  
> -         /* This is a poor man's way to reduce a fraction.  It's far from
> -          * perfect, but it will work well enough for this situation.
> -          */
> +   *numerator = n;
> +   *denominator = d;
> +   return True;
> +}
>  
> -         for (i = 0; f[i] != 0; i++) {
> -            while (n % f[i] == 0 && d % f[i] == 0) {
> -               d /= f[i];
> -               n /= f[i];
> -            }
> -         }
> +static GLboolean RRGetMscRate(struct glx_screen *psc, int32_t *numerator, int32_t *denominator)
> +{
> +   GLboolean ret = False;
> +#ifdef XRANDR
> +   Window root = RootWindow(psc->dpy, psc->scr);
> +   XRRScreenResources *res;
> +   int rr_event, rr_error;
> +   RROutput primary;
> +   RRMode mode = 0;
> +   int n;
> +
> +   if (!XRRQueryExtension(psc->dpy, &rr_event, &rr_error))
> +      return ret;
> +
> +   res = XRRGetScreenResourcesCurrent(psc->dpy, root);
> +   if (res == NULL)
> +      return ret;
> +
> +   /* Use the primary output if specified, otherwise
> +    * use the mode on the first enabled crtc.
> +    */
> +   primary = XRRGetOutputPrimary(psc->dpy, root);
> +   if (primary) {
> +      XRROutputInfo *output;
> +
> +      output = XRRGetOutputInfo(psc->dpy, res, primary);
> +      if (output != NULL) {
> +	 if (output->crtc) {
> +	    XRRCrtcInfo *crtc;
> +
> +	    crtc = XRRGetCrtcInfo(psc->dpy, res, output->crtc);
> +	    if (crtc) {
> +	       mode = crtc->mode;
> +	       XRRFreeCrtcInfo(crtc);
> +	    }
> +	 }
> +	 XRRFreeOutputInfo(output);
>        }
> +   }
>  
> -      *numerator = n;
> -      *denominator = d;
> +   for (n = 0; mode == 0 && n < res->ncrtc; n++) {
> +      XRRCrtcInfo *crtc;
>  
> -      return True;
> +      crtc = XRRGetCrtcInfo(psc->dpy, res, res->crtcs[n]);
> +      if (crtc) {
> +	 mode = crtc->mode;
> +	 XRRFreeCrtcInfo(crtc);
> +      }
>     }
> -   else
> +
> +   for (n = 0; n < res->nmode; n++) {
> +      if (res->modes[n].id == mode) {
> +	 ret = compute_refresh_rate_from_mode(res->modes[n].dotClock,
> +					      res->modes[n].hTotal*res->modes[n].vTotal,
> +					      res->modes[n].modeFlags,
> +					      numerator, denominator);
> +	 break;
> +      }
> +   }
> +
> +   XRRFreeScreenResources(res);
> +#endif
> +   return ret;
> +}
> +
> +static GLboolean VMGetMscRate(struct glx_screen *psc, int32_t *numerator, int32_t *denominator)
> +{
> +#ifdef XF86VIDMODE
> +   XF86VidModeModeLine mode_line;
> +   int dot_clock;
> +   int i;
> +
> +   if (XF86VidModeQueryVersion(psc->dpy, &i, &i) &&
> +       XF86VidModeGetModeLine(psc->dpy, psc->scr, &dot_clock, &mode_line))
> +      return compute_refresh_rate_from_mode(dot_clock * 1000,
> +					    mode_line.vtotal * mode_line.htotal,
> +					    mode_line.flags,
> +					    numerator,
> +					    denominator);
>  #endif
>  
>     return False;
>  }
> +
> +#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
> +_X_HIDDEN GLboolean
> +__glxGetMscRate(struct glx_screen *psc,
> +		int32_t * numerator, int32_t * denominator)
> +{
> +   if (RRGetMscRate(psc, numerator, denominator))
> +	   return True;
> +
> +   if (VMGetMscRate(psc, numerator, denominator))
> +	   return True;
> +
> +   return False;
> +}
>  #endif
>  
>  /**
> -- 
> 2.0.0.rc2
>