[Spice-devel,06/12] qxl-wddm-dod: Registry-based control over VSync

Submitted by Yuri Benditovich on March 12, 2017, 8:45 a.m.

Details

Message ID 1489308309-9728-7-git-send-email-yuri.benditovich@daynix.com
State New
Headers show
Series "Set of patches for further support of VSync" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Yuri Benditovich March 12, 2017, 8:45 a.m.
Registry override of default behavior (disabling when enabled
by default) provided for support and troubleshooting only in case
this feature affects specific user.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 qxldod/driver.cpp | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Patch hide | download patch | download mbox

diff --git a/qxldod/driver.cpp b/qxldod/driver.cpp
index 1946147..01b88d7 100755
--- a/qxldod/driver.cpp
+++ b/qxldod/driver.cpp
@@ -21,6 +21,38 @@ 
 
 int nDebugLevel = TRACE_LEVEL_ERROR;
 
+// registry-based configuration is intended to be manual only
+// for VSync suppression during support and troubleshooting
+// and not expected to be made default
+static void QueryVSyncSetting(BOOLEAN& b, UNICODE_STRING *path)
+{
+    PAGED_CODE();
+    WCHAR buffer[MAX_PATH];
+    ULONG val = b;
+    RTL_QUERY_REGISTRY_TABLE QueryTable[3] = {};
+    if (path->Length >= sizeof(buffer))
+        return;
+
+    QueryTable[0].Flags = RTL_QUERY_REGISTRY_SUBKEY;
+    QueryTable[0].Name = L"Parameters";
+    QueryTable[1].Flags = RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_TYPECHECK | RTL_QUERY_REGISTRY_REQUIRED;
+    QueryTable[1].Name = L"EnableVSync";
+    QueryTable[1].DefaultType = REG_DWORD << 24;
+    QueryTable[1].EntryContext = &val;
+
+    RtlCopyMemory(buffer, path->Buffer, path->Length);
+    buffer[path->Length/2] = 0;
+    NTSTATUS status = RtlQueryRegistryValues(RTL_REGISTRY_ABSOLUTE, buffer, QueryTable, NULL, NULL);
+    if (NT_SUCCESS(status))
+    {
+        DbgPrint(TRACE_LEVEL_WARNING, ("%s: val = %d\n", __FUNCTION__, val));
+        b = !!val;
+    }
+    else
+    {
+        DbgPrint(TRACE_LEVEL_WARNING, ("%s: status = %X\n", __FUNCTION__, status));
+    }
+}
 
 extern "C"
 NTSTATUS
