[Mesa-dev] [PATCH] glx: Report the current refresh rate for GetMscRateOML
Jamey Sharp
jamey at minilop.net
Sat May 17 09:01:58 PDT 2014
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 at chris-wilson.co.uk>
> Cc: TheoH <Theo0x48 at gmail.com>
> Cc: Jamey Sharp <jamey at 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140517/d13275ba/attachment.sig>
More information about the mesa-dev
mailing list