| Message ID | 20121011103237.21e6637c@jbarnes-desktop (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi 2012/10/11 Jesse Barnes <jbarnes@virtuousgeek.org>: > I've only lightly tested this so far, but the corruption seems to be > gone if I write the GFX_FLSH_CNTL reg after binding an object. This > register should control the TLB for the system agent, which is what CPU > mapped objects will go through. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> I'm not sure if this is the patch you asked me to test on IRC, but, well this is the patch I tested :) I tested it on HSW on top of dinq + some other patches. Booted the machine, ran mostly xfterm4 under Xfce, but I also ran firefox, gnome-shell and restarted X a few times. No GPU hangs so far. I'm testing this for about 3-4 hours. I do have to say that I could not find this register on the HSW documentation, but I also did no see any "Unclaimed write" messages containing the 101008 address... I'm a little confused. And the interesting thing: intel_gpu_tools is useless now. I keep getting "Couldn't map MMIO region: Resource temporarily unavailable". Do you see this too? > > diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h > index 6ec0fff..95f0d4d 100644 > --- a/drivers/char/agp/intel-agp.h > +++ b/drivers/char/agp/intel-agp.h > @@ -99,6 +99,9 @@ > #define GFX_FLSH_CNTL 0x2170 /* 915+ */ > #define GFX_FLSH_CNTL_VLV 0x101008 > > +#define GFX_FLSH_CNTL 0x101008 > +#define GFX_FLSH_CNTL_EN (1<<0) > + > #define I810_DRAM_CTL 0x3000 > #define I810_DRAM_ROW_0 0x00000001 > #define I810_DRAM_ROW_0_SDRAM 0x00000001 > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > index e01f5ea..08844d6 100644 > --- a/drivers/char/agp/intel-gtt.c > +++ b/drivers/char/agp/intel-gtt.c > @@ -667,12 +667,8 @@ static int intel_gtt_init(void) > gtt_map_size = intel_private.base.gtt_total_entries * 4; > > intel_private.gtt = NULL; > - if (INTEL_GTT_GEN < 6) > - intel_private.gtt = ioremap_wc(intel_private.gtt_bus_addr, > - gtt_map_size); > - if (intel_private.gtt == NULL) > - intel_private.gtt = ioremap(intel_private.gtt_bus_addr, > - gtt_map_size); > + intel_private.gtt = ioremap_wc(intel_private.gtt_bus_addr, > + gtt_map_size); > if (intel_private.gtt == NULL) { > intel_private.driver->cleanup(); > iounmap(intel_private.registers); > @@ -897,6 +893,7 @@ void intel_gtt_insert_sg_entries(struct sg_table *st, > } > } > readl(intel_private.gtt+j-1); > + writel(GFX_FLSH_CNTL_EN, intel_private.registers + GFX_FLSH_CNTL); > } > EXPORT_SYMBOL(intel_gtt_insert_sg_entries); > > @@ -913,6 +910,7 @@ static void intel_gtt_insert_pages(unsigned int first_entry, > j, flags); > } > readl(intel_private.gtt+j-1); > + writel(GFX_FLSH_CNTL_EN, intel_private.registers + GFX_FLSH_CNTL); > } > > static int intel_fake_agp_insert_entries(struct agp_memory *mem, > @@ -1256,7 +1254,7 @@ static int i9xx_setup(void) > > reg_addr &= 0xfff80000; > > - if (INTEL_GTT_GEN >= 7) > + if (INTEL_GTT_GEN >= 6) > size = MB(2); > > intel_private.registers = ioremap(reg_addr, size); > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 11 Oct 2012 20:29:47 -0300 Paulo Zanoni <przanoni@gmail.com> wrote: > Hi > > 2012/10/11 Jesse Barnes <jbarnes@virtuousgeek.org>: > > I've only lightly tested this so far, but the corruption seems to be > > gone if I write the GFX_FLSH_CNTL reg after binding an object. This > > register should control the TLB for the system agent, which is what CPU > > mapped objects will go through. > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > I'm not sure if this is the patch you asked me to test on IRC, but, > well this is the patch I tested :) > > I tested it on HSW on top of dinq + some other patches. Booted the > machine, ran mostly xfterm4 under Xfce, but I also ran firefox, > gnome-shell and restarted X a few times. No GPU hangs so far. I'm > testing this for about 3-4 hours. > > I do have to say that I could not find this register on the HSW > documentation, but I also did no see any "Unclaimed write" messages > containing the 101008 address... I'm a little confused. > > And the interesting thing: intel_gpu_tools is useless now. I keep > getting "Couldn't map MMIO region: Resource temporarily unavailable". > Do you see this too? No I haven't seen that, I ran some tests today with the tip of the tree too... Dunno why the resource files would return EBUSY? Maybe because a driver is bound?
On Thu, Oct 11, 2012 at 7:54 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > On Thu, 11 Oct 2012 20:29:47 -0300 > Paulo Zanoni <przanoni@gmail.com> wrote: > >> Hi >> >> 2012/10/11 Jesse Barnes <jbarnes@virtuousgeek.org>: >> > I've only lightly tested this so far, but the corruption seems to be >> > gone if I write the GFX_FLSH_CNTL reg after binding an object. This >> > register should control the TLB for the system agent, which is what CPU >> > mapped objects will go through. >> > >> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> >> >> I'm not sure if this is the patch you asked me to test on IRC, but, >> well this is the patch I tested :) >> >> I tested it on HSW on top of dinq + some other patches. Booted the >> machine, ran mostly xfterm4 under Xfce, but I also ran firefox, >> gnome-shell and restarted X a few times. No GPU hangs so far. I'm >> testing this for about 3-4 hours. >> >> I do have to say that I could not find this register on the HSW >> documentation, but I also did no see any "Unclaimed write" messages >> containing the 101008 address... I'm a little confused. >> >> And the interesting thing: intel_gpu_tools is useless now. I keep >> getting "Couldn't map MMIO region: Resource temporarily unavailable". >> Do you see this too? > > No I haven't seen that, I ran some tests today with the tip of the tree > too... Dunno why the resource files would return EBUSY? Maybe because > a driver is bound? More likely because of some WB/WC/UC collision. We don't like aliases. Dave.
On Fri, Oct 12, 2012 at 03:54:50AM -0400, Dave Airlie wrote: > On Thu, Oct 11, 2012 at 7:54 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > On Thu, 11 Oct 2012 20:29:47 -0300 > > Paulo Zanoni <przanoni@gmail.com> wrote: > > > >> Hi > >> > >> 2012/10/11 Jesse Barnes <jbarnes@virtuousgeek.org>: > >> > I've only lightly tested this so far, but the corruption seems to be > >> > gone if I write the GFX_FLSH_CNTL reg after binding an object. This > >> > register should control the TLB for the system agent, which is what CPU > >> > mapped objects will go through. > >> > > >> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > >> > >> I'm not sure if this is the patch you asked me to test on IRC, but, > >> well this is the patch I tested :) > >> > >> I tested it on HSW on top of dinq + some other patches. Booted the > >> machine, ran mostly xfterm4 under Xfce, but I also ran firefox, > >> gnome-shell and restarted X a few times. No GPU hangs so far. I'm > >> testing this for about 3-4 hours. > >> > >> I do have to say that I could not find this register on the HSW > >> documentation, but I also did no see any "Unclaimed write" messages > >> containing the 101008 address... I'm a little confused. > >> > >> And the interesting thing: intel_gpu_tools is useless now. I keep > >> getting "Couldn't map MMIO region: Resource temporarily unavailable". > >> Do you see this too? > > > > No I haven't seen that, I ran some tests today with the tip of the tree > > too... Dunno why the resource files would return EBUSY? Maybe because > > a driver is bound? > > More likely because of some WB/WC/UC collision. We don't like aliases. Yep, you need to upgrade, latest i-g-t should handle the wc/uc split correctly. The important testcase is gem_gtt_cpu_tlb, if that one works this patch is good. -Daniel
Hi 2012/10/12 Daniel Vetter <daniel@ffwll.ch>: > On Fri, Oct 12, 2012 at 03:54:50AM -0400, Dave Airlie wrote: >> On Thu, Oct 11, 2012 at 7:54 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: >> > On Thu, 11 Oct 2012 20:29:47 -0300 >> > Paulo Zanoni <przanoni@gmail.com> wrote: >> > >> >> Hi >> >> >> >> 2012/10/11 Jesse Barnes <jbarnes@virtuousgeek.org>: >> >> > I've only lightly tested this so far, but the corruption seems to be >> >> > gone if I write the GFX_FLSH_CNTL reg after binding an object. This >> >> > register should control the TLB for the system agent, which is what CPU >> >> > mapped objects will go through. >> >> > >> >> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> >> >> >> >> I'm not sure if this is the patch you asked me to test on IRC, but, >> >> well this is the patch I tested :) >> >> >> >> I tested it on HSW on top of dinq + some other patches. Booted the >> >> machine, ran mostly xfterm4 under Xfce, but I also ran firefox, >> >> gnome-shell and restarted X a few times. No GPU hangs so far. I'm >> >> testing this for about 3-4 hours. >> >> >> >> I do have to say that I could not find this register on the HSW >> >> documentation, but I also did no see any "Unclaimed write" messages >> >> containing the 101008 address... I'm a little confused. >> >> >> >> And the interesting thing: intel_gpu_tools is useless now. I keep >> >> getting "Couldn't map MMIO region: Resource temporarily unavailable". >> >> Do you see this too? >> > >> > No I haven't seen that, I ran some tests today with the tip of the tree >> > too... Dunno why the resource files would return EBUSY? Maybe because >> > a driver is bound? >> >> More likely because of some WB/WC/UC collision. We don't like aliases. > > Yep, you need to upgrade, latest i-g-t should handle the wc/uc split > correctly. The important testcase is gem_gtt_cpu_tlb, if that one works > this patch is good. You are right, thanks. A git pull on i-g-t makes everything work again :) And gem_gtt_cpu_tlb also passes. Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
2012/10/11 Jesse Barnes <jbarnes@virtuousgeek.org>: > I've only lightly tested this so far, but the corruption seems to be > gone if I write the GFX_FLSH_CNTL reg after binding an object. This > register should control the TLB for the system agent, which is what CPU > mapped objects will go through. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h > index 6ec0fff..95f0d4d 100644 > --- a/drivers/char/agp/intel-agp.h > +++ b/drivers/char/agp/intel-agp.h > @@ -99,6 +99,9 @@ > #define GFX_FLSH_CNTL 0x2170 /* 915+ */ > #define GFX_FLSH_CNTL_VLV 0x101008 > > +#define GFX_FLSH_CNTL 0x101008 > +#define GFX_FLSH_CNTL_EN (1<<0) Notice that there's a redefinition of GFX_FLSH_CNTL with a different value just 3 lines above. We're probably fixing gen6+ and breaking gen5- We write 0 to this reg in some places, but I believe that shouldn't matter. > + > #define I810_DRAM_CTL 0x3000 > #define I810_DRAM_ROW_0 0x00000001 > #define I810_DRAM_ROW_0_SDRAM 0x00000001 > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > index e01f5ea..08844d6 100644 > --- a/drivers/char/agp/intel-gtt.c > +++ b/drivers/char/agp/intel-gtt.c > @@ -667,12 +667,8 @@ static int intel_gtt_init(void) > gtt_map_size = intel_private.base.gtt_total_entries * 4; > > intel_private.gtt = NULL; > - if (INTEL_GTT_GEN < 6) > - intel_private.gtt = ioremap_wc(intel_private.gtt_bus_addr, > - gtt_map_size); > - if (intel_private.gtt == NULL) > - intel_private.gtt = ioremap(intel_private.gtt_bus_addr, > - gtt_map_size); > + intel_private.gtt = ioremap_wc(intel_private.gtt_bus_addr, > + gtt_map_size); > if (intel_private.gtt == NULL) { > intel_private.driver->cleanup(); > iounmap(intel_private.registers); > @@ -897,6 +893,7 @@ void intel_gtt_insert_sg_entries(struct sg_table *st, > } > } > readl(intel_private.gtt+j-1); > + writel(GFX_FLSH_CNTL_EN, intel_private.registers + GFX_FLSH_CNTL); > } > EXPORT_SYMBOL(intel_gtt_insert_sg_entries); > > @@ -913,6 +910,7 @@ static void intel_gtt_insert_pages(unsigned int first_entry, > j, flags); > } > readl(intel_private.gtt+j-1); > + writel(GFX_FLSH_CNTL_EN, intel_private.registers + GFX_FLSH_CNTL); > } > > static int intel_fake_agp_insert_entries(struct agp_memory *mem, > @@ -1256,7 +1254,7 @@ static int i9xx_setup(void) > > reg_addr &= 0xfff80000; > > - if (INTEL_GTT_GEN >= 7) > + if (INTEL_GTT_GEN >= 6) > size = MB(2); > > intel_private.registers = ioremap(reg_addr, size); > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Oct 16, 2012 at 9:11 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > 2012/10/11 Jesse Barnes <jbarnes@virtuousgeek.org>: >> I've only lightly tested this so far, but the corruption seems to be >> gone if I write the GFX_FLSH_CNTL reg after binding an object. This >> register should control the TLB for the system agent, which is what CPU >> mapped objects will go through. >> >> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> >> >> diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h >> index 6ec0fff..95f0d4d 100644 >> --- a/drivers/char/agp/intel-agp.h >> +++ b/drivers/char/agp/intel-agp.h >> @@ -99,6 +99,9 @@ >> #define GFX_FLSH_CNTL 0x2170 /* 915+ */ >> #define GFX_FLSH_CNTL_VLV 0x101008 >> >> +#define GFX_FLSH_CNTL 0x101008 >> +#define GFX_FLSH_CNTL_EN (1<<0) > > Notice that there's a redefinition of GFX_FLSH_CNTL with a different > value just 3 lines above. We're probably fixing gen6+ and breaking > gen5- Yeah, Jesse needs to fix up this patch (or me, since it'll conflict anyway with -fixes, so I need a backmerge first). > We write 0 to this reg in some places, but I believe that shouldn't matter. This is one of the magic registers we have: Read always returns 0, it doesn't matter what you write into it, but something special happens as a side-effect of writing to it. In this case we flush the cpu gtt tlb. We have a few others of this kind. -Daniel
On Thu, Oct 11, 2012 at 10:32:37AM -0700, Jesse Barnes wrote: > I've only lightly tested this so far, but the corruption seems to be > gone if I write the GFX_FLSH_CNTL reg after binding an object. This > register should control the TLB for the system agent, which is what CPU > mapped objects will go through. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Ok, I've pushed out a new dinq with -rc2 backmerged. Can you please resubmit, with the conflicts resolved and GFX_FLSH_CNTL bikeshedded to GFX_FLSH_CNTL_GEN6? There's also a _VLV #define variant which needs to be killed since it's redundant now ... Thanks, Daniel > > diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h > index 6ec0fff..95f0d4d 100644 > --- a/drivers/char/agp/intel-agp.h > +++ b/drivers/char/agp/intel-agp.h > @@ -99,6 +99,9 @@ > #define GFX_FLSH_CNTL 0x2170 /* 915+ */ > #define GFX_FLSH_CNTL_VLV 0x101008 > > +#define GFX_FLSH_CNTL 0x101008 > +#define GFX_FLSH_CNTL_EN (1<<0) > + > #define I810_DRAM_CTL 0x3000 > #define I810_DRAM_ROW_0 0x00000001 > #define I810_DRAM_ROW_0_SDRAM 0x00000001 > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > index e01f5ea..08844d6 100644 > --- a/drivers/char/agp/intel-gtt.c > +++ b/drivers/char/agp/intel-gtt.c > @@ -667,12 +667,8 @@ static int intel_gtt_init(void) > gtt_map_size = intel_private.base.gtt_total_entries * 4; > > intel_private.gtt = NULL; > - if (INTEL_GTT_GEN < 6) > - intel_private.gtt = ioremap_wc(intel_private.gtt_bus_addr, > - gtt_map_size); > - if (intel_private.gtt == NULL) > - intel_private.gtt = ioremap(intel_private.gtt_bus_addr, > - gtt_map_size); > + intel_private.gtt = ioremap_wc(intel_private.gtt_bus_addr, > + gtt_map_size); > if (intel_private.gtt == NULL) { > intel_private.driver->cleanup(); > iounmap(intel_private.registers); > @@ -897,6 +893,7 @@ void intel_gtt_insert_sg_entries(struct sg_table *st, > } > } > readl(intel_private.gtt+j-1); > + writel(GFX_FLSH_CNTL_EN, intel_private.registers + GFX_FLSH_CNTL); > } > EXPORT_SYMBOL(intel_gtt_insert_sg_entries); > > @@ -913,6 +910,7 @@ static void intel_gtt_insert_pages(unsigned int first_entry, > j, flags); > } > readl(intel_private.gtt+j-1); > + writel(GFX_FLSH_CNTL_EN, intel_private.registers + GFX_FLSH_CNTL); > } > > static int intel_fake_agp_insert_entries(struct agp_memory *mem, > @@ -1256,7 +1254,7 @@ static int i9xx_setup(void) > > reg_addr &= 0xfff80000; > > - if (INTEL_GTT_GEN >= 7) > + if (INTEL_GTT_GEN >= 6) > size = MB(2); > > intel_private.registers = ioremap(reg_addr, size); > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h index 6ec0fff..95f0d4d 100644 --- a/drivers/char/agp/intel-agp.h +++ b/drivers/char/agp/intel-agp.h @@ -99,6 +99,9 @@ #define GFX_FLSH_CNTL 0x2170 /* 915+ */ #define GFX_FLSH_CNTL_VLV 0x101008 +#define GFX_FLSH_CNTL 0x101008 +#define GFX_FLSH_CNTL_EN (1<<0) + #define I810_DRAM_CTL 0x3000 #define I810_DRAM_ROW_0 0x00000001 #define I810_DRAM_ROW_0_SDRAM 0x00000001 diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c index e01f5ea..08844d6 100644 --- a/drivers/char/agp/intel-gtt.c +++ b/drivers/char/agp/intel-gtt.c @@ -667,12 +667,8 @@ static int intel_gtt_init(void) gtt_map_size = intel_private.base.gtt_total_entries * 4; intel_private.gtt = NULL; - if (INTEL_GTT_GEN < 6) - intel_private.gtt = ioremap_wc(intel_private.gtt_bus_addr, - gtt_map_size); - if (intel_private.gtt == NULL) - intel_private.gtt = ioremap(intel_private.gtt_bus_addr, - gtt_map_size); + intel_private.gtt = ioremap_wc(intel_private.gtt_bus_addr, + gtt_map_size); if (intel_private.gtt == NULL) { intel_private.driver->cleanup(); iounmap(intel_private.registers); @@ -897,6 +893,7 @@ void intel_gtt_insert_sg_entries(struct sg_table *st, } } readl(intel_private.gtt+j-1); + writel(GFX_FLSH_CNTL_EN, intel_private.registers + GFX_FLSH_CNTL); } EXPORT_SYMBOL(intel_gtt_insert_sg_entries); @@ -913,6 +910,7 @@ static void intel_gtt_insert_pages(unsigned int first_entry, j, flags); } readl(intel_private.gtt+j-1); + writel(GFX_FLSH_CNTL_EN, intel_private.registers + GFX_FLSH_CNTL); } static int intel_fake_agp_insert_entries(struct agp_memory *mem, @@ -1256,7 +1254,7 @@ static int i9xx_setup(void) reg_addr &= 0xfff80000; - if (INTEL_GTT_GEN >= 7) + if (INTEL_GTT_GEN >= 6) size = MB(2); intel_private.registers = ioremap(reg_addr, size);
I've only lightly tested this so far, but the corruption seems to be gone if I write the GFX_FLSH_CNTL reg after binding an object. This register should control the TLB for the system agent, which is what CPU mapped objects will go through. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>