[PATCH] agp/intel: Fix dma mask for Sandybridge

[PATCH] agp/intel: Fix dma mask for Sandybridge

[Takashi Iwai]

Sandybridge requires 36bit dma mask, but the current code checks only
against i965, thus it gives Oops with i915 probing on 32bit machine:

   nommu_map_sg: overflow 14a000000+4096 of device mask ffffffff
   [drm:drm_agp_bind_pages] *ERROR* Failed to bind AGP memory: -12
   BUG: unable to handle kernel paging request at fffffff8
   IP: [<f7fac57f>] i915_gem_evict_something+0xef/0x230 [i915]
   ...

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/char/agp/intel-agp.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index ddf5def..c563a60 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -845,7 +845,8 @@ static int __devinit intel_gmch_probe(struct pci_dev *pdev,
 
 	dev_info(&pdev->dev, "Intel %s Chipset\n", intel_agp_chipsets[i].name);
 
-	if (bridge->driver->mask_memory == intel_i965_mask_memory) {
+	if (bridge->driver->mask_memory == intel_i965_mask_memory ||
+	    bridge->driver->mask_memory == intel_gen6_mask_memory) {
 		if (pci_set_dma_mask(intel_private.pcidev, DMA_BIT_MASK(36)))
 			dev_err(&intel_private.pcidev->dev,
 				"set gfx device dma mask 36bit failed!\n");
-- 
1.7.2.1

Re: [PATCH] agp/intel: Fix dma mask for Sandybridge

[Zhenyu Wang]

[-- Attachment #1: Type: text/plain, Size: 656 bytes --]

On 2010.08.20 17:36:08 +0200, Takashi Iwai wrote:
> Sandybridge requires 36bit dma mask, but the current code checks only
> against i965, thus it gives Oops with i915 probing on 32bit machine:
> 
>    nommu_map_sg: overflow 14a000000+4096 of device mask ffffffff
>    [drm:drm_agp_bind_pages] *ERROR* Failed to bind AGP memory: -12
>    BUG: unable to handle kernel paging request at fffffff8
>    IP: [<f7fac57f>] i915_gem_evict_something+0xef/0x230 [i915]
>    ...

Sandybridge can do 40-bit dma mask. This has been fixed upstream now.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] agp/intel: Fix dma mask for Sandybridge

[Takashi Iwai]

At Mon, 23 Aug 2010 09:35:07 +0800,
Zhenyu Wang wrote:
> 
> On 2010.08.20 17:36:08 +0200, Takashi Iwai wrote:
> > Sandybridge requires 36bit dma mask, but the current code checks only
> > against i965, thus it gives Oops with i915 probing on 32bit machine:
> > 
> >    nommu_map_sg: overflow 14a000000+4096 of device mask ffffffff
> >    [drm:drm_agp_bind_pages] *ERROR* Failed to bind AGP memory: -12
> >    BUG: unable to handle kernel paging request at fffffff8
> >    IP: [<f7fac57f>] i915_gem_evict_something+0xef/0x230 [i915]
> >    ...
> 
> Sandybridge can do 40-bit dma mask. This has been fixed upstream now.

Could you point where is the upstream GIT tree and the corresponding
commit id?


thanks,

Takashi

Re: [PATCH] agp/intel: Fix dma mask for Sandybridge

[Zhenyu Wang]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

On 2010.08.23 07:29:29 +0200, Takashi Iwai wrote:
> > Sandybridge can do 40-bit dma mask. This has been fixed upstream now.
> 
> Could you point where is the upstream GIT tree and the corresponding
> commit id?
> 

Linus's tree:

commit 877fdacf8291d7627f339885b5ae52c2f6061734
Author: Zhenyu Wang <zhenyuw@linux.intel.com>
Date:   Thu Aug 19 09:46:13 2010 +0800

agp/intel: set 40-bit dma mask on Sandybridge
        
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
Signed-off-by: Eric Anholt <eric@anholt.net>

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] agp/intel: Fix dma mask for Sandybridge

[Takashi Iwai]

At Mon, 23 Aug 2010 13:43:03 +0800,
Zhenyu Wang wrote:
> 
> On 2010.08.23 07:29:29 +0200, Takashi Iwai wrote:
> > > Sandybridge can do 40-bit dma mask. This has been fixed upstream now.
> > 
> > Could you point where is the upstream GIT tree and the corresponding
> > commit id?
> > 
> 
> Linus's tree:
> 
> commit 877fdacf8291d7627f339885b5ae52c2f6061734
> Author: Zhenyu Wang <zhenyuw@linux.intel.com>
> Date:   Thu Aug 19 09:46:13 2010 +0800
> 
> agp/intel: set 40-bit dma mask on Sandybridge
>         
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> Signed-off-by: Eric Anholt <eric@anholt.net>

Thanks.

But, isn't it better to add .dma_mask field to struct
agp_bridge_driver?

Also, I don't understand the logic of 40bit addr calculation:

> static unsigned long intel_gen6_mask_memory(struct agp_bridge_data *bridge,
>                                             dma_addr_t addr, int type)
> {
>         /* Shift high bits down */
>        addr |= (addr >> 28) & 0xff;

Isn't it 0xff0?


Takashi

Re: [PATCH] agp/intel: Fix dma mask for Sandybridge

[Takashi Iwai]

At Mon, 23 Aug 2010 08:02:42 +0200,
I wrote:
> 
> At Mon, 23 Aug 2010 13:43:03 +0800,
> Zhenyu Wang wrote:
> > 
> > On 2010.08.23 07:29:29 +0200, Takashi Iwai wrote:
> > > > Sandybridge can do 40-bit dma mask. This has been fixed upstream now.
> > > 
> > > Could you point where is the upstream GIT tree and the corresponding
> > > commit id?
> > > 
> > 
> > Linus's tree:
> > 
> > commit 877fdacf8291d7627f339885b5ae52c2f6061734
> > Author: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Date:   Thu Aug 19 09:46:13 2010 +0800
> > 
> > agp/intel: set 40-bit dma mask on Sandybridge
> >         
> > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Signed-off-by: Eric Anholt <eric@anholt.net>
> 
> Thanks.
> 
> But, isn't it better to add .dma_mask field to struct
> agp_bridge_driver?
> 
> Also, I don't understand the logic of 40bit addr calculation:
> 
> > static unsigned long intel_gen6_mask_memory(struct agp_bridge_data *bridge,
> >                                             dma_addr_t addr, int type)
> > {
> >         /* Shift high bits down */
> >        addr |= (addr >> 28) & 0xff;
> 
> Isn't it 0xff0?

Or, it's rather meant (addr >> 32) & 0xff?
If so, better to be upper_32_bits(addr) & 0xff...


thanks,

Takashi

Re: [PATCH] agp/intel: Fix dma mask for Sandybridge

[Zhenyu Wang]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

On 2010.08.23 08:02:42 +0200, Takashi Iwai wrote:
> 
> Also, I don't understand the logic of 40bit addr calculation:
> 
> > static unsigned long intel_gen6_mask_memory(struct agp_bridge_data *bridge,
> >                                             dma_addr_t addr, int type)
> > {
> >         /* Shift high bits down */
> >        addr |= (addr >> 28) & 0xff;
> 
> Isn't it 0xff0?
> 

No. This depends on hw 32bit PTE format for sandybridge.

bit 31			 bit 11			 bit 4			bit 0
   |<-physical addr 31:12->|<-physical addr 39:32->|<-cache ctl 3:1->|valid|

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] agp/intel: Fix dma mask for Sandybridge

[Takashi Iwai]

At Mon, 23 Aug 2010 14:19:22 +0800,
Zhenyu Wang wrote:
> 
> On 2010.08.23 08:02:42 +0200, Takashi Iwai wrote:
> > 
> > Also, I don't understand the logic of 40bit addr calculation:
> > 
> > > static unsigned long intel_gen6_mask_memory(struct agp_bridge_data *bridge,
> > >                                             dma_addr_t addr, int type)
> > > {
> > >         /* Shift high bits down */
> > >        addr |= (addr >> 28) & 0xff;
> > 
> > Isn't it 0xff0?
> > 
> 
> No. This depends on hw 32bit PTE format for sandybridge.
> 
> bit 31			 bit 11			 bit 4			bit 0
>    |<-physical addr 31:12->|<-physical addr 39:32->|<-cache ctl 3:1->|valid|

Then I really don't understand why it works.
You shift 28bit and mask with 0xff.  Obviously it overwrite bits 0:3
with original 28:31 bits.  Masking 0xff0 fixes the issue.

And, the information like above would be greatly helpful if put into
either changelog or comment...


thanks,

Takashi

Re: [PATCH] agp/intel: Fix dma mask for Sandybridge

[Zhenyu Wang]

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

On 2010.08.23 08:31:42 +0200, Takashi Iwai wrote:
> > bit 31			 bit 11			 bit 4			bit 0
> >    |<-physical addr 31:12->|<-physical addr 39:32->|<-cache ctl 3:1->|valid|
> 
> Then I really don't understand why it works.
> You shift 28bit and mask with 0xff.  Obviously it overwrite bits 0:3
> with original 28:31 bits.  Masking 0xff0 fixes the issue.
> 

ah, sorry, my stupid. Thanks for catching this!

Subject: [PATCH] agp/intel: fix physical address mask bits for sandybridge

It should shift bit 39-32 into pte's bit 11-4.

Reported-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/char/agp/intel-gtt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index d22ffb8..0edfc87 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1333,8 +1333,8 @@ static unsigned long intel_i965_mask_memory(struct agp_bridge_data *bridge,
 static unsigned long intel_gen6_mask_memory(struct agp_bridge_data *bridge,
 					    dma_addr_t addr, int type)
 {
-	/* Shift high bits down */
-	addr |= (addr >> 28) & 0xff;
+	/* gen6 has bit11-4 for physical addr bit39-32 */
+	addr |= (addr >> 28) & 0xff0;
 
 	/* Type checking must be done elsewhere */
 	return addr | bridge->driver->masks[type].mask;
-- 
1.7.0.4

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] agp/intel: Fix dma mask for Sandybridge

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

On Mon, 23 Aug 2010 14:48:54 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> On 2010.08.23 08:31:42 +0200, Takashi Iwai wrote:
> > > bit 31			 bit 11			 bit 4			bit 0
> > >    |<-physical addr 31:12->|<-physical addr 39:32->|<-cache ctl 3:1->|valid|
> > 
> > Then I really don't understand why it works.
> > You shift 28bit and mask with 0xff.  Obviously it overwrite bits 0:3
> > with original 28:31 bits.  Masking 0xff0 fixes the issue.
> > 
> 
> ah, sorry, my stupid. Thanks for catching this!
> 
> Subject: [PATCH] agp/intel: fix physical address mask bits for sandybridge
> 
> It should shift bit 39-32 into pte's bit 11-4.

Applied.  Thanks!

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]