Re: [Intel-gfx] [PATCH xf86-video-intel v2] SNA: fix PRIME output support since xserver 1.20

2019-11-16 Thread Peter Wu
On Fri, Nov 15, 2019 at 08:14:05PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 15, 2019 at 04:32:47PM +0100, Peter Wu wrote:
> > Since "Make PixmapDirtyUpdateRec::src a DrawablePtr" in xserver, the
> > "src" pointer might point to the root window (created by the server)
> > instead of a pixmap (as created by xf86-video-intel). Use
> > get_drawable_pixmap to handle both cases.
> > 
> > When built with -fsanitize=address, the following test on a hybrid
> > graphics laptop will trigger a heap-buffer-overflow error due to
> > to_sna_from_pixmap receiving a window instead of a pixmap:
> > 
> > xrandr --setprovideroutputsource modesetting Intel
> > xrandr --output DP-1-1 --mode 2560x1440  # should not crash
> > glxgears  # should display gears on both screens
> > 
> > With nouveau instead of modesetting, it does not crash but the external
> > monitor remains blank aside from a mouse cursor. This patch fixes both.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100086
> 
> Also
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111976

I marked this bug as duplicate of the former since it is the same issue.

About the CI failure
(https://lists.freedesktop.org/archives/intel-gfx/2019-November/220187.html),
should I be concerned? I can't see what tree it is trying to apply the
patch to. Is it actually trying to apply it to xf86-video-intel, or is
it trying the Linux kernel instead?

> > Signed-off-by: Peter Wu 
> > ---
> > v1: https://lists.freedesktop.org/archives/intel-gfx/2018-August/173522.html
> > v2: rebased on current master (2.99.917-893-gbff5eca4), reworded commit.
> > 
> > This patch has been tested at https://bugs.archlinux.org/task/64238, I
> > have additionally tested it with both modesetting and nouveau under
> > ASAN, the modesetting ASAN trace for unpatched intel can be found at:
> > https://bugs.freedesktop.org/show_bug.cgi?id=100086#c24
> > 
> > commit 2.99.917-891-g581ddc5d ("sna: Fix compiler warnings due to
> > DrawablePtr vs. PixmapPtr") incorporated all compiler warning fixes from
> > v1 of this patch, but unfortunately lacks this crucial bugfix.
> > ---
> >  src/sna/sna_accel.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
> > index fa386ff6..ee857a14 100644
> > --- a/src/sna/sna_accel.c
> > +++ b/src/sna/sna_accel.c
> > @@ -17684,10 +17684,10 @@ static void sna_accel_post_damage(struct sna *sna)
> > continue;
> >  
> >  #ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
> > -   assert(dirty->src->type == DRAWABLE_PIXMAP);
> > +   src = get_drawable_pixmap(dirty->src);
> > +#else
> > +   src = dirty->src;
> >  #endif
> > -
> > -   src = (PixmapPtr)dirty->src;
> > dst = dirty->slave_dst->master_pixmap;
> >  
> > region.extents.x1 = dirty->x;
> > -- 
> > 2.23.0
> 
> -- 
> Ville Syrjälä
> Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH xf86-video-intel v2] SNA: fix PRIME output support since xserver 1.20

2019-11-15 Thread Peter Wu
Since "Make PixmapDirtyUpdateRec::src a DrawablePtr" in xserver, the
"src" pointer might point to the root window (created by the server)
instead of a pixmap (as created by xf86-video-intel). Use
get_drawable_pixmap to handle both cases.

When built with -fsanitize=address, the following test on a hybrid
graphics laptop will trigger a heap-buffer-overflow error due to
to_sna_from_pixmap receiving a window instead of a pixmap:

xrandr --setprovideroutputsource modesetting Intel
xrandr --output DP-1-1 --mode 2560x1440  # should not crash
glxgears  # should display gears on both screens

With nouveau instead of modesetting, it does not crash but the external
monitor remains blank aside from a mouse cursor. This patch fixes both.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100086
Signed-off-by: Peter Wu 
---
v1: https://lists.freedesktop.org/archives/intel-gfx/2018-August/173522.html
v2: rebased on current master (2.99.917-893-gbff5eca4), reworded commit.

This patch has been tested at https://bugs.archlinux.org/task/64238, I
have additionally tested it with both modesetting and nouveau under
ASAN, the modesetting ASAN trace for unpatched intel can be found at:
https://bugs.freedesktop.org/show_bug.cgi?id=100086#c24

commit 2.99.917-891-g581ddc5d ("sna: Fix compiler warnings due to
DrawablePtr vs. PixmapPtr") incorporated all compiler warning fixes from
v1 of this patch, but unfortunately lacks this crucial bugfix.
---
 src/sna/sna_accel.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index fa386ff6..ee857a14 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -17684,10 +17684,10 @@ static void sna_accel_post_damage(struct sna *sna)
continue;
 
 #ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
-   assert(dirty->src->type == DRAWABLE_PIXMAP);
+   src = get_drawable_pixmap(dirty->src);
+#else
+   src = dirty->src;
 #endif
-
-   src = (PixmapPtr)dirty->src;
dst = dirty->slave_dst->master_pixmap;
 
region.extents.x1 = dirty->x;
-- 
2.23.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH xf86-video-intel] SNA: fix PRIME output support since xserver 1.20

2018-09-11 Thread Peter Wu
In meantime this patch has been picked up by the Arch Linux packages:
https://bugs.archlinux.org/task/58895

Is there any chance that this patch can be reviewed/applied? The diff is
quite small and should be easy to review.

On Wed, Aug 29, 2018 at 12:22:21PM +0200, Peter Wu wrote:
> Ping.
> 
> This patch was independently verified working (see the linked bug
> report) and is essential for Xorg 1.20 using this driver.
> 
> On Tue, Aug 14, 2018 at 02:16:07AM +0200, Peter Wu wrote:
> > Since xorg-server 1.20, an external monitor would remain blank when used
> > in a PRIME output slave setup. Only a cursor was visible. The cause is
> > "Make PixmapDirtyUpdateRec::src a DrawablePtr" in xserver, the "src"
> > pointer might point to the root window (created by the server) instead
> > of a pixmap (as created by xf86-video-intel). Use get_drawable_pixmap to
> > handle both cases.
> > 
> > When built with -fsanitize=address, the following test will trigger a
> > heap-buffer-overflow error due to to_sna_from_pixmap receiving a window
> > instead of a pixmap.
> > 
> > Test on a hybrid graphics laptop (Intel + modesetting/nouveau):
> > 
> > xrandr --setprovideroutputsource modesetting Intel
> > xrandr --output DP-1-1 --mode 2560x1440  # should not crash
> > glxgears  # should display gears on both screens
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100086
> > Signed-off-by: Peter Wu 
> > ---
> > Tested with xserver 1.20.1 with ASAN enabled. Survives multiple
> > resolution changes, works with a Plasma desktop session, it seems
> > stable. Something like this patch is required to make multi-monitor
> > setups usable in a hybrid graphics setting with Xorg 1.20.
> > ---
> >  src/sna/sna_accel.c | 18 ++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
> > index 2f669bcf..80b116a3 100644
> > --- a/src/sna/sna_accel.c
> > +++ b/src/sna/sna_accel.c
> > @@ -17510,7 +17510,11 @@ static bool has_offload_slaves(struct sna *sna)
> > PixmapDirtyUpdatePtr dirty;
> >  
> > xorg_list_for_each_entry(dirty, &screen->pixmap_dirty_list, ent) {
> > +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
> > +   assert(dirty->src == &sna->front->drawable);
> > +#else
> > assert(dirty->src == sna->front);
> > +#endif
> > if (RegionNotEmpty(DamageRegion(dirty->damage)))
> > return true;
> > }
> > @@ -17671,7 +17675,11 @@ static void sna_accel_post_damage(struct sna *sna)
> > if (RegionNil(damage))
> > continue;
> >  
> > +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
> > +   src = get_drawable_pixmap(dirty->src);
> > +#else
> > src = dirty->src;
> > +#endif
> > dst = dirty->slave_dst->master_pixmap;
> >  
> > region.extents.x1 = dirty->x;
> > @@ -17922,9 +17930,15 @@ migrate_dirty_tracking(PixmapPtr old_front, 
> > PixmapPtr new_front)
> > PixmapDirtyUpdatePtr dirty, safe;
> >  
> > xorg_list_for_each_entry_safe(dirty, safe, &screen->pixmap_dirty_list, 
> > ent) {
> > +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
> > +   assert(dirty->src == &old_front->drawable);
> > +   if (dirty->src != &old_front->drawable)
> > +   continue;
> > +#else
> > assert(dirty->src == old_front);
> > if (dirty->src != old_front)
> > continue;
> > +#endif
> >  
> > DamageUnregister(&dirty->src->drawable, dirty->damage);
> > DamageDestroy(dirty->damage);
> > @@ -17939,7 +17953,11 @@ migrate_dirty_tracking(PixmapPtr old_front, 
> > PixmapPtr new_front)
> > }
> >  
> > DamageRegister(&new_front->drawable, dirty->damage);
> > +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
> > +   dirty->src = &new_front->drawable;
> > +#else
> > dirty->src = new_front;
> > +#endif
> > }
> >  #endif
> >  }
> > -- 
> > 2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH xf86-video-intel] SNA: fix PRIME output support since xserver 1.20

2018-08-29 Thread Peter Wu
Ping.

This patch was independently verified working (see the linked bug
report) and is essential for Xorg 1.20 using this driver.

On Tue, Aug 14, 2018 at 02:16:07AM +0200, Peter Wu wrote:
> Since xorg-server 1.20, an external monitor would remain blank when used
> in a PRIME output slave setup. Only a cursor was visible. The cause is
> "Make PixmapDirtyUpdateRec::src a DrawablePtr" in xserver, the "src"
> pointer might point to the root window (created by the server) instead
> of a pixmap (as created by xf86-video-intel). Use get_drawable_pixmap to
> handle both cases.
> 
> When built with -fsanitize=address, the following test will trigger a
> heap-buffer-overflow error due to to_sna_from_pixmap receiving a window
> instead of a pixmap.
> 
> Test on a hybrid graphics laptop (Intel + modesetting/nouveau):
> 
> xrandr --setprovideroutputsource modesetting Intel
> xrandr --output DP-1-1 --mode 2560x1440  # should not crash
> glxgears  # should display gears on both screens
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100086
> Signed-off-by: Peter Wu 
> ---
> Tested with xserver 1.20.1 with ASAN enabled. Survives multiple
> resolution changes, works with a Plasma desktop session, it seems
> stable. Something like this patch is required to make multi-monitor
> setups usable in a hybrid graphics setting with Xorg 1.20.
> ---
>  src/sna/sna_accel.c | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
> index 2f669bcf..80b116a3 100644
> --- a/src/sna/sna_accel.c
> +++ b/src/sna/sna_accel.c
> @@ -17510,7 +17510,11 @@ static bool has_offload_slaves(struct sna *sna)
>   PixmapDirtyUpdatePtr dirty;
>  
>   xorg_list_for_each_entry(dirty, &screen->pixmap_dirty_list, ent) {
> +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
> + assert(dirty->src == &sna->front->drawable);
> +#else
>   assert(dirty->src == sna->front);
> +#endif
>   if (RegionNotEmpty(DamageRegion(dirty->damage)))
>   return true;
>   }
> @@ -17671,7 +17675,11 @@ static void sna_accel_post_damage(struct sna *sna)
>   if (RegionNil(damage))
>   continue;
>  
> +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
> + src = get_drawable_pixmap(dirty->src);
> +#else
>   src = dirty->src;
> +#endif
>   dst = dirty->slave_dst->master_pixmap;
>  
>   region.extents.x1 = dirty->x;
> @@ -17922,9 +17930,15 @@ migrate_dirty_tracking(PixmapPtr old_front, 
> PixmapPtr new_front)
>   PixmapDirtyUpdatePtr dirty, safe;
>  
>   xorg_list_for_each_entry_safe(dirty, safe, &screen->pixmap_dirty_list, 
> ent) {
> +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
> + assert(dirty->src == &old_front->drawable);
> + if (dirty->src != &old_front->drawable)
> + continue;
> +#else
>   assert(dirty->src == old_front);
>   if (dirty->src != old_front)
>   continue;
> +#endif
>  
>   DamageUnregister(&dirty->src->drawable, dirty->damage);
>   DamageDestroy(dirty->damage);
> @@ -17939,7 +17953,11 @@ migrate_dirty_tracking(PixmapPtr old_front, 
> PixmapPtr new_front)
>   }
>  
>   DamageRegister(&new_front->drawable, dirty->damage);
> +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
> + dirty->src = &new_front->drawable;
> +#else
>   dirty->src = new_front;
> +#endif
>   }
>  #endif
>  }
> -- 
> 2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH xserver] randr: fix RRCrtcDetachScanoutPixmap crash on server exit

2018-08-13 Thread Peter Wu
The following crash was observed with xserver 1.20.1 on exiting xserver
after enabling a PRIME output source with the Intel driver:

Old value = (WindowPtr) 0x612000159dc0
New value = (WindowPtr) 0x0 // pWin->drawable.pScreen->root = NULL;
DeleteWindow (value=0x612000159dc0, wid=) at 
dix/window.c:1112
1112dixFreeObjectWithPrivates(pWin, PRIVATE_WINDOW);
(gdb) bt
#0  DeleteWindow (value=0x612000159dc0, wid=) at 
dix/window.c:1112
#1  0x557e7842535b in doFreeResource (res=0x6030ebf0, 
skip=) at dix/resource.c:880
#2  0x557e784289ed in FreeClientResources (client=0x60e00040) at 
dix/resource.c:1146
#3  0x557e78428c46 in FreeAllResources () at dix/resource.c:1161
#4  0x557e783c25d8 in dix_main (argc=, argv=, envp=) at dix/main.c:292
...

Thread 1 "Xorg" received signal SIGSEGV, Segmentation fault.
0x557e7841138c in PixmapStopDirtyTracking (src=0x0, 
slave_dst=0x6112ea80) at dix/pixmap.c:251
251 ScreenPtr screen = src->pScreen;
(gdb) bt
#0  0x558e598a938c in PixmapStopDirtyTracking (src=0x0, 
slave_dst=0x61138d00) at ../xserver/dix/pixmap.c:251
#1  0x558e5990ccd5 in RRCrtcDetachScanoutPixmap (crtc=0x61704680) 
at ../xserver/randr/rrcrtc.c:413
#2  0x558e5990d001 in RRCrtcDestroyResource (value=0x61704680, 
pid=) at ../xserver/randr/rrcrtc.c:900
#3  0x558e598bd35b in doFreeResource (res=0x6030a2a0, 
skip=) at ../xserver/dix/resource.c:880
#4  0x558e598c09ed in FreeClientResources (client=0x60e00040) at 
../xserver/dix/resource.c:1146
#5  0x558e598c0c46 in FreeAllResources () at 
../xserver/dix/resource.c:1161
#6  0x558e5985a5d8 in dix_main (argc=, argv=, envp=) at ../xserver/dix/main.c:292

For some reason, the Window resource ends up being freed before a pixmap
when using the intel driver. It does not occur with modesetting (there
RRCrtcDestroyResource is called before deleting the root window).

Before "Make PixmapDirtyUpdateRec::src a DrawablePtr" the "src" argument
was "master->GetScreenPixmap(master)". After that commit, it becomes the
root window drawable which can be NULL as shown above.

Signed-off-by: Peter Wu 
---
 randr/rrcrtc.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index 5d9026266..d5dc235b7 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -398,20 +398,22 @@ RRCrtcDetachScanoutPixmap(RRCrtcPtr crtc)
 ScreenPtr master = crtc->pScreen->current_master;
 DrawablePtr mrootdraw = &master->root->drawable;
 
-if (crtc->scanout_pixmap_back) {
-pScrPriv->rrDisableSharedPixmapFlipping(crtc);
+if (mrootdraw) {
+if (crtc->scanout_pixmap_back) {
+pScrPriv->rrDisableSharedPixmapFlipping(crtc);
 
-master->StopFlippingPixmapTracking(mrootdraw,
-   crtc->scanout_pixmap,
-   crtc->scanout_pixmap_back);
+master->StopFlippingPixmapTracking(mrootdraw,
+   crtc->scanout_pixmap,
+   crtc->scanout_pixmap_back);
 
-rrDestroySharedPixmap(crtc, crtc->scanout_pixmap_back);
-crtc->scanout_pixmap_back = NULL;
-}
-else {
-pScrPriv->rrCrtcSetScanoutPixmap(crtc, NULL);
-master->StopPixmapTracking(mrootdraw,
-   crtc->scanout_pixmap);
+rrDestroySharedPixmap(crtc, crtc->scanout_pixmap_back);
+crtc->scanout_pixmap_back = NULL;
+}
+else {
+pScrPriv->rrCrtcSetScanoutPixmap(crtc, NULL);
+master->StopPixmapTracking(mrootdraw,
+   crtc->scanout_pixmap);
+}
 }
 
 rrDestroySharedPixmap(crtc, crtc->scanout_pixmap);
-- 
2.18.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH xf86-video-intel] SNA: fix PRIME output support since xserver 1.20

2018-08-13 Thread Peter Wu
Since xorg-server 1.20, an external monitor would remain blank when used
in a PRIME output slave setup. Only a cursor was visible. The cause is
"Make PixmapDirtyUpdateRec::src a DrawablePtr" in xserver, the "src"
pointer might point to the root window (created by the server) instead
of a pixmap (as created by xf86-video-intel). Use get_drawable_pixmap to
handle both cases.

When built with -fsanitize=address, the following test will trigger a
heap-buffer-overflow error due to to_sna_from_pixmap receiving a window
instead of a pixmap.

Test on a hybrid graphics laptop (Intel + modesetting/nouveau):

xrandr --setprovideroutputsource modesetting Intel
xrandr --output DP-1-1 --mode 2560x1440  # should not crash
glxgears  # should display gears on both screens

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100086
Signed-off-by: Peter Wu 
---
Tested with xserver 1.20.1 with ASAN enabled. Survives multiple
resolution changes, works with a Plasma desktop session, it seems
stable. Something like this patch is required to make multi-monitor
setups usable in a hybrid graphics setting with Xorg 1.20.
---
 src/sna/sna_accel.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 2f669bcf..80b116a3 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -17510,7 +17510,11 @@ static bool has_offload_slaves(struct sna *sna)
PixmapDirtyUpdatePtr dirty;
 
xorg_list_for_each_entry(dirty, &screen->pixmap_dirty_list, ent) {
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+   assert(dirty->src == &sna->front->drawable);
+#else
assert(dirty->src == sna->front);
+#endif
if (RegionNotEmpty(DamageRegion(dirty->damage)))
return true;
}
@@ -17671,7 +17675,11 @@ static void sna_accel_post_damage(struct sna *sna)
if (RegionNil(damage))
continue;
 
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+   src = get_drawable_pixmap(dirty->src);
+#else
src = dirty->src;
+#endif
dst = dirty->slave_dst->master_pixmap;
 
region.extents.x1 = dirty->x;
@@ -17922,9 +17930,15 @@ migrate_dirty_tracking(PixmapPtr old_front, PixmapPtr 
new_front)
PixmapDirtyUpdatePtr dirty, safe;
 
xorg_list_for_each_entry_safe(dirty, safe, &screen->pixmap_dirty_list, 
ent) {
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+   assert(dirty->src == &old_front->drawable);
+   if (dirty->src != &old_front->drawable)
+   continue;
+#else
assert(dirty->src == old_front);
if (dirty->src != old_front)
continue;
+#endif
 
DamageUnregister(&dirty->src->drawable, dirty->damage);
DamageDestroy(dirty->damage);
@@ -17939,7 +17953,11 @@ migrate_dirty_tracking(PixmapPtr old_front, PixmapPtr 
new_front)
}
 
DamageRegister(&new_front->drawable, dirty->damage);
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+   dirty->src = &new_front->drawable;
+#else
dirty->src = new_front;
+#endif
}
 #endif
 }
-- 
2.18.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 0/3] drm/i915/opregion: proper handling of DIDL, and some hacks on CADL

2016-11-21 Thread Peter Wu
Hi Jani,

I can also confirm that patches 1+2 (applied in top of v4.9-rc5ish) fixes it 
for me (Clevo P651RA).
The previous issue where the keys broke with PRIME/monitor hotplugging is also 
gone.
After system suspend/resume stuff is also still working.

Thanks!

Kind regards,
Peter
https://lekensteyn.nl
(pardon my brevity, top-posting and formatting, sent from my phone)


On 18 November 2016 09:27:07 CET, Jani Nikula  wrote:
>On Thu, 17 Nov 2016, Paolo Stivanin  wrote:
>> Hello,
>> I can confirm that patch 3 is not needed.
>> I applied only patch 1 and 2 and everything works super fine!
>
>Thanks for confirming this.
>
>BR,
>Jani.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 0/2] drm/i915/opregion: proper handling of DIDL and CADL

2016-08-27 Thread Peter Wu
Hi Jani,

The method is somehow not reliable. At boot I was still able to capture
brightness hotkeys (via acpi_listen). After starting SSDM and logging
in, it still worked. Then I played a bit with PRIME output slaves, Xorg
crashed at some points and hotkeys stopped working.

Inspection of CADL (via acpidbg) showed that it contained zeroes instead
of 0x400. By poking more, suddenly the expected 0x400 value returned
into CADL and hotkeys functioned again. At later moments it reverted to
0 again and hotkeys broke again.

Tracking down setters of active_crcts, I noticed that
intel_modeset_readout_hw_state somehow does not detect eDP1 as active.
Full dmesg log (9MiB) with drm.debug=0x1e (via sysfs) is available at

https://lekensteyn.nl/files/logs/journal-v4.8-rc3-183-g5e608a0-plus-2-cadl-patches.txt

Kind regards,
Peter

On Thu, Aug 25, 2016 at 03:53:02PM +0300, Jani Nikula wrote:
> This is the next iteration of [1] and [2]. Please review and/or test,
> according to your abilities.
> 
> Thanks,
> Jani.
> 
> Cc: Peter Wu 
> Cc: Rainer Koenig 
> Cc: Jan-Marek Glogowski 
> Cc: Maarten Lankhorst 
> Cc: Marcos Paulo de Souza 
> Cc: Paolo Stivanin 
> 
> [1] http://mid.mail-archive.com/cover.1467214151.git.jani.nikula@intel.com
> [2] 
> 1471315782-925-1-git-send-email-marcos.souza.org@gmail.com">http://mid.mail-archive.com/1471315782-925-1-git-send-email-marcos.souza.org@gmail.com
> 
> Jani Nikula (2):
>   drm/i915: make i915 the source of acpi device ids for _DOD
>   drm/i915/opregion: update cadl based on actually active outputs
> 
>  drivers/gpu/drm/i915/i915_drv.h   |   4 +
>  drivers/gpu/drm/i915/intel_display.c  |   6 ++
>  drivers/gpu/drm/i915/intel_drv.h  |   3 +
>  drivers/gpu/drm/i915/intel_opregion.c | 157 
> +-
>  4 files changed, 74 insertions(+), 96 deletions(-)
> 
> -- 
> 2.1.4
> 

-- 
Kind regards,
Peter Wu
https://lekensteyn.nl
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4 0/2] drm/i915/opregion: proper handling of DIDL and CADL

2016-06-29 Thread Peter Wu
Hi Jani,

I just tested your branch (2f9a317) and can confirm that the fix is still 
valid. evtest reports brightness up/down events.

Kind regards,
Peter
https://lekensteyn.nl
(pardon my brevity, top-posting and formatting, sent from my phone)


On 29 June 2016 17:36:40 CEST, Jani Nikula  wrote:
>This is v4 of [1]. The first three have already been pushed to
>drm-intel-next-queued. The only change here is the atomic commit.
>
>Review and testing would be much appreciated to move this forward. For
>testing, I've pushed this to opregion-didl-v4 branch of my repo at [2].
>
>Maarten, please check the hunk touching the atomic code in patch 2.
>
>BR,
>Jani.
>
>
>[1] http://mid.gmane.org/cover.1465810007.git.jani.nik...@intel.com
>[2] https://cgit.freedesktop.org/~jani/drm/
>
>Jani Nikula (2):
>  drm/i915: make i915 the source of acpi device ids for _DOD
>  drm/i915/opregion: update cadl based on actually active outputs
>
> drivers/gpu/drm/i915/i915_drv.h   |   2 +
> drivers/gpu/drm/i915/intel_display.c  |   4 +
> drivers/gpu/drm/i915/intel_drv.h  |   3 +
>drivers/gpu/drm/i915/intel_opregion.c | 155
>+-
> 4 files changed, 69 insertions(+), 95 deletions(-)

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [RFC PATCH 4/5] drm/i915: make i915 the source of acpi device ids for _DOD

2016-05-27 Thread Peter Wu
_SIZE(opregion->acpi->didl) +
>   ARRAY_SIZE(opregion->acpi->did2);
>  
> - list_for_each_entry(acpi_cdev, &acpi_video_bus->children, node) {
> - if (i >= max_outputs) {
> - DRM_DEBUG_KMS("More than %u outputs detected via 
> ACPI\n",
> -   max_outputs);
> - return;
> - }
> - status = acpi_evaluate_integer(acpi_cdev->handle, "_ADR",
> -NULL, &device_id);
> - if (ACPI_SUCCESS(status)) {
> - if (!device_id)
> - goto blind_set;
> - set_did(opregion, i++, (u32)(device_id & 0x0f0f));

Previously the device ID included just the display index (0x00f) and
display type (0xf00).

> - }
> + for_each_intel_connector(dev, connector) {
> + u32 device_id, type;
> +
> + device_id = acpi_display_type(connector);
> + device_id |= ACPI_DEVICE_ID_SCHEME;

Now it includes an additional bit. The Clevo P651RA firmware expects
that the contents contain a value masked with 0xf0f. If this line is
removed, the Brightness Down/Up Notification is sent to the display
device:

ElseIf ((^^^GFX0.CDDS (0x0410) == 0x1F)) {
Notify (^^^GFX0.DD1F, 0x87)
}

(The items in _DOD are constructed with 0x8000 | (DIDx & 0xf0f).)

I have tested it also on an older Clevo B7130 laptop which needed the
hack that is removed in patch 6.  There is no regression on that device.
For some reason the old laptop exposed an "acpi_video0" device, pressing
hotkeys generates both a KEY_BRIGHTNESSDOWN event (measured via evtest)
and changes the brightness itself.

On the new laptop (Clevo P651RA) there is an "intel_backlight" device
and only the hotkeys are sent, the actual brightness was not affected.
I guess that a desktop environment should handle it, but this
unfortunately does not really work for the text consoles.

Anyway, these series (with the above fix) are
Reviewed-and-tested-by: Peter Wu 

Peter

> + /* Use display type specific display index. */
> + type = (device_id & ACPI_DISPLAY_TYPE_MASK)
> + >> ACPI_DISPLAY_TYPE_SHIFT;
> + device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT;
> 
> + connector->acpi_device_id = device_id;
> + if (i < max_outputs)
> + set_did(opregion, i, device_id);
> + i++;
>   }
>  
> -end:
>   DRM_DEBUG_KMS("%d outputs detected\n", i);
>  
> + if (i > max_outputs)
> + DRM_ERROR("More than %d outputs in connector list\n",
> +   max_outputs);
> +
>   /* If fewer than max outputs, the list must be null terminated */
>   if (i < max_outputs)
>   set_did(opregion, i, 0);
> - return;
> -
> -blind_set:
> - i = 0;
> - list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> - int display_type = acpi_display_type(connector);
> -
> - if (i >= max_outputs) {
> - DRM_DEBUG_KMS("More than %u outputs in connector 
> list\n",
> -   max_outputs);
> - return;
> - }
> -
> - temp = get_did(opregion, i);
> - set_did(opregion, i, temp | (1 << 31) | display_type | i);
> - i++;
> - }
> - goto end;
>  }
>  
>  static void intel_setup_cadls(struct drm_device *dev)
> -- 
> 2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] i915: fix ACPI _DSM warning

2013-08-01 Thread Peter Wu
Since commit 29a241c (ACPICA: Add argument typechecking for all
predefined ACPI names), _DSM parameters are validated which trigger the
following warning:

ACPI Warning: \_SB_.PCI0.GFX0._DSM: Argument #4 type mismatch - Found 
[Integer], ACPI requires [Package] (20130517/nsarguments-95)
ACPI Warning: \_SB_.PCI0.GFX0._DSM: Argument #4 type mismatch - Found 
[Integer], ACPI requires [Package] (20130517/nsarguments-95)
ACPI Warning: \_SB_.PCI0.P0P2.PEGP._DSM: Argument #4 type mismatch - Found 
[Integer], ACPI requires [Package] (20130517/nsarguments-95)
ACPI Warning: \_SB_.PCI0.P0P2.PEGP._DSM: Argument #4 type mismatch - Found 
[Integer], ACPI requires [Package] (20130517/nsarguments-95)

As the Intel _DSM method seems to ignore this parameter, let's comply to
the ACPI spec and use a Package instead.

Signed-off-by: Peter Wu 
---
What is this code useful for? It seems unfinished, all it does it
printing some information when a mux is available, but besides that
there is no interaction with the driver.
---
 drivers/gpu/drm/i915/intel_acpi.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
b/drivers/gpu/drm/i915/intel_acpi.c
index bcbbaea..57fe1ae 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -28,7 +28,7 @@ static const u8 intel_dsm_guid[] = {
0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
 };
 
-static int intel_dsm(acpi_handle handle, int func, int arg)
+static int intel_dsm(acpi_handle handle, int func)
 {
struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
struct acpi_object_list input;
@@ -46,8 +46,9 @@ static int intel_dsm(acpi_handle handle, int func, int arg)
params[1].integer.value = INTEL_DSM_REVISION_ID;
params[2].type = ACPI_TYPE_INTEGER;
params[2].integer.value = func;
-   params[3].type = ACPI_TYPE_INTEGER;
-   params[3].integer.value = arg;
+   params[3].type = ACPI_TYPE_PACKAGE;
+   params[3].package.count = 0;
+   params[3].package.elements = NULL;
 
ret = acpi_evaluate_object(handle, "_DSM", &input, &output);
if (ret) {
@@ -151,8 +152,9 @@ static void intel_dsm_platform_mux_info(void)
params[1].integer.value = INTEL_DSM_REVISION_ID;
params[2].type = ACPI_TYPE_INTEGER;
params[2].integer.value = INTEL_DSM_FN_PLATFORM_MUX_INFO;
-   params[3].type = ACPI_TYPE_INTEGER;
-   params[3].integer.value = 0;
+   params[3].type = ACPI_TYPE_PACKAGE;
+   params[3].package.count = 0;
+   params[3].package.elements = NULL;
 
ret = acpi_evaluate_object(intel_dsm_priv.dhandle, "_DSM", &input,
   &output);
@@ -205,7 +207,7 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
return false;
}
 
-   ret = intel_dsm(dhandle, INTEL_DSM_FN_SUPPORTED_FUNCTIONS, 0);
+   ret = intel_dsm(dhandle, INTEL_DSM_FN_SUPPORTED_FUNCTIONS);
if (ret < 0) {
DRM_DEBUG_KMS("failed to get supported _DSM functions\n");
return false;
-- 
1.8.3.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx