Re: [PATCH 2/2] nouveau: Acknowledge HPD irq in handler, not bottom half

[Ben Skeggs]

On Wed, 2010-11-10 at 17:51 -0500, Andrew Lutomirski wrote:
 On Wed, Nov 10, 2010 at 5:35 PM, Ben Skeggs bske...@redhat.com wrote:
  On Wed, 2010-11-10 at 17:25 -0500, Andrew Lutomirski wrote:
  On Wed, Nov 10, 2010 at 5:10 PM, Ben Skeggs bske...@redhat.com wrote:
   On Wed, 2010-11-10 at 16:32 -0500, Andy Lutomirski wrote:
   The old code generated an interrupt storm bad enough to completely
   take down my system.
  
   This only fixes the bits that are defined nouveau_regs.h.  Newer 
   hardware
   uses another register that isn't described, and I don't have that 
   hardware
   to test.
   Thanks for looking at this.  I'll take a closer look at the problem
   today and see what I can come up with too, that'll work with the newer
   hardware too.
 
  It should be as simple as adding an hpd1 field to the hpd_state and
  making exactly the same change.  (It would be nice to put the register
  definitions into nouveau_regs.h as well -- I didn't really want to
  muck around with a bunch of magic numbers that I can't test.)
  Yes, it is.  I can confirm the problem on another card, but it doesn't
  actually cause any crashes here.  If you can rework the patch to support
  the newer chips too, that'd be great.
 
  As for magic numbers, the register names for those regs are wrong
  anyway.  The joy of reverse-engineering the support.  It doesn't really
  matter if you want to stick to them or go back to magic numbers.
 
 That explains why INTR and CTRL seemed backwards :)  I'll leave the
 magic numbers for the 0xe07? stuff.
That sounds good, it'll all get a cleanup at some point and switched to
proper (well, our best guess, you'd have to ask NVIDIA about the real
ones) names.

Ben.
 
 Also, I accidentally dropped the  enabled_bits part -- I'll put that back.
 
 Patch to follow after I boot and test it here.
 
 --Andy


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/ttm: track kmap io_reserve(), only unreserve on unmap where needed

[Ben Skeggs]

On Thu, 2010-11-04 at 19:34 +0100, Thomas Hellstrom wrote:
> Ben,
> 
> I had something like the attached in mind, although it might be more 
> beneficial to do the actual refcounting in drivers that needs it. Atomic 
> incs and decs are expensive, but I'm not sure how expensive relative to 
> function pointer calls.
Thomas,

Thanks for that :)  It looks good to me, and appears to work as it
should.

Ben.
> 
> Patch is only compile-tested
> 
> /Thomas
> 
> 
> On 11/04/2010 02:08 PM, Thomas Hellstrom wrote:
> > On 11/04/2010 01:03 AM, Ben Skeggs wrote:
> >> From: Ben Skeggs
> >>
> >> If the driver kmaps an object userspace is expecting to be mapped, the
> >> unmap would have called down into the drivers io_unreserve() function
> >> and potentially unmapped the pages from its BARs (for example) and 
> >> they'd
> >> no longer be accessible for the userspace mapping.
> >>
> >> Signed-off-by: Ben Skeggs
> >> ---
> >>   drivers/gpu/drm/ttm/ttm_bo_util.c |   14 ++
> >>   include/drm/ttm/ttm_bo_api.h  |1 +
> >>   2 files changed, 11 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
> >> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> >> index ff358ad..e9dbe8b 100644
> >> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> >> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> >> @@ -467,9 +467,12 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
> >>   if (num_pages>  1&&  !DRM_SUSER(DRM_CURPROC))
> >>   return -EPERM;
> >>   #endif
> >> -ret = ttm_mem_io_reserve(bo->bdev,>mem);
> >> -if (ret)
> >> -return ret;
> >> +if (!bo->mem.bus.io_reserved) {
> >> +ret = ttm_mem_io_reserve(bo->bdev,>mem);
> >> +if (ret)
> >> +return ret;
> >> +map->io_reserved = true;
> >> +}
> >>   if (!bo->mem.bus.is_iomem) {
> >>   return ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
> >>   } else {
> >> @@ -487,7 +490,10 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
> >>   switch (map->bo_kmap_type) {
> >>   case ttm_bo_map_iomap:
> >>   iounmap(map->virtual);
> >> -ttm_mem_io_free(map->bo->bdev,>bo->mem);
> >> +if (map->io_reserved) {
> >> +ttm_mem_io_free(map->bo->bdev,>bo->mem);
> >> +map->io_reserved = false;
> >> +}
> >>   break;
> >>   case ttm_bo_map_vmap:
> >>   vunmap(map->virtual);
> >> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> >> index 5afa5b5..ce998ac 100644
> >> --- a/include/drm/ttm/ttm_bo_api.h
> >> +++ b/include/drm/ttm/ttm_bo_api.h
> >> @@ -300,6 +300,7 @@ struct ttm_bo_kmap_obj {
> >>   ttm_bo_map_premapped= 4 | TTM_BO_MAP_IOMEM_MASK,
> >>   } bo_kmap_type;
> >>   struct ttm_buffer_object *bo;
> >> +bool io_reserved;
> >>   };
> >>
> >>   /**
> >>
> >
> > This doesn't solve the problem unfortunately. Consider the sequence
> >
> > kmap->io_mem_reserve
> > fault()->
> > kunmap->io_mem_free
> > user_space_access()-> Invalid.
> >
> > I think this needs to be fixed by us maintaining an 
> > mem:io_reserved_count, where all user-space triggered io_reserves 
> > count as 1. A mem::user_space_io_reserved flag could be protected by 
> > the bo::reserve lock, whereas a reserved_count can't, since strictly 
> > you're allowed to kmap a bo without reserving it, but only if it's pinned
> >
> > /Thomas
> > .
> >
> >
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCH 2/2] drm/ttm: track kmap io_reserve(), only unreserve on unmap where needed

[Ben Skeggs]

On Thu, 2010-11-04 at 19:34 +0100, Thomas Hellstrom wrote:
 Ben,
 
 I had something like the attached in mind, although it might be more 
 beneficial to do the actual refcounting in drivers that needs it. Atomic 
 incs and decs are expensive, but I'm not sure how expensive relative to 
 function pointer calls.
Thomas,

Thanks for that :)  It looks good to me, and appears to work as it
should.

Ben.
 
 Patch is only compile-tested
 
 /Thomas
 
 
 On 11/04/2010 02:08 PM, Thomas Hellstrom wrote:
  On 11/04/2010 01:03 AM, Ben Skeggs wrote:
  From: Ben Skeggsbske...@redhat.com
 
  If the driver kmaps an object userspace is expecting to be mapped, the
  unmap would have called down into the drivers io_unreserve() function
  and potentially unmapped the pages from its BARs (for example) and 
  they'd
  no longer be accessible for the userspace mapping.
 
  Signed-off-by: Ben Skeggsbske...@redhat.com
  ---
drivers/gpu/drm/ttm/ttm_bo_util.c |   14 ++
include/drm/ttm/ttm_bo_api.h  |1 +
2 files changed, 11 insertions(+), 4 deletions(-)
 
  diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
  b/drivers/gpu/drm/ttm/ttm_bo_util.c
  index ff358ad..e9dbe8b 100644
  --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
  +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
  @@ -467,9 +467,12 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
if (num_pages  1  !DRM_SUSER(DRM_CURPROC))
return -EPERM;
#endif
  -ret = ttm_mem_io_reserve(bo-bdev,bo-mem);
  -if (ret)
  -return ret;
  +if (!bo-mem.bus.io_reserved) {
  +ret = ttm_mem_io_reserve(bo-bdev,bo-mem);
  +if (ret)
  +return ret;
  +map-io_reserved = true;
  +}
if (!bo-mem.bus.is_iomem) {
return ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
} else {
  @@ -487,7 +490,10 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
switch (map-bo_kmap_type) {
case ttm_bo_map_iomap:
iounmap(map-virtual);
  -ttm_mem_io_free(map-bo-bdev,map-bo-mem);
  +if (map-io_reserved) {
  +ttm_mem_io_free(map-bo-bdev,map-bo-mem);
  +map-io_reserved = false;
  +}
break;
case ttm_bo_map_vmap:
vunmap(map-virtual);
  diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
  index 5afa5b5..ce998ac 100644
  --- a/include/drm/ttm/ttm_bo_api.h
  +++ b/include/drm/ttm/ttm_bo_api.h
  @@ -300,6 +300,7 @@ struct ttm_bo_kmap_obj {
ttm_bo_map_premapped= 4 | TTM_BO_MAP_IOMEM_MASK,
} bo_kmap_type;
struct ttm_buffer_object *bo;
  +bool io_reserved;
};
 
/**
 
 
  This doesn't solve the problem unfortunately. Consider the sequence
 
  kmap-io_mem_reserve
  fault()-
  kunmap-io_mem_free
  user_space_access()- Invalid.
 
  I think this needs to be fixed by us maintaining an 
  mem:io_reserved_count, where all user-space triggered io_reserves 
  count as 1. A mem::user_space_io_reserved flag could be protected by 
  the bo::reserve lock, whereas a reserved_count can't, since strictly 
  you're allowed to kmap a bo without reserving it, but only if it's pinned
 
  /Thomas
  .
 
 
  ___
  dri-devel mailing list
  dri-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/dri-devel
 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/ttm: track kmap io_reserve(), only unreserve on unmap where needed

[Ben Skeggs]

From: Ben Skeggs <bske...@redhat.com>

If the driver kmaps an object userspace is expecting to be mapped, the
unmap would have called down into the drivers io_unreserve() function
and potentially unmapped the pages from its BARs (for example) and they'd
no longer be accessible for the userspace mapping.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c |   14 ++
 include/drm/ttm/ttm_bo_api.h  |1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index ff358ad..e9dbe8b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -467,9 +467,12 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
if (num_pages > 1 && !DRM_SUSER(DRM_CURPROC))
return -EPERM;
 #endif
-   ret = ttm_mem_io_reserve(bo->bdev, >mem);
-   if (ret)
-   return ret;
+   if (!bo->mem.bus.io_reserved) {
+   ret = ttm_mem_io_reserve(bo->bdev, >mem);
+   if (ret)
+   return ret;
+   map->io_reserved = true;
+   }
if (!bo->mem.bus.is_iomem) {
return ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
} else {
@@ -487,7 +490,10 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
switch (map->bo_kmap_type) {
case ttm_bo_map_iomap:
iounmap(map->virtual);
-   ttm_mem_io_free(map->bo->bdev, >bo->mem);
+   if (map->io_reserved) {
+   ttm_mem_io_free(map->bo->bdev, >bo->mem);
+   map->io_reserved = false;
+   }
break;
case ttm_bo_map_vmap:
vunmap(map->virtual);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 5afa5b5..ce998ac 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -300,6 +300,7 @@ struct ttm_bo_kmap_obj {
ttm_bo_map_premapped= 4 | TTM_BO_MAP_IOMEM_MASK,
} bo_kmap_type;
struct ttm_buffer_object *bo;
+   bool io_reserved;
 };

 /**
-- 
1.7.3.1

[PATCH 1/2] drm/ttm: ensure ttm_mem_io_free is called on bo destruction

[Ben Skeggs]

From: Ben Skeggs <bske...@redhat.com>

Nouveau will start to use ttm_mem_io_reserve to allocate BAR VM space
for VRAM mappings, and without this call GPU address space gets leaked.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/ttm/ttm_bo.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index dfa163b..c373cf9 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -472,6 +472,7 @@ static void ttm_bo_cleanup_memtype_use(struct 
ttm_buffer_object *bo)
wake_up_all(>event_queue);
spin_unlock(>lru_lock);

+   ttm_mem_io_free(bo->bdev, _mem);
ttm_bo_mem_put(bo, _mem);
 }

-- 
1.7.3.1

[PATCH 1/2] drm/ttm: ensure ttm_mem_io_free is called on bo destruction

[Ben Skeggs]

From: Ben Skeggs bske...@redhat.com

Nouveau will start to use ttm_mem_io_reserve to allocate BAR VM space
for VRAM mappings, and without this call GPU address space gets leaked.

Signed-off-by: Ben Skeggs bske...@redhat.com
---
 drivers/gpu/drm/ttm/ttm_bo.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index dfa163b..c373cf9 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -472,6 +472,7 @@ static void ttm_bo_cleanup_memtype_use(struct 
ttm_buffer_object *bo)
wake_up_all(bo-event_queue);
spin_unlock(glob-lru_lock);
 
+   ttm_mem_io_free(bo-bdev, tmp_mem);
ttm_bo_mem_put(bo, tmp_mem);
 }
 
-- 
1.7.3.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/ttm: track kmap io_reserve(), only unreserve on unmap where needed

[Ben Skeggs]

From: Ben Skeggs bske...@redhat.com

If the driver kmaps an object userspace is expecting to be mapped, the
unmap would have called down into the drivers io_unreserve() function
and potentially unmapped the pages from its BARs (for example) and they'd
no longer be accessible for the userspace mapping.

Signed-off-by: Ben Skeggs bske...@redhat.com
---
 drivers/gpu/drm/ttm/ttm_bo_util.c |   14 ++
 include/drm/ttm/ttm_bo_api.h  |1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index ff358ad..e9dbe8b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -467,9 +467,12 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
if (num_pages  1  !DRM_SUSER(DRM_CURPROC))
return -EPERM;
 #endif
-   ret = ttm_mem_io_reserve(bo-bdev, bo-mem);
-   if (ret)
-   return ret;
+   if (!bo-mem.bus.io_reserved) {
+   ret = ttm_mem_io_reserve(bo-bdev, bo-mem);
+   if (ret)
+   return ret;
+   map-io_reserved = true;
+   }
if (!bo-mem.bus.is_iomem) {
return ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
} else {
@@ -487,7 +490,10 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
switch (map-bo_kmap_type) {
case ttm_bo_map_iomap:
iounmap(map-virtual);
-   ttm_mem_io_free(map-bo-bdev, map-bo-mem);
+   if (map-io_reserved) {
+   ttm_mem_io_free(map-bo-bdev, map-bo-mem);
+   map-io_reserved = false;
+   }
break;
case ttm_bo_map_vmap:
vunmap(map-virtual);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 5afa5b5..ce998ac 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -300,6 +300,7 @@ struct ttm_bo_kmap_obj {
ttm_bo_map_premapped= 4 | TTM_BO_MAP_IOMEM_MASK,
} bo_kmap_type;
struct ttm_buffer_object *bo;
+   bool io_reserved;
 };
 
 /**
-- 
1.7.3.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Intel-gfx] [PATCH 1/4] drm/edid: add helper function to detect monitor audio capability

[Ben Skeggs]

On Sun, 2010-09-19 at 15:56 +0800, Zhenyu Wang wrote:
> On 2010.09.19 08:38:07 +0100, Chris Wilson wrote:
> > 
> > This should be cc'ed for Adam Jackson's attention as well.
> 
> yep, I think I cc-ed ajax in git-send-mail command line...I'll amend the 
> commit.
> 
> > > +bool drm_detect_monitor_audio(struct edid *edid)
> > 
> > drm_edid_has_monitor_audio()? That is a little more self-descriptive.
> > (I also think drm_detect_hdmi_monitor is poorly named.)
> 
> yeah, that looks better. thanks.
> 
> > The last time Adam had a chance to comment on this EDID check, he
> > suggested that we cannot rely on has_audio being a reliable indicator of
> > the sink's properties. If the EDID has no audio support, then return
> > early, otherwise perform the secondary check that extension block contains
> > audio data.
> 
> That's also the problem we stuck for quite some time, and until recently
> we got reply on this issue. Adam is right, 'basic audio' bit might not be
> reliable, so we try to search audio block too, if there's any existence,
> we will accept 'basic audio' support as well.
Also, would you be able to export drm_find_cea_extension(), I have
pending nouveau patches that will require this symbol (to build a HDA
ELD from the EDID data for HDMI/DP audio).

With that, you can add my Reviewed-by :)

Thanks,
Ben.
> 
> Thanks for the review!
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/4] drm/edid: add helper function to detect monitor audio capability

[Ben Skeggs]

On Sun, 2010-09-19 at 15:56 +0800, Zhenyu Wang wrote:
 On 2010.09.19 08:38:07 +0100, Chris Wilson wrote:
  
  This should be cc'ed for Adam Jackson's attention as well.
 
 yep, I think I cc-ed ajax in git-send-mail command line...I'll amend the 
 commit.
 
   +bool drm_detect_monitor_audio(struct edid *edid)
  
  drm_edid_has_monitor_audio()? That is a little more self-descriptive.
  (I also think drm_detect_hdmi_monitor is poorly named.)
 
 yeah, that looks better. thanks.
 
  The last time Adam had a chance to comment on this EDID check, he
  suggested that we cannot rely on has_audio being a reliable indicator of
  the sink's properties. If the EDID has no audio support, then return
  early, otherwise perform the secondary check that extension block contains
  audio data.
 
 That's also the problem we stuck for quite some time, and until recently
 we got reply on this issue. Adam is right, 'basic audio' bit might not be
 reliable, so we try to search audio block too, if there's any existence,
 we will accept 'basic audio' support as well.
Also, would you be able to export drm_find_cea_extension(), I have
pending nouveau patches that will require this symbol (to build a HDA
ELD from the EDID data for HDMI/DP audio).

With that, you can add my Reviewed-by :)

Thanks,
Ben.
 
 Thanks for the review!
 
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

GEM - radeon cs ioctl deadlock

[Ben Skeggs]

On Thu, 2010-10-14 at 08:14 +1000, Dave Airlie wrote:
> On Wed, 2010-10-13 at 17:57 -0400, Jerome Glisse wrote:
> > So we are facing a deadlock with the radeon cs ioctl. When a buffer is given
> > a name (with flink) we could endup with 2 handle pointing to the same
> > object (flink object and open it from same file descriptor). Would it be ok
> > if i change gem open to first look if we already have an handle for the
> > object and to use that handle instead of creating a new one ? Or could
> > this break intel driver ?
> 
> I think r300g worked around this already, maybe we should just avoid
> doing it from userspace if possible.
We had this in nouveau at some point.  In the end, I prevented the
deadlock from occuring by checking that a process doesn't reserve the
same buffer twice (we store file_priv in a reserved_by field in the bo
as we reserve the buffers), and then just fixed userspace.

Ben.
> 
> Dave.
> 
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: GEM - radeon cs ioctl deadlock

[Ben Skeggs]

On Thu, 2010-10-14 at 08:14 +1000, Dave Airlie wrote:
 On Wed, 2010-10-13 at 17:57 -0400, Jerome Glisse wrote:
  So we are facing a deadlock with the radeon cs ioctl. When a buffer is given
  a name (with flink) we could endup with 2 handle pointing to the same
  object (flink object and open it from same file descriptor). Would it be ok
  if i change gem open to first look if we already have an handle for the
  object and to use that handle instead of creating a new one ? Or could
  this break intel driver ?
 
 I think r300g worked around this already, maybe we should just avoid
 doing it from userspace if possible.
We had this in nouveau at some point.  In the end, I prevented the
deadlock from occuring by checking that a process doesn't reserve the
same buffer twice (we store file_priv in a reserved_by field in the bo
as we reserve the buffers), and then just fixed userspace.

Ben.
 
 Dave.
 
 
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/1] nouveau: ratelimit IRQ messages

[Ben Skeggs]

On Mon, 2010-10-04 at 15:11 +0200, Jiri Slaby wrote:
> There are two messages in the ISR of nouveau which might be printed out
> hundred times in a second. Ratelimit them. (We need to move
> nouveau_ratelimit to the top of the file.)
Looks good to me.  If you could rebase this on nouveau git
(git.freedesktop.org/git/nouveau/linux-2.6), I'll push it.

Thanks,
Ben.
> 
> Signed-off-by: Jiri Slaby 
> Cc: Ben Skeggs 
> Cc: Marcin Slusarz 
> ---
>  drivers/gpu/drm/nouveau/nouveau_irq.c |   23 +--
>  1 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_irq.c 
> b/drivers/gpu/drm/nouveau/nouveau_irq.c
> index 794b0ee..5b097b4 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_irq.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_irq.c
> @@ -41,6 +41,13 @@
>  #include "nouveau_connector.h"
>  #include "nv50_display.h"
>  
> +static DEFINE_RATELIMIT_STATE(nouveau_ratelimit_state, 3 * HZ, 20);
> +
> +static int nouveau_ratelimit(void)
> +{
> + return __ratelimit(_ratelimit_state);
> +}
> +
>  void
>  nouveau_irq_preinstall(struct drm_device *dev)
>  {
> @@ -200,7 +207,9 @@ nouveau_fifo_irq_handler(struct drm_device *dev)
>   }
>  
>   if (status & NV_PFIFO_INTR_DMA_PUSHER) {
> - NV_INFO(dev, "PFIFO_DMA_PUSHER - Ch %d\n", chid);
> + if (nouveau_ratelimit())
> + NV_INFO(dev, "PFIFO_DMA_PUSHER - Ch %d\n",
> + chid);
>  
>   status &= ~NV_PFIFO_INTR_DMA_PUSHER;
>   nv_wr32(dev, NV03_PFIFO_INTR_0,
> @@ -227,8 +236,9 @@ nouveau_fifo_irq_handler(struct drm_device *dev)
>   }
>  
>   if (status) {
> - NV_INFO(dev, "PFIFO_INTR 0x%08x - Ch %d\n",
> - status, chid);
> + if (nouveau_ratelimit())
> + NV_INFO(dev, "PFIFO_INTR 0x%08x - Ch %d\n",
> + status, chid);
>   nv_wr32(dev, NV03_PFIFO_INTR_0, status);
>   status = 0;
>   }
> @@ -505,13 +515,6 @@ nouveau_pgraph_intr_notify(struct drm_device *dev, 
> uint32_t nsource)
>   nouveau_graph_dump_trap_info(dev, "PGRAPH_NOTIFY", );
>  }
>  
> -static DEFINE_RATELIMIT_STATE(nouveau_ratelimit_state, 3 * HZ, 20);
> -
> -static int nouveau_ratelimit(void)
> -{
> - return __ratelimit(_ratelimit_state);
> -}
> -
>  
>  static inline void
>  nouveau_pgraph_intr_error(struct drm_device *dev, uint32_t nsource)

Re: [PATCH v2 1/1] nouveau: ratelimit IRQ messages

[Ben Skeggs]

On Mon, 2010-10-04 at 15:11 +0200, Jiri Slaby wrote:
 There are two messages in the ISR of nouveau which might be printed out
 hundred times in a second. Ratelimit them. (We need to move
 nouveau_ratelimit to the top of the file.)
Looks good to me.  If you could rebase this on nouveau git
(git.freedesktop.org/git/nouveau/linux-2.6), I'll push it.

Thanks,
Ben.
 
 Signed-off-by: Jiri Slaby jsl...@suse.cz
 Cc: Ben Skeggs bske...@redhat.com
 Cc: Marcin Slusarz marcin.slus...@gmail.com
 ---
  drivers/gpu/drm/nouveau/nouveau_irq.c |   23 +--
  1 files changed, 13 insertions(+), 10 deletions(-)
 
 diff --git a/drivers/gpu/drm/nouveau/nouveau_irq.c 
 b/drivers/gpu/drm/nouveau/nouveau_irq.c
 index 794b0ee..5b097b4 100644
 --- a/drivers/gpu/drm/nouveau/nouveau_irq.c
 +++ b/drivers/gpu/drm/nouveau/nouveau_irq.c
 @@ -41,6 +41,13 @@
  #include nouveau_connector.h
  #include nv50_display.h
  
 +static DEFINE_RATELIMIT_STATE(nouveau_ratelimit_state, 3 * HZ, 20);
 +
 +static int nouveau_ratelimit(void)
 +{
 + return __ratelimit(nouveau_ratelimit_state);
 +}
 +
  void
  nouveau_irq_preinstall(struct drm_device *dev)
  {
 @@ -200,7 +207,9 @@ nouveau_fifo_irq_handler(struct drm_device *dev)
   }
  
   if (status  NV_PFIFO_INTR_DMA_PUSHER) {
 - NV_INFO(dev, PFIFO_DMA_PUSHER - Ch %d\n, chid);
 + if (nouveau_ratelimit())
 + NV_INFO(dev, PFIFO_DMA_PUSHER - Ch %d\n,
 + chid);
  
   status = ~NV_PFIFO_INTR_DMA_PUSHER;
   nv_wr32(dev, NV03_PFIFO_INTR_0,
 @@ -227,8 +236,9 @@ nouveau_fifo_irq_handler(struct drm_device *dev)
   }
  
   if (status) {
 - NV_INFO(dev, PFIFO_INTR 0x%08x - Ch %d\n,
 - status, chid);
 + if (nouveau_ratelimit())
 + NV_INFO(dev, PFIFO_INTR 0x%08x - Ch %d\n,
 + status, chid);
   nv_wr32(dev, NV03_PFIFO_INTR_0, status);
   status = 0;
   }
 @@ -505,13 +515,6 @@ nouveau_pgraph_intr_notify(struct drm_device *dev, 
 uint32_t nsource)
   nouveau_graph_dump_trap_info(dev, PGRAPH_NOTIFY, trap);
  }
  
 -static DEFINE_RATELIMIT_STATE(nouveau_ratelimit_state, 3 * HZ, 20);
 -
 -static int nouveau_ratelimit(void)
 -{
 - return __ratelimit(nouveau_ratelimit_state);
 -}
 -
  
  static inline void
  nouveau_pgraph_intr_error(struct drm_device *dev, uint32_t nsource)


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/nouveau: fix panels using straps-based mode detection

[Ben Skeggs]

From: Ben Skeggs <bske...@redhat.com>

nouveau_bios_fp_mode() zeroes the mode struct before filling in relevant
entries.  This nukes the mode id initialised by drm_mode_create(), and
causes warnings from idr when we try to remove the mode.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nouveau_connector.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 98c214c..4b286a8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -594,8 +594,10 @@ nouveau_connector_get_modes(struct drm_connector 
*connector)
if (nv_encoder->dcb->type == OUTPUT_LVDS &&
(nv_encoder->dcb->lvdsconf.use_straps_for_mode ||
 dev_priv->vbios.fp_no_ddc) && nouveau_bios_fp_mode(dev, NULL)) {
-   nv_connector->native_mode = drm_mode_create(dev);
-   nouveau_bios_fp_mode(dev, nv_connector->native_mode);
+   struct drm_display_mode mode;
+
+   nouveau_bios_fp_mode(dev, );
+   nv_connector->native_mode = drm_mode_duplicate(dev, );
}

/* Find the native mode if this is a digital panel, if we didn't
-- 
1.7.2.2

idr_remove called for id=0 which is not allocated

[Ben Skeggs]

On Wed, 2010-09-22 at 22:56 -0700, Andrew Morton wrote:
> On Wed, 22 Sep 2010 23:10:10 +0200 Alessandro Guido  alessandroguido.name> wrote:
> 
> > I have this traces in my logs (full dmesg attached).
> > 
> > idr_remove called for id=0 which is not allocated.
> > Pid: 1136, comm: Xorg Not tainted 2.6.36-rc5-49-gc79bd89 #1
> > Call Trace:
> >   [] ? printk+0x18/0x1a
> >   [] idr_remove+0x73/0x1c0
> >   [] drm_mode_object_put+0x2f/0x50
> >   [] drm_mode_destroy+0xe/0x20
> >   [] nouveau_connector_get_modes+0x2b/0x390
> >   [] ? acpi_lid_open+0x22/0x3c
> >   [] ? queue_delayed_work+0x1b/0x30
> >   [] drm_helper_probe_single_connector_modes+0xc4/0x360
> >   [] drm_mode_getconnector+0x2a7/0x350
> >   [] drm_ioctl+0x1c2/0x4b0
> >   [] ? filemap_fault+0x81/0x3c0
> >   [] ? drm_mode_getconnector+0x0/0x350
> >   [] ? handle_mm_fault+0x13f/0x670
> >   [] ? drm_ioctl+0x0/0x4b0
> >   [] do_vfs_ioctl+0x7d/0x5f0
> >   [] ? do_page_fault+0x17c/0x3c0
> >   [] ? vfs_write+0xfd/0x140
> >   [] ? do_sync_write+0x0/0xe0
> >   [] sys_ioctl+0x39/0x60
> >   [] sysenter_do_call+0x12/0x26
> 
> I assume this is a regression.  2.6.35 didn't do this?
Yes, I suspect 6e86e0419471d11ed3d4d46039ee90e8cb85806c is the culprit.

I'll send a patch to Dave, but I can't test it personally, I don't have
any hardware the manufacturer has configured in the way it'd need to be
to trigger this.

Ben.
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: idr_remove called for id=0 which is not allocated

[Ben Skeggs]

On Wed, 2010-09-22 at 22:56 -0700, Andrew Morton wrote:
 On Wed, 22 Sep 2010 23:10:10 +0200 Alessandro Guido 
 a...@alessandroguido.name wrote:
 
  I have this traces in my logs (full dmesg attached).
  
  idr_remove called for id=0 which is not allocated.
  Pid: 1136, comm: Xorg Not tainted 2.6.36-rc5-49-gc79bd89 #1
  Call Trace:
[c1379e16] ? printk+0x18/0x1a
[c113b8f3] idr_remove+0x73/0x1c0
[c11b8d6f] drm_mode_object_put+0x2f/0x50
[c11b8f6e] drm_mode_destroy+0xe/0x20
[c11eb24b] nouveau_connector_get_modes+0x2b/0x390
[c1185b6f] ? acpi_lid_open+0x22/0x3c
[c103800b] ? queue_delayed_work+0x1b/0x30
[c11abf34] drm_helper_probe_single_connector_modes+0xc4/0x360
[c11bb6a7] drm_mode_getconnector+0x2a7/0x350
[c11b00d2] drm_ioctl+0x1c2/0x4b0
[c1068071] ? filemap_fault+0x81/0x3c0
[c11bb400] ? drm_mode_getconnector+0x0/0x350
[c107bedf] ? handle_mm_fault+0x13f/0x670
[c11aff10] ? drm_ioctl+0x0/0x4b0
[c109c90d] do_vfs_ioctl+0x7d/0x5f0
[c101a14c] ? do_page_fault+0x17c/0x3c0
[c108fc6d] ? vfs_write+0xfd/0x140
[c108f1c0] ? do_sync_write+0x0/0xe0
[c109ceb9] sys_ioctl+0x39/0x60
[c1002b90] sysenter_do_call+0x12/0x26
 
 I assume this is a regression.  2.6.35 didn't do this?
Yes, I suspect 6e86e0419471d11ed3d4d46039ee90e8cb85806c is the culprit.

I'll send a patch to Dave, but I can't test it personally, I don't have
any hardware the manufacturer has configured in the way it'd need to be
to trigger this.

Ben.
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

nouveau/ttm: BUG in ttm_bo_release_list

[Ben Skeggs]

On Fri, 2010-09-17 at 19:43 +0200, Marcin Slusarz wrote:
> Hi
> Since upgrade from 2.6.35 to 2.6.36-rc3 (nouveau tree) I'm hitting this bug a 
> couple of times a day:
> 
> [ 2869.618504] [ cut here ]
> [ 2869.618532] kernel BUG at drivers/gpu/drm/ttm/ttm_bo.c:153!
> [ 2869.618560] invalid opcode:  [#1] PREEMPT SMP 
> [ 2869.618600] last sysfs file: 
> /sys/devices/pci:00/:00:03.0/:02:00.0/pm_status
> [ 2869.618637] CPU 0 
> [ 2869.618649] Modules linked in: nouveau ttm drm_kms_helper 
> snd_hda_codec_realtek snd_hda_intel snd_hda_codec
> [ 2869.618730] 
> [ 2869.618742] Pid: 11, comm: kworker/0:1 Not tainted 2.6.36-rc3-nv+ #485 P6T 
> SE/System Product Name
> [ 2869.618781] RIP: 0010:[]  [] 
> ttm_bo_release_list+0x37/0xcf [ttm]
> [ 2869.618830] RSP: 0018:8801bfd85d40  EFLAGS: 00010202
> [ 2869.618855] RAX: 0001 RBX: 8801b1e50c00 RCX: 
> 8801b7500330
> [ 2869.618887] RDX: 0001 RSI: 0037 RDI: 
> 8801b1e50c44
> [ 2869.618918] RBP: 8801bfd85d60 R08: 0002 R09: 
> 8801bf683000
> [ 2869.618950] R10: 8801b1e50c70 R11:  R12: 
> 8801b1e50c44
> [ 2869.618981] R13: 8801b7500188 R14:  R15: 
> 8801b7500738
> [ 2869.619013] FS:  () GS:880001e0() 
> knlGS:
> [ 2869.619048] CS:  0010 DS:  ES:  CR0: 8005003b
> [ 2869.619074] CR2: 7f2cc0f6b000 CR3: 0001bfe8a000 CR4: 
> 06f0
> [ 2869.619106] DR0:  DR1:  DR2: 
> 
> [ 2869.619137] DR3:  DR6: 0ff0 DR7: 
> 0400
> [ 2869.619168] Process kworker/0:1 (pid: 11, threadinfo 8801bfd84000, 
> task 8801bfd88000)
> [ 2869.619205] Stack:
> [ 2869.619217]   8801b1e50c44 a0082c67 
> 8801bdcb8ab0
> [ 2869.619262] <0> 8801bfd85d80 8121c83a 8801b1e5 
> 8801b1e50c00
> [ 2869.619318] <0> 8801bfd85dd0 a00839f4 880001e0ebc0 
> 0001
> [ 2869.619374] Call Trace:
> [ 2869.619392]  [] ? ttm_bo_release_list+0x0/0xcf [ttm]
> [ 2869.619425]  [] kref_put+0x43/0x4d
> [ 2869.619451]  [] ttm_bo_delayed_delete+0xa2/0xf9 [ttm]
> [ 2869.619484]  [] ? ttm_bo_delayed_workqueue+0x0/0x30 [ttm]
> [ 2869.619517]  [] ttm_bo_delayed_workqueue+0x1a/0x30 [ttm]
> [ 2869.619551]  [] process_one_work+0x29f/0x448
> [ 2869.619580]  [] worker_thread+0x1d6/0x349
> [ 2869.619607]  [] ? worker_thread+0x0/0x349
> [ 2869.619635]  [] kthread+0x7d/0x85
> [ 2869.619661]  [] kernel_thread_helper+0x4/0x10
> [ 2869.619692]  [] ? finish_task_switch+0x49/0xb2
> [ 2869.619722]  [] ? _raw_spin_unlock_irq+0x19/0x34
> [ 2869.619752]  [] ? restore_args+0x0/0x30
> [ 2869.619779]  [] ? kthread+0x0/0x85
> [ 2869.619805]  [] ? kernel_thread_helper+0x0/0x10
> [ 2869.619832] Code: 48 8d 5f bc 48 83 ec 08 8b 07 4c 8b 6f c4 85 c0 74 04 0f 
> 0b eb fe 8b 47 fc 85 c0 74 04 0f 0b eb fe 8b 87 a8 00 00 00 85 c0 74 04 <0f> 
> 0b eb fe 48 83 bb 38 01 00 00 00 74 04 0f 0b eb fe 48 83 bb 
> [ 2869.620255] RIP  [] ttm_bo_release_list+0x37/0xcf [ttm]
> [ 2869.620295]  RSP 
> [ 2869.629610] ---[ end trace 302a6257f0da8cdc ]---
> 
> this is BUG_ON(atomic_read(>cpu_writers));
> 
> I'm on b642f07208988270ac402d2548d42dab1d5fec92 "drm/nv50: fix 100c90 write 
> on nva3".
> If you have any patches / ideas how to debug this, let me know.
I've actually seen this a couple of times.  It appears I was incorrectly
blaming some patches I have in progress for it, the problem appeared to
go away when I reverted them.  Perhaps it's more random.  I have no
current ideas however.

Ben.
> 
> Marcin
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: nouveau/ttm: BUG in ttm_bo_release_list

[Ben Skeggs]

On Fri, 2010-09-17 at 19:43 +0200, Marcin Slusarz wrote:
 Hi
 Since upgrade from 2.6.35 to 2.6.36-rc3 (nouveau tree) I'm hitting this bug a 
 couple of times a day:
 
 [ 2869.618504] [ cut here ]
 [ 2869.618532] kernel BUG at drivers/gpu/drm/ttm/ttm_bo.c:153!
 [ 2869.618560] invalid opcode:  [#1] PREEMPT SMP 
 [ 2869.618600] last sysfs file: 
 /sys/devices/pci:00/:00:03.0/:02:00.0/pm_status
 [ 2869.618637] CPU 0 
 [ 2869.618649] Modules linked in: nouveau ttm drm_kms_helper 
 snd_hda_codec_realtek snd_hda_intel snd_hda_codec
 [ 2869.618730] 
 [ 2869.618742] Pid: 11, comm: kworker/0:1 Not tainted 2.6.36-rc3-nv+ #485 P6T 
 SE/System Product Name
 [ 2869.618781] RIP: 0010:[a0082c9e]  [a0082c9e] 
 ttm_bo_release_list+0x37/0xcf [ttm]
 [ 2869.618830] RSP: 0018:8801bfd85d40  EFLAGS: 00010202
 [ 2869.618855] RAX: 0001 RBX: 8801b1e50c00 RCX: 
 8801b7500330
 [ 2869.618887] RDX: 0001 RSI: 0037 RDI: 
 8801b1e50c44
 [ 2869.618918] RBP: 8801bfd85d60 R08: 0002 R09: 
 8801bf683000
 [ 2869.618950] R10: 8801b1e50c70 R11:  R12: 
 8801b1e50c44
 [ 2869.618981] R13: 8801b7500188 R14:  R15: 
 8801b7500738
 [ 2869.619013] FS:  () GS:880001e0() 
 knlGS:
 [ 2869.619048] CS:  0010 DS:  ES:  CR0: 8005003b
 [ 2869.619074] CR2: 7f2cc0f6b000 CR3: 0001bfe8a000 CR4: 
 06f0
 [ 2869.619106] DR0:  DR1:  DR2: 
 
 [ 2869.619137] DR3:  DR6: 0ff0 DR7: 
 0400
 [ 2869.619168] Process kworker/0:1 (pid: 11, threadinfo 8801bfd84000, 
 task 8801bfd88000)
 [ 2869.619205] Stack:
 [ 2869.619217]   8801b1e50c44 a0082c67 
 8801bdcb8ab0
 [ 2869.619262] 0 8801bfd85d80 8121c83a 8801b1e5 
 8801b1e50c00
 [ 2869.619318] 0 8801bfd85dd0 a00839f4 880001e0ebc0 
 0001
 [ 2869.619374] Call Trace:
 [ 2869.619392]  [a0082c67] ? ttm_bo_release_list+0x0/0xcf [ttm]
 [ 2869.619425]  [8121c83a] kref_put+0x43/0x4d
 [ 2869.619451]  [a00839f4] ttm_bo_delayed_delete+0xa2/0xf9 [ttm]
 [ 2869.619484]  [a0083a4b] ? ttm_bo_delayed_workqueue+0x0/0x30 [ttm]
 [ 2869.619517]  [a0083a65] ttm_bo_delayed_workqueue+0x1a/0x30 [ttm]
 [ 2869.619551]  [8107eb83] process_one_work+0x29f/0x448
 [ 2869.619580]  [8107f0e5] worker_thread+0x1d6/0x349
 [ 2869.619607]  [8107ef0f] ? worker_thread+0x0/0x349
 [ 2869.619635]  [81082198] kthread+0x7d/0x85
 [ 2869.619661]  [8102edd4] kernel_thread_helper+0x4/0x10
 [ 2869.619692]  [8105c10b] ? finish_task_switch+0x49/0xb2
 [ 2869.619722]  [814333d1] ? _raw_spin_unlock_irq+0x19/0x34
 [ 2869.619752]  [814339ad] ? restore_args+0x0/0x30
 [ 2869.619779]  [8108211b] ? kthread+0x0/0x85
 [ 2869.619805]  [8102edd0] ? kernel_thread_helper+0x0/0x10
 [ 2869.619832] Code: 48 8d 5f bc 48 83 ec 08 8b 07 4c 8b 6f c4 85 c0 74 04 0f 
 0b eb fe 8b 47 fc 85 c0 74 04 0f 0b eb fe 8b 87 a8 00 00 00 85 c0 74 04 0f 
 0b eb fe 48 83 bb 38 01 00 00 00 74 04 0f 0b eb fe 48 83 bb 
 [ 2869.620255] RIP  [a0082c9e] ttm_bo_release_list+0x37/0xcf [ttm]
 [ 2869.620295]  RSP 8801bfd85d40
 [ 2869.629610] ---[ end trace 302a6257f0da8cdc ]---
 
 this is BUG_ON(atomic_read(bo-cpu_writers));
 
 I'm on b642f07208988270ac402d2548d42dab1d5fec92 drm/nv50: fix 100c90 write 
 on nva3.
 If you have any patches / ideas how to debug this, let me know.
I've actually seen this a couple of times.  It appears I was incorrectly
blaming some patches I have in progress for it, the problem appeared to
go away when I reverted them.  Perhaps it's more random.  I have no
current ideas however.

Ben.
 
 Marcin
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

PROBLEM: oops in nouveau driver in 2.6.36-rc*

[Ben Skeggs]

On Mon, 2010-08-23 at 17:35 +0200, Stephane Casset wrote:
> Hi,
Hi,

> 
> Since 2.6.36-rc1 I have an oops when loading the nouveau driver.
> The system woks ok expect for the nouveau driver and the console gets
> corrupted :(
> 
> I attach relevant informations in attached files :
>  o dmesg output for 2.6.36-rc2-git1
>  o ksymoops output for the above file
> 
> If you need more informations please let me know.
> If you want me to test patches let me know.
> If you want me to run a git bissect let me know.
The reason is known, and fixed in nouveau git.  The patches have been
sent to Linus to be merged, but I'm not sure if he has yet.

The relevant commit is "drm/nouveau: fix earlier mistake when fixing
merge conflict"

Ben.
> 
> thanx for your help.
> 
> Other relevant informations:
> 
> Output of ver_linux :
> Linux flocon 2.6.36-rc2-git1 #81 SMP PREEMPT Mon Aug 23 15:14:05 CEST 2010 
> i686 GNU/Linux
>  
> Gnu C  4.4.5
> Gnu make   3.81
> binutils   2.20.1
> util-linux 2.17.2
> mount  support
> module-init-tools  3.12
> e2fsprogs  1.41.12
> jfsutils   1.1.12
> xfsprogs   3.1.2
> pcmciautils015
> Linux C Library2.11.2
> Dynamic linker (ldd)   2.11.2
> Procps 3.2.8
> Net-tools  1.60
> Console-tools  0.2.3
> Sh-utils   8.5
> wireless-tools 30
> Modules Loaded nfsd lockd nfs_acl auth_rpcgss sunrpc exportfs fuse 
> snd_intel8x0 snd_ac97_codec nouveau ac97_bus snd_pcm_oss snd_mixer_oss 
> snd_pcm ttm firewire_ohci snd_seq sg cb710_mmc uhci_hcd snd_timer 
> firewire_core mmc_core drm_kms_helper sr_mod 8139too ehci_hcd snd_seq_device 
> snd crc_itu_t usbcore cb710 ohci1394 cdrom 8139cp soundcore cfbcopyarea 
> snd_page_alloc cfbimgblt ieee1394 intel_agp cfbfillrect battery ac btrfs 
> crc32c libcrc32c sd_mod
> 
> Output of lspci :
> 
> 00:00.0 Host bridge: Intel Corporation 82865G/PE/P DRAM Controller/Host-Hub 
> Interface (rev 02)
>   Subsystem: Hewlett-Packard Company NX9500
>   Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
> Stepping- SERR+ FastB2B- DisINTx-
>   Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort-  SERR-Latency: 0
>   Region 0: Memory at d800 (32-bit, prefetchable) [size=128M]
>   Capabilities: [e4] Vendor Specific Information: Len=06 
>   Capabilities: [a0] AGP version 3.0
>   Status: RQ=32 Iso- ArqSz=2 Cal=2 SBA+ ITACoh- GART64- HTrans- 
> 64bit- FW+ AGP3+ Rate=x4,x8
>   Command: RQ=1 ArqSz=0 Cal=2 SBA- AGP+ GART64- 64bit- FW- 
> Rate=
>   Kernel driver in use: agpgart-intel
> 
> 00:01.0 PCI bridge: Intel Corporation 82865G/PE/P PCI to AGP Controller (rev 
> 02) (prog-if 00 [Normal decode])
>   Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
> Stepping- SERR+ FastB2B- DisINTx-
>   Status: Cap- 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort-  SERR-Latency: 96
>   Bus: primary=00, secondary=01, subordinate=01, sec-latency=80
>   I/O behind bridge: f000-0fff
>   Memory behind bridge: d100-d1ff
>   Prefetchable memory behind bridge: e000-efff
>   Secondary status: 66MHz+ FastB2B+ ParErr- DEVSEL=medium >TAbort- 
>BridgeCtl: Parity- SERR- NoISA+ VGA+ MAbort- >Reset- FastB2B-
>   PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
> 
> 00:06.0 System peripheral: Intel Corporation 82865G/PE/P Processor to I/O 
> Memory Interface (rev ff) (prog-if ff)
>   !!! Unknown header type 7f
> 
> 00:1d.0 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI 
> Controller #1 (rev 02) (prog-if 00 [UHCI])
>   Subsystem: Hewlett-Packard Company NX9500
>   Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
> Stepping- SERR- FastB2B- DisINTx-
>   Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
> SERR-Latency: 0
>   Interrupt: pin A routed to IRQ 16
>   Region 4: I/O ports at 1cc0 [size=32]
>   Kernel driver in use: uhci_hcd
> 
> 00:1d.1 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI 
> Controller #2 (rev 02) (prog-if 00 [UHCI])
>   Subsystem: Hewlett-Packard Company NX9500
>   Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
> Stepping- SERR- FastB2B- DisINTx-
>   Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
> SERR-Latency: 0
>   Interrupt: pin B routed to IRQ 19
>   Region 4: I/O ports at 1ce0 [size=32]
>   Kernel driver in use: uhci_hcd
> 
> 00:1d.2 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI 
> Controller #3 (rev 02) (prog-if 00 [UHCI])
>   Subsystem: Hewlett-Packard Company NX9500
>   Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
> Stepping- SERR- FastB2B- DisINTx-
>   Status: Cap- 

Re: PROBLEM: oops in nouveau driver in 2.6.36-rc*

[Ben Skeggs]

On Mon, 2010-08-23 at 17:35 +0200, Stephane Casset wrote:
 Hi,
Hi,

 
 Since 2.6.36-rc1 I have an oops when loading the nouveau driver.
 The system woks ok expect for the nouveau driver and the console gets
 corrupted :(
 
 I attach relevant informations in attached files :
  o dmesg output for 2.6.36-rc2-git1
  o ksymoops output for the above file
 
 If you need more informations please let me know.
 If you want me to test patches let me know.
 If you want me to run a git bissect let me know.
The reason is known, and fixed in nouveau git.  The patches have been
sent to Linus to be merged, but I'm not sure if he has yet.

The relevant commit is drm/nouveau: fix earlier mistake when fixing
merge conflict

Ben.
 
 thanx for your help.
 
 Other relevant informations:
 
 Output of ver_linux :
 Linux flocon 2.6.36-rc2-git1 #81 SMP PREEMPT Mon Aug 23 15:14:05 CEST 2010 
 i686 GNU/Linux
  
 Gnu C  4.4.5
 Gnu make   3.81
 binutils   2.20.1
 util-linux 2.17.2
 mount  support
 module-init-tools  3.12
 e2fsprogs  1.41.12
 jfsutils   1.1.12
 xfsprogs   3.1.2
 pcmciautils015
 Linux C Library2.11.2
 Dynamic linker (ldd)   2.11.2
 Procps 3.2.8
 Net-tools  1.60
 Console-tools  0.2.3
 Sh-utils   8.5
 wireless-tools 30
 Modules Loaded nfsd lockd nfs_acl auth_rpcgss sunrpc exportfs fuse 
 snd_intel8x0 snd_ac97_codec nouveau ac97_bus snd_pcm_oss snd_mixer_oss 
 snd_pcm ttm firewire_ohci snd_seq sg cb710_mmc uhci_hcd snd_timer 
 firewire_core mmc_core drm_kms_helper sr_mod 8139too ehci_hcd snd_seq_device 
 snd crc_itu_t usbcore cb710 ohci1394 cdrom 8139cp soundcore cfbcopyarea 
 snd_page_alloc cfbimgblt ieee1394 intel_agp cfbfillrect battery ac btrfs 
 crc32c libcrc32c sd_mod
 
 Output of lspci :
 
 00:00.0 Host bridge: Intel Corporation 82865G/PE/P DRAM Controller/Host-Hub 
 Interface (rev 02)
   Subsystem: Hewlett-Packard Company NX9500
   Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
 Stepping- SERR+ FastB2B- DisINTx-
   Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast TAbort- TAbort- 
 MAbort+ SERR- PERR- INTx-
   Latency: 0
   Region 0: Memory at d800 (32-bit, prefetchable) [size=128M]
   Capabilities: [e4] Vendor Specific Information: Len=06 ?
   Capabilities: [a0] AGP version 3.0
   Status: RQ=32 Iso- ArqSz=2 Cal=2 SBA+ ITACoh- GART64- HTrans- 
 64bit- FW+ AGP3+ Rate=x4,x8
   Command: RQ=1 ArqSz=0 Cal=2 SBA- AGP+ GART64- 64bit- FW- 
 Rate=none
   Kernel driver in use: agpgart-intel
 
 00:01.0 PCI bridge: Intel Corporation 82865G/PE/P PCI to AGP Controller (rev 
 02) (prog-if 00 [Normal decode])
   Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
 Stepping- SERR+ FastB2B- DisINTx-
   Status: Cap- 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=fast TAbort- TAbort- 
 MAbort- SERR- PERR- INTx-
   Latency: 96
   Bus: primary=00, secondary=01, subordinate=01, sec-latency=80
   I/O behind bridge: f000-0fff
   Memory behind bridge: d100-d1ff
   Prefetchable memory behind bridge: e000-efff
   Secondary status: 66MHz+ FastB2B+ ParErr- DEVSEL=medium TAbort- 
 TAbort- MAbort+ SERR- PERR-
   BridgeCtl: Parity- SERR- NoISA+ VGA+ MAbort- Reset- FastB2B-
   PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
 
 00:06.0 System peripheral: Intel Corporation 82865G/PE/P Processor to I/O 
 Memory Interface (rev ff) (prog-if ff)
   !!! Unknown header type 7f
 
 00:1d.0 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI 
 Controller #1 (rev 02) (prog-if 00 [UHCI])
   Subsystem: Hewlett-Packard Company NX9500
   Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
 Stepping- SERR- FastB2B- DisINTx-
   Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium TAbort- 
 TAbort- MAbort- SERR- PERR- INTx-
   Latency: 0
   Interrupt: pin A routed to IRQ 16
   Region 4: I/O ports at 1cc0 [size=32]
   Kernel driver in use: uhci_hcd
 
 00:1d.1 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI 
 Controller #2 (rev 02) (prog-if 00 [UHCI])
   Subsystem: Hewlett-Packard Company NX9500
   Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
 Stepping- SERR- FastB2B- DisINTx-
   Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium TAbort- 
 TAbort- MAbort- SERR- PERR- INTx-
   Latency: 0
   Interrupt: pin B routed to IRQ 19
   Region 4: I/O ports at 1ce0 [size=32]
   Kernel driver in use: uhci_hcd
 
 00:1d.2 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI 
 Controller #3 (rev 02) (prog-if 00 [UHCI])
   Subsystem: Hewlett-Packard Company NX9500
   Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
 Stepping- SERR- FastB2B- DisINTx-
   

[PATCHv2] drm/ttm: restructure to allow driver to plug in alternate memory manager

[Ben Skeggs]

From: Ben Skeggs <bske...@redhat.com>

Nouveau will need this on GeForce 8 and up to account for the GPU
reordering physical VRAM for some memory types.

v2: rebase to include nvc0_instmem.c changes

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c   |   12 ++-
 drivers/gpu/drm/nouveau/nouveau_channel.c  |6 +-
 drivers/gpu/drm/nouveau/nouveau_notifier.c |2 +-
 drivers/gpu/drm/nouveau/nouveau_sgdma.c|4 +-
 drivers/gpu/drm/nouveau/nv50_crtc.c|3 +-
 drivers/gpu/drm/nouveau/nv50_display.c |2 +-
 drivers/gpu/drm/nouveau/nv50_instmem.c |2 +-
 drivers/gpu/drm/nouveau/nvc0_instmem.c |2 +-
 drivers/gpu/drm/radeon/radeon_object.c |6 +-
 drivers/gpu/drm/radeon/radeon_ttm.c|   16 ++--
 drivers/gpu/drm/ttm/Makefile   |3 +-
 drivers/gpu/drm/ttm/ttm_agp_backend.c  |3 +-
 drivers/gpu/drm/ttm/ttm_bo.c   |  100 ---
 drivers/gpu/drm/ttm/ttm_bo_manager.c   |  148 
 drivers/gpu/drm/ttm/ttm_bo_util.c  |3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |3 +-
 include/drm/ttm/ttm_bo_api.h   |3 +-
 include/drm/ttm/ttm_bo_driver.h|   21 -
 18 files changed, 226 insertions(+), 113 deletions(-)
 create mode 100644 drivers/gpu/drm/ttm/ttm_bo_manager.c

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index c2497bf..522a891 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -399,6 +399,7 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
man->default_caching = TTM_PL_FLAG_CACHED;
break;
case TTM_PL_VRAM:
+   man->func = _bo_manager_func;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
 TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_UNCACHED |
@@ -407,6 +408,7 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
man->gpu_offset = dev_priv->vm_vram_base;
break;
case TTM_PL_TT:
+   man->func = _bo_manager_func;
switch (dev_priv->gart_info.type) {
case NOUVEAU_GART_AGP:
man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
@@ -509,8 +511,8 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int 
evict, bool intr,
if (!chan || nvbo->tile_flags || nvbo->no_vm)
chan = dev_priv->channel;

-   src_offset = old_mem->mm_node->start << PAGE_SHIFT;
-   dst_offset = new_mem->mm_node->start << PAGE_SHIFT;
+   src_offset = old_mem->start << PAGE_SHIFT;
+   dst_offset = new_mem->start << PAGE_SHIFT;
if (chan != dev_priv->channel) {
if (old_mem->mem_type == TTM_PL_TT)
src_offset += dev_priv->vm_gart_base;
@@ -659,7 +661,7 @@ nouveau_bo_vm_bind(struct ttm_buffer_object *bo, struct 
ttm_mem_reg *new_mem,
return 0;
}

-   offset = new_mem->mm_node->start << PAGE_SHIFT;
+   offset = new_mem->start << PAGE_SHIFT;

if (dev_priv->card_type == NV_50) {
ret = nv50_mem_vm_bind_linear(dev,
@@ -773,14 +775,14 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, 
struct ttm_mem_reg *mem)
case TTM_PL_TT:
 #if __OS_HAS_AGP
if (dev_priv->gart_info.type == NOUVEAU_GART_AGP) {
-   mem->bus.offset = mem->mm_node->start << PAGE_SHIFT;
+   mem->bus.offset = mem->start << PAGE_SHIFT;
mem->bus.base = dev_priv->gart_info.aper_base;
mem->bus.is_iomem = true;
}
 #endif
break;
case TTM_PL_VRAM:
-   mem->bus.offset = mem->mm_node->start << PAGE_SHIFT;
+   mem->bus.offset = mem->start << PAGE_SHIFT;
mem->bus.base = pci_resource_start(dev->pdev, 1);
mem->bus.is_iomem = true;
break;
diff --git a/drivers/gpu/drm/nouveau/nouveau_channel.c 
b/drivers/gpu/drm/nouveau/nouveau_channel.c
index 90fdcda..90d3450 100644
--- a/drivers/gpu/drm/nouveau/nouveau_channel.c
+++ b/drivers/gpu/drm/nouveau/nouveau_channel.c
@@ -48,14 +48,14 @@ nouveau_channel_pushbuf_ctxdma_init(struct nouveau_channel 
*chan)
  dev_priv->gart_info.aper_size,
  NV_DMA_ACCESS_RO, ,
  NULL);
-   chan->pushbuf_base = pb->bo.mem.mm_node->start << PAGE_SHIFT;
+   chan->pushbuf_base = pb->bo.mem.start <<

question regarding nvc0_instmem_suspend()

[Ben Skeggs]

On Fri, 2010-08-13 at 23:59 +0200, Luca Tettamanti wrote:
> On Fri, Aug 13, 2010 at 11:39 PM, Dan Carpenter  wrote:
> > Smatch thinks there is a buffer overflow in nvc0_instmem_suspend() and
> > I've looked at it, but I don't understand the code.
> >
> > drivers/gpu/drm/nouveau/nvc0_instmem.c +152 nvc0_instmem_suspend(10)
> >error: buffer overflow 'dev_priv->susres.ramin_copy' 16384 <= 1835008
> >
> >   141  int
> >   142  nvc0_instmem_suspend(struct drm_device *dev)
> >   143  {
> >   144  struct drm_nouveau_private *dev_priv = dev->dev_private;
> >   145  int i;
> >   146
> >   147  dev_priv->susres.ramin_copy = vmalloc(65536);
> >
> >dev_priv->susres.ramin_copy is an array of 16384 u32 elements
> >(65536 bytes).
> >
> >   148  if (!dev_priv->susres.ramin_copy)
> >   149  return -ENOMEM;
> >   150
> >   151  for (i = 0x70; i < 0x71; i += 4)
> >   152  dev_priv->susres.ramin_copy[i/4] = nv_rd32(dev, i);
> >
> >0x70 / 4 is 1835008 so we're way past the end of the array
> >and then we get larger.
> 
> I guess that it should be something like:
> 
> base = 0x70;
> for (i = 0; i < 0x1; i += 4)
> dev_priv->susres.ramin_copy[i/4] = nv_rd32(dev, base + i);
Oops, what a thinko.  I've pushed a fix to nouveau git, I'll send it on
for inclusion in 2.6.36.

Ben.
> 
> 
> Luca

Re: question regarding nvc0_instmem_suspend()

[Ben Skeggs]

On Fri, 2010-08-13 at 23:59 +0200, Luca Tettamanti wrote:
 On Fri, Aug 13, 2010 at 11:39 PM, Dan Carpenter erro...@gmail.com wrote:
  Smatch thinks there is a buffer overflow in nvc0_instmem_suspend() and
  I've looked at it, but I don't understand the code.
 
  drivers/gpu/drm/nouveau/nvc0_instmem.c +152 nvc0_instmem_suspend(10)
 error: buffer overflow 'dev_priv-susres.ramin_copy' 16384 = 1835008
 
141  int
142  nvc0_instmem_suspend(struct drm_device *dev)
143  {
144  struct drm_nouveau_private *dev_priv = dev-dev_private;
145  int i;
146
147  dev_priv-susres.ramin_copy = vmalloc(65536);
 
 dev_priv-susres.ramin_copy is an array of 16384 u32 elements
 (65536 bytes).
 
148  if (!dev_priv-susres.ramin_copy)
149  return -ENOMEM;
150
151  for (i = 0x70; i  0x71; i += 4)
152  dev_priv-susres.ramin_copy[i/4] = nv_rd32(dev, i);
 
 0x70 / 4 is 1835008 so we're way past the end of the array
 and then we get larger.
 
 I guess that it should be something like:
 
 base = 0x70;
 for (i = 0; i  0x1; i += 4)
 dev_priv-susres.ramin_copy[i/4] = nv_rd32(dev, base + i);
Oops, what a thinko.  I've pushed a fix to nouveau git, I'll send it on
for inclusion in 2.6.36.

Ben.
 
 
 Luca


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/ttm: restructure to allow driver to plug in alternate memory manager

[Ben Skeggs]

From: Ben Skeggs <bske...@redhat.com>

Nouveau will need this on GeForce 8 and up to account for the GPU
reordering physical VRAM for some memory types.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c   |   12 ++-
 drivers/gpu/drm/nouveau/nouveau_channel.c  |6 +-
 drivers/gpu/drm/nouveau/nouveau_notifier.c |2 +-
 drivers/gpu/drm/nouveau/nouveau_sgdma.c|4 +-
 drivers/gpu/drm/nouveau/nv50_crtc.c|3 +-
 drivers/gpu/drm/nouveau/nv50_display.c |2 +-
 drivers/gpu/drm/nouveau/nv50_instmem.c |2 +-
 drivers/gpu/drm/radeon/radeon_object.c |6 +-
 drivers/gpu/drm/radeon/radeon_ttm.c|   16 ++--
 drivers/gpu/drm/ttm/Makefile   |3 +-
 drivers/gpu/drm/ttm/ttm_agp_backend.c  |3 +-
 drivers/gpu/drm/ttm/ttm_bo.c   |  100 ---
 drivers/gpu/drm/ttm/ttm_bo_manager.c   |  148 
 drivers/gpu/drm/ttm/ttm_bo_util.c  |3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |3 +-
 include/drm/ttm/ttm_bo_api.h   |3 +-
 include/drm/ttm/ttm_bo_driver.h|   21 -
 17 files changed, 225 insertions(+), 112 deletions(-)
 create mode 100644 drivers/gpu/drm/ttm/ttm_bo_manager.c

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 92290fa..f403737 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -390,6 +390,7 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
man->default_caching = TTM_PL_FLAG_CACHED;
break;
case TTM_PL_VRAM:
+   man->func = _bo_manager_func;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
 TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_UNCACHED |
@@ -398,6 +399,7 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
man->gpu_offset = dev_priv->vm_vram_base;
break;
case TTM_PL_TT:
+   man->func = _bo_manager_func;
switch (dev_priv->gart_info.type) {
case NOUVEAU_GART_AGP:
man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
@@ -500,8 +502,8 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int 
evict, bool intr,
if (!chan || nvbo->tile_flags || nvbo->no_vm)
chan = dev_priv->channel;

-   src_offset = old_mem->mm_node->start << PAGE_SHIFT;
-   dst_offset = new_mem->mm_node->start << PAGE_SHIFT;
+   src_offset = old_mem->start << PAGE_SHIFT;
+   dst_offset = new_mem->start << PAGE_SHIFT;
if (chan != dev_priv->channel) {
if (old_mem->mem_type == TTM_PL_TT)
src_offset += dev_priv->vm_gart_base;
@@ -650,7 +652,7 @@ nouveau_bo_vm_bind(struct ttm_buffer_object *bo, struct 
ttm_mem_reg *new_mem,
return 0;
}

-   offset = new_mem->mm_node->start << PAGE_SHIFT;
+   offset = new_mem->start << PAGE_SHIFT;

if (dev_priv->card_type == NV_50) {
ret = nv50_mem_vm_bind_linear(dev,
@@ -764,14 +766,14 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, 
struct ttm_mem_reg *mem)
case TTM_PL_TT:
 #if __OS_HAS_AGP
if (dev_priv->gart_info.type == NOUVEAU_GART_AGP) {
-   mem->bus.offset = mem->mm_node->start << PAGE_SHIFT;
+   mem->bus.offset = mem->start << PAGE_SHIFT;
mem->bus.base = dev_priv->gart_info.aper_base;
mem->bus.is_iomem = true;
}
 #endif
break;
case TTM_PL_VRAM:
-   mem->bus.offset = mem->mm_node->start << PAGE_SHIFT;
+   mem->bus.offset = mem->start << PAGE_SHIFT;
mem->bus.base = pci_resource_start(dev->pdev, 1);
mem->bus.is_iomem = true;
break;
diff --git a/drivers/gpu/drm/nouveau/nouveau_channel.c 
b/drivers/gpu/drm/nouveau/nouveau_channel.c
index 90fdcda..90d3450 100644
--- a/drivers/gpu/drm/nouveau/nouveau_channel.c
+++ b/drivers/gpu/drm/nouveau/nouveau_channel.c
@@ -48,14 +48,14 @@ nouveau_channel_pushbuf_ctxdma_init(struct nouveau_channel 
*chan)
  dev_priv->gart_info.aper_size,
  NV_DMA_ACCESS_RO, ,
  NULL);
-   chan->pushbuf_base = pb->bo.mem.mm_node->start << PAGE_SHIFT;
+   chan->pushbuf_base = pb->bo.mem.start << PAGE_SHIFT;
} else
if (dev_priv->card_type 

[PATCH 1/2] drm/ttm: introduce utility function to free an allocated memory node

[Ben Skeggs]

From: Ben Skeggs <bske...@redhat.com>

Existing core code/drivers call drm_mm_put_block on ttm_mem_reg.mm_node
directly.  Future patches will modify TTM behaviour in such a way that
ttm_mem_reg.mm_node doesn't necessarily belong to drm_mm.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c |   14 ++
 drivers/gpu/drm/radeon/radeon_ttm.c  |   18 ++
 drivers/gpu/drm/ttm/ttm_bo.c |   26 +++---
 drivers/gpu/drm/ttm/ttm_bo_util.c|9 +
 include/drm/ttm/ttm_bo_driver.h  |4 
 5 files changed, 24 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 3ca8343..92290fa 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -597,12 +597,7 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool 
evict, bool intr,

ret = ttm_bo_move_ttm(bo, evict, no_wait_reserve, no_wait_gpu, new_mem);
 out:
-   if (tmp_mem.mm_node) {
-   spin_lock(>bdev->glob->lru_lock);
-   drm_mm_put_block(tmp_mem.mm_node);
-   spin_unlock(>bdev->glob->lru_lock);
-   }
-
+   ttm_bo_mem_put(bo, _mem);
return ret;
 }

@@ -635,12 +630,7 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool 
evict, bool intr,
goto out;

 out:
-   if (tmp_mem.mm_node) {
-   spin_lock(>bdev->glob->lru_lock);
-   drm_mm_put_block(tmp_mem.mm_node);
-   spin_unlock(>bdev->glob->lru_lock);
-   }
-
+   ttm_bo_mem_put(bo, _mem);
return ret;
 }

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 84c53e4..cc19aba 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -326,14 +326,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object 
*bo,
}
r = ttm_bo_move_ttm(bo, true, no_wait_reserve, no_wait_gpu, new_mem);
 out_cleanup:
-   if (tmp_mem.mm_node) {
-   struct ttm_bo_global *glob = rdev->mman.bdev.glob;
-
-   spin_lock(>lru_lock);
-   drm_mm_put_block(tmp_mem.mm_node);
-   spin_unlock(>lru_lock);
-   return r;
-   }
+   ttm_bo_mem_put(bo, _mem);
return r;
 }

@@ -372,14 +365,7 @@ static int radeon_move_ram_vram(struct ttm_buffer_object 
*bo,
goto out_cleanup;
}
 out_cleanup:
-   if (tmp_mem.mm_node) {
-   struct ttm_bo_global *glob = rdev->mman.bdev.glob;
-
-   spin_lock(>lru_lock);
-   drm_mm_put_block(tmp_mem.mm_node);
-   spin_unlock(>lru_lock);
-   return r;
-   }
+   ttm_bo_mem_put(bo, _mem);
return r;
 }

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index cb4cf7e..80d37b4 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -475,11 +475,8 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object 
*bo, bool remove_all)
list_del_init(>ddestroy);
++put_count;
}
-   if (bo->mem.mm_node) {
-   drm_mm_put_block(bo->mem.mm_node);
-   bo->mem.mm_node = NULL;
-   }
spin_unlock(>lru_lock);
+   ttm_bo_mem_put(bo, >mem);

atomic_set(>reserved, 0);

@@ -621,7 +618,6 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, bool 
interruptible,
bool no_wait_reserve, bool no_wait_gpu)
 {
struct ttm_bo_device *bdev = bo->bdev;
-   struct ttm_bo_global *glob = bo->glob;
struct ttm_mem_reg evict_mem;
struct ttm_placement placement;
int ret = 0;
@@ -667,12 +663,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, bool 
interruptible,
if (ret) {
if (ret != -ERESTARTSYS)
printk(KERN_ERR TTM_PFX "Buffer eviction failed\n");
-   spin_lock(>lru_lock);
-   if (evict_mem.mm_node) {
-   drm_mm_put_block(evict_mem.mm_node);
-   evict_mem.mm_node = NULL;
-   }
-   spin_unlock(>lru_lock);
+   ttm_bo_mem_put(bo, _mem);
goto out;
}
bo->evicted = true;
@@ -769,6 +760,19 @@ static int ttm_bo_man_get_node(struct ttm_buffer_object 
*bo,
return 0;
 }

+void ttm_bo_mem_put(struct ttm_buffer_object *bo, struct ttm_mem_reg *mem)
+{
+   struct ttm_bo_global *glob = bo->glob;
+
+   if (mem->mm_node) {
+   spin_lock(>lru_lock);
+   drm_mm_put_block(mem->mm_node);
+   spin_unlock(>lru_lock);
+   mem->mm_node = NULL;
+

[PATCH 0/2] RFC: allow driver to plugin alternative to drm_mm

[Ben Skeggs]

From: Ben Skeggs <bske...@redhat.com>

In order to properly deal with GPU reordering of blocks in physical VRAM,
Nouveau needs to be able to have better control over VRAM allocations.

Currently nouveau is extremely wasteful and forces massive amounts of
padding/alignment to avoid buffer corruption issues.

radeon/vmwgfx haven't been tested, but nouveau works as well as it did
previously.

Ben Skeggs (2):
  drm/ttm: introduce utility function to free an allocated memory node
  drm/ttm: restructure to allow driver to plug in alternate memory
manager

 drivers/gpu/drm/nouveau/nouveau_bo.c   |   26 ++---
 drivers/gpu/drm/nouveau/nouveau_channel.c  |6 +-
 drivers/gpu/drm/nouveau/nouveau_notifier.c |2 +-
 drivers/gpu/drm/nouveau/nouveau_sgdma.c|4 +-
 drivers/gpu/drm/nouveau/nv50_crtc.c|3 +-
 drivers/gpu/drm/nouveau/nv50_display.c |2 +-
 drivers/gpu/drm/nouveau/nv50_instmem.c |2 +-
 drivers/gpu/drm/radeon/radeon_object.c |6 +-
 drivers/gpu/drm/radeon/radeon_ttm.c|   34 ++-
 drivers/gpu/drm/ttm/Makefile   |3 +-
 drivers/gpu/drm/ttm/ttm_agp_backend.c  |3 +-
 drivers/gpu/drm/ttm/ttm_bo.c   |  104 +---
 drivers/gpu/drm/ttm/ttm_bo_manager.c   |  148 
 drivers/gpu/drm/ttm/ttm_bo_util.c  |   12 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |3 +-
 include/drm/ttm/ttm_bo_api.h   |3 +-
 include/drm/ttm/ttm_bo_driver.h|   25 +-
 17 files changed, 238 insertions(+), 148 deletions(-)
 create mode 100644 drivers/gpu/drm/ttm/ttm_bo_manager.c

-- 
1.7.2

[PATCH 2/2] drm/ttm: restructure to allow driver to plug in alternate memory manager

[Ben Skeggs]

From: Ben Skeggs bske...@redhat.com

Nouveau will need this on GeForce 8 and up to account for the GPU
reordering physical VRAM for some memory types.

Signed-off-by: Ben Skeggs bske...@redhat.com
---
 drivers/gpu/drm/nouveau/nouveau_bo.c   |   12 ++-
 drivers/gpu/drm/nouveau/nouveau_channel.c  |6 +-
 drivers/gpu/drm/nouveau/nouveau_notifier.c |2 +-
 drivers/gpu/drm/nouveau/nouveau_sgdma.c|4 +-
 drivers/gpu/drm/nouveau/nv50_crtc.c|3 +-
 drivers/gpu/drm/nouveau/nv50_display.c |2 +-
 drivers/gpu/drm/nouveau/nv50_instmem.c |2 +-
 drivers/gpu/drm/radeon/radeon_object.c |6 +-
 drivers/gpu/drm/radeon/radeon_ttm.c|   16 ++--
 drivers/gpu/drm/ttm/Makefile   |3 +-
 drivers/gpu/drm/ttm/ttm_agp_backend.c  |3 +-
 drivers/gpu/drm/ttm/ttm_bo.c   |  100 ---
 drivers/gpu/drm/ttm/ttm_bo_manager.c   |  148 
 drivers/gpu/drm/ttm/ttm_bo_util.c  |3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |3 +-
 include/drm/ttm/ttm_bo_api.h   |3 +-
 include/drm/ttm/ttm_bo_driver.h|   21 -
 17 files changed, 225 insertions(+), 112 deletions(-)
 create mode 100644 drivers/gpu/drm/ttm/ttm_bo_manager.c

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 92290fa..f403737 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -390,6 +390,7 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
man-default_caching = TTM_PL_FLAG_CACHED;
break;
case TTM_PL_VRAM:
+   man-func = ttm_bo_manager_func;
man-flags = TTM_MEMTYPE_FLAG_FIXED |
 TTM_MEMTYPE_FLAG_MAPPABLE;
man-available_caching = TTM_PL_FLAG_UNCACHED |
@@ -398,6 +399,7 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
man-gpu_offset = dev_priv-vm_vram_base;
break;
case TTM_PL_TT:
+   man-func = ttm_bo_manager_func;
switch (dev_priv-gart_info.type) {
case NOUVEAU_GART_AGP:
man-flags = TTM_MEMTYPE_FLAG_MAPPABLE;
@@ -500,8 +502,8 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int 
evict, bool intr,
if (!chan || nvbo-tile_flags || nvbo-no_vm)
chan = dev_priv-channel;
 
-   src_offset = old_mem-mm_node-start  PAGE_SHIFT;
-   dst_offset = new_mem-mm_node-start  PAGE_SHIFT;
+   src_offset = old_mem-start  PAGE_SHIFT;
+   dst_offset = new_mem-start  PAGE_SHIFT;
if (chan != dev_priv-channel) {
if (old_mem-mem_type == TTM_PL_TT)
src_offset += dev_priv-vm_gart_base;
@@ -650,7 +652,7 @@ nouveau_bo_vm_bind(struct ttm_buffer_object *bo, struct 
ttm_mem_reg *new_mem,
return 0;
}
 
-   offset = new_mem-mm_node-start  PAGE_SHIFT;
+   offset = new_mem-start  PAGE_SHIFT;
 
if (dev_priv-card_type == NV_50) {
ret = nv50_mem_vm_bind_linear(dev,
@@ -764,14 +766,14 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, 
struct ttm_mem_reg *mem)
case TTM_PL_TT:
 #if __OS_HAS_AGP
if (dev_priv-gart_info.type == NOUVEAU_GART_AGP) {
-   mem-bus.offset = mem-mm_node-start  PAGE_SHIFT;
+   mem-bus.offset = mem-start  PAGE_SHIFT;
mem-bus.base = dev_priv-gart_info.aper_base;
mem-bus.is_iomem = true;
}
 #endif
break;
case TTM_PL_VRAM:
-   mem-bus.offset = mem-mm_node-start  PAGE_SHIFT;
+   mem-bus.offset = mem-start  PAGE_SHIFT;
mem-bus.base = pci_resource_start(dev-pdev, 1);
mem-bus.is_iomem = true;
break;
diff --git a/drivers/gpu/drm/nouveau/nouveau_channel.c 
b/drivers/gpu/drm/nouveau/nouveau_channel.c
index 90fdcda..90d3450 100644
--- a/drivers/gpu/drm/nouveau/nouveau_channel.c
+++ b/drivers/gpu/drm/nouveau/nouveau_channel.c
@@ -48,14 +48,14 @@ nouveau_channel_pushbuf_ctxdma_init(struct nouveau_channel 
*chan)
  dev_priv-gart_info.aper_size,
  NV_DMA_ACCESS_RO, pushbuf,
  NULL);
-   chan-pushbuf_base = pb-bo.mem.mm_node-start  PAGE_SHIFT;
+   chan-pushbuf_base = pb-bo.mem.start  PAGE_SHIFT;
} else
if (dev_priv-card_type != NV_04) {
ret = nouveau_gpuobj_dma_new(chan, NV_CLASS_DMA_IN_MEMORY, 0,
 dev_priv-fb_available_size,
 NV_DMA_ACCESS_RO,
 NV_DMA_TARGET_VIDMEM, pushbuf);
-   chan

[PATCH] drm: add "auto" dithering method

[Ben Skeggs]

From: Ben Skeggs <bske...@redhat.com>

There's no convenient/reliable way for drivers to both obey the dithering
mode property, and to be able to attempt to provide a good default in all
cases.

This commit adds an "auto" method to the property which drivers can default
to if they wish, whilst still allowing the user to override the choice as
they do now.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/drm_crtc.c |1 +
 include/drm/drm_mode.h |1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 57cea01..c359c83 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -80,6 +80,7 @@ static struct drm_prop_enum_list 
drm_dithering_mode_enum_list[] =
 {
{ DRM_MODE_DITHERING_OFF, "Off" },
{ DRM_MODE_DITHERING_ON, "On" },
+   { DRM_MODE_DITHERING_AUTO, "Automatic" },
 };

 /*
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index c5ba163..0fc7397 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -74,6 +74,7 @@
 /* Dithering mode options */
 #define DRM_MODE_DITHERING_OFF 0
 #define DRM_MODE_DITHERING_ON  1
+#define DRM_MODE_DITHERING_AUTO 2

 /* Dirty info options */
 #define DRM_MODE_DIRTY_OFF  0
-- 
1.7.1.1

deadlock possiblity introduced by "drm/nouveau: use drm_mm in preference to custom code doing the same thing"

[Ben Skeggs]

On Sun, 2010-07-11 at 01:24 +0200, Marcin Slusarz wrote:
> Hi
> 
> Patch "drm/nouveau: use drm_mm in preference to custom code doing the same 
> thing"
> in nouveau tree introduced new deadlock possibility, for which lockdep 
> complains loudly:
> 
> [ 1541.070202] [drm] nouveau :02:00.0: Allocating FIFO number 3
> [ 1541.084772] [drm] nouveau :02:00.0: nouveau_channel_alloc: initialised 
> FIFO 3
> [ 2417.733440] [drm] nouveau :02:00.0: nouveau_channel_free: freeing fifo 
> 3
> [ 2417.746638] 
> [ 2417.746639] ==
> [ 2417.746642] [ INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected ]
> [ 2417.746644] 2.6.35-rc4-nv+ #379
> [ 2417.746645] --
> [ 2417.746648] warsow/2850 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> [ 2417.746650]  (&(>unused_lock)->rlock){+.+...}, at: 
> [] drm_mm_put_block+0x17a/0x1c0
> [ 2417.746657] 
> [ 2417.746658] and this task is already holding:
> [ 2417.746659]  (&(_priv->context_switch_lock)->rlock){-.}, at: 
> [] nouveau_channel_free+0x10f/0x233 [nouveau]
> [ 2417.746669] which would create a new lock dependency:
> [ 2417.746671]  (&(_priv->context_switch_lock)->rlock){-.} -> 
> (&(>unused_lock)->rlock){+.+...}
> [ 2417.746676] 
> [ 2417.746676] but this new dependency connects a HARDIRQ-irq-safe lock:
> [ 2417.746678]  (&(_priv->context_switch_lock)->rlock){-.}
> [ 2417.746680] ... which became HARDIRQ-irq-safe at:
> [ 2417.746682]   [] __lock_acquire+0x671/0x8f4
> [ 2417.746685]   [] lock_acquire+0x148/0x18d
> [ 2417.746688]   [] _raw_spin_lock_irqsave+0x41/0x53
> [ 2417.746692]   [] nouveau_irq_handler+0x56/0xa48 [nouveau]
> [ 2417.746698]   [] handle_IRQ_event+0xec/0x25d
> [ 2417.746702]   [] handle_fasteoi_irq+0x92/0xd2
> [ 2417.746705]   [] handle_irq+0x83/0x8c
> [ 2417.746707]   [] do_IRQ+0x57/0xbe
> [ 2417.746711]   [] ret_from_intr+0x0/0xf
> [ 2417.746714]   [] cpu_idle+0x5c/0xb3
> [ 2417.746717]   [] start_secondary+0x1c7/0x1cb
> [ 2417.746720] 
> [ 2417.746720] to a HARDIRQ-irq-unsafe lock:
> [ 2417.746722]  (&(>unused_lock)->rlock){+.+...}
> [ 2417.746724] ... which became HARDIRQ-irq-unsafe at:
> [ 2417.746725] ...  [] __lock_acquire+0x6eb/0x8f4
> [ 2417.746729]   [] lock_acquire+0x148/0x18d
> [ 2417.746732]   [] _raw_spin_lock+0x36/0x45
> [ 2417.746735]   [] drm_mm_pre_get+0x20/0x1a7
> [ 2417.746738]   [] ttm_bo_init+0x234/0x385 [ttm]
> [ 2417.746743]   [] nouveau_bo_new+0x366/0x3eb [nouveau]
> [ 2417.746750]   [] nouveau_mem_init+0x262/0x44a [nouveau]
> [ 2417.746756]   [] nouveau_card_init+0xa73/0xd7e [nouveau]
> [ 2417.746761]   [] nouveau_load+0x6ba/0x70a [nouveau]
> [ 2417.746766]   [] drm_get_dev+0x470/0x5a1
> [ 2417.746769]   [] nouveau_pci_probe+0x10/0x12 [nouveau]
> [ 2417.746778]   [] local_pci_probe+0x12/0x16
> [ 2417.746781]   [] pci_device_probe+0x60/0x8f
> [ 2417.746784]   [] driver_probe_device+0xa7/0x136
> [ 2417.746788]   [] __driver_attach+0x5c/0x80
> [ 2417.746790]   [] bus_for_each_dev+0x54/0x89
> [ 2417.746793]   [] driver_attach+0x19/0x1b
> [ 2417.746796]   [] bus_add_driver+0x183/0x2ef
> [ 2417.746799]   [] driver_register+0x98/0x109
> [ 2417.746801]   [] __pci_register_driver+0x63/0xd3
> [ 2417.746805]   [] drm_init+0x6b/0xd1
> [ 2417.746807]   [] 0xa0128048
> [ 2417.746810]   [] do_one_initcall+0x59/0x14e
> [ 2417.746814]   [] sys_init_module+0x9c/0x1dd
> [ 2417.746817]   [] system_call_fastpath+0x16/0x1b
> [ 2417.746820] 
> [ 2417.746820] other info that might help us debug this:
> [ 2417.746821] 
> [ 2417.746823] 1 lock held by warsow/2850:
> [ 2417.746824]  #0:  (&(_priv->context_switch_lock)->rlock){-.}, at: 
> [] nouveau_channel_free+0x10f/0x233 [nouveau]
> [ 2417.746833] 
> [ 2417.746833] the dependencies between HARDIRQ-irq-safe lock and the holding 
> lock:
> [ 2417.746841] -> (&(_priv->context_switch_lock)->rlock){-.} ops: 18 {
> [ 2417.746845]IN-HARDIRQ-W at:
> [ 2417.746847][] 
> __lock_acquire+0x671/0x8f4
> [ 2417.746851][] 
> lock_acquire+0x148/0x18d
> [ 2417.746854][] 
> _raw_spin_lock_irqsave+0x41/0x53
> [ 2417.746858][] 
> nouveau_irq_handler+0x56/0xa48 [nouveau]
> [ 2417.746865][] 
> handle_IRQ_event+0xec/0x25d
> [ 2417.746868][] 
> handle_fasteoi_irq+0x92/0xd2
> [ 2417.746871][] 
> handle_irq+0x83/0x8c
> [ 2417.746874][] 
> do_IRQ+0x57/0xbe
> [ 2417.746878][] 
> ret_from_intr+0x0/0xf
> [ 2417.746881][] 
> cpu_idle+0x5c/0xb3
> [ 2417.746885][] 
> start_secondary+0x1c7/0x1cb
> [ 2417.746888]INITIAL USE at:
> [ 2417.746890]   

Re: deadlock possiblity introduced by drm/nouveau: use drm_mm in preference to custom code doing the same thing

[Ben Skeggs]

On Sun, 2010-07-11 at 01:24 +0200, Marcin Slusarz wrote:
 Hi
 
 Patch drm/nouveau: use drm_mm in preference to custom code doing the same 
 thing
 in nouveau tree introduced new deadlock possibility, for which lockdep 
 complains loudly:
 
 [ 1541.070202] [drm] nouveau :02:00.0: Allocating FIFO number 3
 [ 1541.084772] [drm] nouveau :02:00.0: nouveau_channel_alloc: initialised 
 FIFO 3
 [ 2417.733440] [drm] nouveau :02:00.0: nouveau_channel_free: freeing fifo 
 3
 [ 2417.746638] 
 [ 2417.746639] ==
 [ 2417.746642] [ INFO: HARDIRQ-safe - HARDIRQ-unsafe lock order detected ]
 [ 2417.746644] 2.6.35-rc4-nv+ #379
 [ 2417.746645] --
 [ 2417.746648] warsow/2850 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
 [ 2417.746650]  ((mm-unused_lock)-rlock){+.+...}, at: 
 [8129f0c0] drm_mm_put_block+0x17a/0x1c0
 [ 2417.746657] 
 [ 2417.746658] and this task is already holding:
 [ 2417.746659]  ((dev_priv-context_switch_lock)-rlock){-.}, at: 
 [a00ac502] nouveau_channel_free+0x10f/0x233 [nouveau]
 [ 2417.746669] which would create a new lock dependency:
 [ 2417.746671]  ((dev_priv-context_switch_lock)-rlock){-.} - 
 ((mm-unused_lock)-rlock){+.+...}
 [ 2417.746676] 
 [ 2417.746676] but this new dependency connects a HARDIRQ-irq-safe lock:
 [ 2417.746678]  ((dev_priv-context_switch_lock)-rlock){-.}
 [ 2417.746680] ... which became HARDIRQ-irq-safe at:
 [ 2417.746682]   [8109739e] __lock_acquire+0x671/0x8f4
 [ 2417.746685]   [81097769] lock_acquire+0x148/0x18d
 [ 2417.746688]   [8143b2cd] _raw_spin_lock_irqsave+0x41/0x53
 [ 2417.746692]   [a00b3072] nouveau_irq_handler+0x56/0xa48 [nouveau]
 [ 2417.746698]   [810a7b3b] handle_IRQ_event+0xec/0x25d
 [ 2417.746702]   [810a98e1] handle_fasteoi_irq+0x92/0xd2
 [ 2417.746705]   [81032953] handle_irq+0x83/0x8c
 [ 2417.746707]   [81031f19] do_IRQ+0x57/0xbe
 [ 2417.746711]   [8143bf13] ret_from_intr+0x0/0xf
 [ 2417.746714]   [8102f085] cpu_idle+0x5c/0xb3
 [ 2417.746717]   [81434180] start_secondary+0x1c7/0x1cb
 [ 2417.746720] 
 [ 2417.746720] to a HARDIRQ-irq-unsafe lock:
 [ 2417.746722]  ((mm-unused_lock)-rlock){+.+...}
 [ 2417.746724] ... which became HARDIRQ-irq-unsafe at:
 [ 2417.746725] ...  [81097418] __lock_acquire+0x6eb/0x8f4
 [ 2417.746729]   [81097769] lock_acquire+0x148/0x18d
 [ 2417.746732]   [8143b1d9] _raw_spin_lock+0x36/0x45
 [ 2417.746735]   [8129f641] drm_mm_pre_get+0x20/0x1a7
 [ 2417.746738]   [a00963f2] ttm_bo_init+0x234/0x385 [ttm]
 [ 2417.746743]   [a00b67e4] nouveau_bo_new+0x366/0x3eb [nouveau]
 [ 2417.746750]   [a00ad341] nouveau_mem_init+0x262/0x44a [nouveau]
 [ 2417.746756]   [a00ab8b6] nouveau_card_init+0xa73/0xd7e [nouveau]
 [ 2417.746761]   [a00ac27b] nouveau_load+0x6ba/0x70a [nouveau]
 [ 2417.746766]   [8129b613] drm_get_dev+0x470/0x5a1
 [ 2417.746769]   [a0108b24] nouveau_pci_probe+0x10/0x12 [nouveau]
 [ 2417.746778]   [8121f2c7] local_pci_probe+0x12/0x16
 [ 2417.746781]   [8121faaf] pci_device_probe+0x60/0x8f
 [ 2417.746784]   [812aced7] driver_probe_device+0xa7/0x136
 [ 2417.746788]   [812acfc2] __driver_attach+0x5c/0x80
 [ 2417.746790]   [812ac632] bus_for_each_dev+0x54/0x89
 [ 2417.746793]   [812acd46] driver_attach+0x19/0x1b
 [ 2417.746796]   [812abf48] bus_add_driver+0x183/0x2ef
 [ 2417.746799]   [812ad28f] driver_register+0x98/0x109
 [ 2417.746801]   [8121fd1f] __pci_register_driver+0x63/0xd3
 [ 2417.746805]   [81295f78] drm_init+0x6b/0xd1
 [ 2417.746807]   [a0128048] 0xa0128048
 [ 2417.746810]   [810001ef] do_one_initcall+0x59/0x14e
 [ 2417.746814]   [810a2b0f] sys_init_module+0x9c/0x1dd
 [ 2417.746817]   [8102fdab] system_call_fastpath+0x16/0x1b
 [ 2417.746820] 
 [ 2417.746820] other info that might help us debug this:
 [ 2417.746821] 
 [ 2417.746823] 1 lock held by warsow/2850:
 [ 2417.746824]  #0:  ((dev_priv-context_switch_lock)-rlock){-.}, at: 
 [a00ac502] nouveau_channel_free+0x10f/0x233 [nouveau]
 [ 2417.746833] 
 [ 2417.746833] the dependencies between HARDIRQ-irq-safe lock and the holding 
 lock:
 [ 2417.746841] - ((dev_priv-context_switch_lock)-rlock){-.} ops: 18 {
 [ 2417.746845]IN-HARDIRQ-W at:
 [ 2417.746847][8109739e] 
 __lock_acquire+0x671/0x8f4
 [ 2417.746851][81097769] 
 lock_acquire+0x148/0x18d
 [ 2417.746854][8143b2cd] 
 _raw_spin_lock_irqsave+0x41/0x53
 [ 2417.746858][a00b3072] 
 nouveau_irq_handler+0x56/0xa48 [nouveau]
 [ 2417.746865]

[PATCH] drm: disable encoder rather than dpms off in drm_crtc_prepare_encoders()

[Ben Skeggs]

From: Ben Skeggs <bske...@redhat.com>

Original behaviour will be preserved for drivers that don't implement
disable() hooks for an encoder.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/drm_crtc_helper.c |   22 ++
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index 9b2a541..1eaa315 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -201,6 +201,17 @@ bool drm_helper_crtc_in_use(struct drm_crtc *crtc)
 }
 EXPORT_SYMBOL(drm_helper_crtc_in_use);

+static void
+drm_encoder_disable(struct drm_encoder *encoder)
+{
+   struct drm_encoder_helper_funcs *encoder_funcs = 
encoder->helper_private;
+
+   if (encoder_funcs->disable)
+   (*encoder_funcs->disable)(encoder);
+   else
+   (*encoder_funcs->dpms)(encoder, DRM_MODE_DPMS_OFF);
+}
+
 /**
  * drm_helper_disable_unused_functions - disable unused objects
  * @dev: DRM device
@@ -215,7 +226,6 @@ void drm_helper_disable_unused_functions(struct drm_device 
*dev)
 {
struct drm_encoder *encoder;
struct drm_connector *connector;
-   struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_crtc *crtc;

list_for_each_entry(connector, >mode_config.connector_list, head) {
@@ -226,12 +236,8 @@ void drm_helper_disable_unused_functions(struct drm_device 
*dev)
}

list_for_each_entry(encoder, >mode_config.encoder_list, head) {
-   encoder_funcs = encoder->helper_private;
if (!drm_helper_encoder_in_use(encoder)) {
-   if (encoder_funcs->disable)
-   (*encoder_funcs->disable)(encoder);
-   else
-   (*encoder_funcs->dpms)(encoder, 
DRM_MODE_DPMS_OFF);
+   drm_encoder_disable(encoder);
/* disconnector encoder from any connector */
encoder->crtc = NULL;
}
@@ -292,11 +298,11 @@ drm_crtc_prepare_encoders(struct drm_device *dev)
encoder_funcs = encoder->helper_private;
/* Disable unused encoders */
if (encoder->crtc == NULL)
-   (*encoder_funcs->dpms)(encoder, DRM_MODE_DPMS_OFF);
+   drm_encoder_disable(encoder);
/* Disable encoders whose CRTC is about to change */
if (encoder_funcs->get_crtc &&
encoder->crtc != (*encoder_funcs->get_crtc)(encoder))
-   (*encoder_funcs->dpms)(encoder, DRM_MODE_DPMS_OFF);
+   drm_encoder_disable(encoder);
}
 }

-- 
1.7.1

[PATCH] drm: disable encoder rather than dpms off in drm_crtc_prepare_encoders()

[Ben Skeggs]

From: Ben Skeggs bske...@redhat.com

Original behaviour will be preserved for drivers that don't implement
disable() hooks for an encoder.

Signed-off-by: Ben Skeggs bske...@redhat.com
---
 drivers/gpu/drm/drm_crtc_helper.c |   22 ++
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index 9b2a541..1eaa315 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -201,6 +201,17 @@ bool drm_helper_crtc_in_use(struct drm_crtc *crtc)
 }
 EXPORT_SYMBOL(drm_helper_crtc_in_use);
 
+static void
+drm_encoder_disable(struct drm_encoder *encoder)
+{
+   struct drm_encoder_helper_funcs *encoder_funcs = 
encoder-helper_private;
+
+   if (encoder_funcs-disable)
+   (*encoder_funcs-disable)(encoder);
+   else
+   (*encoder_funcs-dpms)(encoder, DRM_MODE_DPMS_OFF);
+}
+
 /**
  * drm_helper_disable_unused_functions - disable unused objects
  * @dev: DRM device
@@ -215,7 +226,6 @@ void drm_helper_disable_unused_functions(struct drm_device 
*dev)
 {
struct drm_encoder *encoder;
struct drm_connector *connector;
-   struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_crtc *crtc;
 
list_for_each_entry(connector, dev-mode_config.connector_list, head) {
@@ -226,12 +236,8 @@ void drm_helper_disable_unused_functions(struct drm_device 
*dev)
}
 
list_for_each_entry(encoder, dev-mode_config.encoder_list, head) {
-   encoder_funcs = encoder-helper_private;
if (!drm_helper_encoder_in_use(encoder)) {
-   if (encoder_funcs-disable)
-   (*encoder_funcs-disable)(encoder);
-   else
-   (*encoder_funcs-dpms)(encoder, 
DRM_MODE_DPMS_OFF);
+   drm_encoder_disable(encoder);
/* disconnector encoder from any connector */
encoder-crtc = NULL;
}
@@ -292,11 +298,11 @@ drm_crtc_prepare_encoders(struct drm_device *dev)
encoder_funcs = encoder-helper_private;
/* Disable unused encoders */
if (encoder-crtc == NULL)
-   (*encoder_funcs-dpms)(encoder, DRM_MODE_DPMS_OFF);
+   drm_encoder_disable(encoder);
/* Disable encoders whose CRTC is about to change */
if (encoder_funcs-get_crtc 
encoder-crtc != (*encoder_funcs-get_crtc)(encoder))
-   (*encoder_funcs-dpms)(encoder, DRM_MODE_DPMS_OFF);
+   drm_encoder_disable(encoder);
}
 }
 
-- 
1.7.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[patch] drm/nouveau: off by one in init_i2c_device_find()

[Ben Skeggs]

On Tue, 2010-05-25 at 11:52 +0200, Dan Carpenter wrote:
> dcb->i2c[] has DCB_MAX_NUM_I2C_ENTRIES entries.
> 
> Signed-off-by: Dan Carpenter 
Thanks, picked this up in the nouveau tree.

> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c 
> b/drivers/gpu/drm/nouveau/nouveau_bios.c
> index e7e69cc..bf92a1e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bios.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
> @@ -814,7 +814,7 @@ init_i2c_device_find(struct drm_device *dev, int 
> i2c_index)
>   if (i2c_index == 0x81)
>   i2c_index = (dcb->i2c_default_indices & 0xf0) >> 4;
>  
> - if (i2c_index > DCB_MAX_NUM_I2C_ENTRIES) {
> + if (i2c_index >= DCB_MAX_NUM_I2C_ENTRIES) {
>   NV_ERROR(dev, "invalid i2c_index 0x%x\n", i2c_index);
>   return NULL;
>   }

Re: [patch] drm/nouveau: off by one in init_i2c_device_find()

[Ben Skeggs]

On Tue, 2010-05-25 at 11:52 +0200, Dan Carpenter wrote:
 dcb-i2c[] has DCB_MAX_NUM_I2C_ENTRIES entries.
 
 Signed-off-by: Dan Carpenter erro...@gmail.com
Thanks, picked this up in the nouveau tree.

 
 diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c 
 b/drivers/gpu/drm/nouveau/nouveau_bios.c
 index e7e69cc..bf92a1e 100644
 --- a/drivers/gpu/drm/nouveau/nouveau_bios.c
 +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
 @@ -814,7 +814,7 @@ init_i2c_device_find(struct drm_device *dev, int 
 i2c_index)
   if (i2c_index == 0x81)
   i2c_index = (dcb-i2c_default_indices  0xf0)  4;
  
 - if (i2c_index  DCB_MAX_NUM_I2C_ENTRIES) {
 + if (i2c_index = DCB_MAX_NUM_I2C_ENTRIES) {
   NV_ERROR(dev, invalid i2c_index 0x%x\n, i2c_index);
   return NULL;
   }


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH -next] nouveau: fix acpi_lid_open undefined

[Ben Skeggs]

On Sun, 2010-05-23 at 17:00 -0700, Randy Dunlap wrote:
 From: Randy Dunlap randy.dun...@oracle.com
 
 When CONFIG_ACPI_BUTTON=m (and probably when ACPI_BUTTON is not enabled)
 and NOUVEAU is built-in (not as a loadable module):
 
 nouveau_connector.c:(.text+0xe17ce): undefined reference to `acpi_lid_open'
 
 Signed-off-by: Randy Dunlap randy.dun...@oracle.com
Acked-by: Ben Skeggs bske...@redhat.com

 Cc: David Airlie airl...@linux.ie
 Cc: dri-devel@lists.freedesktop.org
 ---
  drivers/gpu/drm/nouveau/nouveau_connector.c |3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 --- linux-next-20100522.orig/drivers/gpu/drm/nouveau/nouveau_connector.c
 +++ linux-next-20100522/drivers/gpu/drm/nouveau/nouveau_connector.c
 @@ -241,7 +241,8 @@ nouveau_connector_detect(struct drm_conn
   if (nv_encoder  nv_connector-native_mode) {
   unsigned status = connector_status_connected;
  
 -#ifdef CONFIG_ACPI
 +#if defined(CONFIG_ACPI_BUTTON) || \
 + (defined(CONFIG_ACPI_BUTTON_MODULE)  defined(MODULE))
   if (!nouveau_ignorelid  !acpi_lid_open())
   status = connector_status_unknown;
  #endif


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel