Help me test fixes for Intel IGD graphical artifacts on Qubes R4.0
40 views
Skip to first unread message
Jinoh Kang
Dec 23, 2020, 9:21:30 PM12/23/20
to qubes-users, qubes-devel, Marek Marczykowski-Górecki, donoban, Patrik Hagara
When using some Intel integrated graphic cards on Qubes R4.0, screen
glitches may manifest after switching VTs or entering suspend mode.
A known workaround does exist for this bug, which is to add a
configuration file with the following contents within
/etc/X11/xorg.conf.d:
> Section "Device"
> Identifier "Intel Graphics"
> Driver "modesetting"
> Option "AccelMethod" "glamor"
> Option "DRI" "3"
> EndSection
However, the X11 modesetting driver version in Fedora 25 has its own
drawbacks:
* It freezes briefly when re-configuring monitors (e.g. plugging in an
external monitor or changing screen resolution)
* XRandR keystone support is buggy
To remediate this, I've patched the Linux i915 driver and it has been
working fine for months. Only the patch for Linux 4.19 has been tested.
If anyone is affected by the issue, please feel free to test the
follow-up patches and give some feedback here.
See also:
* https://github.com/QubesOS/qubes-issues/issues/5244
* https://github.com/QubesOS/qubes-issues/issues/5377
* https://github.com/QubesOS/qubes-issues/issues/5460
Cheers,
Jinoh Kang
glitches may manifest after switching VTs or entering suspend mode.
A known workaround does exist for this bug, which is to add a
configuration file with the following contents within
/etc/X11/xorg.conf.d:
> Section "Device"
> Identifier "Intel Graphics"
> Driver "modesetting"
> Option "AccelMethod" "glamor"
> Option "DRI" "3"
> EndSection
However, the X11 modesetting driver version in Fedora 25 has its own
drawbacks:
* It freezes briefly when re-configuring monitors (e.g. plugging in an
external monitor or changing screen resolution)
* XRandR keystone support is buggy
To remediate this, I've patched the Linux i915 driver and it has been
working fine for months. Only the patch for Linux 4.19 has been tested.
If anyone is affected by the issue, please feel free to test the
follow-up patches and give some feedback here.
See also:
* https://github.com/QubesOS/qubes-issues/issues/5244
* https://github.com/QubesOS/qubes-issues/issues/5377
* https://github.com/QubesOS/qubes-issues/issues/5460
Cheers,
Jinoh Kang
Jinoh Kang
Dec 23, 2020, 9:24:09 PM12/23/20
to qubes-users, qubes-devel, Marek Marczykowski-Górecki, donoban, Patrik Hagara
If GUP-ineligible pages are passed to a GEM userptr object, -EFAULT is
returned only when the object is actually bound.
The xf86-video-intel userspace driver cannot differentiate this
condition, and marks the GPU as wedged. This disables graphics
acceleration and may cripple other functionalities such as VT switch.
Solve this by "prefaulting" user pages on GEM object creation, testing
whether all pages are eligible for get_user_pages() in the process.
On failure, return -EFAULT so that userspace can fallback to software
bliting.
This behavior can be controlled by using a new modparam
"gem_userptr_prefault", which is true by default.
The only known use for this patch is Qubes OS's GUI virtualization.
Userspace is expected to resort to DMA-BUF whenever possible.
Signed-off-by: Jinoh Kang <jinoh....@gmail.com>
---
drivers/gpu/drm/i915/i915_gem_userptr.c | 36 +++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_params.c | 3 +++
drivers/gpu/drm/i915/i915_params.h | 1 +
3 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 961abb6ea18e..c1e680cf037d 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -748,6 +748,34 @@ static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
.release = i915_gem_userptr_release,
};
+static int i915_gem_userptr_prefault(unsigned long start,
+ unsigned long nr_pages,
+ bool readonly)
+{
+ struct mm_struct *mm = current->mm;
+ unsigned int gup_flags = (readonly ? 0 : FOLL_WRITE) | FOLL_NOWAIT;
+ int err = 0;
+
+ down_read(&mm->mmap_sem);
+ while (nr_pages) {
+ long ret;
+
+ ret = get_user_pages(start, nr_pages, gup_flags, NULL, NULL);
+ if (ret < 0) {
+ err = (int)ret;
+ break;
+ }
+ if (ret == 0)
+ ret = 1; /* skip this page */
+
+ start += ret << PAGE_SHIFT;
+ nr_pages -= ret;
+ }
+ up_read(&mm->mmap_sem);
+
+ return err;
+}
+
/*
* Creates a new mm object that wraps some normal memory from the process
* context - user memory.
@@ -815,6 +843,14 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
(char __user *)(unsigned long)args->user_ptr, args->user_size))
return -EFAULT;
+ if (READ_ONCE(i915_modparams.gem_userptr_prefault)) {
+ ret = i915_gem_userptr_prefault((unsigned long)args->user_ptr,
+ args->user_size >> PAGE_SHIFT,
+ args->flags & I915_USERPTR_READ_ONLY);
+ if (ret)
+ return ret;
+ }
+
if (args->flags & I915_USERPTR_READ_ONLY) {
struct i915_hw_ppgtt *ppgtt;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 295e981e4a39..702163d85921 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -174,6 +174,9 @@ i915_param_named(enable_dpcd_backlight, bool, 0600,
i915_param_named(enable_gvt, bool, 0400,
"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
+i915_param_named(gem_userptr_prefault, bool, 0600,
+ "Prefault pages when userptr GEM object is created (default:true)");
+
static __always_inline void _print_param(struct drm_printer *p,
const char *name,
const char *type,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 6c4d4a21474b..86915d1fc303 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -66,6 +66,7 @@ struct drm_printer;
param(bool, disable_display, false) \
param(bool, verbose_state_checks, true) \
param(bool, nuclear_pageflip, false) \
+ param(bool, gem_userptr_prefault, true) \
param(bool, enable_dp_mst, true) \
param(bool, enable_dpcd_backlight, false) \
param(bool, enable_gvt, false)
--
2.26.2
returned only when the object is actually bound.
The xf86-video-intel userspace driver cannot differentiate this
condition, and marks the GPU as wedged. This disables graphics
acceleration and may cripple other functionalities such as VT switch.
Solve this by "prefaulting" user pages on GEM object creation, testing
whether all pages are eligible for get_user_pages() in the process.
On failure, return -EFAULT so that userspace can fallback to software
bliting.
This behavior can be controlled by using a new modparam
"gem_userptr_prefault", which is true by default.
The only known use for this patch is Qubes OS's GUI virtualization.
Userspace is expected to resort to DMA-BUF whenever possible.
Signed-off-by: Jinoh Kang <jinoh....@gmail.com>
---
drivers/gpu/drm/i915/i915_gem_userptr.c | 36 +++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_params.c | 3 +++
drivers/gpu/drm/i915/i915_params.h | 1 +
3 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 961abb6ea18e..c1e680cf037d 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -748,6 +748,34 @@ static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
.release = i915_gem_userptr_release,
};
+static int i915_gem_userptr_prefault(unsigned long start,
+ unsigned long nr_pages,
+ bool readonly)
+{
+ struct mm_struct *mm = current->mm;
+ unsigned int gup_flags = (readonly ? 0 : FOLL_WRITE) | FOLL_NOWAIT;
+ int err = 0;
+
+ down_read(&mm->mmap_sem);
+ while (nr_pages) {
+ long ret;
+
+ ret = get_user_pages(start, nr_pages, gup_flags, NULL, NULL);
+ if (ret < 0) {
+ err = (int)ret;
+ break;
+ }
+ if (ret == 0)
+ ret = 1; /* skip this page */
+
+ start += ret << PAGE_SHIFT;
+ nr_pages -= ret;
+ }
+ up_read(&mm->mmap_sem);
+
+ return err;
+}
+
/*
* Creates a new mm object that wraps some normal memory from the process
* context - user memory.
@@ -815,6 +843,14 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
(char __user *)(unsigned long)args->user_ptr, args->user_size))
return -EFAULT;
+ if (READ_ONCE(i915_modparams.gem_userptr_prefault)) {
+ ret = i915_gem_userptr_prefault((unsigned long)args->user_ptr,
+ args->user_size >> PAGE_SHIFT,
+ args->flags & I915_USERPTR_READ_ONLY);
+ if (ret)
+ return ret;
+ }
+
if (args->flags & I915_USERPTR_READ_ONLY) {
struct i915_hw_ppgtt *ppgtt;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 295e981e4a39..702163d85921 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -174,6 +174,9 @@ i915_param_named(enable_dpcd_backlight, bool, 0600,
i915_param_named(enable_gvt, bool, 0400,
"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
+i915_param_named(gem_userptr_prefault, bool, 0600,
+ "Prefault pages when userptr GEM object is created (default:true)");
+
static __always_inline void _print_param(struct drm_printer *p,
const char *name,
const char *type,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 6c4d4a21474b..86915d1fc303 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -66,6 +66,7 @@ struct drm_printer;
param(bool, disable_display, false) \
param(bool, verbose_state_checks, true) \
param(bool, nuclear_pageflip, false) \
+ param(bool, gem_userptr_prefault, true) \
param(bool, enable_dp_mst, true) \
param(bool, enable_dpcd_backlight, false) \
param(bool, enable_gvt, false)
--
2.26.2
Jinoh Kang
Dec 23, 2020, 9:24:49 PM12/23/20
to qubes-users, qubes-devel, Marek Marczykowski-Górecki, donoban, Patrik Hagara
If GUP-ineligible pages are passed to a GEM userptr object, -EFAULT is
returned only when the object is actually bound.
The xf86-video-intel userspace driver cannot differentiate this
condition, and marks the GPU as wedged. This disables graphics
acceleration and may cripple other functionalities such as VT switch.
Solve this by "prefaulting" user pages on GEM object creation, testing
whether all pages are eligible for get_user_pages() in the process.
On failure, return -EFAULT so that userspace can fallback to software
bliting.
This behavior can be controlled by using a new modparam
"gem_userptr_prefault", which is true by default.
The only known use for this patch is Qubes OS's GUI virtualization.
Userspace is expected to resort to DMA-BUF whenever possible.
Signed-off-by: Jinoh Kang <jinoh....@gmail.com>
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 36 +++++++++++++++++++++
returned only when the object is actually bound.
The xf86-video-intel userspace driver cannot differentiate this
condition, and marks the GPU as wedged. This disables graphics
acceleration and may cripple other functionalities such as VT switch.
Solve this by "prefaulting" user pages on GEM object creation, testing
whether all pages are eligible for get_user_pages() in the process.
On failure, return -EFAULT so that userspace can fallback to software
bliting.
This behavior can be controlled by using a new modparam
"gem_userptr_prefault", which is true by default.
The only known use for this patch is Qubes OS's GUI virtualization.
Userspace is expected to resort to DMA-BUF whenever possible.
Signed-off-by: Jinoh Kang <jinoh....@gmail.com>
---
drivers/gpu/drm/i915/i915_params.c | 3 ++
drivers/gpu/drm/i915/i915_params.h | 1 +
3 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
3 files changed, 40 insertions(+)
index 6d0cc90401c0..c86862a6f592 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -739,6 +739,34 @@ static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
if (!access_ok((char __user *)(unsigned long)args->user_ptr, args->user_size))
return -EFAULT;
+ if (READ_ONCE(i915_modparams.gem_userptr_prefault)) {
+ ret = i915_gem_userptr_prefault((unsigned long)args->user_ptr,
+ args->user_size >> PAGE_SHIFT,
+ args->flags & I915_USERPTR_READ_ONLY);
+ if (ret)
+ return ret;
+ }
+
if (args->flags & I915_USERPTR_READ_ONLY) {
struct i915_address_space *vm;
+ if (READ_ONCE(i915_modparams.gem_userptr_prefault)) {
+ ret = i915_gem_userptr_prefault((unsigned long)args->user_ptr,
+ args->user_size >> PAGE_SHIFT,
+ args->flags & I915_USERPTR_READ_ONLY);
+ if (ret)
+ return ret;
+ }
+
if (args->flags & I915_USERPTR_READ_ONLY) {
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 296452f9efe4..90a03cb8ca57 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -178,6 +178,9 @@ i915_param_named(enable_gvt, bool, 0400,
"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
#endif
+i915_param_named(gem_userptr_prefault, bool, 0600,
+ "Prefault pages when userptr GEM object is created (default:true)");
+
static __always_inline void _print_param(struct drm_printer *p,
const char *name,
const char *type,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -76,6 +76,7 @@ struct drm_printer;
param(bool, disable_display, false) \
param(bool, verbose_state_checks, true) \
param(bool, nuclear_pageflip, false) \
+ param(bool, gem_userptr_prefault, true) \
param(bool, enable_dp_mst, true) \
param(bool, enable_gvt, false)
param(bool, verbose_state_checks, true) \
param(bool, nuclear_pageflip, false) \
+ param(bool, gem_userptr_prefault, true) \
param(bool, enable_dp_mst, true) \
--
2.26.2
Jinoh Kang
Dec 23, 2020, 9:25:07 PM12/23/20
to qubes-users, qubes-devel, Marek Marczykowski-Górecki, donoban, Patrik Hagara
If GUP-ineligible pages are passed to a GEM userptr object, -EFAULT is
returned only when the object is actually bound.
The xf86-video-intel userspace driver cannot differentiate this
condition, and marks the GPU as wedged. This disables graphics
acceleration and may cripple other functionalities such as VT switch.
Solve this by "prefaulting" user pages on GEM object creation, testing
whether all pages are eligible for get_user_pages() in the process.
On failure, return -EFAULT so that userspace can fallback to software
bliting.
This behavior can be controlled by using a new modparam
"gem_userptr_prefault", which is true by default.
The only known use for this patch is Qubes OS's GUI virtualization.
Userspace is expected to resort to DMA-BUF whenever possible.
Signed-off-by: Jinoh Kang <jinoh....@gmail.com>
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 36 +++++++++++++++++++++
drivers/gpu/drm/i915/i915_params.c | 3 ++
drivers/gpu/drm/i915/i915_params.h | 1 +
3 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index f2eaed6aca3d..65596d2b284f 100644
returned only when the object is actually bound.
The xf86-video-intel userspace driver cannot differentiate this
condition, and marks the GPU as wedged. This disables graphics
acceleration and may cripple other functionalities such as VT switch.
Solve this by "prefaulting" user pages on GEM object creation, testing
whether all pages are eligible for get_user_pages() in the process.
On failure, return -EFAULT so that userspace can fallback to software
bliting.
This behavior can be controlled by using a new modparam
"gem_userptr_prefault", which is true by default.
The only known use for this patch is Qubes OS's GUI virtualization.
Userspace is expected to resort to DMA-BUF whenever possible.
Signed-off-by: Jinoh Kang <jinoh....@gmail.com>
---
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 36 +++++++++++++++++++++
drivers/gpu/drm/i915/i915_params.c | 3 ++
drivers/gpu/drm/i915/i915_params.h | 1 +
3 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -712,6 +712,34 @@ static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
if (!access_ok((char __user *)(unsigned long)args->user_ptr, args->user_size))
return -EFAULT;
+ if (READ_ONCE(i915_modparams.gem_userptr_prefault)) {
+ ret = i915_gem_userptr_prefault((unsigned long)args->user_ptr,
+ args->user_size >> PAGE_SHIFT,
+ args->flags & I915_USERPTR_READ_ONLY);
+ if (ret)
+ return ret;
+ }
+
if (args->flags & I915_USERPTR_READ_ONLY) {
/*
return -EFAULT;
+ if (READ_ONCE(i915_modparams.gem_userptr_prefault)) {
+ ret = i915_gem_userptr_prefault((unsigned long)args->user_ptr,
+ args->user_size >> PAGE_SHIFT,
+ args->flags & I915_USERPTR_READ_ONLY);
+ if (ret)
+ return ret;
+ }
+
if (args->flags & I915_USERPTR_READ_ONLY) {
* On almost all of the older hw, we cannot tell the GPU that
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 7f139ea4a90b..c39766adeda2 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -197,6 +197,9 @@ i915_param_named_unsafe(fake_lmem_start, ulong, 0400,
"Fake LMEM start offset (default: 0)");
#endif
+i915_param_named(gem_userptr_prefault, bool, 0600,
+ "Prefault pages when userptr GEM object is created (default:true)");
+
static __always_inline void _print_param(struct drm_printer *p,
const char *name,
const char *type,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 330c03e2b4f7..1169a610a73c 100644
+i915_param_named(gem_userptr_prefault, bool, 0600,
+ "Prefault pages when userptr GEM object is created (default:true)");
+
static __always_inline void _print_param(struct drm_printer *p,
const char *name,
const char *type,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -79,6 +79,7 @@ struct drm_printer;
param(bool, disable_display, false, 0400) \
param(bool, verbose_state_checks, true, 0) \
param(bool, nuclear_pageflip, false, 0400) \
+ param(bool, gem_userptr_prefault, true) \
param(bool, enable_dp_mst, true, 0600) \
param(bool, enable_gvt, false, 0400)
--
2.26.2
Marek Marczykowski-Górecki
Jan 10, 2021, 5:23:19 PM1/10/21
to Jinoh Kang, qubes-users, qubes-devel, donoban, Patrik Hagara
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On Thu, Dec 24, 2020 at 02:24:59AM +0000, Jinoh Kang wrote:
> If GUP-ineligible pages are passed to a GEM userptr object, -EFAULT is
> returned only when the object is actually bound.
>
> The xf86-video-intel userspace driver cannot differentiate this
> condition, and marks the GPU as wedged. This disables graphics
> acceleration and may cripple other functionalities such as VT switch.
>
> Solve this by "prefaulting" user pages on GEM object creation, testing
> whether all pages are eligible for get_user_pages() in the process.
> On failure, return -EFAULT so that userspace can fallback to software
> bliting.
>
> This behavior can be controlled by using a new modparam
> "gem_userptr_prefault", which is true by default.
>
> The only known use for this patch is Qubes OS's GUI virtualization.
> Userspace is expected to resort to DMA-BUF whenever possible.
>
> Signed-off-by: Jinoh Kang <jinoh....@gmail.com>
Thanks for the path. I've tried this one on 5.10.5, and it needs a
couple fixes to build, see below.
But then, it seems to fix the issue with VT switch and disabling
acceleration.
Independently, starting with 5.9.x (without the patch) the system freezes
with specific monitor configuration (two monitors side by side, but one
scaled x1.5). It happens immediately after configuring the second
display (and I still got the message about it in Xorg.0.log). Not
setting scaling on the monitor workarounds the issue.
Switching to modesetting driver fixes it. This issue wasn't here on
5.8.x or earlier and looks to be unrelated to this patch. I'm not sure
whether it's purely kernel driver issue, or some kernel<->Xorg
incompatibility.
> ---
> drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 36 +++++++++++++++++++++
> drivers/gpu/drm/i915/i915_params.c | 3 ++
> drivers/gpu/drm/i915/i915_params.h | 1 +
> 3 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> index f2eaed6aca3d..65596d2b284f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> @@ -712,6 +712,34 @@ static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
> .release = i915_gem_userptr_release,
> };
>
> +static int i915_gem_userptr_prefault(unsigned long start,
> + unsigned long nr_pages,
> + bool readonly)
> +{
> + struct mm_struct *mm = current->mm;
> + unsigned int gup_flags = (readonly ? 0 : FOLL_WRITE) | FOLL_NOWAIT;
> + int err = 0;
> +
> + down_read(&mm->mmap_sem);
mmap_read_lock(mm);
> + while (nr_pages) {
> + long ret;
> +
> + ret = get_user_pages(start, nr_pages, gup_flags, NULL, NULL);
> + if (ret < 0) {
> + err = (int)ret;
> + break;
> + }
> + if (ret == 0)
> + ret = 1; /* skip this page */
> +
> + start += ret << PAGE_SHIFT;
> + nr_pages -= ret;
> + }
> + up_read(&mm->mmap_sem);
mmap_read_unlock(mm);
param(bool, gem_userptr_prefault, true, 0600)
> param(bool, enable_dp_mst, true, 0600) \
> param(bool, enable_gvt, false, 0400)
>
- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAl/7fkwACgkQ24/THMrX
1yyJ+Qf+Kp1NqR/RruBKW3pKbNEZy2Y92viOcKkMcOq96fjLEn/boRevpQHjBpjQ
bdBa5wZasgk0aHV6UTGB1GrLMQupbupMcI2kffmJnvo0/uleRdad1QgPqlcWhO+x
Y0CtPioefWLEwBNITIJGr1emtyM/pO7NpVkFJt3jjei7DfG/jFEkmD36oKb/ea+P
GVmug75CpJKPpmYZT39RzqfoI6ZwCq7Lq70I+/kjQBNiyo2N5/xTKiBkt7NDkQas
lmrtBXAQRn0UGps7SU2tfsdkU/vYJWohNGK7NzLTSN4jyBsH8CBTyJ0x6DxTvYox
W9BPiiLAjBSYJ6jeZ4x8Ly89s0rw8Q==
=EF0b
-----END PGP SIGNATURE-----
Hash: SHA256
On Thu, Dec 24, 2020 at 02:24:59AM +0000, Jinoh Kang wrote:
> If GUP-ineligible pages are passed to a GEM userptr object, -EFAULT is
> returned only when the object is actually bound.
>
> The xf86-video-intel userspace driver cannot differentiate this
> condition, and marks the GPU as wedged. This disables graphics
> acceleration and may cripple other functionalities such as VT switch.
>
> Solve this by "prefaulting" user pages on GEM object creation, testing
> whether all pages are eligible for get_user_pages() in the process.
> On failure, return -EFAULT so that userspace can fallback to software
> bliting.
>
> This behavior can be controlled by using a new modparam
> "gem_userptr_prefault", which is true by default.
>
> The only known use for this patch is Qubes OS's GUI virtualization.
> Userspace is expected to resort to DMA-BUF whenever possible.
>
> Signed-off-by: Jinoh Kang <jinoh....@gmail.com>
couple fixes to build, see below.
But then, it seems to fix the issue with VT switch and disabling
acceleration.
Independently, starting with 5.9.x (without the patch) the system freezes
with specific monitor configuration (two monitors side by side, but one
scaled x1.5). It happens immediately after configuring the second
display (and I still got the message about it in Xorg.0.log). Not
setting scaling on the monitor workarounds the issue.
Switching to modesetting driver fixes it. This issue wasn't here on
5.8.x or earlier and looks to be unrelated to this patch. I'm not sure
whether it's purely kernel driver issue, or some kernel<->Xorg
incompatibility.
> ---
> drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 36 +++++++++++++++++++++
> drivers/gpu/drm/i915/i915_params.c | 3 ++
> drivers/gpu/drm/i915/i915_params.h | 1 +
> 3 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> index f2eaed6aca3d..65596d2b284f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> @@ -712,6 +712,34 @@ static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
> .release = i915_gem_userptr_release,
> };
>
> +static int i915_gem_userptr_prefault(unsigned long start,
> + unsigned long nr_pages,
> + bool readonly)
> +{
> + struct mm_struct *mm = current->mm;
> + unsigned int gup_flags = (readonly ? 0 : FOLL_WRITE) | FOLL_NOWAIT;
> + int err = 0;
> +
> + down_read(&mm->mmap_sem);
> + while (nr_pages) {
> + long ret;
> +
> + ret = get_user_pages(start, nr_pages, gup_flags, NULL, NULL);
> + if (ret < 0) {
> + err = (int)ret;
> + break;
> + }
> + if (ret == 0)
> + ret = 1; /* skip this page */
> +
> + start += ret << PAGE_SHIFT;
> + nr_pages -= ret;
> + }
> + up_read(&mm->mmap_sem);
> param(bool, enable_dp_mst, true, 0600) \
> param(bool, enable_gvt, false, 0400)
>
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAl/7fkwACgkQ24/THMrX
1yyJ+Qf+Kp1NqR/RruBKW3pKbNEZy2Y92viOcKkMcOq96fjLEn/boRevpQHjBpjQ
bdBa5wZasgk0aHV6UTGB1GrLMQupbupMcI2kffmJnvo0/uleRdad1QgPqlcWhO+x
Y0CtPioefWLEwBNITIJGr1emtyM/pO7NpVkFJt3jjei7DfG/jFEkmD36oKb/ea+P
GVmug75CpJKPpmYZT39RzqfoI6ZwCq7Lq70I+/kjQBNiyo2N5/xTKiBkt7NDkQas
lmrtBXAQRn0UGps7SU2tfsdkU/vYJWohNGK7NzLTSN4jyBsH8CBTyJ0x6DxTvYox
W9BPiiLAjBSYJ6jeZ4x8Ly89s0rw8Q==
=EF0b
-----END PGP SIGNATURE-----
Marek Marczykowski-Górecki
Jan 11, 2021, 6:03:17 PM1/11/21
to Jinoh Kang, qubes-users, qubes-devel, donoban, Patrik Hagara
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hash: SHA256
On Thu, Dec 24, 2020 at 02:21:22AM +0000, Jinoh Kang wrote:
> When using some Intel integrated graphic cards on Qubes R4.0, screen
> glitches may manifest after switching VTs or entering suspend mode.
>
> A known workaround does exist for this bug, which is to add a
> configuration file with the following contents within
> /etc/X11/xorg.conf.d:
>
> > Section "Device"
> > Identifier "Intel Graphics"
> > Driver "modesetting"
> > Option "AccelMethod" "glamor"
> > Option "DRI" "3"
> > EndSection
>
> However, the X11 modesetting driver version in Fedora 25 has its own
> drawbacks:
>
> * It freezes briefly when re-configuring monitors (e.g. plugging in an
> external monitor or changing screen resolution)
> * XRandR keystone support is buggy
>
> To remediate this, I've patched the Linux i915 driver and it has been
> working fine for months. Only the patch for Linux 4.19 has been tested.
>
> If anyone is affected by the issue, please feel free to test the
> follow-up patches and give some feedback here.
So, I can confirm the (fixed) 5.10 patch also improves the situation.
> When using some Intel integrated graphic cards on Qubes R4.0, screen
> glitches may manifest after switching VTs or entering suspend mode.
>
> A known workaround does exist for this bug, which is to add a
> configuration file with the following contents within
> /etc/X11/xorg.conf.d:
>
> > Section "Device"
> > Identifier "Intel Graphics"
> > Driver "modesetting"
> > Option "AccelMethod" "glamor"
> > Option "DRI" "3"
> > EndSection
>
> However, the X11 modesetting driver version in Fedora 25 has its own
> drawbacks:
>
> * It freezes briefly when re-configuring monitors (e.g. plugging in an
> external monitor or changing screen resolution)
> * XRandR keystone support is buggy
>
> To remediate this, I've patched the Linux i915 driver and it has been
> working fine for months. Only the patch for Linux 4.19 has been tested.
>
> If anyone is affected by the issue, please feel free to test the
> follow-up patches and give some feedback here.
Have you sent it upstream? I do consider including it in our standard
kernel package, but I'd like to see i915 driver maintainer opinion
first.
- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
1yz/XQf9GbeTq3KJpoO/smK7tLJ+EE8Q61G+nejAm5d7VZ+IBofLjWxds2cEn4kJ
xjEpjXxiqTL40cBRa1NkXoLLW7Dcesb/G/7MW+73qYm2DjVYyDQFAQOmnJDXT30L
Vdai3tXb1miTQ6gAme/Zaffe6RLsLzp1Qrq1ieEpQIjJk+tBSWRVTKyNKQAZDkt3
siznMtbre3te7XybIbShUpgXoiwCqpnjZwEmMJg93nFAre5K6XukIksZg+w3Nt1T
/INdhTR6DebTGLtn+pkV9PTGFDRLL+bmWQGallNI2tQnttWogolH9BfEKhkZq+Ja
KUIDySAOIjDhj1UfaGM6m73oIcRc9A==
=TSr5
-----END PGP SIGNATURE-----
Jinoh Kang
Jan 13, 2021, 8:23:24 AM1/13/21
to Marek Marczykowski-Górecki, qubes-users, qubes-devel, donoban, Patrik Hagara
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hash: SHA256
On 1/11/21 11:03 PM, Marek Marczykowski-Górecki wrote:
> So, I can confirm the (fixed) 5.10 patch also improves the situation.
Sounds good. Thanks for testing!
> So, I can confirm the (fixed) 5.10 patch also improves the situation.
> Have you sent it upstream?
posted this.
I guess chances of these patches being merged upstream would not be
that great.
After all, we're not going to need it with Qubes R4.1.
> I do consider including it in our standard
> kernel package, but I'd like to see i915 driver maintainer opinion
> first.
Tested-by: and/or Reviewed-by: lines for the trailer from you.
I'm fine as well if you'd rather just submit it yourself.
Otherwise I suppose I shall only CC' the maintainers and not the list?
Cheers,
Jinoh Kang
-----BEGIN PGP SIGNATURE-----
iQJMBAEBCAA2FiEEzGktrvc/U3kFXd9AGlqQRGyEq/UFAl/+860YHGppbm9oLmth
bmcua3JAZ21haWwuY29tAAoJEBpakERshKv1WEMQAMKzWpjyis08x3Ph8VVYVhbY
I+GjDaU29Y9xF5lJ8lSvJn1mWUwEiLMyz2ypW4xQE7Z5fC1TgDuz2VS2Bju5SSWv
m1gpsxT3rkydn3Cgr9gl8wmL/HgRltz3HB/zd3aoivqKBK7EUY/McDOqGo/Ntat6
1WogjRZwSQgy5X1oXee7Sn+Uo3Sz6sG/+aszT+oaS4hgrKccp6ImeJtDfH8axJKQ
Ud7BCNZTPHKaX7hw5YYA4IdSebceLWzzIL+Znz5M8g3Rf8Sqn6kuVPu/Q84NESBi
m5RxcZ0AFNINiQLlGDubys36vqPlPcUoitCDAa7sYC7LtKtuuuvaQb9wG7IS/OpG
XsiiLxGwSQ3mOzw2izfz42FjycNjIZ0nMBpFibqzg6aanWP2DWWJlo2L/AtxKUb1
463lJKEomBreAmO0YK3RQUkz6hZ12iOdCfay53X7T2tCNfgC4c7NgpEHhICiBwVL
DQpq1khCswyh+zb/boC/FQr3kp3kDQvtWAf5QTrZ24uM+SK1pd9IqRULuewuz/+r
+c4q9NSlMR8GxK7pXWxpowtWM/54ByeLMayXNYAIISkshN1cHRyIFDQ6OJ2ke7hr
rkRR8DdeR2M3ncZUJ5aWzVERlThzk2oJNIp41dTa2X6nh0A0XFfYLbLAnl6psf7f
/QjQTzjzVsSie06PTwzT
=SeI1
-----END PGP SIGNATURE-----
Marek Marczykowski-Górecki
Jan 13, 2021, 10:25:31 AM1/13/21
to Jinoh Kang, qubes-users, qubes-devel, donoban, Patrik Hagara
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hash: SHA256
On Wed, Jan 13, 2021 at 01:21:51PM +0000, Jinoh Kang wrote:
> On 1/11/21 11:03 PM, Marek Marczykowski-Górecki wrote:
> > So, I can confirm the (fixed) 5.10 patch also improves the situation.
>
> Sounds good. Thanks for testing!
>
> > Have you sent it upstream?
>
> No, qubes-users and qubes-devel are the only mailing list where I
> posted this.
>
> I guess chances of these patches being merged upstream would not be
> that great.
If that bug indeed affects only Qubes OS, there is a greater chance to
> On 1/11/21 11:03 PM, Marek Marczykowski-Górecki wrote:
> > So, I can confirm the (fixed) 5.10 patch also improves the situation.
>
> Sounds good. Thanks for testing!
>
> > Have you sent it upstream?
>
> No, qubes-users and qubes-devel are the only mailing list where I
> posted this.
>
> I guess chances of these patches being merged upstream would not be
> that great.
accept the patch, if the option defaults to false.
> After all, we're not going to need it with Qubes R4.1.
may be necessary. On the other hand, your patch description suggests
it's just any VM-mapped window triggers the faulty path in the
xf86-video-intel driver, that later affects all of the output.
> > I do consider including it in our standard
> > kernel package, but I'd like to see i915 driver maintainer opinion
> > first.
>
> If you mean you'd prefer to have it upstreamed, I'd appreciate some
> Tested-by: and/or Reviewed-by: lines for the trailer from you.
(5.11-rc3, or recent 5.10)? I'll re-test and add my Tested-by:.
> I'm fine as well if you'd rather just submit it yourself.
>
> Otherwise I suppose I shall only CC' the maintainers and not the list?
lists, which do include some mailing lists. I highly recommend using
scripts/get_maintainer.pl script for that purpose (if you use git
send-email, that's as easy as --cc-cmd=scripts/get_maintainer.pl).
PS The other (independent) issue I mentioned seems to be
https://bugzilla.suse.com/show_bug.cgi?id=1180543, which is supposed to
be already fixed in >=5.10.6. I've already uploaded 5.10.7, but haven't
tested it on this particular machine yet.
- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
1yxXWgf8DkpeBlpx3kQgXn+FFPsQGpLLkX9O3arm4WEcU71y02J0wmOml8XUj1oZ
4y3p6Wmk1KT8nC74SgG4igkCqcb7ay1m1L0D8AjrY8o4CaaJmErnd0kxYXJMfrnN
T2js+Hlh/kax0y7iphCCpX1IGH1QSPHThDKuMs/40blvKMIDLmymkq8BtoduVEwQ
nZzquV2vRZSFYgl79xWtnxr0QF8yzisIwbYgeEgl256G+ivtmhqLlej6eCUZe6FH
U6j7UwalfXTjWVTnUdtuvmt2rgsV8jZ69eUBJuqqBPfkt3XqMGxNKkAd0hFTBGoZ
f9XtU34qHMwk1vxZCddjsJYi/EPERg==
=teyW
-----END PGP SIGNATURE-----
Jinoh Kang
Jan 15, 2021, 11:14:43 AM1/15/21
to Marek Marczykowski-Górecki, qubes-users, qubes-devel, donoban, Patrik Hagara
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hash: SHA256
On 1/13/21 3:25 PM, Marek Marczykowski-Górecki wrote:
> On Wed, Jan 13, 2021 at 01:21:51PM +0000, Jinoh Kang wrote:
>> On 1/11/21 11:03 PM, Marek Marczykowski-Górecki wrote:
>>> So, I can confirm the (fixed) 5.10 patch also improves the situation.
>
>> Sounds good. Thanks for testing!
>
>>> Have you sent it upstream?
>
>> No, qubes-users and qubes-devel are the only mailing list where I
>> posted this.
>
>> I guess chances of these patches being merged upstream would not be
>> that great.
>
> If that bug indeed affects only Qubes OS, there is a greater chance to
> accept the patch, if the option defaults to false.
>
>> After all, we're not going to need it with Qubes R4.1.
>
> Are you sure?
Now that I think about it, I just realised that we are still going to
> On Wed, Jan 13, 2021 at 01:21:51PM +0000, Jinoh Kang wrote:
>> On 1/11/21 11:03 PM, Marek Marczykowski-Górecki wrote:
>>> So, I can confirm the (fixed) 5.10 patch also improves the situation.
>
>> Sounds good. Thanks for testing!
>
>>> Have you sent it upstream?
>
>> No, qubes-users and qubes-devel are the only mailing list where I
>> posted this.
>
>> I guess chances of these patches being merged upstream would not be
>> that great.
>
> If that bug indeed affects only Qubes OS, there is a greater chance to
> accept the patch, if the option defaults to false.
>
>> After all, we're not going to need it with Qubes R4.1.
>
> Are you sure?
this patch in Qubes R4.1, while migrating old VMs still using MFNDUMP.
But then, Fedora has moved away from xf86-video-intel to modesetting, so
we aren't going to need this patch either way (unless we are going to
switch to Wayland tomorrow)?
I believe everything else has been covered in qubes-issues#5909 [1].
> The issue affects dom0 windows, which suggests it still
> may be necessary.
> On the other hand, your patch description suggests
> it's just any VM-mapped window triggers the faulty path in the
> xf86-video-intel driver, that later affects all of the output.
>
>>> I do consider including it in our standard
>>> kernel package, but I'd like to see i915 driver maintainer opinion
>>> first.
>
>> If you mean you'd prefer to have it upstreamed, I'd appreciate some
>> Tested-by: and/or Reviewed-by: lines for the trailer from you.
>
> Can you send a fixed patch (that builds), rebased on top of recent Linux
> (5.11-rc3, or recent 5.10)? I'll re-test and add my Tested-by:.
>
>> I'm fine as well if you'd rather just submit it yourself.
>
>> Otherwise I suppose I shall only CC' the maintainers and not the list?
>
> Generally, Linux patches should be sent to whoever MAINTAINERS file
> lists, which do include some mailing lists. I highly recommend using
> scripts/get_maintainer.pl script for that purpose (if you use git
> send-email, that's as easy as --cc-cmd=scripts/get_maintainer.pl).
first stage my changes in my local IMAP daemon's mbox and bounce it via
Thunderbird. This is admittedly very annoying, and perhaps I should
tinker with (neo)mutt + gmail-oauth2-tools some time. Or maybe switch
to a "real" mail account. This will also explain my past embedded PGP
signing mistake. Sorry about that...;(
>
> PS The other (independent) issue I mentioned seems to be
> https://bugzilla.suse.com/show_bug.cgi?id=1180543, which is supposed to
> be already fixed in >=5.10.6. I've already uploaded 5.10.7, but haven't
> tested it on this particular machine yet.
- --
[1] https://github.com/QubesOS/qubes-issues/issues/5909
- --
Sincerely,
Jinoh Kang
-----BEGIN PGP SIGNATURE-----
iQJMBAEBCAA2FiEEzGktrvc/U3kFXd9AGlqQRGyEq/UFAmABvzEYHGppbm9oLmth
-----BEGIN PGP SIGNATURE-----
bmcua3JAZ21haWwuY29tAAoJEBpakERshKv1MIcQAI3TZ5Lp1kfIxEW9EasMgLQH
lKu+YH/ca5iWkoMGn3/uJKvKF2gEkuJp0u85nAu+ns7DWrBjggJnZ+A0y/uZtAFN
FcFQy4OMdURRwPVUTwAOr5WJkkpm04PEhNLZJrgD+8crb/fTYiwu5Ii3XO7egFPd
PQB71cxS4rzWoi+Dz8WVrhn1/wIg/jqL15//iXYM44NXLs8355lWI/QT5/L0dfn1
7g4vPJkbpeMUdjCo5xGyDDc59VUyyzauCPskRW7+G0y6JagzFi8wOI4b6kkoABau
YUgsC/CPGu+JKYPqbtROmA6M5+Mp8McA3oX2DbyIeYfTK58qd32/+bLI0Q6PY14r
8MGXzLFvN8kBA+e9mGCxX2SdX4sb6bYwlAaUs64B37Eo7ybugapUz6GUpL1U64+o
yyQIVoSZ1Hd4zA8KXMP0uocBuH1zrePU0EJ6nlp33X5HPOJXyPdMIFk+CJmPR6yk
3yDmGnQeXxOA+j8uXhH4yOF+zcwyE3+BDOsTKhsVxr99A24TYW92pVUXhN2KTkZQ
3dIYZfaYH2TySA67Z3n9bQ7aoyxe00Q8UcGuO87bFU8vGzUeRGlwyfMYKUzd6Nuz
E+w9g8DYfYfT7HXP1TfzMgAoF9hJfMLN3IQb56BBsIU2je5/A1HgF5kRcmulVKU4
CHThsvFTdVjydLZPuPuR
=8295
-----END PGP SIGNATURE-----
Jinoh Kang
Jan 15, 2021, 12:30:06 PM1/15/21
to Marek Marczykowski-Górecki, qubes-users, qubes-devel, donoban, Patrik Hagara
Is qubes-xorg-x11-drv-intel an option? Upstream hasn't released for years after all...
--
Sincerely,
Jinoh Kang
--
Sincerely,
Jinoh Kang
Marek Marczykowski-Górecki
Jan 15, 2021, 3:06:50 PM1/15/21
to Jinoh Kang, qubes-users, qubes-devel, donoban, Patrik Hagara
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hash: SHA256
On Fri, Jan 15, 2021 at 05:29:43PM +0000, Jinoh Kang wrote:
> Is qubes-xorg-x11-drv-intel an option? Upstream hasn't released for years after all...
Something like this. In fact the current (Fedora) package is already
> Is qubes-xorg-x11-drv-intel an option? Upstream hasn't released for years after all...
built from git snapshot.
We do backport this package from newer Fedora already:
https://github.com/QubesOS/qubes-linux-dom0-updates
But I would prefer to get it upstream anyway (and then possibly build
xorg-x11-drv-intel from newer git snapshot).
- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
1yxNRgf7B2nc2Qomgnqi2/lwiUmv0Mqx7e54cl2zQNtQl57TsVuDu+mWEbef15Ry
gtSBg9c8uXuDq8acbGTP5sqRAJmKlCtWyDdGf5jiZEWATCpZXcVyao/9b8pkuDkY
PZSaTEQU+GekWzSrbuoxHJj4HlrPGRxR4CrGGtaqCyqTzJ3V8rV39jbhG5+hxpdF
HBS0XBxZUHd1Lzxl0l/qbXkyiMSTJvuJ0a6Hl7rvPCbmNbaIAhXru4zM6ZCVTxC9
W00+hUyirnqz0lfXEhBUD2w42rwfO6Hs67yn8Te2/u9QnE9XxFKSVaRVZqfH6EUw
zrh+5BaGaAt4TeyiPxb9FdBdo8/wqQ==
=iNFz
-----END PGP SIGNATURE-----
Jinoh Kang
Jan 15, 2021, 8:49:50 PM1/15/21
to Marek Marczykowski-Górecki, qubes-users, qubes-devel, donoban, Patrik Hagara
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hash: SHA256
On 1/15/21 8:06 PM, Marek Marczykowski-Górecki wrote:
> On Fri, Jan 15, 2021 at 05:29:43PM +0000, Jinoh Kang wrote:
>> Is qubes-xorg-x11-drv-intel an option? Upstream hasn't released for years after all...
>
> Something like this. In fact the current (Fedora) package is already
> built from git snapshot.
Here's the catch: Fedora hasn't been bumping gitdate for almost a year,
> On Fri, Jan 15, 2021 at 05:29:43PM +0000, Jinoh Kang wrote:
>> Is qubes-xorg-x11-drv-intel an option? Upstream hasn't released for years after all...
>
> Something like this. In fact the current (Fedora) package is already
> built from git snapshot.
as seen in Pagure [1].
> We do backport this package from newer Fedora already:
> https://github.com/QubesOS/qubes-linux-dom0-updates
>
> But I would prefer to get it upstream anyway (and then possibly build
> xorg-x11-drv-intel from newer git snapshot).
diff --git a/src/sna/kgem.c b/src/sna/kgem.c
index 6a35067c..8a7af809 100644
- --- a/src/sna/kgem.c
+++ b/src/sna/kgem.c
@@ -7023,6 +7023,8 @@ struct kgem_bo *kgem_create_map(struct kgem *kgem,
struct kgem_bo *bo;
uintptr_t first_page, last_page;
uint32_t handle;
+ struct drm_i915_gem_set_domain set_domain;
+ bool move_to_gtt = false;
assert(MAP(ptr) == ptr);
@@ -7043,20 +7045,10 @@ struct kgem_bo *kgem_create_map(struct kgem *kgem,
read_only);
if (handle == 0) {
if (read_only && kgem->has_wc_mmap) {
- - struct drm_i915_gem_set_domain set_domain;
- -
handle = gem_userptr(kgem->fd,
(void *)first_page, last_page-first_page,
false);
- -
- - VG_CLEAR(set_domain);
- - set_domain.handle = handle;
- - set_domain.read_domains = I915_GEM_DOMAIN_GTT;
- - set_domain.write_domain = 0;
- - if (do_ioctl(kgem->fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, &set_domain)) {
- - gem_close(kgem->fd, handle);
- - handle = 0;
- - }
+ move_to_gtt = true;
}
if (handle == 0) {
DBG(("%s: import failed, errno=%d\n", __FUNCTION__, errno));
@@ -7064,6 +7056,21 @@ struct kgem_bo *kgem_create_map(struct kgem *kgem,
}
}
+ VG_CLEAR(set_domain);
+ set_domain.handle = handle;
+ if (move_to_gtt) {
+ set_domain.read_domains = I915_GEM_DOMAIN_GTT;
+ set_domain.write_domain = 0;
+ } else {
+ set_domain.read_domains = I915_GEM_DOMAIN_CPU;
+ set_domain.write_domain = I915_GEM_DOMAIN_CPU;
+ }
+ if (do_ioctl(kgem->fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, &set_domain)) {
+ gem_close(kgem->fd, handle);
+ DBG(("%s: set_domain in import failed, errno=%d\n", __FUNCTION__, errno));
+ return NULL;
+ }
+
bo = __kgem_bo_alloc(handle, (last_page - first_page) / PAGE_SIZE);
if (bo == NULL) {
gem_close(kgem->fd, handle);
- ---
[1] https://src.fedoraproject.org/rpms/xorg-x11-drv-intel/blob/master/f/xorg-x11-drv-intel.spec#_3
- --
Sincerely,
Jinoh Kang
-----BEGIN PGP SIGNATURE-----
bmcua3JAZ21haWwuY29tAAoJEBpakERshKv1rUcP/iymJetru96CXG3m0PTDLH37
fUenxVbuSc6J9xjPCzV7p39QBziQ2LBaIA6uL4csp7scaw+AHqKqQhxSSFb7Z4x0
zyiax7UyYuH5yGx6+Jn9E5Bf/sWarWw5fq3v61vgTs/0VUQs7tm3zKCj3uhyR4rG
pdhQEaFtqKFZVB26qNBiUsDwv1hibi0dqVvy+LHgXQqhsR39pCSBzrYFwLgf+7C8
9uD+Kbl1/CVmq+bQR9eYmRUQlkibmzzwGgtGrfHUT3dwYrGM+Q3VgsVHr9lOWQa5
F+AET7KkGLqq/iq81BWvJOOKBwPEpczp4TK1DBbI60RR/xIwShHMBxsTxRbvITIw
3ezYdRj7ArROA7XBkLw2LZo09jctG2BqO8USmWdif2wah82LLJrUmswTEWWE3R1Z
lxVnQKEA8LgNc+RICMr3bfN8+MxMErMs4rByF+Od7UUye8u/TjJvTMITvDpnskLW
L9kRrzCL5CZQoklyysvbIgyIwdb+n4OsIUhZ2guMGPly7rksmKOhfXmqFMCCMQCm
EXMDJ7mYQqVN+ajDKOWyQVUlMX9IsD37QtMLhGAA7+uvQJVGGUWiXeF+fgdtiE2v
wBC+EwuEFCnUV1Ich9T9+dBiXB3BX9gKkWJa9+YPLfw0yEwHziiBSMF10V7V/A4k
or2d3f2JMHUnVT4yJBES
=Edlh
-----END PGP SIGNATURE-----
Marek Marczykowski-Górecki
Jan 15, 2021, 8:53:44 PM1/15/21
to Jinoh Kang, qubes-users, qubes-devel, donoban, Patrik Hagara
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hash: SHA256
On Sat, Jan 16, 2021 at 01:49:25AM +0000, Jinoh Kang wrote:
> On 1/15/21 8:06 PM, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 15, 2021 at 05:29:43PM +0000, Jinoh Kang wrote:
> >> Is qubes-xorg-x11-drv-intel an option? Upstream hasn't released for years after all...
> >
> > Something like this. In fact the current (Fedora) package is already
> > built from git snapshot.
>
> Here's the catch: Fedora hasn't been bumping gitdate for almost a year,
> as seen in Pagure [1].
>
> > We do backport this package from newer Fedora already:
> > https://github.com/QubesOS/qubes-linux-dom0-updates
>
> That one from Fedora 28 is a bit behind, too.
>
> >
> > But I would prefer to get it upstream anyway (and then possibly build
> > xorg-x11-drv-intel from newer git snapshot).
>
> Something like this? (haven't built it yet, will fix later)
I guess, yes.
> On 1/15/21 8:06 PM, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 15, 2021 at 05:29:43PM +0000, Jinoh Kang wrote:
> >> Is qubes-xorg-x11-drv-intel an option? Upstream hasn't released for years after all...
> >
> > Something like this. In fact the current (Fedora) package is already
> > built from git snapshot.
>
> Here's the catch: Fedora hasn't been bumping gitdate for almost a year,
> as seen in Pagure [1].
>
> > We do backport this package from newer Fedora already:
> > https://github.com/QubesOS/qubes-linux-dom0-updates
>
> That one from Fedora 28 is a bit behind, too.
>
> >
> > But I would prefer to get it upstream anyway (and then possibly build
> > xorg-x11-drv-intel from newer git snapshot).
>
> Something like this? (haven't built it yet, will fix later)
> diff --git a/src/sna/kgem.c b/src/sna/kgem.c
> index 6a35067c..8a7af809 100644
> --- a/src/sna/kgem.c
> +++ b/src/sna/kgem.c
> @@ -7023,6 +7023,8 @@ struct kgem_bo *kgem_create_map(struct kgem *kgem,
> struct kgem_bo *bo;
> uintptr_t first_page, last_page;
> uint32_t handle;
> + struct drm_i915_gem_set_domain set_domain;
> + bool move_to_gtt = false;
>
> assert(MAP(ptr) == ptr);
>
> @@ -7043,20 +7045,10 @@ struct kgem_bo *kgem_create_map(struct kgem *kgem,
> read_only);
> if (handle == 0) {
> if (read_only && kgem->has_wc_mmap) {
> - struct drm_i915_gem_set_domain set_domain;
> +++ b/src/sna/kgem.c
> @@ -7023,6 +7023,8 @@ struct kgem_bo *kgem_create_map(struct kgem *kgem,
> struct kgem_bo *bo;
> uintptr_t first_page, last_page;
> uint32_t handle;
> + struct drm_i915_gem_set_domain set_domain;
> + bool move_to_gtt = false;
>
> assert(MAP(ptr) == ptr);
>
> @@ -7043,20 +7045,10 @@ struct kgem_bo *kgem_create_map(struct kgem *kgem,
> read_only);
> if (handle == 0) {
> if (read_only && kgem->has_wc_mmap) {
> -
> handle = gem_userptr(kgem->fd,
> (void *)first_page, last_page-first_page,
> false);
> -
> - VG_CLEAR(set_domain);
> handle = gem_userptr(kgem->fd,
> (void *)first_page, last_page-first_page,
> false);
> -
> - set_domain.handle = handle;
> - set_domain.read_domains = I915_GEM_DOMAIN_GTT;
> - set_domain.write_domain = 0;
> - if (do_ioctl(kgem->fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, &set_domain)) {
> - gem_close(kgem->fd, handle);
> - if (do_ioctl(kgem->fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, &set_domain)) {
> - handle = 0;
> - }
> - }
> + move_to_gtt = true;
> }
> if (handle == 0) {
> DBG(("%s: import failed, errno=%d\n", __FUNCTION__, errno));
> @@ -7064,6 +7056,21 @@ struct kgem_bo *kgem_create_map(struct kgem *kgem,
> }
> }
>
> + VG_CLEAR(set_domain);
> + set_domain.handle = handle;
> + if (move_to_gtt) {
> + set_domain.read_domains = I915_GEM_DOMAIN_GTT;
> + set_domain.write_domain = 0;
> + } else {
> + set_domain.read_domains = I915_GEM_DOMAIN_CPU;
> + set_domain.write_domain = I915_GEM_DOMAIN_CPU;
> + }
> + if (do_ioctl(kgem->fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, &set_domain)) {
> + gem_close(kgem->fd, handle);
> + DBG(("%s: set_domain in import failed, errno=%d\n", __FUNCTION__, errno));
> + return NULL;
> + }
> +
> bo = __kgem_bo_alloc(handle, (last_page - first_page) / PAGE_SIZE);
> if (bo == NULL) {
> gem_close(kgem->fd, handle);
>
> }
> if (handle == 0) {
> DBG(("%s: import failed, errno=%d\n", __FUNCTION__, errno));
> @@ -7064,6 +7056,21 @@ struct kgem_bo *kgem_create_map(struct kgem *kgem,
> }
> }
>
> + VG_CLEAR(set_domain);
> + set_domain.handle = handle;
> + if (move_to_gtt) {
> + set_domain.read_domains = I915_GEM_DOMAIN_GTT;
> + set_domain.write_domain = 0;
> + } else {
> + set_domain.read_domains = I915_GEM_DOMAIN_CPU;
> + set_domain.write_domain = I915_GEM_DOMAIN_CPU;
> + }
> + if (do_ioctl(kgem->fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, &set_domain)) {
> + gem_close(kgem->fd, handle);
> + DBG(("%s: set_domain in import failed, errno=%d\n", __FUNCTION__, errno));
> + return NULL;
> + }
> +
> bo = __kgem_bo_alloc(handle, (last_page - first_page) / PAGE_SIZE);
> if (bo == NULL) {
> gem_close(kgem->fd, handle);
>
> ---
>
> [1] https://src.fedoraproject.org/rpms/xorg-x11-drv-intel/blob/master/f/xorg-x11-drv-intel.spec#_3
>
- --
>
> [1] https://src.fedoraproject.org/rpms/xorg-x11-drv-intel/blob/master/f/xorg-x11-drv-intel.spec#_3
>
- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmACRyEACgkQ24/THMrX
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
1ywP6wgAlKaJitGmJHIgzkCpdGqEh3XjoqS2QOyIvsnzkn98v9E/cWrIrCMgrYAC
U2IIYx4e9vrqAW1JwyNLii7ws5/+yI1Y2H7r7In237hedWQ7rCWJRs0UYsAGrtJx
p/rNlxDhDBDWc2IWyZHE21bdEb1eKhl2W3EUzxsUGJ7ZxVDX8J8EgKS3PvZGLdC2
JdT2rcsy9ZWZ8YEmwm7k9GxHmuFMbAXJzgIVv3NxVWBQ4IJeNOfJrHrW1RFUMoyC
BtdkHNUzBtsMLNlGczRMMPE3LdL6n9E8KnXX6RqXgudsDibdm8ixAagas5E6Cvxq
zPgbcftI5MvpDHYdb4QZsCF6kFVxbQ==
=R/oj
-----END PGP SIGNATURE-----
Vít Šesták
Jan 21, 2021, 11:05:29 AM1/21/21
to qubes-devel
Hi,
I've decided to test the workaround without your patches on my i7 7500U (Intel HD Graphics 620). I don't need keystone effect correction and brief freeze when reconfiguring monitors is acceptable tradeoff for me. However, I've found one side effect: It has renamed HDMI1 to HDMI-1 and eDP1 to eDP-1. As a result, it broke my screen configuration scripts, so that my external screen was just scaled-up fullHD mirror of my laptop screen. But the solution was pretty easy: just adjust the screen names in the scripts.
I have observed no other drawbacks so far.
Regards,
Vít Šesták 'v6ak'
Reply all
Reply to author
Forward
0 new messages