Re: bug in inteldrm on braswell

2017-12-26 Thread Martin Ziemer 
On Tue, Dec 26, 2017 at 02:24:04PM +0100, Mark Kettenis wrote:
> I've committed the first bit now.  Let me know if you manage to
> trigger the WARN_ON before the 2nd bit.
I am running now since Thursday a kernel with only the first bit and
had no Problems so far. (And I stressed this notebook a lot)

I guess, the second bit is really not needed.

Thanks for the commit!

Re: bug in inteldrm on braswell

2017-12-26 Thread Mark Kettenis 
> Date: Thu, 21 Dec 2017 15:15:35 +0100
> From: Martin Ziemer 
> 
> > Sorry.  I want to test this myself before I commit it, but I keep
> > getting distracted with other stuff.
> No Sorry. I think its cool, you find the time to test everything
> yourself. (Everybody knows the Problem of no time and too much things
> to do)
> 
> I just repeated my mail, because there was no response and I thought,
> it was not seen by those, who can submit fixes.
> 
> > That said, the first bit (pt_vaddr) looks correct, but the second bit
> > doesn't make sense.  Have you actually seen the WARM_ON(!pt) being
> > triggered?
> No, i have not seen the Warning triggered. I just copied this Line
> from DragonflyBSD, because it was in Matt Dillons fix. (Perhaps he
> fixed two Issues there)
> 
> I will today compile a kernel with only the first part of the patch and
> try it. If it does not fail until after christmas i assume the first
> part is enough.

I've committed the first bit now.  Let me know if you manage to
trigger the WARN_ON before the 2nd bit.

Cheers,

Mark

Re: bug in inteldrm on braswell

2017-12-21 Thread Martin Ziemer 
> Sorry.  I want to test this myself before I commit it, but I keep
> getting distracted with other stuff.
No Sorry. I think its cool, you find the time to test everything
yourself. (Everybody knows the Problem of no time and too much things
to do)

I just repeated my mail, because there was no response and I thought,
it was not seen by those, who can submit fixes.

> That said, the first bit (pt_vaddr) looks correct, but the second bit
> doesn't make sense.  Have you actually seen the WARM_ON(!pt) being
> triggered?
No, i have not seen the Warning triggered. I just copied this Line
from DragonflyBSD, because it was in Matt Dillons fix. (Perhaps he
fixed two Issues there)

I will today compile a kernel with only the first part of the patch and
try it. If it does not fail until after christmas i assume the first
part is enough.

Re: bug in inteldrm on braswell

2017-12-21 Thread Jonathan Gray 
On Thu, Dec 21, 2017 at 02:44:44PM +0100, Mark Kettenis wrote:
> > Date: Thu, 21 Dec 2017 13:06:00 +0100
> > From: Martin Ziemer 
> > 
> > My system crashed regularly on usage of graphical Browsers. Other
> > people had the same issues:
> > https://marc.info/?l=openbsd-misc=150916174632762=2
> > 
> > After searching for the problem I found the solution in DragonflyBSD
> > and ported those changes to our inteldrm:
> > https://www.mail-archive.com/misc@openbsd.org/msg157533.html
> > 
> > The patch fixed the issue not only for me, but for at least two other
> > users.
> > 
> > Could someone please apply those changes to our inteldrm? (Or tell me,
> > why those changes are not fitting)
> 
> Sorry.  I want to test this myself before I commit it, but I keep
> getting distracted with other stuff.
> 
> That said, the first bit (pt_vaddr) looks correct, but the second bit
> doesn't make sense.  Have you actually seen the WARM_ON(!pt) being
> triggered?

Only the first bit is in the linux commit

commit 44a7102484db0ddfa6f855b57ffe0566f739b55a
Author: Matthew Auld 
Date:   Tue Apr 12 16:57:42 2016 +0100

drm/i915: call kunmap_px on pt_vaddr

We need to kunmap pt_vaddr and not pt itself, otherwise we end up
mapping a bunch of pages without ever unmapping them.

Cc: Chris Wilson 
Cc: Joonas Lahtinen 
Fixes: d1c54acd67dc ("drm/i915/gtt: Introduce kmap|kunmap for dma page")
Signed-off-by: Matthew Auld 
Reviewed-by: Joonas Lahtinen 
Signed-off-by: Mika Kuoppala 
Link: 
http://patchwork.freedesktop.org/patch/msgid/1460476663-24890-4-git-send-email-matthew.a...@intel.com

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9f165feb54ae..eebdb28acfa9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -745,7 +745,7 @@ static void gen8_ppgtt_clear_pte_range(struct 
i915_address_space *vm,
num_entries--;
}
 
-   kunmap_px(ppgtt, pt);
+   kunmap_px(ppgtt, pt_vaddr);
 
pte = 0;
if (++pde == I915_PDES) {

> 
> > Index: i915_gem_gtt.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_gem_gtt.c,v
> > retrieving revision 1.15
> > diff -u -p -r1.15 i915_gem_gtt.c
> > --- i915_gem_gtt.c  1 Jul 2017 16:14:10 -   1.15
> > +++ i915_gem_gtt.c  16 Nov 2017 15:51:02 -
> > @@ -778,7 +778,10 @@ static void gen8_ppgtt_clear_pte_range(s
> > num_entries--;
> > }
> >  
> > -   kunmap_px(ppgtt, pt);
> > +   kunmap_px(ppgtt, pt_vaddr); /* XXX dillon, out of order
> > + * patch from linux
> > + * 44a71024 12-Apr-2016
> > + */
> >  
> > pte = 0;
> > if (++pde == I915_PDES) {
> > @@ -1317,6 +1320,8 @@ static int gen8_alloc_va_range_3lvl(stru
> > gen8_for_each_pde(pt, pd, pd_start, pd_len, temp, pde) {
> > /* Same reasoning as pd */
> > WARN_ON(!pt);
> > +if (pt == NULL) /* XXX dillon hack */
> > +continue;   /* XXX dillon hack */
> > WARN_ON(!pd_len);
> > WARN_ON(!gen8_pte_count(pd_start, pd_len));
> > 
> > 
>

Re: bug in inteldrm on braswell

2017-12-21 Thread Mark Kettenis 
> Date: Thu, 21 Dec 2017 13:06:00 +0100
> From: Martin Ziemer 
> 
> My system crashed regularly on usage of graphical Browsers. Other
> people had the same issues:
> https://marc.info/?l=openbsd-misc=150916174632762=2
> 
> After searching for the problem I found the solution in DragonflyBSD
> and ported those changes to our inteldrm:
> https://www.mail-archive.com/misc@openbsd.org/msg157533.html
> 
> The patch fixed the issue not only for me, but for at least two other
> users.
> 
> Could someone please apply those changes to our inteldrm? (Or tell me,
> why those changes are not fitting)

Sorry.  I want to test this myself before I commit it, but I keep
getting distracted with other stuff.

That said, the first bit (pt_vaddr) looks correct, but the second bit
doesn't make sense.  Have you actually seen the WARM_ON(!pt) being
triggered?

> Index: i915_gem_gtt.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_gem_gtt.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 i915_gem_gtt.c
> --- i915_gem_gtt.c1 Jul 2017 16:14:10 -   1.15
> +++ i915_gem_gtt.c16 Nov 2017 15:51:02 -
> @@ -778,7 +778,10 @@ static void gen8_ppgtt_clear_pte_range(s
>   num_entries--;
>   }
>  
> - kunmap_px(ppgtt, pt);
> + kunmap_px(ppgtt, pt_vaddr); /* XXX dillon, out of order
> + * patch from linux
> + * 44a71024 12-Apr-2016
> + */
>  
>   pte = 0;
>   if (++pde == I915_PDES) {
> @@ -1317,6 +1320,8 @@ static int gen8_alloc_va_range_3lvl(stru
>   gen8_for_each_pde(pt, pd, pd_start, pd_len, temp, pde) {
>   /* Same reasoning as pd */
>   WARN_ON(!pt);
> +if (pt == NULL) /* XXX dillon hack */
> +continue;   /* XXX dillon hack */
>   WARN_ON(!pd_len);
>   WARN_ON(!gen8_pte_count(pd_start, pd_len));
> 
>