@@ -52,6 +84,9 @@  DriverEntry(
         // related to enabled VSync control in Win10RS1
 
         //g_bSupportVSync = TRUE;
+
+        // for support/troubleshooting be able to disable VSync on specific machine
+        QueryVSyncSetting(g_bSupportVSync, pRegistryPath);
     }
     DbgPrint(TRACE_LEVEL_WARNING, ("VSync support %sabled for %d.%d.%d\n",
         g_bSupportVSync ? "en" : "dis",

Comments

> 
> Registry override of default behavior (disabling when enabled
> by default) provided for support and troubleshooting only in case
> this feature affects specific user.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  qxldod/driver.cpp | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/qxldod/driver.cpp b/qxldod/driver.cpp
> index 1946147..01b88d7 100755
> --- a/qxldod/driver.cpp
> +++ b/qxldod/driver.cpp
> @@ -21,6 +21,38 @@
>  
>  int nDebugLevel = TRACE_LEVEL_ERROR;
>  
> +// registry-based configuration is intended to be manual only
> +// for VSync suppression during support and troubleshooting
> +// and not expected to be made default
> +static void QueryVSyncSetting(BOOLEAN& b, UNICODE_STRING *path)
> +{
> +    PAGED_CODE();
> +    WCHAR buffer[MAX_PATH];
> +    ULONG val = b;
> +    RTL_QUERY_REGISTRY_TABLE QueryTable[3] = {};
> +    if (path->Length >= sizeof(buffer))
> +        return;
> +
> +    QueryTable[0].Flags = RTL_QUERY_REGISTRY_SUBKEY;
> +    QueryTable[0].Name = L"Parameters";
> +    QueryTable[1].Flags = RTL_QUERY_REGISTRY_DIRECT |
> RTL_QUERY_REGISTRY_TYPECHECK | RTL_QUERY_REGISTRY_REQUIRED;
> +    QueryTable[1].Name = L"EnableVSync";
> +    QueryTable[1].DefaultType = REG_DWORD << 24;
> +    QueryTable[1].EntryContext = &val;
> +
> +    RtlCopyMemory(buffer, path->Buffer, path->Length);
> +    buffer[path->Length/2] = 0;
> +    NTSTATUS status = RtlQueryRegistryValues(RTL_REGISTRY_ABSOLUTE, buffer,
> QueryTable, NULL, NULL);
> +    if (NT_SUCCESS(status))
> +    {
> +        DbgPrint(TRACE_LEVEL_WARNING, ("%s: val = %d\n", __FUNCTION__,
> val));

I would say INFORMATION level, nothing to worry about

> +        b = !!val;
> +    }
> +    else
> +    {
> +        DbgPrint(TRACE_LEVEL_WARNING, ("%s: status = %X\n", __FUNCTION__,
> status));

Even this, mostly of the time should be that the value is not set which is
normal. Unless is supposed to always to present (from the inf file for instance).

> +    }
> +}
>  
>  extern "C"
>  NTSTATUS
> @@ -52,6 +84,9 @@ DriverEntry(
>          // related to enabled VSync control in Win10RS1
>  
>          //g_bSupportVSync = TRUE;

Maybe this line now can be removed?

> +
> +        // for support/troubleshooting be able to disable VSync on specific
> machine
> +        QueryVSyncSetting(g_bSupportVSync, pRegistryPath);
>      }
>      DbgPrint(TRACE_LEVEL_WARNING, ("VSync support %sabled for %d.%d.%d\n",
>          g_bSupportVSync ? "en" : "dis",

Frediano
On Tue, Mar 21, 2017 at 2:21 PM, Frediano Ziglio <fziglio@redhat.com> wrote:

> >
> > Registry override of default behavior (disabling when enabled
> > by default) provided for support and troubleshooting only in case
> > this feature affects specific user.
> >
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >  qxldod/driver.cpp | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git a/qxldod/driver.cpp b/qxldod/driver.cpp
> > index 1946147..01b88d7 100755
> > --- a/qxldod/driver.cpp
> > +++ b/qxldod/driver.cpp
> > @@ -21,6 +21,38 @@
> >
> >  int nDebugLevel = TRACE_LEVEL_ERROR;
> >
> > +// registry-based configuration is intended to be manual only
> > +// for VSync suppression during support and troubleshooting
> > +// and not expected to be made default
> > +static void QueryVSyncSetting(BOOLEAN& b, UNICODE_STRING *path)
> > +{
> > +    PAGED_CODE();
> > +    WCHAR buffer[MAX_PATH];
> > +    ULONG val = b;
> > +    RTL_QUERY_REGISTRY_TABLE QueryTable[3] = {};
> > +    if (path->Length >= sizeof(buffer))
> > +        return;
> > +
> > +    QueryTable[0].Flags = RTL_QUERY_REGISTRY_SUBKEY;
> > +    QueryTable[0].Name = L"Parameters";
> > +    QueryTable[1].Flags = RTL_QUERY_REGISTRY_DIRECT |
> > RTL_QUERY_REGISTRY_TYPECHECK | RTL_QUERY_REGISTRY_REQUIRED;
> > +    QueryTable[1].Name = L"EnableVSync";
> > +    QueryTable[1].DefaultType = REG_DWORD << 24;
> > +    QueryTable[1].EntryContext = &val;
> > +
> > +    RtlCopyMemory(buffer, path->Buffer, path->Length);
> > +    buffer[path->Length/2] = 0;
> > +    NTSTATUS status = RtlQueryRegistryValues(RTL_REGISTRY_ABSOLUTE,
> buffer,
> > QueryTable, NULL, NULL);
> > +    if (NT_SUCCESS(status))
> > +    {
> > +        DbgPrint(TRACE_LEVEL_WARNING, ("%s: val = %d\n", __FUNCTION__,
> > val));
>
> I would say INFORMATION level, nothing to worry about
>
> > +        b = !!val;
> > +    }
> > +    else
> > +    {
> > +        DbgPrint(TRACE_LEVEL_WARNING, ("%s: status = %X\n",
> __FUNCTION__,
> > status));
>
> Even this, mostly of the time should be that the value is not set which is
> normal. Unless is supposed to always to present (from the inf file for
> instance).
>

This value is planned as override of default behavior, and so normally not
present.


>
> > +    }
> > +}
> >
> >  extern "C"
> >  NTSTATUS
> > @@ -52,6 +84,9 @@ DriverEntry(
> >          // related to enabled VSync control in Win10RS1
> >
> >          //g_bSupportVSync = TRUE;
>
> Maybe this line now can be removed?
>
>
This line is planned to be uncommented in future.
Registry value is normally not set (as it is explained in the comment)


> > +
> > +        // for support/troubleshooting be able to disable VSync on
> specific
> > machine
> > +        QueryVSyncSetting(g_bSupportVSync, pRegistryPath);
> >      }
> >      DbgPrint(TRACE_LEVEL_WARNING, ("VSync support %sabled for
> %d.%d.%d\n",
> >          g_bSupportVSync ? "en" : "dis",
>
> Frediano
>
On Sun, Mar 12, 2017 at 10:45:03AM +0200, Yuri Benditovich wrote:
> Registry override of default behavior (disabling when enabled
> by default) provided for support and troubleshooting only in case
> this feature affects specific user.

I'm afraid it's still unclear to me why vsync support is required for
qxl-wddm-dod. Looking at
https://msdn.microsoft.com/en-us/library/windows/hardware/jj673962(v=vs.85).aspx
it seems in this case vsync is optional?
"Note that if a KMDOD does not support the VSync control feature, it
should not implement certain functions"

I'd assume Microsoft would not document this if they are not willing to
let such drivers pass HCK? Do you have any reference explaining why
vsync is mandatory for HCK?

Christophe

> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  qxldod/driver.cpp | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/qxldod/driver.cpp b/qxldod/driver.cpp
> index 1946147..01b88d7 100755
> --- a/qxldod/driver.cpp
> +++ b/qxldod/driver.cpp
> @@ -21,6 +21,38 @@
>  
>  int nDebugLevel = TRACE_LEVEL_ERROR;
>  
> +// registry-based configuration is intended to be manual only
> +// for VSync suppression during support and troubleshooting
> +// and not expected to be made default
> +static void QueryVSyncSetting(BOOLEAN& b, UNICODE_STRING *path)
> +{
> +    PAGED_CODE();
> +    WCHAR buffer[MAX_PATH];
> +    ULONG val = b;
> +    RTL_QUERY_REGISTRY_TABLE QueryTable[3] = {};
> +    if (path->Length >= sizeof(buffer))
> +        return;
> +
> +    QueryTable[0].Flags = RTL_QUERY_REGISTRY_SUBKEY;
> +    QueryTable[0].Name = L"Parameters";
> +    QueryTable[1].Flags = RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_TYPECHECK | RTL_QUERY_REGISTRY_REQUIRED;
> +    QueryTable[1].Name = L"EnableVSync";
> +    QueryTable[1].DefaultType = REG_DWORD << 24;
> +    QueryTable[1].EntryContext = &val;
> +
> +    RtlCopyMemory(buffer, path->Buffer, path->Length);
> +    buffer[path->Length/2] = 0;
> +    NTSTATUS status = RtlQueryRegistryValues(RTL_REGISTRY_ABSOLUTE, buffer, QueryTable, NULL, NULL);
> +    if (NT_SUCCESS(status))
> +    {
> +        DbgPrint(TRACE_LEVEL_WARNING, ("%s: val = %d\n", __FUNCTION__, val));
> +        b = !!val;
> +    }
> +    else
> +    {
> +        DbgPrint(TRACE_LEVEL_WARNING, ("%s: status = %X\n", __FUNCTION__, status));
> +    }
> +}
>  
>  extern "C"
>  NTSTATUS
> @@ -52,6 +84,9 @@ DriverEntry(
>          // related to enabled VSync control in Win10RS1
>  
>          //g_bSupportVSync = TRUE;
> +
> +        // for support/troubleshooting be able to disable VSync on specific machine
> +        QueryVSyncSetting(g_bSupportVSync, pRegistryPath);
>      }
>      DbgPrint(TRACE_LEVEL_WARNING, ("VSync support %sabled for %d.%d.%d\n",
>          g_bSupportVSync ? "en" : "dis",
> -- 
> 2.7.0.windows.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel