Re: [PATCH] drm/radeon/bo: add some fallback placements for VRAM only objects. (v2)

2010-04-27 Thread Michel Dänzer
On Die, 2010-04-27 at 12:34 +1000, Dave Airlie wrote: 
 From: Dave Airlie airl...@redhat.com
 
 On constrained r100 systems compiz would fail to start due to a lack
 of memory, we can just fallback place the objects rather than completely
 failing it works a lot better.
 
 v2:
 fixes issue identified by Michel when pinning could happen in a busy 
 placement domain.

[...]


 diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
 b/drivers/gpu/drm/radeon/radeon_object.c
 index 6a8617b..ab3bc7b 100644
 --- a/drivers/gpu/drm/radeon/radeon_object.c
 +++ b/drivers/gpu/drm/radeon/radeon_object.c
 @@ -110,7 +114,7 @@ int radeon_bo_create(struct radeon_device *rdev, struct 
 drm_gem_object *gobj,
   bo-surface_reg = -1;
   INIT_LIST_HEAD(bo-list);
  
 - radeon_ttm_placement_from_domain(bo, domain);
 + radeon_ttm_placement_from_domain(bo, domain, false);
   /* Kernel allocation are uninterruptible */
   r = ttm_bo_init(rdev-mman.bdev, bo-tbo, size, type,
   bo-placement, 0, 0, !kernel, NULL, size,

Not sure it's a good idea to unconditionally allow BOs to be created in
GTT if userspace requested VRAM. E.g. this would at least defeat the
purpose of UploadToScreen if not turn it into pure overhead.


 @@ -325,10 +329,10 @@ int radeon_bo_list_validate(struct list_head *head)
   if (!bo-pin_count) {
   if (lobj-wdomain) {
   radeon_ttm_placement_from_domain(bo,
 - lobj-wdomain);
 +  lobj-wdomain, 
 false);
   } else {
   radeon_ttm_placement_from_domain(bo,
 - lobj-rdomain);
 +  lobj-rdomain, 
 false);
   }
   r = ttm_bo_validate(bo-tbo, bo-placement,
   true, false, false);

How about something like the below (completely untested) instead? This
should try to respect the userspace request first and only allow falling
back from VRAM to GTT if that failed. (Maybe the new bool parameter
shouldn't be called 'pinned' then but something like
'allow_vram_fallback' and its sense inverted)

diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
b/drivers/gpu/drm/radeon/radeon_object.c
index d10f246..b6f5177 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -322,17 +322,25 @@ int radeon_bo_list_validate(struct list_head *head)
list_for_each_entry(lobj, head, list) {
bo = lobj-bo;
if (!bo-pin_count) {
+   bool pinned = true;
+
+retry:
if (lobj-wdomain) {
radeon_ttm_placement_from_domain(bo,
-   lobj-wdomain);
+lobj-wdomain, 
pinned);
} else {
radeon_ttm_placement_from_domain(bo,
-   lobj-rdomain);
+lobj-rdomain, 
pinned);
}
r = ttm_bo_validate(bo-tbo, bo-placement,
true, false);
-   if (unlikely(r))
+   if (unlikely(r)) {
+   if (pinned) {
+   pinned = false;
+   goto retry;
+   }
return r;
+   }
}
lobj-gpu_offset = radeon_bo_gpu_offset(bo);
lobj-tiling_flags = bo-tiling_flags;


 @@ -516,7 +520,7 @@ int radeon_bo_fault_reserve_notify(struct 
 ttm_buffer_object *bo)
   offset = bo-mem.mm_node-start  PAGE_SHIFT;
   if ((offset + size)  rdev-mc.visible_vram_size) {
   /* hurrah the memory is not visible ! */
 - radeon_ttm_placement_from_domain(rbo, 
 RADEON_GEM_DOMAIN_VRAM);
 + radeon_ttm_placement_from_domain(rbo, 
 RADEON_GEM_DOMAIN_VRAM, false);
   rbo-placement.lpfn = rdev-mc.visible_vram_size  
 PAGE_SHIFT;
   r = ttm_bo_validate(bo, rbo-placement, false, true, 
 false);
   if (unlikely(r != 0))

This will impose rbo-placement.lpfn for GTT as well, but it's only
required for VRAM.


-- 
Earthling Michel Dänzer   |http://www.vmware.com
Libre software enthusiast |  Debian, X and DRI developer


[Bug 15181] Radeon: *ERROR* Unable to locate a BIOS ROM

2010-04-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=15181





--- Comment #11 from Jérôme Glisse gli...@freedesktop.org  2010-04-27 
08:47:15 ---
Please attach drivers/gpu/drm/radeon/radeon_bios.c it seems different from the
one upstream and it's useless to add warn to rom.c.

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.
--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [PATCH] drm/radeon/bo: add some fallback placements for VRAM only objects. (v2)

2010-04-27 Thread Michel Dänzer
[ Moving to the new list ]

On Die, 2010-04-27 at 12:34 +1000, Dave Airlie wrote: 
 From: Dave Airlie airl...@redhat.com
 
 On constrained r100 systems compiz would fail to start due to a lack
 of memory, we can just fallback place the objects rather than completely
 failing it works a lot better.
 
 v2:
 fixes issue identified by Michel when pinning could happen in a busy 
 placement domain.

[...]


 diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
 b/drivers/gpu/drm/radeon/radeon_object.c
 index 6a8617b..ab3bc7b 100644
 --- a/drivers/gpu/drm/radeon/radeon_object.c
 +++ b/drivers/gpu/drm/radeon/radeon_object.c
 @@ -64,17 +64,21 @@ bool radeon_ttm_bo_is_radeon_bo(struct ttm_buffer_object 
 *bo)
   return false;
  }
  
 -void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
 +void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain, 
 bool pinned)
  {
 - u32 c = 0;
 + u32 c = 0, b = 0;
  
   rbo-placement.fpfn = 0;
   rbo-placement.lpfn = 0;
   rbo-placement.placement = rbo-placements;
 - rbo-placement.busy_placement = rbo-placements;
 + rbo-placement.busy_placement = rbo-busy_placements;
   if (domain  RADEON_GEM_DOMAIN_VRAM)
   rbo-placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
   TTM_PL_FLAG_VRAM;
 + /* add busy placement to TTM if VRAM is only option */
 + if (domain == RADEON_GEM_DOMAIN_VRAM  pinned == false) {
 + rbo-busy_placements[b++] = TTM_PL_MASK_CACHING | 
 TTM_PL_FLAG_TT;
 + }
   if (domain  RADEON_GEM_DOMAIN_GTT)
   rbo-placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
   if (domain  RADEON_GEM_DOMAIN_CPU)
 @@ -82,7 +86,7 @@ void radeon_ttm_placement_from_domain(struct radeon_bo 
 *rbo, u32 domain)
   if (!c)
   rbo-placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
   rbo-placement.num_placement = c;
 - rbo-placement.num_busy_placement = c;
 + rbo-placement.num_busy_placement = b;
  }
  
  int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,

BTW, this means there won't be any busy placements if (domain !=
RADEON_GEM_DOMAIN_VRAM || pinned). Not sure if that's a problem.


-- 
Earthling Michel Dänzer   |http://www.vmware.com
Libre software enthusiast |  Debian, X and DRI developer

--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[Bug 15166] Changing brightness of backlight freezes kernel with radeon kms enabled.

2010-04-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=15166





--- Comment #38 from Aurélien Couderc zecou...@free.fr  2010-04-27 16:51:41 
---
To answer Alex Deucher's questions:
#32 NoAccel doen't solve the problem (driver 6.13.0)
#33 disabling KMS solves the problem and ums+acceleration works fine

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.
--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [PATCH 5/7] drm/i915: use vblank and vsync interrupts on 945

2010-04-27 Thread Jesse Barnes
On Fri, 26 Mar 2010 11:07:19 -0700
Jesse Barnes jbar...@virtuousgeek.org wrote:

 On 945, vblank delivery alone seems unreliable.  The PIPE*STAT bits get
 set correctly, but interrupts occur at a low frequency relative to
 refresh.  If we enable VSYNC interrupts as well however (even though we
 only check for VBLANK interrupts when handling) we get the right
 frequency.  Increases OA performance on my AspireOne by about 300% with
 the new DRI2 bits, which rely on high frequency vblank events.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---

Ignore this patch; it's still broken.  Working on a fix now.

-- 
Jesse Barnes, Intel Open Source Technology Center

--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[Bug 15166] Changing brightness of backlight freezes kernel with radeon kms enabled.

2010-04-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=15166





--- Comment #39 from Alex Deucher alexdeuc...@gmail.com  2010-04-27 17:35:11 
---
Created an attachment (id=26161)
 -- (https://bugzilla.kernel.org/attachment.cgi?id=26161)
add accel parameter

This patch adds an accel parameter that allows you to disable acceleration in
the drm.  boot with radeon.accel=0 and see if you still get the hang.

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.

--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[Bug 15181] Radeon: *ERROR* Unable to locate a BIOS ROM

2010-04-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=15181





--- Comment #12 from Matteo rootki...@yahoo.it  2010-04-27 20:32:37 ---
Created an attachment (id=26165)
 -- (https://bugzilla.kernel.org/attachment.cgi?id=26165)
drivers/gpu/drm/radeon/radeon_bios.c

drivers/gpu/drm/radeon/radeon_bios.c

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.

--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[patch 2/5] drivers/gpu/drm/via/via_video.c: fix off by one issue

2010-04-27 Thread akpm
From: Dan Carpenter erro...@gmail.com

fx-lock is used as the index in dev_priv-decoder_queue[fx-lock]
which is an array of VIA_NR_XVMC_LOCKS elements.

Signed-off-by: Dan Carpenter erro...@gmail.com
Cc: David Airlie airl...@linux.ie
Signed-off-by: Andrew Morton a...@linux-foundation.org
---

 drivers/gpu/drm/via/via_video.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN 
drivers/gpu/drm/via/via_video.c~drivers-gpu-drm-via-via_videoc-fix-off-by-one-issue
 drivers/gpu/drm/via/via_video.c
--- 
a/drivers/gpu/drm/via/via_video.c~drivers-gpu-drm-via-via_videoc-fix-off-by-one-issue
+++ a/drivers/gpu/drm/via/via_video.c
@@ -75,7 +75,7 @@ int via_decoder_futex(struct drm_device 
 
DRM_DEBUG(\n);
 
-   if (fx-lock  VIA_NR_XVMC_LOCKS)
+   if (fx-lock = VIA_NR_XVMC_LOCKS)
return -EFAULT;
 
lock = (volatile int *)XVMCLOCKPTR(sAPriv, fx-lock);
_

--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[patch 1/5] gpu: vga_switcheroo, fix lock imbalance

2010-04-27 Thread akpm
From: Jiri Slaby jsl...@suse.cz

Stanse found that one error path in vga_switcheroo_debugfs_write omits to
unlock vgasr_mutex.  Fix that.

Signed-off-by: Jiri Slaby jsl...@suse.cz
Cc: Dave Airlie airl...@redhat.com
Signed-off-by: Andrew Morton a...@linux-foundation.org
---

 drivers/gpu/vga/vga_switcheroo.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff -puN 
drivers/gpu/vga/vga_switcheroo.c~gpu-vga_switcheroo-fix-lock-imbalance 
drivers/gpu/vga/vga_switcheroo.c
--- a/drivers/gpu/vga/vga_switcheroo.c~gpu-vga_switcheroo-fix-lock-imbalance
+++ a/drivers/gpu/vga/vga_switcheroo.c
@@ -276,8 +276,10 @@ vga_switcheroo_debugfs_write(struct file
 
mutex_lock(vgasr_mutex);
 
-   if (!vgasr_priv.active)
-   return -EINVAL;
+   if (!vgasr_priv.active) {
+   cnt = -EINVAL;
+   goto out;
+   }
 
/* pwr off the device not in use */
if (strncmp(usercmd, OFF, 3) == 0) {
_

--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[patch 3/5] drivers/gpu/drm/radeon/radeon_atombios.c: range check issues

2010-04-27 Thread akpm
From: Dan Carpenter erro...@gmail.com

This change makes the array larger, MAX_SUPPORTED_TV_TIMING_V1_2 is 3
and the original size MAX_SUPPORTED_TV_TIMING is 2.

Also there were checks that were off by one.

Signed-off-by: Dan Carpenter erro...@gmail.com
Cc: David Airlie airl...@linux.ie
Acked-by: Alex Deucher alexdeuc...@gmail.com
Signed-off-by: Andrew Morton a...@linux-foundation.org
---

 drivers/gpu/drm/radeon/atombios.h|2 +-
 drivers/gpu/drm/radeon/radeon_atombios.c |4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff -puN 
drivers/gpu/drm/radeon/atombios.h~drivers-gpu-drm-radeon-radeon_atombiosc-range-check-issues
 drivers/gpu/drm/radeon/atombios.h
--- 
a/drivers/gpu/drm/radeon/atombios.h~drivers-gpu-drm-radeon-radeon_atombiosc-range-check-issues
+++ a/drivers/gpu/drm/radeon/atombios.h
@@ -2912,7 +2912,7 @@ typedef struct _ATOM_ANALOG_TV_INFO_V1_2
   UCHARucTV_BootUpDefaultStandard; 
   UCHARucExt_TV_ASIC_ID;
   UCHARucExt_TV_ASIC_SlaveAddr;
-  ATOM_DTD_FORMAT  aModeTimings[MAX_SUPPORTED_TV_TIMING];
+  ATOM_DTD_FORMAT  aModeTimings[MAX_SUPPORTED_TV_TIMING_V1_2];
 }ATOM_ANALOG_TV_INFO_V1_2;
 
 typedef struct _ATOM_DPCD_INFO
diff -puN 
drivers/gpu/drm/radeon/radeon_atombios.c~drivers-gpu-drm-radeon-radeon_atombiosc-range-check-issues
 drivers/gpu/drm/radeon/radeon_atombios.c
--- 
a/drivers/gpu/drm/radeon/radeon_atombios.c~drivers-gpu-drm-radeon-radeon_atombiosc-range-check-issues
+++ a/drivers/gpu/drm/radeon/radeon_atombios.c
@@ -1264,7 +1264,7 @@ bool radeon_atom_get_tv_timings(struct r
switch (crev) {
case 1:
tv_info = (ATOM_ANALOG_TV_INFO *)(mode_info-atom_context-bios 
+ data_offset);
-   if (index  MAX_SUPPORTED_TV_TIMING)
+   if (index = MAX_SUPPORTED_TV_TIMING)
return false;
 
mode-crtc_htotal = 
le16_to_cpu(tv_info-aModeTimings[index].usCRTC_H_Total);
@@ -1302,7 +1302,7 @@ bool radeon_atom_get_tv_timings(struct r
break;
case 2:
tv_info_v1_2 = (ATOM_ANALOG_TV_INFO_V1_2 
*)(mode_info-atom_context-bios + data_offset);
-   if (index  MAX_SUPPORTED_TV_TIMING_V1_2)
+   if (index = MAX_SUPPORTED_TV_TIMING_V1_2)
return false;
 
dtd_timings = tv_info_v1_2-aModeTimings[index];
_

--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[patch 4/5] drivers/gpu/drm/drm_sysfs.c: sysfs files error handling

2010-04-27 Thread akpm
From: Dan Carpenter erro...@gmail.com

In the original code we used j as an iterator but we used i as an
index.

-   for (j = 0; j  i; j++)
-   device_remove_file(connector-kdev,
-  connector_attrs[i]);

Smatch complained about that because i was potentially passed the end of
the array.  Which makes sense if we should be using j there.

I also thought that we should remove the files for connector_attrs_opt1
but to do that I had to add separate iterators for connector_attrs and
connector_attrs_opt1.

Signed-off-by: Dan Carpenter erro...@gmail.com
Cc: David Airlie airl...@linux.ie
Cc: Greg Kroah-Hartman gre...@suse.de
Signed-off-by: Andrew Morton a...@linux-foundation.org
---

 drivers/gpu/drm/drm_sysfs.c |   21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff -puN 
drivers/gpu/drm/drm_sysfs.c~drivers-gpu-drm-drm_sysfsc-sysfs-files-error-handling
 drivers/gpu/drm/drm_sysfs.c
--- 
a/drivers/gpu/drm/drm_sysfs.c~drivers-gpu-drm-drm_sysfsc-sysfs-files-error-handling
+++ a/drivers/gpu/drm/drm_sysfs.c
@@ -354,7 +354,10 @@ static struct bin_attribute edid_attr = 
 int drm_sysfs_connector_add(struct drm_connector *connector)
 {
struct drm_device *dev = connector-dev;
-   int ret = 0, i, j;
+   int attr_cnt = 0;
+   int opt_cnt = 0;
+   int i;
+   int ret = 0;
 
/* We shouldn't get called more than once for the same connector */
BUG_ON(device_is_registered(connector-kdev));
@@ -377,8 +380,8 @@ int drm_sysfs_connector_add(struct drm_c
 
/* Standard attributes */
 
-   for (i = 0; i  ARRAY_SIZE(connector_attrs); i++) {
-   ret = device_create_file(connector-kdev, connector_attrs[i]);
+   for (attr_cnt = 0; attr_cnt  ARRAY_SIZE(connector_attrs); attr_cnt++) {
+   ret = device_create_file(connector-kdev, 
connector_attrs[attr_cnt]);
if (ret)
goto err_out_files;
}
@@ -394,8 +397,8 @@ int drm_sysfs_connector_add(struct drm_c
case DRM_MODE_CONNECTOR_SVIDEO:
case DRM_MODE_CONNECTOR_Component:
case DRM_MODE_CONNECTOR_TV:
-   for (i = 0; i  ARRAY_SIZE(connector_attrs_opt1); i++) {
-   ret = device_create_file(connector-kdev, 
connector_attrs_opt1[i]);
+   for (opt_cnt = 0; opt_cnt  
ARRAY_SIZE(connector_attrs_opt1); opt_cnt++) {
+   ret = device_create_file(connector-kdev, 
connector_attrs_opt1[opt_cnt]);
if (ret)
goto err_out_files;
}
@@ -414,10 +417,10 @@ int drm_sysfs_connector_add(struct drm_c
return 0;
 
 err_out_files:
-   if (i  0)
-   for (j = 0; j  i; j++)
-   device_remove_file(connector-kdev,
-  connector_attrs[i]);
+   for (i = 0; i  opt_cnt; i++)
+   device_remove_file(connector-kdev, connector_attrs_opt1[i]);
+   for (i = 0; i  attr_cnt; i++)
+   device_remove_file(connector-kdev, connector_attrs[i]);
device_unregister(connector-kdev);
 
 out:
_

--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[Bug 15166] Changing brightness of backlight freezes kernel with radeon kms enabled.

2010-04-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=15166





--- Comment #40 from Alex Deucher alexdeuc...@gmail.com  2010-04-28 03:38:58 
---
I managed to get remote access to an affected machine today, and to start off
with I tried Linus' git tree, bc113f151a73cb2195c2fb40d7d70acf8e2f9208, to be
exact.  And, low and behold, the backlight worked fine. Switched back to an
older kernel and it hung.  So it appears to be fixed, at least as of that
commit.  Can anyone else confirm and if so, help bisect to see what fixed it?

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are watching the assignee of the bug.

--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel