* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
@ 2010-11-16 17:05 Sedat Dilek
2010-11-16 17:30 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Sedat Dilek @ 2010-11-16 17:05 UTC (permalink / raw)
To: DRI; +Cc: daniel.vetter, Arnd Bergmann
[ CCing Arnd Bergmann ]
Hi,
I have tested both patchsets from Daniel (see [1] and [2]) again on a
Radeon RV250 in a none-BKL-config and it looks like
agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch
is the culprit in combination with the below listed drm patches.
I have switched to a "normal" DDX (w/o pageflip-support).
The OpenArena benchmarks is now between 17.5 - 18.6 [fps] @1024x768 resolution.
( w/o below patchset: 840 frames 50.6 seconds 16.6 fps 12.0/60.2/267.0/21.3 ms )
Kind Regards,
- Sedat -
[1] http://lists.freedesktop.org/archives/dri-devel/2010-November/005420.html
[2] http://lists.freedesktop.org/archives/dri-devel/2010-November/005441.html
$ cd $HOME/src/linux-2.6/linux-2.6.37-rc2/debian/build/source_i386_none/
$ cat .pc/applied-patches
danvet-drm-for-sedat-dilek/0001-drm-nouveau-don-t-munge-in-drm_mm-internals.patch
danvet-drm-for-sedat-dilek/0002-drm_mm-add-support-for-range-restricted-fair-lru-sca.patch
danvet-drm-for-sedat-dilek/0003-drm-mm-track-free-areas-implicitly.patch
danvet-drm-for-sedat-dilek/0004-drm-mm-extract-node-insert-helper-functions.patch
danvet-drm-for-sedat-dilek/0005-drm-mm-add-api-for-embedding-struct-drm_mm_node.patch
danvet-drm-for-sedat-dilek/0006-drm-mm-add-helper-to-unwind-scan-state.patch
danvet-embed-drm_gem_object-into-radeon_bo/1-3-drm-radeon-embed-struct-drm_gem_object.patch
danvet-embed-drm_gem_object-into-radeon_bo/2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
danvet-embed-drm_gem_object-into-radeon_bo/3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch
drm-vblank-timestamping/0001-drm-vblank-Add-support-for-precise-vblank-timestampi.patch
drm-vblank-timestamping/0002-drm-radeon-Add-support-for-precise-vblank-timestampi.patch
for-drm-radeon-testing/drm-radeon-kms-enable-writeback-on-radeon-AGP-boards.patch
$ cd $HOME/src/mesa/
$ ./scripts/run_openarena-benchmark.sh
840 frames 46.5 seconds 18.1 fps 10.0/55.3/147.0/18.9 ms
$ grep OK LATEST_linux-2.6/logs/setup_linux-2.6_git0.sd.1.log
(+) OK bkl-config/0002-drm-i810-remove-the-BKL.patch
(+) OK bkl-config/0003-staging-stradis-mark-as-depends-on-BKL.patch
(+) OK bkl-config/0004-BKL-remove-extraneous-include-smp_lock.h.patch
(+) OK bkl-config/0005-BKL-remove-references-to-lock_kernel-from-comments.patch
(+) OK bkl-config/0006-BKL-disable-by-default.patch
(+) OK bkl-config/0007-BKL-mark-lock_kernel-as-deprecated.patch
(+) OK bkl-config/0008-BKL-move-CONFIG_BKL-to-staging.patch
(+) OK debian/version.patch
(+) OK debian/kernelvariables-2.6.37.patch
(+) OK debian/doc-build-parallel.patch
(+) OK bugfix/ia64/hardcode-arch-script-output.patch
(+) OK bugfix/mips/disable-advansys.patch
(+) OK bugfix/arm/disable-scsi_acard.patch
(+) OK debian/mips-disable-werror.patch
(+) OK bugfix/powerpc/lpar-console.patch
(+) OK features/all/i915-autoload-without-CONFIG_DRM_I915_KMS.patch
(+) OK debian/arch-sh4-fix-uimage-build.patch
(+) OK bugfix/mips/mips-ide-flush-dcache.patch
(+) OK bugfix/all/qla4xxx-Fix-build-on-some-architectures-lacking-64-bit-I-O.patch
(+) OK bugfix/x86/Skip-looking-for-ioapic-overrides-when-ioapics-are-not-present.patch
( NOTE: bkl-config/0001-preempt-fix-kernel-build-with-CONFIG_BKL.patch
is already in Linux 2.6.37-rc2 )
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
2010-11-16 17:05 [PATCH 0/3] embed drm_gem_object into radeon_bo Sedat Dilek
@ 2010-11-16 17:30 ` Daniel Vetter
2010-11-16 19:37 ` Sedat Dilek
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2010-11-16 17:30 UTC (permalink / raw)
To: sedat.dilek; +Cc: daniel.vetter, Arnd Bergmann, DRI
On Tue, Nov 16, 2010 at 06:05:25PM +0100, Sedat Dilek wrote:
> I have tested both patchsets from Daniel (see [1] and [2]) again on a
> Radeon RV250 in a none-BKL-config and it looks like
>
> agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch
>
> is the culprit in combination with the below listed drm patches.
Likely a gem_bo->driver_private access. My patches set this to NULL to
catch conversion bugs.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
2010-11-16 17:30 ` Daniel Vetter
@ 2010-11-16 19:37 ` Sedat Dilek
2010-11-16 19:54 ` Sedat Dilek
0 siblings, 1 reply; 11+ messages in thread
From: Sedat Dilek @ 2010-11-16 19:37 UTC (permalink / raw)
To: Daniel Vetter; +Cc: daniel.vetter, Arnd Bergmann, DRI
[-- Attachment #1: Type: text/plain, Size: 2280 bytes --]
On Tue, Nov 16, 2010 at 6:30 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Nov 16, 2010 at 06:05:25PM +0100, Sedat Dilek wrote:
>> I have tested both patchsets from Daniel (see [1] and [2]) again on a
>> Radeon RV250 in a none-BKL-config and it looks like
>>
>> agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch
>>
>> is the culprit in combination with the below listed drm patches.
> Likely a gem_bo->driver_private access. My patches set this to NULL to
> catch conversion bugs.
> -Daniel
> --
> Daniel Vetter
> Mail: daniel@ffwll.ch
> Mobile: +41 (0)79 365 57 48
>
[ CCing Alex Deucher ]
With the attached diff to the original patch from [1], OpenArena works
with pageflip-support for radeon-KMS.
Unfortunately, there is a drop in fps from 18.5 down to 13.5.
- Sedat -
[1] http://people.freedesktop.org/~agd5f/pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch
$ cd ~/src/linux-2.6/linux-2.6.37-rc2/debian/build/source_i386_none/
$ cat .pc/applied-patches
danvet-drm-for-sedat-dilek/0001-drm-nouveau-don-t-munge-in-drm_mm-internals.patch
danvet-drm-for-sedat-dilek/0002-drm_mm-add-support-for-range-restricted-fair-lru-sca.patch
danvet-drm-for-sedat-dilek/0003-drm-mm-track-free-areas-implicitly.patch
danvet-drm-for-sedat-dilek/0004-drm-mm-extract-node-insert-helper-functions.patch
danvet-drm-for-sedat-dilek/0005-drm-mm-add-api-for-embedding-struct-drm_mm_node.patch
danvet-drm-for-sedat-dilek/0006-drm-mm-add-helper-to-unwind-scan-state.patch
danvet-embed-drm_gem_object-into-radeon_bo/1-3-drm-radeon-embed-struct-drm_gem_object.patch
danvet-embed-drm_gem_object-into-radeon_bo/2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
danvet-embed-drm_gem_object-into-radeon_bo/3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch
drm-vblank-timestamping/0001-drm-vblank-Add-support-for-precise-vblank-timestampi.patch
drm-vblank-timestamping/0002-drm-radeon-Add-support-for-precise-vblank-timestampi.patch
for-drm-radeon-testing/drm-radeon-kms-enable-writeback-on-radeon-AGP-boards.patch
agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support-for-danvet-v2.patch
$ cd ~/src/mesa/
$ ./scripts/run_openarena-benchmark.sh
840 frames 62.6 seconds 13.4 fps 16.0/74.5/224.0/19.7 ms
[-- Attachment #2: agd5f-pflip.diff --]
[-- Type: text/x-diff, Size: 841 bytes --]
diff -Naur 0002-drm-radeon-kms-add-pageflip-ioctl-support.patch 0002-drm-radeon-kms-add-pageflip-ioctl-support-for-danvet-v2.patch
--- 0002-drm-radeon-kms-add-pageflip-ioctl-support.patch 2010-10-31 18:53:38.000000000 +0100
+++ 0002-drm-radeon-kms-add-pageflip-ioctl-support-for-danvet-v2.patch 2010-11-16 20:07:12.380393000 +0100
@@ -1319,7 +1319,7 @@
+ new_radeon_fb = to_radeon_framebuffer(fb);
+ /* schedule unpin of the old buffer */
+ obj = old_radeon_fb->obj;
-+ rbo = obj->driver_private;
++ rbo = gem_to_radeon_bo(obj);
+ work->old_rbo = rbo;
+ INIT_WORK(&work->work, radeon_unpin_work_func);
+
@@ -1338,7 +1338,7 @@
+
+ /* pin the new buffer */
+ obj = new_radeon_fb->obj;
-+ rbo = obj->driver_private;
++ rbo = gem_to_radeon_bo(obj);
+ r = radeon_bo_reserve(rbo, false);
+ if (unlikely(r != 0))
+ goto pflip_cleanup;
[-- Attachment #3: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
2010-11-16 19:37 ` Sedat Dilek
@ 2010-11-16 19:54 ` Sedat Dilek
0 siblings, 0 replies; 11+ messages in thread
From: Sedat Dilek @ 2010-11-16 19:54 UTC (permalink / raw)
To: Daniel Vetter; +Cc: daniel.vetter, Arnd Bergmann, DRI
[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]
On Tue, Nov 16, 2010 at 8:37 PM, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> On Tue, Nov 16, 2010 at 6:30 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Tue, Nov 16, 2010 at 06:05:25PM +0100, Sedat Dilek wrote:
>>> I have tested both patchsets from Daniel (see [1] and [2]) again on a
>>> Radeon RV250 in a none-BKL-config and it looks like
>>>
>>> agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch
>>>
>>> is the culprit in combination with the below listed drm patches.
>> Likely a gem_bo->driver_private access. My patches set this to NULL to
>> catch conversion bugs.
>> -Daniel
>> --
>> Daniel Vetter
>> Mail: daniel@ffwll.ch
>> Mobile: +41 (0)79 365 57 48
>>
>
> [ CCing Alex Deucher ]
>
> With the attached diff to the original patch from [1], OpenArena works
> with pageflip-support for radeon-KMS.
> Unfortunately, there is a drop in fps from 18.5 down to 13.5.
>
> - Sedat -
>
> [1] http://people.freedesktop.org/~agd5f/pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch
>
> $ cd ~/src/linux-2.6/linux-2.6.37-rc2/debian/build/source_i386_none/
>
> $ cat .pc/applied-patches
> danvet-drm-for-sedat-dilek/0001-drm-nouveau-don-t-munge-in-drm_mm-internals.patch
> danvet-drm-for-sedat-dilek/0002-drm_mm-add-support-for-range-restricted-fair-lru-sca.patch
> danvet-drm-for-sedat-dilek/0003-drm-mm-track-free-areas-implicitly.patch
> danvet-drm-for-sedat-dilek/0004-drm-mm-extract-node-insert-helper-functions.patch
> danvet-drm-for-sedat-dilek/0005-drm-mm-add-api-for-embedding-struct-drm_mm_node.patch
> danvet-drm-for-sedat-dilek/0006-drm-mm-add-helper-to-unwind-scan-state.patch
> danvet-embed-drm_gem_object-into-radeon_bo/1-3-drm-radeon-embed-struct-drm_gem_object.patch
> danvet-embed-drm_gem_object-into-radeon_bo/2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
> danvet-embed-drm_gem_object-into-radeon_bo/3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch
> drm-vblank-timestamping/0001-drm-vblank-Add-support-for-precise-vblank-timestampi.patch
> drm-vblank-timestamping/0002-drm-radeon-Add-support-for-precise-vblank-timestampi.patch
> for-drm-radeon-testing/drm-radeon-kms-enable-writeback-on-radeon-AGP-boards.patch
> agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support-for-danvet-v2.patch
>
> $ cd ~/src/mesa/
>
> $ ./scripts/run_openarena-benchmark.sh
> 840 frames 62.6 seconds 13.4 fps 16.0/74.5/224.0/19.7 ms
>
Attached are all the patches I used with quilt-series file and kernel-config.
- Sedat -
[-- Attachment #2: for-danvet_patches.tar.xz --]
[-- Type: application/octet-stream, Size: 39512 bytes --]
[-- Attachment #3: for-danvet_patches.tar.xz.sha256sum --]
[-- Type: application/octet-stream, Size: 92 bytes --]
626533723a3828b5d54303216fe81e5e28bb9fa8a5b30dd9cd1fb63d37df04ed for-danvet_patches.tar.xz
[-- Attachment #4: bkl-config.tar.xz --]
[-- Type: application/octet-stream, Size: 40432 bytes --]
[-- Attachment #5: bkl-config.tar.xz.sha256sum --]
[-- Type: application/octet-stream, Size: 84 bytes --]
e8ccf66b3eda0a33c87bf025491493a7600a3234c7ce0db77a8856faeb818157 bkl-config.tar.xz
[-- Attachment #6: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/3] embed drm_gem_object into radeon_bo
@ 2010-11-28 0:29 Sedat Dilek
0 siblings, 0 replies; 11+ messages in thread
From: Sedat Dilek @ 2010-11-28 0:29 UTC (permalink / raw)
To: daniel.vetter, DRI
Hi Daniel,
I have tested this upgraded patchset again with linux-next
(next-20101126), they work fine.
Can you next time label the complete series as "v2": [PATCH 0/3 v2]
embed drm_gem_object into radeon_bo (don't ask me if git can create
this automatically for you).
Feel free to add:
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
Unfortunately, with
drm-radeon-kms-improve-pflip-precision-on-r1xx-r4xx.patch from [1] in
addition and running Eric Anholt's OpenArena benchmark... it is really
smoother with pflip now but after benchmark I cannot return to
desktop.
That is a problem not of your patchset, Daniel. I already reported on
IRC to Alex.
Hope to see also your embed-gtt-space patchset soonish in drm-next (I
have still the patches from [2] in my kernel patch-series).
Kind Regards,
- Sedat -
[1] https://patchwork.kernel.org/patch/348981/
[2] http://cgit.freedesktop.org/~danvet/drm/log/?h=for-sedat-dilek
$ cd src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/
$ cat .pc/applied-patches
danvet-embed-drm_gem_object-into-radeon_bo/1-3-drm-radeon-embed-struct-drm_gem_object.patch
danvet-embed-drm_gem_object-into-radeon_bo/2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
danvet-embed-drm_gem_object-into-radeon_bo/3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch
$ ls -l patches/danvet-embed-drm_gem_object-into-radeon_bo/insgesamt 32
-rw-r--r-- 1 sd sd 12520 28. Nov 00:51
1-3-drm-radeon-embed-struct-drm_gem_object.patch
-rw-r--r-- 1 sd sd 11560 28. Nov 00:53
2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
-rw-r--r-- 1 sd sd 2556 28. Nov 00:53
3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/3] embed drm_gem_object into radeon_bo
@ 2010-11-27 9:47 Daniel Vetter
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2010-11-27 9:47 UTC (permalink / raw)
To: airlied; +Cc: Daniel Vetter, dri-devel
Hi Dave,
As promised rebased on top of latest drm-next to resolve a conflict the
pageflipping code.
Tested on my agp rv570.
Please review and consider merging for -next.
Thanks, Daniel
Daniel Vetter (3):
drm/radeon: embed struct drm_gem_object
drm/radeon: introduce gem_to_radeon_bo helper
drm/radeon: kill radeon_bo->gobj pointer
drivers/gpu/drm/radeon/atombios_crtc.c | 8 ++--
drivers/gpu/drm/radeon/evergreen_blit_kms.c | 2 +-
drivers/gpu/drm/radeon/r600.c | 2 +-
drivers/gpu/drm/radeon/r600_blit_kms.c | 2 +-
drivers/gpu/drm/radeon/radeon.h | 3 +-
drivers/gpu/drm/radeon/radeon_benchmark.c | 4 +-
drivers/gpu/drm/radeon/radeon_cs.c | 2 +-
drivers/gpu/drm/radeon/radeon_device.c | 4 +-
drivers/gpu/drm/radeon/radeon_display.c | 4 +-
drivers/gpu/drm/radeon/radeon_fb.c | 10 +++---
drivers/gpu/drm/radeon/radeon_gart.c | 2 +-
drivers/gpu/drm/radeon/radeon_gem.c | 43 ++++++++++++---------------
drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 4 +-
drivers/gpu/drm/radeon/radeon_object.c | 24 +++++++--------
drivers/gpu/drm/radeon/radeon_object.h | 7 ++--
drivers/gpu/drm/radeon/radeon_ring.c | 4 +-
drivers/gpu/drm/radeon/radeon_test.c | 4 +-
drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
drivers/gpu/drm/radeon/rv770.c | 2 +-
19 files changed, 63 insertions(+), 70 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
@ 2010-11-15 10:20 Sedat Dilek
0 siblings, 0 replies; 11+ messages in thread
From: Sedat Dilek @ 2010-11-15 10:20 UTC (permalink / raw)
To: DRI; +Cc: daniel.vetter
Hi Daniel,
I have tried this patchset on a RV250 with linux-next (next-20101115)
in combination w/ patchset from "[PATCH 0/9] make struct drm_mm_node
embeddable" [1].
glxgears works nice.
2nd test-case is Eric Anholt's OpenArena benchmark.
The screen gets blank and system is unusable (cold start, poweroff-button).
Mesa is from master GIT branch:
$ cd ~/src/mesa
$ grep -A4 "git log" mesa.log
+ git log --pretty=short -1
commit 9cf25b3d1cd2910ae33e1faafa04629638bff0fe
Author: Marek Olšák <maraeo@gmail.com>
r300g: return shader caps from Draw for SWTCL vertex shaders
Daniel requested me to bisect:
bad: 3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch +
2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
good: 1-3-drm-radeon-embed-struct-drm_gem_object.patch
$cd ~/src/linux-2.6/linux-2.6.37-rc1/debian/build/source_i386_none
$ cat .pc/applied-patches
danvet-embed-drm_gem_object-into-radeon_bo/1-3-drm-radeon-embed-struct-drm_gem_object.patch
Looks like patch #2 is culprit according to Daniel.
Kind Regards,
- Sedat -
[1] http://lists.freedesktop.org/archives/dri-devel/2010-November/005420.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/3] embed drm_gem_object into radeon_bo
@ 2010-11-13 20:57 Daniel Vetter
2010-11-15 7:25 ` Thomas Hellstrom
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2010-11-13 20:57 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
Hi all,
This patch series embeds drm_gem_object into struct radeon_bo and adjusts
any references. I've decided against embedding the gem stuff into struct
ttm_bo because
- vmwgfx doesn't use gem and
- ttm is used for implementing the memory management, whereas gem provides
the userspace interface (I know, there's some overlap there, but that's
not really a new problem).
Please review and consider merging for -next.
Yours, Daniel
Daniel Vetter (3):
drm/radeon: embed struct drm_gem_object
drm/radeon: introduce gem_to_radeon_bo helper
drm/radeon: kill radeon_bo->gobj pointer
drivers/gpu/drm/radeon/atombios_crtc.c | 8 ++--
drivers/gpu/drm/radeon/evergreen_blit_kms.c | 2 +-
drivers/gpu/drm/radeon/r600.c | 2 +-
drivers/gpu/drm/radeon/r600_blit_kms.c | 2 +-
drivers/gpu/drm/radeon/radeon.h | 3 +-
drivers/gpu/drm/radeon/radeon_benchmark.c | 4 +-
drivers/gpu/drm/radeon/radeon_cs.c | 2 +-
drivers/gpu/drm/radeon/radeon_device.c | 4 +-
drivers/gpu/drm/radeon/radeon_fb.c | 10 +++---
drivers/gpu/drm/radeon/radeon_gart.c | 2 +-
drivers/gpu/drm/radeon/radeon_gem.c | 43 ++++++++++++---------------
drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 4 +-
drivers/gpu/drm/radeon/radeon_object.c | 24 +++++++--------
drivers/gpu/drm/radeon/radeon_object.h | 2 +-
drivers/gpu/drm/radeon/radeon_ring.c | 4 +-
drivers/gpu/drm/radeon/radeon_test.c | 4 +-
drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
drivers/gpu/drm/radeon/rv770.c | 2 +-
18 files changed, 59 insertions(+), 65 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
2010-11-13 20:57 Daniel Vetter
@ 2010-11-15 7:25 ` Thomas Hellstrom
2010-11-15 18:45 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Thomas Hellstrom @ 2010-11-15 7:25 UTC (permalink / raw)
To: Daniel Vetter; +Cc: dri-devel
Hi, Daniel,
My main concerns previously for embedding GEM objects as user-space
references for TTM has been twofold and implementation specific.
1) The locking has been using global mutexes where local spin- or RCU
locks have been more appropriate. It looks like this has finally been /
is finally going to be addressed.
2) The gem object is too specialized for general purpose use:
a) The shmem member has no natural use with TTM except possibly as a
persistent swap storage, but in an ideal world, TTM would talk to the
swap cache directly so in the longer term there is no need for the shmem
object at all.
b) Implementations may want to use other user-space visible objects than
buffer objects:
For example fence objects or CPU synchronization objects. The common
traits of / operations on these are user-space visibility, inter-process
visibility, refcounting and destruction when the relevant file is closed.
Hence a class that provides only the user-space handles, naming (flink),
refcounting and registering with a file handle would be the best choice
of a "base" class. Traditional Gem objects could derive from those and
provide any extra *generic* members needed for buffer objects.
This doesn't really affect your work though. Just some comments on why
vmwgfx don't use GEM objects by default and how they could be made
optimal for TTM-aware drivers.
Thanks,
/Thomas
On 11/13/2010 09:57 PM, Daniel Vetter wrote:
> Hi all,
>
> This patch series embeds drm_gem_object into struct radeon_bo and adjusts
> any references. I've decided against embedding the gem stuff into struct
> ttm_bo because
> - vmwgfx doesn't use gem and
> - ttm is used for implementing the memory management, whereas gem provides
> the userspace interface (I know, there's some overlap there, but that's
> not really a new problem).
>
> Please review and consider merging for -next.
>
> Yours, Daniel
>
> Daniel Vetter (3):
> drm/radeon: embed struct drm_gem_object
> drm/radeon: introduce gem_to_radeon_bo helper
> drm/radeon: kill radeon_bo->gobj pointer
>
> drivers/gpu/drm/radeon/atombios_crtc.c | 8 ++--
> drivers/gpu/drm/radeon/evergreen_blit_kms.c | 2 +-
> drivers/gpu/drm/radeon/r600.c | 2 +-
> drivers/gpu/drm/radeon/r600_blit_kms.c | 2 +-
> drivers/gpu/drm/radeon/radeon.h | 3 +-
> drivers/gpu/drm/radeon/radeon_benchmark.c | 4 +-
> drivers/gpu/drm/radeon/radeon_cs.c | 2 +-
> drivers/gpu/drm/radeon/radeon_device.c | 4 +-
> drivers/gpu/drm/radeon/radeon_fb.c | 10 +++---
> drivers/gpu/drm/radeon/radeon_gart.c | 2 +-
> drivers/gpu/drm/radeon/radeon_gem.c | 43 ++++++++++++---------------
> drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 4 +-
> drivers/gpu/drm/radeon/radeon_object.c | 24 +++++++--------
> drivers/gpu/drm/radeon/radeon_object.h | 2 +-
> drivers/gpu/drm/radeon/radeon_ring.c | 4 +-
> drivers/gpu/drm/radeon/radeon_test.c | 4 +-
> drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
> drivers/gpu/drm/radeon/rv770.c | 2 +-
> 18 files changed, 59 insertions(+), 65 deletions(-)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
2010-11-15 7:25 ` Thomas Hellstrom
@ 2010-11-15 18:45 ` Daniel Vetter
2010-11-15 20:48 ` Thomas Hellstrom
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2010-11-15 18:45 UTC (permalink / raw)
To: Thomas Hellstrom; +Cc: Daniel Vetter, dri-devel
Hi Thomas,
Thanks for your comments about ttm and vmwgfx. Some of my own ideas about
where this might all be heading below.
On Mon, Nov 15, 2010 at 08:25:16AM +0100, Thomas Hellstrom wrote:
> Hi, Daniel,
>
> My main concerns previously for embedding GEM objects as user-space
> references for TTM has been twofold and implementation specific.
>
> 1) The locking has been using global mutexes where local spin- or
> RCU locks have been more appropriate. It looks like this has finally
> been / is finally going to be addressed.
gem object lookup / insert has always (iirc) been protected by a spinlock.
drm/i915 is just a bit inefficient with lookups, hence this spinlock
showing up in profiling.
> 2) The gem object is too specialized for general purpose use:
> a) The shmem member has no natural use with TTM except possibly as a
> persistent swap storage, but in an ideal world, TTM would talk to
> the swap cache directly so in the longer term there is no need for
> the shmem object at all.
Chris Wilson is working on a gem_vm for i915 that employs a address_space
per bo and directly manages swap entries and has its own page allocator
(actualy cflushed page cache). I haven't yet looked into this closely
(especially the ttm part), but this might (at least in parts) be shareable
between i915 and ttm. At least it gets rid of the shmfs inode in struct
drm_gem_object!
> b) Implementations may want to use other user-space visible objects
> than buffer objects:
> For example fence objects or CPU synchronization objects. The common
> traits of / operations on these are user-space visibility,
> inter-process visibility, refcounting and destruction when the
> relevant file is closed.
>
> Hence a class that provides only the user-space handles, naming
> (flink), refcounting and registering with a file handle would be the
> best choice of a "base" class. Traditional Gem objects could derive
> from those and provide any extra *generic* members needed for buffer
> objects.
>
> This doesn't really affect your work though. Just some comments on
> why vmwgfx don't use GEM objects by default and how they could be
> made optimal for TTM-aware drivers.
Yeah, I've noticed that vmwgfx is the (sometimes) sole user of many of the
more fancy stuff in ttm. And I also don't like the duplication between gem
and ttm. My plan (i.e. wishful thinking) is to have a common set of helper
functions that can be shared between i915, radeon and nouveau and any
other driver with a gem-like interface (i.e. buffer-objects + execbuffer,
gpu<->cpu sync abstracted away by the kernel). Leaving the cracy stuff for
drivers that need it (vmwgfx). Nothing concrete though (besides a new
testing rig to start hacking on nouveau), I'll intend to simply plow
along, hunting for common patterns.
> Thanks,
> /Thomas
Yours, Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
2010-11-15 18:45 ` Daniel Vetter
@ 2010-11-15 20:48 ` Thomas Hellstrom
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Hellstrom @ 2010-11-15 20:48 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, dri-devel
On 11/15/2010 07:45 PM, Daniel Vetter wrote:
> Hi Thomas,
>
> Thanks for your comments about ttm and vmwgfx. Some of my own ideas about
> where this might all be heading below.
>
> On Mon, Nov 15, 2010 at 08:25:16AM +0100, Thomas Hellstrom wrote:
>
>> Hi, Daniel,
>>
>> My main concerns previously for embedding GEM objects as user-space
>> references for TTM has been twofold and implementation specific.
>>
>> 1) The locking has been using global mutexes where local spin- or
>> RCU locks have been more appropriate. It looks like this has finally
>> been / is finally going to be addressed.
>>
> gem object lookup / insert has always (iirc) been protected by a spinlock.
> drm/i915 is just a bit inefficient with lookups, hence this spinlock
> showing up in profiling.
>
>
>> 2) The gem object is too specialized for general purpose use:
>> a) The shmem member has no natural use with TTM except possibly as a
>> persistent swap storage, but in an ideal world, TTM would talk to
>> the swap cache directly so in the longer term there is no need for
>> the shmem object at all.
>>
> Chris Wilson is working on a gem_vm for i915 that employs a address_space
> per bo and directly manages swap entries and has its own page allocator
> (actualy cflushed page cache). I haven't yet looked into this closely
> (especially the ttm part), but this might (at least in parts) be shareable
> between i915 and ttm. At least it gets rid of the shmfs inode in struct
> drm_gem_object!
>
>
>> b) Implementations may want to use other user-space visible objects
>> than buffer objects:
>> For example fence objects or CPU synchronization objects. The common
>> traits of / operations on these are user-space visibility,
>> inter-process visibility, refcounting and destruction when the
>> relevant file is closed.
>>
>> Hence a class that provides only the user-space handles, naming
>> (flink), refcounting and registering with a file handle would be the
>> best choice of a "base" class. Traditional Gem objects could derive
>> from those and provide any extra *generic* members needed for buffer
>> objects.
>>
>> This doesn't really affect your work though. Just some comments on
>> why vmwgfx don't use GEM objects by default and how they could be
>> made optimal for TTM-aware drivers.
>>
> Yeah, I've noticed that vmwgfx is the (sometimes) sole user of many of the
> more fancy stuff in ttm. And I also don't like the duplication between gem
> and ttm. My plan (i.e. wishful thinking) is to have a common set of helper
> functions that can be shared between i915, radeon and nouveau and any
> other driver with a gem-like interface (i.e. buffer-objects + execbuffer,
> gpu<->cpu sync abstracted away by the kernel). Leaving the cracy stuff for
> drivers that need it (vmwgfx). Nothing concrete though (besides a new
> testing rig to start hacking on nouveau), I'll intend to simply plow
> along, hunting for common patterns.
>
Sounds good. I actually think the only thing which is currently
vmwgfx-specific are the ttm_objects and the ttm_locks. TTM objects could
really be replaced by gem objects with the bo-specific part stripped, as
discussed above. And then there is the ttm lock to allow concurrent
execbufs running.
/Thomas
>
>> Thanks,
>> /Thomas
>>
> Yours, Daniel
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-11-28 0:29 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-16 17:05 [PATCH 0/3] embed drm_gem_object into radeon_bo Sedat Dilek
2010-11-16 17:30 ` Daniel Vetter
2010-11-16 19:37 ` Sedat Dilek
2010-11-16 19:54 ` Sedat Dilek
-- strict thread matches above, loose matches on Subject: below --
2010-11-28 0:29 Sedat Dilek
2010-11-27 9:47 Daniel Vetter
2010-11-15 10:20 Sedat Dilek
2010-11-13 20:57 Daniel Vetter
2010-11-15 7:25 ` Thomas Hellstrom
2010-11-15 18:45 ` Daniel Vetter
2010-11-15 20:48 ` Thomas Hellstrom
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).