Re: [MMTests] IO metadata on XFS
On Mon, Jul 02, 2012 at 08:35:16PM +0100, Mel Gorman wrote: > Adding dri-devel and a few others because an i915 patch contributed to > the regression. > > On Mon, Jul 02, 2012 at 03:32:15PM +0100, Mel Gorman wrote: > > On Mon, Jul 02, 2012 at 02:32:26AM -0400, Christoph Hellwig wrote: > > > > It increases the CPU overhead (dirty_inode can be called up to 4 > > > > times per write(2) call, IIRC), so with limited numbers of > > > > threads/limited CPU power it will result in lower performance. Where > > > > you have lots of CPU power, there will be little difference in > > > > performance... > > > > > > When I checked it it could only be called twice, and we'd already > > > optimize away the second call. I'd defintively like to track down where > > > the performance changes happend, at least to a major version but even > > > better to a -rc or git commit. > > > > > > > By all means feel free to run the test yourself and run the bisection :) > > > > It's rare but on this occasion the test machine is idle so I started an > > automated git bisection. As you know the milage with an automated bisect > > varies so it may or may not find the right commit. Test machine is sandy so > > http://www.csn.ul.ie/~mel/postings/mmtests-20120424/global-dhp__io-metadata-xfs/sandy/comparison.html > > is the report of interest. The script is doing a full search between v3.3 > > and > > v3.4 for a point where average files/sec for fsmark-single drops below > > 25000. > > I did not limit the search to fs/xfs on the off-chance that it is an > > apparently unrelated patch that caused the problem. > > > > It was obvious very quickly that there were two distinct regression so I > ran two bisections. One led to a XFS and the other led to an i915 patch > that enables RC6 to reduce power usage. > > [aa464191: drm/i915: enable plain RC6 on Sandy Bridge by default] Doesn't seem to be the major cause of the regression. By itself, it has impact, but the majority comes from the XFS change... > [c999a223: xfs: introduce an allocation workqueue] Which indicates that there is workqueue scheduling issues, I think. The same amount of work is being done, but half of it is being pushed off into a workqueue to avoid stack overflow issues (*). I tested the above patch in anger on an 8p machine, similar to the machine you saw no regressions on, but the workload didn't drive it to being completely CPU bound (only about 90%) so the allocation work was probably always scheduled quickly. How many worker threads have been spawned on these machines that are showing the regression? What is the context switch rate on the machines whenteh test is running? Can you run latencytop to see if there is excessive starvation/wait times for allocation completion? A pert top profile comparison might be informative, too... (*) The stack usage below submit_bio() can be more than 5k (DM, MD, SCSI, driver, memory allocation), so it's really not safe to do allocation anywhere below about 3k of kernel stack being used. e.g. on a relatively trivial storage setup without the above commit: [142296.384921] flush-253:4 used greatest stack depth: 360 bytes left Fundamentally, 8k stacks on x86-64 are too small for our increasingly complex storage layers and the 100+ function deep call chains that occur. Cheers, Dave. -- Dave Chinner da...@fromorbit.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: gma500 suspend to ram fails (3.4)
On Tue, Jul 03, 2012 at 07:11:31AM +0900, Mattia Dongili wrote: > On Mon, Jul 02, 2012 at 11:43:04AM +0100, Alan Cox wrote: > > On Mon, 2 Jul 2012 07:06:59 +0900 > > Mattia Dongili wrote: > ... > > > a did put some printks here and there and psb_driver_init seems to > > > never return from gma_backlight_init. > > > > Is it ok if you just comment out gma_backlight_init (at which point it > > should just skip backlight handling and leave the firmware configured > > one) > > ha! that gave me a good lead. > The problem is not gma_backlight_init but rather psb_modeset_init that > calls the errata for the chip that in turn sets the brightness before > any initialization is done. > > Commenting out the call to dev_priv->ops->errata in framebuffer.c makes > boot get to the end like it used to on 3.4. It seems to be just an > ordering problem in the end. and back to the original issue, with 3.5 the laptop suspends to ram just fine! finally this vaio X may be of some use!! I can test patches for the errata call issue if you need any confirmation. -- mattia :wq! ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: gma500 suspend to ram fails (3.4)
On Mon, Jul 02, 2012 at 11:43:04AM +0100, Alan Cox wrote: > On Mon, 2 Jul 2012 07:06:59 +0900 > Mattia Dongili wrote: ... > > a did put some printks here and there and psb_driver_init seems to > > never return from gma_backlight_init. > > Is it ok if you just comment out gma_backlight_init (at which point it > should just skip backlight handling and leave the firmware configured > one) ha! that gave me a good lead. The problem is not gma_backlight_init but rather psb_modeset_init that calls the errata for the chip that in turn sets the brightness before any initialization is done. Commenting out the call to dev_priv->ops->errata in framebuffer.c makes boot get to the end like it used to on 3.4. It seems to be just an ordering problem in the end. -- mattia :wq! ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] pci_regs: define LNKSTA2 pcie cap + bits.
On Thu, Jun 28, 2012 at 3:45 AM, Dave Airlie wrote: > On Wed, Jun 27, 2012 at 8:35 AM, Dave Airlie wrote: >> From: Dave Airlie >> >> We need these for detecting the max link speed for drm drivers. > > Hi Bjorn, > > Can you ack this patch so I can carry it in the drm-next tree? we need > these regs for getting the PCIE v2/v3 supported link speeds. Sure. Note that I already have a patch in my "next" tree that will conflict with this because it contains this hunk: #define PCI_EXP_OBFF_WAKE_EN 0x6000 /* OBFF using WAKE# signaling */ +#define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 44 /* v2 endpoints end here */ #define PCI_EXP_LNKCTL248 /* Link Control 2 */ at this commit: http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=commitdiff;h=a0dee2ed0cdc666b5622f1fc74979355a6b36850 I think the comments would make more sense as "2.5GT/s supported", etc., since these bits don't tell you anything about the "Current Link Speed". Maybe it would make sense for me to incorporate this patch into my "next" branch, and you could carry both commits in your drm-next tree? I don't know what results in the easiest merge later. But if you need it: Acked-by: Bjorn Helgaas Bjorn >> Signed-off-by: Dave Airlie >> --- >> include/linux/pci_regs.h | 5 + >> 1 files changed, 5 insertions(+), 0 deletions(-) >> >> diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h >> index 4b608f5..7f04132 100644 >> --- a/include/linux/pci_regs.h >> +++ b/include/linux/pci_regs.h >> @@ -521,6 +521,11 @@ >> #define PCI_EXP_OBFF_MSGA_EN 0x2000 /* OBFF enable with Message type A */ >> #define PCI_EXP_OBFF_MSGB_EN 0x4000 /* OBFF enable with Message type B */ >> #define PCI_EXP_OBFF_WAKE_EN 0x6000 /* OBFF using WAKE# signaling */ >> +#define PCI_EXP_LNKCAP2 44 /* Link Capability 2 */ >> +#define PCI_EXP_LNKCAP2_SLS_2_5GB 0x01 /* Current Link Speed >> 2.5GT/s */ >> +#define PCI_EXP_LNKCAP2_SLS_5_0GB 0x02 /* Current Link Speed >> 5.0GT/s */ >> +#define PCI_EXP_LNKCAP2_SLS_8_0GB 0x04 /* Current Link Speed >> 8.0GT/s */ >> +#define PCI_EXP_LNKCAP2_CROSSLINK 0x100 /* Crosslink supported */ >> #define PCI_EXP_LNKCTL2 48 /* Link Control 2 */ >> #define PCI_EXP_SLTCTL2 56 /* Slot Control 2 */ >> >> -- >> 1.7.7.6 >> >> ___ >> 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
Re: [MMTests] IO metadata on XFS
Adding dri-devel and a few others because an i915 patch contributed to the regression. On Mon, Jul 02, 2012 at 03:32:15PM +0100, Mel Gorman wrote: > On Mon, Jul 02, 2012 at 02:32:26AM -0400, Christoph Hellwig wrote: > > > It increases the CPU overhead (dirty_inode can be called up to 4 > > > times per write(2) call, IIRC), so with limited numbers of > > > threads/limited CPU power it will result in lower performance. Where > > > you have lots of CPU power, there will be little difference in > > > performance... > > > > When I checked it it could only be called twice, and we'd already > > optimize away the second call. I'd defintively like to track down where > > the performance changes happend, at least to a major version but even > > better to a -rc or git commit. > > > > By all means feel free to run the test yourself and run the bisection :) > > It's rare but on this occasion the test machine is idle so I started an > automated git bisection. As you know the milage with an automated bisect > varies so it may or may not find the right commit. Test machine is sandy so > http://www.csn.ul.ie/~mel/postings/mmtests-20120424/global-dhp__io-metadata-xfs/sandy/comparison.html > is the report of interest. The script is doing a full search between v3.3 and > v3.4 for a point where average files/sec for fsmark-single drops below 25000. > I did not limit the search to fs/xfs on the off-chance that it is an > apparently unrelated patch that caused the problem. > It was obvious very quickly that there were two distinct regression so I ran two bisections. One led to a XFS and the other led to an i915 patch that enables RC6 to reduce power usage. [c999a223: xfs: introduce an allocation workqueue] [aa464191: drm/i915: enable plain RC6 on Sandy Bridge by default] gdm was running on the machine so i915 would have been in use. In case it is of interest this is the log of the bisection. Lines beginning with # are notes I made and all other lines are from the bisection script. The second-last column is the files/sec recorded by fsmark. # MARK v3.3..v3.4 Search for BAD files/sec -lt 28000 # BAD 16536 # GOOD 34757 Mon Jul 2 15:46:13 IST 2012 sandy xfsbisect 141124c02059eee9dbc5c86ea797b1ca888e77f7 37454 good Mon Jul 2 15:56:06 IST 2012 sandy xfsbisect 55a320308902f7a0746569ee57eeb3f254e6ed16 25192 bad Mon Jul 2 16:08:34 IST 2012 sandy xfsbisect 281b05392fc2cb26209b4d85abaf4889ab1991f3 38807 good Mon Jul 2 16:18:02 IST 2012 sandy xfsbisect a8364db2030d093cde0f07951628e55454e1 37553 good Mon Jul 2 16:27:22 IST 2012 sandy xfsbisect d2a2fc18d98d8ee2dec1542efc7f47beec256144 36676 good Mon Jul 2 16:36:48 IST 2012 sandy xfsbisect 2e7580b0e75d771d93e24e681031a165b1d31071 37756 good Mon Jul 2 16:46:36 IST 2012 sandy xfsbisect 532bfc851a7475fb6a36c1e953aa395798a7cca7 25416 bad Mon Jul 2 16:56:10 IST 2012 sandy xfsbisect 0c9aac08261512d70d7d4817bd222abca8b6bdd6 38486 good Mon Jul 2 17:05:40 IST 2012 sandy xfsbisect 0fc9d1040313047edf6a39fd4d7c7defdca97c62 37970 good Mon Jul 2 17:16:01 IST 2012 sandy xfsbisect 5a5881cdeec2c019b5c9a307800218ee029f7f61 24493 bad Mon Jul 2 17:21:15 IST 2012 sandy xfsbisect f616137519feb17b849894fcbe634a021d3fa7db 24405 bad Mon Jul 2 17:26:16 IST 2012 sandy xfsbisect 5575acc7807595687288b3bbac15103f2a5462e1 37336 good Mon Jul 2 17:31:25 IST 2012 sandy xfsbisect c999a223c2f0d31c64ef7379814cea1378b2b800 24552 bad Mon Jul 2 17:36:34 IST 2012 sandy xfsbisect 1a1d772433d42aaff7315b3468fef5951604f5c6 36872 good # c999a223c2f0d31c64ef7379814cea1378b2b800 is the first bad commit # [c999a223: xfs: introduce an allocation workqueue] # # MARK c999a223c2f0d31c64ef7379814cea1378b2b800..v3.4 Search for BAD files/sec -lt 2 # BAD 16536 # GOOD 24552 Mon Jul 2 17:48:39 IST 2012 sandy xfsbisect b2094ef840697bc8ca5d17a83b7e30fad5f1e9fa 37435 good Mon Jul 2 17:58:12 IST 2012 sandy xfsbisect d2a2fc18d98d8ee2dec1542efc7f47beec256144 38303 good Mon Jul 2 18:08:18 IST 2012 sandy xfsbisect 5d32c88f0b94061b3af2e3ade92422407282eb12 16718 bad Mon Jul 2 18:18:02 IST 2012 sandy xfsbisect 2f7fa1be66dce77608330c5eb918d6360b5525f2 24964 good Mon Jul 2 18:24:14 IST 2012 sandy xfsbisect 923f79743c76583ed4684e2c80c8da51a7268af3 24963 good Mon Jul 2 18:33:49 IST 2012 sandy xfsbisect b61c37f57988567c84359645f8202a7c84bc798a 24824 good Mon Jul 2 18:40:20 IST 2012 sandy xfsbisect 20a2a811602b16c42ce88bada3d52712cdfb988b 17155 bad Mon Jul 2 18:50:12 IST 2012 sandy xfsbisect 78fb72f7936c01d5b426c03a691eca082b03f2b9 38494 good Mon Jul 2 19:00:24 IST 2012 sandy xfsbisect e1a7eb08ee097e97e928062a242b0de5b2599a11 25033 good Mon Jul 2 19:10:24 IST 2012 sandy xfsbisect 97effadb65ed08809e1720c8d3ee80b73a93665c 16520 bad Mon Jul 2 19:16:16 IST 2012 sandy xfsbisect 25e341cfc33d94435472983825163e97fe370a6c 16748 bad Mon Jul 2 19:21:52 IST 2012 sandy xfsbisect 7dd4906586274f3945f2aeaaa5a33b451c3b4bba 24957 good Mon Jul 2 19:27:35 IST 2012 sandy xfsbisect aa46419186992e6b8b8010319f0ca7f40a0d13f5 17088 bad Mon Jul 2
[MMTests] IO metadata on XFS
Adding dri-devel and a few others because an i915 patch contributed to the regression. On Mon, Jul 02, 2012 at 03:32:15PM +0100, Mel Gorman wrote: > On Mon, Jul 02, 2012 at 02:32:26AM -0400, Christoph Hellwig wrote: > > > It increases the CPU overhead (dirty_inode can be called up to 4 > > > times per write(2) call, IIRC), so with limited numbers of > > > threads/limited CPU power it will result in lower performance. Where > > > you have lots of CPU power, there will be little difference in > > > performance... > > > > When I checked it it could only be called twice, and we'd already > > optimize away the second call. I'd defintively like to track down where > > the performance changes happend, at least to a major version but even > > better to a -rc or git commit. > > > > By all means feel free to run the test yourself and run the bisection :) > > It's rare but on this occasion the test machine is idle so I started an > automated git bisection. As you know the milage with an automated bisect > varies so it may or may not find the right commit. Test machine is sandy so > http://www.csn.ul.ie/~mel/postings/mmtests-20120424/global-dhp__io-metadata-xfs/sandy/comparison.html > is the report of interest. The script is doing a full search between v3.3 and > v3.4 for a point where average files/sec for fsmark-single drops below 25000. > I did not limit the search to fs/xfs on the off-chance that it is an > apparently unrelated patch that caused the problem. > It was obvious very quickly that there were two distinct regression so I ran two bisections. One led to a XFS and the other led to an i915 patch that enables RC6 to reduce power usage. [c999a223: xfs: introduce an allocation workqueue] [aa464191: drm/i915: enable plain RC6 on Sandy Bridge by default] gdm was running on the machine so i915 would have been in use. In case it is of interest this is the log of the bisection. Lines beginning with # are notes I made and all other lines are from the bisection script. The second-last column is the files/sec recorded by fsmark. # MARK v3.3..v3.4 Search for BAD files/sec -lt 28000 # BAD 16536 # GOOD 34757 Mon Jul 2 15:46:13 IST 2012 sandy xfsbisect 141124c02059eee9dbc5c86ea797b1ca888e77f7 37454 good Mon Jul 2 15:56:06 IST 2012 sandy xfsbisect 55a320308902f7a0746569ee57eeb3f254e6ed16 25192 bad Mon Jul 2 16:08:34 IST 2012 sandy xfsbisect 281b05392fc2cb26209b4d85abaf4889ab1991f3 38807 good Mon Jul 2 16:18:02 IST 2012 sandy xfsbisect a8364db2030d093cde0f07951628e55454e1 37553 good Mon Jul 2 16:27:22 IST 2012 sandy xfsbisect d2a2fc18d98d8ee2dec1542efc7f47beec256144 36676 good Mon Jul 2 16:36:48 IST 2012 sandy xfsbisect 2e7580b0e75d771d93e24e681031a165b1d31071 37756 good Mon Jul 2 16:46:36 IST 2012 sandy xfsbisect 532bfc851a7475fb6a36c1e953aa395798a7cca7 25416 bad Mon Jul 2 16:56:10 IST 2012 sandy xfsbisect 0c9aac08261512d70d7d4817bd222abca8b6bdd6 38486 good Mon Jul 2 17:05:40 IST 2012 sandy xfsbisect 0fc9d1040313047edf6a39fd4d7c7defdca97c62 37970 good Mon Jul 2 17:16:01 IST 2012 sandy xfsbisect 5a5881cdeec2c019b5c9a307800218ee029f7f61 24493 bad Mon Jul 2 17:21:15 IST 2012 sandy xfsbisect f616137519feb17b849894fcbe634a021d3fa7db 24405 bad Mon Jul 2 17:26:16 IST 2012 sandy xfsbisect 5575acc7807595687288b3bbac15103f2a5462e1 37336 good Mon Jul 2 17:31:25 IST 2012 sandy xfsbisect c999a223c2f0d31c64ef7379814cea1378b2b800 24552 bad Mon Jul 2 17:36:34 IST 2012 sandy xfsbisect 1a1d772433d42aaff7315b3468fef5951604f5c6 36872 good # c999a223c2f0d31c64ef7379814cea1378b2b800 is the first bad commit # [c999a223: xfs: introduce an allocation workqueue] # # MARK c999a223c2f0d31c64ef7379814cea1378b2b800..v3.4 Search for BAD files/sec -lt 2 # BAD 16536 # GOOD 24552 Mon Jul 2 17:48:39 IST 2012 sandy xfsbisect b2094ef840697bc8ca5d17a83b7e30fad5f1e9fa 37435 good Mon Jul 2 17:58:12 IST 2012 sandy xfsbisect d2a2fc18d98d8ee2dec1542efc7f47beec256144 38303 good Mon Jul 2 18:08:18 IST 2012 sandy xfsbisect 5d32c88f0b94061b3af2e3ade92422407282eb12 16718 bad Mon Jul 2 18:18:02 IST 2012 sandy xfsbisect 2f7fa1be66dce77608330c5eb918d6360b5525f2 24964 good Mon Jul 2 18:24:14 IST 2012 sandy xfsbisect 923f79743c76583ed4684e2c80c8da51a7268af3 24963 good Mon Jul 2 18:33:49 IST 2012 sandy xfsbisect b61c37f57988567c84359645f8202a7c84bc798a 24824 good Mon Jul 2 18:40:20 IST 2012 sandy xfsbisect 20a2a811602b16c42ce88bada3d52712cdfb988b 17155 bad Mon Jul 2 18:50:12 IST 2012 sandy xfsbisect 78fb72f7936c01d5b426c03a691eca082b03f2b9 38494 good Mon Jul 2 19:00:24 IST 2012 sandy xfsbisect e1a7eb08ee097e97e928062a242b0de5b2599a11 25033 good Mon Jul 2 19:10:24 IST 2012 sandy xfsbisect 97effadb65ed08809e1720c8d3ee80b73a93665c 16520 bad Mon Jul 2 19:16:16 IST 2012 sandy xfsbisect 25e341cfc33d94435472983825163e97fe370a6c 16748 bad Mon Jul 2 19:21:52 IST 2012 sandy xfsbisect 7dd4906586274f3945f2aeaaa5a33b451c3b4bba 24957 good Mon Jul 2 19:27:35 IST 2012 sandy xfsbisect aa46419186992e6b8b8010319f0ca7f40a0d13f5 17088 bad Mon Jul 2
[Bug 33309] [855GM] GPU freeze due to overlay hang
https://bugs.freedesktop.org/show_bug.cgi?id=33309 --- Comment #29 from Daniel Vetter 2012-07-02 13:29:18 PDT --- (In reply to comment #28) > are there any news on this issue? > The 3.4 and 3.5-rc series seem stable wrt this issue, > but unfortunately something broke resume from s2ram badly, > the backlight stays off and the machine does not respond > even to SysRq and I need to do a hard power-off. That's good&bad news. Can you try to bisect the backlight regression that has been introduce in 3.4 and open a new bug report? That usually helps in fixing it ... -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 28622] radeon video lockup
https://bugzilla.kernel.org/show_bug.cgi?id=28622 --- Comment #11 from Alex Deucher 2012-07-02 20:21:02 --- Is this still an issue with a more recent kernel (3.x)? -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug.
[Bug 33309] [855GM] GPU freeze due to overlay hang
https://bugs.freedesktop.org/show_bug.cgi?id=33309 --- Comment #28 from stefan 2012-07-02 13:13:59 PDT --- Hi, are there any news on this issue? The 3.4 and 3.5-rc series seem stable wrt this issue, but unfortunately something broke resume from s2ram badly, the backlight stays off and the machine does not respond even to SysRq and I need to do a hard power-off. Cheers, Stefan. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[PATCH] drm/radeon: add an exclusive lock for GPU reset
On 02.07.2012 18:41, Jerome Glisse wrote: > On Mon, Jul 2, 2012 at 12:26 PM, Christian K?nig > wrote: >> On 02.07.2012 17:39, j.glisse at gmail.com wrote: >>> From: Jerome Glisse >>> >>> GPU reset need to be exclusive, one happening at a time. For this >>> add a rw semaphore so that any path that trigger GPU activities >>> have to take the semaphore as a reader thus allowing concurency. >>> >>> The GPU reset path take the semaphore as a writer ensuring that >>> no concurrent reset take place. >>> >>> Signed-off-by: Jerome Glisse >> NAK, that isn't as bad as the cs mutex was but still to complicated. It's >> just too far up in the call stack, e.g. it tries to catch ioctl operations, >> instead of catching the underlying hardware operation which is caused by the >> ioctl/ttm/etc... >> >> Why not just take the ring look as I suggested? >> >> > No we can't use ring lock, we need to protect suspend/resume path and > we need an exclusive lock for that so we need a reset mutex at the > very least. But instead of having a reset mutex i prefer using a rw > lock so that we can stop ioctl until a reset goes through an let > pending ioctl take proper action. Think about multiple context trying > to reset GPU ... > > Really this is the best option, the rw locking wont induce any lock > contention execept in gpu reset case which is anyway bad news. Why? That makes no sense to me. Well I don't want to prevent lock contention, but understand why we should add locking at the ioctl level. That violates locking rule number one "lock data instead of code" (or in our case "lock hardware access instead of code path") and it really is the reason why we ended up with the cs_mutex protecting practically everything. Multiple context trying to reset the GPU should be pretty fine, current it would just reset the GPU twice, but in the future asic_reset should be much more fine grained and only reset the parts of the GPU which really needs an reset. Cheers, Christian.
[PATCH] drm/radeon: add an exclusive lock for GPU reset
On 02.07.2012 17:39, j.glisse at gmail.com wrote: > From: Jerome Glisse > > GPU reset need to be exclusive, one happening at a time. For this > add a rw semaphore so that any path that trigger GPU activities > have to take the semaphore as a reader thus allowing concurency. > > The GPU reset path take the semaphore as a writer ensuring that > no concurrent reset take place. > > Signed-off-by: Jerome Glisse NAK, that isn't as bad as the cs mutex was but still to complicated. It's just too far up in the call stack, e.g. it tries to catch ioctl operations, instead of catching the underlying hardware operation which is caused by the ioctl/ttm/etc... Why not just take the ring look as I suggested? Christian. > --- > drivers/gpu/drm/radeon/radeon.h|1 + > drivers/gpu/drm/radeon/radeon_cs.c |5 + > drivers/gpu/drm/radeon/radeon_device.c |2 ++ > drivers/gpu/drm/radeon/radeon_gem.c|7 +++ > 4 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index 77b4519b..29d6986 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -1446,6 +1446,7 @@ struct radeon_device { > struct device *dev; > struct drm_device *ddev; > struct pci_dev *pdev; > + struct rw_semaphore exclusive_lock; > /* ASIC */ > union radeon_asic_configconfig; > enum radeon_family family; > diff --git a/drivers/gpu/drm/radeon/radeon_cs.c > b/drivers/gpu/drm/radeon/radeon_cs.c > index f1b7527..7ee6491 100644 > --- a/drivers/gpu/drm/radeon/radeon_cs.c > +++ b/drivers/gpu/drm/radeon/radeon_cs.c > @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > struct radeon_cs_parser parser; > int r; > > + down_read(&rdev->exclusive_lock); > if (!rdev->accel_working) { > + up_read(&rdev->exclusive_lock); > return -EBUSY; > } > /* initialize parser */ > @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > if (r) { > DRM_ERROR("Failed to initialize parser !\n"); > radeon_cs_parser_fini(&parser, r); > + up_read(&rdev->exclusive_lock); > r = radeon_cs_handle_lockup(rdev, r); > return r; > } > @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > if (r != -ERESTARTSYS) > DRM_ERROR("Failed to parse relocation %d!\n", r); > radeon_cs_parser_fini(&parser, r); > + up_read(&rdev->exclusive_lock); > r = radeon_cs_handle_lockup(rdev, r); > return r; > } > @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > } > out: > radeon_cs_parser_fini(&parser, r); > + up_read(&rdev->exclusive_lock); > r = radeon_cs_handle_lockup(rdev, r); > return r; > } > diff --git a/drivers/gpu/drm/radeon/radeon_device.c > b/drivers/gpu/drm/radeon/radeon_device.c > index f654ba8..c8fdb40 100644 > --- a/drivers/gpu/drm/radeon/radeon_device.c > +++ b/drivers/gpu/drm/radeon/radeon_device.c > @@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) > int r; > int resched; > > + down_write(&rdev->exclusive_lock); > radeon_save_bios_scratch_regs(rdev); > /* block TTM */ > resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev); > @@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) > dev_info(rdev->dev, "GPU reset failed\n"); > } > > + up_write(&rdev->exclusive_lock); > return r; > } > > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c > b/drivers/gpu/drm/radeon/radeon_gem.c > index d9b0809..f99db63 100644 > --- a/drivers/gpu/drm/radeon/radeon_gem.c > +++ b/drivers/gpu/drm/radeon/radeon_gem.c > @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, > void *data, > uint32_t handle; > int r; > > + down_read(&rdev->exclusive_lock); > /* create a gem object to contain this object in */ > args->size = roundup(args->size, PAGE_SIZE); > r = radeon_gem_object_create(rdev, args->size, args->alignment, > args->initial_domain, false, > false, &gobj); > if (r) { > + up_read(&rdev->exclusive_lock); > r = radeon_gem_handle_lockup(rdev, r); > return r; > } > @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, > void *data, > /* drop reference from allocate - handle holds it now */ > drm_gem_object_unreference_unlocked(gobj); > if (r) { > + up_read(&r
[RFC v2] implicit drm synchronization wip with dma-buf
Well, V2 time! I was hinted to look at ttm_execbuf_util, and it does indeed contain some nice code. My goal was to extend dma-buf in a generic way now, some elements from v1 are remaining, most notably a dma-buf used for syncing. However it is expected now that dma-buf syncing will work in a very specific way now, slightly more strict than in v1. Instead of each buffer having their own dma-buf I put in 1 per command submission. This submission hasn't been run-time tested yet, but I expect the api to go something like this. Intended to be used like this: list_init(&head); add_list_tail(&validate1->entry, &head); add_list_tail(&validate2->entry, &head); add_list_tail(&validate3->entry, &head); r = dmabufmgr_eu_reserve_buffers(&head); if (r) return r; // add waits on cpu or gpu list_for_each_entry(validate, ...) { if (!validate->sync_buf) continue; // Check attachments to see if we already imported sync_buf // somewhere, if not attach to it. // waiting until cur_seq - dmabuf->sync_val >= 0 on either cpu // or as command submitted to gpu // sync_buf itself is a dma-buf, so it should be trivial // TODO: sync_buf should NEVER be validated, add is_sync_buf to dma_buf? // If this step fails: dmabufmgr_eu_backoff_reservation // else: // dmabufmgr_eu_fence_buffer_objects(our_own_sync_buf, // hwchannel * max(minhwalign, 4), ++counter[hwchannel]); // XXX: Do we still require a minimum alignment? I set up 16 for nouveau, // but this is no longer needed in this design since it only matters // for writes for which nouveau would already control the offset. } // Some time after execbuffer was executed, doesn't have to be right away but before // getting in the danger of our own counter wrapping around: // grab dmabufmgr.lru_lock, and cleanup by unreffing sync_buf when // sync_buf == ownbuf, sync_ofs == ownofs, and sync_val == saved_counter // In the meantime someone else or even us might have reserved this dma_buf // again, which is why all those checks are needed before unreffing. diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 5aa2d70..86e7598 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -10,7 +10,7 @@ obj-$(CONFIG_CMA) += dma-contiguous.o obj-y += power/ obj-$(CONFIG_HAS_DMA) += dma-mapping.o obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o -obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o +obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o dma-buf-mgr.o dma-buf-mgr-eu.o obj-$(CONFIG_ISA) += isa.o obj-$(CONFIG_FW_LOADER)+= firmware_class.o obj-$(CONFIG_NUMA) += node.o diff --git a/drivers/base/dma-buf-mgr-eu.c b/drivers/base/dma-buf-mgr-eu.c new file mode 100644 index 000..27ebc68 --- /dev/null +++ b/drivers/base/dma-buf-mgr-eu.c @@ -0,0 +1,170 @@ +/* + * Copyright (C) 2012 Canonical Ltd + * + * Based on ttm_bo.c which bears the following copyright notice, + * but is dual licensed: + * + * Copyright (c) 2006-2009 VMware, Inc., Palo Alto, CA., USA + * All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sub license, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice (including the + * next paragraph) shall be included in all copies or substantial portions + * of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE + * USE OR OTHER DEALINGS IN THE SOFTWARE. + * + **/ + +#include +#include +#include + +static void dmabufmgr_eu_backoff_reservation_locked(struct list_head *list) +{ + struct dmabufmgr_validate *entry; + + list_for_each_entry(entry, list, head) { + struct dma_buf *bo = entry->bo; + if (!entry->reserved) + continue; + + entry->reserved = false; + atomic_set(&bo->reserved, 0); + wake_up_all(&bo->event_queue); + if (entry->sync_buf) + dma_buf_put(entry->sync_buf); + entry->sync_buf =
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #8 from Eugene St Leger 2012-07-02 11:04:54 PDT --- A summary of all of the patches follows. r200 essential patches (1st 3 patches) for gnome shell: "Essential patch to disable texture formats that are reported unrenderable elsewhere in driver." "Essential patch to allow 2048 pixel blits." "Essential patch to reapply dirtied texenv registers." suggested minor fixes patch (4th patch): "Unessential fixes/enhancements patch." proposed r200 blit patch (5th patch) - unknown importance: "Proposed blit register dirtying patch." untested but probably essential r100/radeon patch (7th patch) for gnome shell: "Untested but probably essential patch to allow 2048 pixel blits on r100." optional unrecommended patches to supplement blit patches already mentioned above (6th & 8th patches): "Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on r200." "Optional untested patch to warn (once) when a blit with 2048 pixel dimension occurs on r100." -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[PATCH 1/3] drm/radeon: move ring locking out of reset path
On 02.07.2012 17:41, Jerome Glisse wrote: > On Fri, Jun 29, 2012 at 12:15 PM, Michel D?nzer wrote: >> On Fre, 2012-06-29 at 17:18 +0200, Christian K?nig wrote: >>> On 29.06.2012 17:09, Michel D?nzer wrote: On Fre, 2012-06-29 at 16:45 +0200, Christian K?nig wrote: > Hold the ring lock the whole time the reset is in progress, > otherwise another process can submit new jobs. Sounds good, but doesn't this create other paths (e.g. initialization, resume) where the ring is being accessed without holding the lock? Isn't that a problem? >>> Thought about that also. >>> >>> For init I'm pretty sure that no application can submit commands before >>> we are done, otherwise we are doomed anyway. >>> >>> For resume I'm not really sure, but I think that applications are >>> resumed after the hardware driver had a chance of doing so. >> I hope you're right... but if it's not too much trouble, it might be >> better to be safe than sorry and take the lock for those paths as well. >> >> > NAK this is the wrong way to solve the issue, we need a global lock on > all path that can trigger gpu activities. Previously it was the cs > mutex, but i haven't thought about it too much when it got removed. So > to fix the situation i am sending a patch with rw semaphore. So what I'm missing? What else can trigger GPU activity when not the rings? I'm currently working on ring-partial resets and also resets where you only skip over a single faulty IB instead of flushing the whole ring. And my current idea for that to work is that we hold the ring lock while we do suspend, ring_save, asic_reset, resume and ring_restore. Christian. > > Cheers, > Jerome >
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #7 from Eugene St Leger 2012-07-02 10:51:10 PDT --- Created attachment 63718 --> https://bugs.freedesktop.org/attachment.cgi?id=63718 Optional untested patch to warn (once) when a blit with 2048 pixel dimension occurs on r100. If 2048 pixel blits cause graphical glitches/problems on r100, this patch can be used to provide a single warning. This patch is untested. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #6 from Eugene St Leger 2012-07-02 10:49:07 PDT --- Created attachment 63717 --> https://bugs.freedesktop.org/attachment.cgi?id=63717 Untested but probably essential patch to allow 2048 pixel blits on r100. Without this patch, gnome shell is expected to crash. This patch is untested. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #4 from Eugene St Leger 2012-07-02 10:40:51 PDT --- Created attachment 63715 --> https://bugs.freedesktop.org/attachment.cgi?id=63715 Proposed blit register dirtying patch. It appears bliting dirties some registers without notifying. This proposed patch notifies of register dirtying. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #3 from Eugene St Leger 2012-07-02 10:37:47 PDT --- Created attachment 63714 --> https://bugs.freedesktop.org/attachment.cgi?id=63714 Unessential fixes/enhancements patch. Minor spelling fixes & enhancements. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #2 from Eugene St Leger 2012-07-02 10:34:41 PDT --- Created attachment 63713 --> https://bugs.freedesktop.org/attachment.cgi?id=63713 Essential patch to reapply dirtied texenv registers. Without this patch, colour corruption happens with xv. xf86-video-ati textured xv dirties texenv registers and r200 DRI does not reapply them. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #1 from Eugene St Leger 2012-07-02 10:29:44 PDT --- Created attachment 63712 --> https://bugs.freedesktop.org/attachment.cgi?id=63712 Essential patch to allow 2048 pixel blits. Without this patch, gnome shell crashes. 2048 pixel blits are used. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 51658] New: r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 Bug #: 51658 Summary: r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3 Classification: Unclassified Product: Mesa Version: 8.0 Platform: All OS/Version: All Status: NEW Severity: major Priority: medium Component: Drivers/DRI/r200 AssignedTo: dri-devel at lists.freedesktop.org ReportedBy: grimrc at yahoo.com Created attachment 63711 --> https://bugs.freedesktop.org/attachment.cgi?id=63711 Essential patch to disable texture formats that are reported unrenderable elsewhere in driver. Without r200-tex-format-fix.patch, gnome shell crashes. Texture formats are selected by radeonChooseTexFormat but reported as unrenderable by radeonIsFormatRenderable. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[PATCH] staging: drm/omap: add rotation properties
On Fri, Jun 29, 2012 at 07:17:23AM -0500, Rob Clark wrote: > On Fri, Jun 29, 2012 at 5:46 AM, Tomi Valkeinen > wrote: > > On Wed, 2012-06-27 at 09:06 -0500, Rob Clark wrote: > >> From: Rob Clark > >> > >> Use tiled buffers for rotated/reflected scanout, with CRTC and plane > >> properties as the interface for userspace to configure rotation. > >> > >> Signed-off-by: Rob Clark > > > >> +/* this should probably be in drm-core to standardize amongst drivers */ > >> +#define DRM_ROTATE_0 0 > >> +#define DRM_ROTATE_90 ? ? ? ?1 > >> +#define DRM_ROTATE_180 ? ? ? 2 > >> +#define DRM_ROTATE_270 ? ? ? 3 > >> +#define DRM_REFLECT_X ? ? ? ?4 > >> +#define DRM_REFLECT_Y ? ? ? ?5 > > > > Are both reflect X and Y needed? You can get all the possible > > orientations with just one of the reflects. > > > > And I think the word "mirror" represents nicely what the reflect does, > > i.e. if you look at the mirror, the image you see is flipped > > horizontally. > > fwiw these values are aligned with what is used in userspace xrandr.. > an earlier version of this patch used just 3 bits, which where aligned > with what the omap hw uses and can describe all possible combinations > of mirroring and isomorphic rotation (x-invert, y-invert, and > xy-flip). But the advantage of the xrandr approach is you can more > easily leave out bits for rotation/mirroring modes that your hw does > not support.. for example if some hw supports only certain rotations > or does not support mirror/reflect. When I hear "mirror" I always think of it as the happening after rotation, but that's not how xrandr does things. I suppose "reflection" has more or less the same connotation for me, but since it's already used by xrandr, I know I have to do the necessary mental gymnastics. If you think about it in terms of matrix multiplication, xrandr does things like this: x' = Rot * Ref * x, whereas for me the more natural order (for whatever reason) would be x' = Ref * Rot * x, where x is the source coordinates, and x' the destination coordinates. IIRC in dss mirroring was specified in the post-rotation way, and the direction of the rotation was also opposite to xrandr (cw in dss, ccw in xrandr). So FWIW, my vote goes to "reflect" if we want to match xrandr semantics. Also I agree on the number of bits, six is better than three since it allows you to describe the hw capabilities. -- Ville Syrj?l? Intel OTC
[PATCH v3] DRM: Add DRM kms/fb cma helper
This patchset introduces a set of helper function for implementing the KMS framebuffer layer for drivers which use the drm gem CMA helper function. Signed-off-by: Lars-Peter Clausen --- Note: This patch depends on Sascha's "DRM: add drm gem CMA helper" patch Changes since v2: * Adapt to changes in the GEM CMA helper * Add basic buffer size checking in drm_fb_cma_create Changes since v1: * Some spelling fixes * Add missing kfree in drm_fb_cma_alloc error path * Add multi-plane support --- drivers/gpu/drm/Kconfig | 10 + drivers/gpu/drm/Makefile|1 + drivers/gpu/drm/drm_fb_cma_helper.c | 393 +++ include/drm/drm_fb_cma_helper.h | 27 +++ 4 files changed, 431 insertions(+) create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c create mode 100644 include/drm/drm_fb_cma_helper.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 41bbd95..e511c9a 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -41,6 +41,16 @@ config DRM_GEM_CMA_HELPER help Choose this if you need the GEM CMA helper functions +config DRM_KMS_CMA_HELPER + tristate + select DRM_GEM_CMA_HELPER + select DRM_KMS_HELPER + select FB_SYS_FILLRECT + select FB_SYS_COPYAREA + select FB_SYS_IMAGEBLIT + help + Choose this if you need the KMS cma helper functions + config DRM_TDFX tristate "3dfx Banshee/Voodoo3+" depends on DRM && PCI diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 6e9e948..5dcb1a5 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c new file mode 100644 index 000..9042233 --- /dev/null +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -0,0 +1,393 @@ +/* + * drm kms/fb cma (contiguous memory allocator) helper functions + * + * Copyright (C) 2012 Analog Device Inc. + * Author: Lars-Peter Clausen + * + * Based on udl_fbdev.c + * Copyright (C) 2012 Red Hat + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +struct drm_fb_cma { + struct drm_framebuffer fb; + struct drm_gem_cma_object *obj[4]; +}; + +struct drm_fbdev_cma { + struct drm_fb_helperfb_helper; + struct drm_fb_cma *fb; +}; + +static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper) +{ + return container_of(helper, struct drm_fbdev_cma, fb_helper); +} + +static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb) +{ + return container_of(fb, struct drm_fb_cma, fb); +} + +static void drm_fb_cma_destroy(struct drm_framebuffer *fb) +{ + struct drm_fb_cma *fb_cma = to_fb_cma(fb); + int i; + + for (i = 0; i < 4; i++) { + if (fb_cma->obj[i]) + drm_gem_object_unreference_unlocked(&fb_cma->obj[i]->base); + } + + drm_framebuffer_cleanup(fb); + kfree(fb_cma); +} + +static int drm_fb_cma_create_handle(struct drm_framebuffer *fb, + struct drm_file *file_priv, unsigned int *handle) +{ + struct drm_fb_cma *fb_cma = to_fb_cma(fb); + + return drm_gem_handle_create(file_priv, + &fb_cma->obj[0]->base, handle); +} + +static struct drm_framebuffer_funcs drm_fb_cma_funcs = { + .destroy= drm_fb_cma_destroy, + .create_handle = drm_fb_cma_create_handle, +}; + +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, + struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_object **obj, + unsigned int num_planes) +{ + struct drm_fb_cma *fb_cma; + int ret; + int i; + + fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL); + if (!fb_cma) + return ERR_PTR(-ENOMEM); + + ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs); + if (ret) { + dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret); + kfree(fb_cma); + return ERR_PTR(ret); + } + + drm_helper
[PATCH] drm/radeon: fix rare segfault
On Mon, Jul 2, 2012 at 12:40 PM, wrote: > From: Jerome Glisse > > In gem idle/busy ioctl the radeon object was derefenced after > drm_gem_object_unreference_unlocked which in case the object > have been destroyed lead to use of a possibly free pointer with > possibly wrong data. > > Signed-off-by: Jerome Glisse Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/radeon/radeon_gem.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c > b/drivers/gpu/drm/radeon/radeon_gem.c > index 74176c5..c8838fc 100644 > --- a/drivers/gpu/drm/radeon/radeon_gem.c > +++ b/drivers/gpu/drm/radeon/radeon_gem.c > @@ -325,6 +325,7 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void > *data, > int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > { > + struct radeon_device *rdev = dev->dev_private; > struct drm_radeon_gem_busy *args = data; > struct drm_gem_object *gobj; > struct radeon_bo *robj; > @@ -350,13 +351,14 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void > *data, > break; > } > drm_gem_object_unreference_unlocked(gobj); > - r = radeon_gem_handle_lockup(robj->rdev, r); > + r = radeon_gem_handle_lockup(rdev, r); > return r; > } > > int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > { > + struct radeon_device *rdev = dev->dev_private; > struct drm_radeon_gem_wait_idle *args = data; > struct drm_gem_object *gobj; > struct radeon_bo *robj; > @@ -369,10 +371,10 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, > void *data, > robj = gem_to_radeon_bo(gobj); > r = radeon_bo_wait(robj, NULL, false); > /* callback hw specific functions if any */ > - if (robj->rdev->asic->ioctl_wait_idle) > - robj->rdev->asic->ioctl_wait_idle(robj->rdev, robj); > + if (rdev->asic->ioctl_wait_idle) > + robj->rdev->asic->ioctl_wait_idle(rdev, robj); > drm_gem_object_unreference_unlocked(gobj); > - r = radeon_gem_handle_lockup(robj->rdev, r); > + r = radeon_gem_handle_lockup(rdev, r); > return r; > } > > -- > 1.7.10.2 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Bogus video resolution in Linux 3.5-rc4
On 6/26/12 3:21 AM, Takashi Iwai wrote: > From: Takashi Iwai > Subject: [PATCH] drm: edid: Don't add inferred modes with higher resolution > > When a monitor EDID doesn't give the preferred bit, driver assumes > that the mode with the higest resolution and rate is the preferred > mode. Meanwhile the recent changes for allowing more modes in the > GFT/CVT ranges give actually more modes, and some modes may be over > the native size. Thus such a mode would be picked up as the preferred > mode although it's no native resolution. > > For avoiding such a problem, this patch limits the addition of > inferred modes by checking not to be greater than other modes. > Also, it checks the duplicated mode entry at the same time. This is a little aggressive on CRTs, but whatever, better than what's there. Reviewed-by: Adam Jackson - ajax
Re: [PATCH libdrm] intel: Fix build failure in test_decode.c
On Sat, 30 Jun 2012 13:12:45 +0300 Lauri Kasanen wrote: > Hi list > > The recently released libdrm 2.4.37 does not compile the Intel part: > > test_decode.c: In function 'compare_batch': > test_decode.c:107: error: implicit declaration of function 'open_memstream' > > PS: Please CC me. > > Signed-off-by: Lauri Kasanen > --- > intel/test_decode.c |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/intel/test_decode.c b/intel/test_decode.c > index c9ab7ad..0fcdf3b 100644 > --- a/intel/test_decode.c > +++ b/intel/test_decode.c > @@ -21,6 +21,8 @@ > * IN THE SOFTWARE. > */ > > +#define _GNU_SOURCE > + > #include > #include > #include I can't reproduce this. Can anyone else confirm this is broken, and if so that the above patch fixes it? -- Ben Widawsky, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] intel: Fix build failure in test_decode.c
On Sat, 30 Jun 2012 13:12:45 +0300 Lauri Kasanen wrote: > Hi list > > The recently released libdrm 2.4.37 does not compile the Intel part: > > test_decode.c: In function 'compare_batch': > test_decode.c:107: error: implicit declaration of function 'open_memstream' > > PS: Please CC me. > > Signed-off-by: Lauri Kasanen > --- > intel/test_decode.c |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/intel/test_decode.c b/intel/test_decode.c > index c9ab7ad..0fcdf3b 100644 > --- a/intel/test_decode.c > +++ b/intel/test_decode.c > @@ -21,6 +21,8 @@ > * IN THE SOFTWARE. > */ > > +#define _GNU_SOURCE > + > #include > #include > #include I can't reproduce this. Can anyone else confirm this is broken, and if so that the above patch fixes it? -- Ben Widawsky, Intel Open Source Technology Center
[PATCH 1/3] pci_regs: define LNKSTA2 pcie cap + bits.
On Thu, Jun 28, 2012 at 3:45 AM, Dave Airlie wrote: > On Wed, Jun 27, 2012 at 8:35 AM, Dave Airlie wrote: >> From: Dave Airlie >> >> We need these for detecting the max link speed for drm drivers. > > Hi Bjorn, > > Can you ack this patch so I can carry it in the drm-next tree? we need > these regs for getting the PCIE v2/v3 supported link speeds. Sure. Note that I already have a patch in my "next" tree that will conflict with this because it contains this hunk: #define PCI_EXP_OBFF_WAKE_EN 0x6000 /* OBFF using WAKE# signaling */ +#define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 44 /* v2 endpoints end here */ #define PCI_EXP_LNKCTL248 /* Link Control 2 */ at this commit: http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=commitdiff;h=a0dee2ed0cdc666b5622f1fc74979355a6b36850 I think the comments would make more sense as "2.5GT/s supported", etc., since these bits don't tell you anything about the "Current Link Speed". Maybe it would make sense for me to incorporate this patch into my "next" branch, and you could carry both commits in your drm-next tree? I don't know what results in the easiest merge later. But if you need it: Acked-by: Bjorn Helgaas Bjorn >> Signed-off-by: Dave Airlie >> --- >> ?include/linux/pci_regs.h | ? ?5 + >> ?1 files changed, 5 insertions(+), 0 deletions(-) >> >> diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h >> index 4b608f5..7f04132 100644 >> --- a/include/linux/pci_regs.h >> +++ b/include/linux/pci_regs.h >> @@ -521,6 +521,11 @@ >> ?#define ?PCI_EXP_OBFF_MSGA_EN ?0x2000 ?/* OBFF enable with Message type A */ >> ?#define ?PCI_EXP_OBFF_MSGB_EN ?0x4000 ?/* OBFF enable with Message type B */ >> ?#define ?PCI_EXP_OBFF_WAKE_EN ?0x6000 ?/* OBFF using WAKE# signaling */ >> +#define PCI_EXP_LNKCAP2 ? ? ? ? ? ? ? ?44 ? ? ?/* Link Capability 2 */ >> +#define ?PCI_EXP_LNKCAP2_SLS_2_5GB 0x01 ? ? ? ?/* Current Link Speed >> 2.5GT/s */ >> +#define ?PCI_EXP_LNKCAP2_SLS_5_0GB 0x02 ? ? ? ?/* Current Link Speed >> 5.0GT/s */ >> +#define ?PCI_EXP_LNKCAP2_SLS_8_0GB 0x04 ? ? ? ?/* Current Link Speed >> 8.0GT/s */ >> +#define ?PCI_EXP_LNKCAP2_CROSSLINK 0x100 /* Crosslink supported */ >> ?#define PCI_EXP_LNKCTL2 ? ? ? ? ? ? ? ?48 ? ? ?/* Link Control 2 */ >> ?#define PCI_EXP_SLTCTL2 ? ? ? ? ? ? ? ?56 ? ? ?/* Slot Control 2 */ >> >> -- >> 1.7.7.6 >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 28622] radeon video lockup
https://bugzilla.kernel.org/show_bug.cgi?id=28622 Alan changed: What|Removed |Added CC||alan at lxorguk.ukuu.org.uk Kernel Version|2.6.37 |2.6.39 -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug.
[Bug 33309] [855GM] GPU freeze due to overlay hang
https://bugs.freedesktop.org/show_bug.cgi?id=33309 --- Comment #29 from Daniel Vetter 2012-07-02 13:29:18 PDT --- (In reply to comment #28) > are there any news on this issue? > The 3.4 and 3.5-rc series seem stable wrt this issue, > but unfortunately something broke resume from s2ram badly, > the backlight stays off and the machine does not respond > even to SysRq and I need to do a hard power-off. That's good&bad news. Can you try to bisect the backlight regression that has been introduce in 3.4 and open a new bug report? That usually helps in fixing it ... -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: add an exclusive lock for GPU reset
On Mon, Jul 2, 2012 at 1:05 PM, Christian K?nig wrote: > On 02.07.2012 18:41, Jerome Glisse wrote: >> >> On Mon, Jul 2, 2012 at 12:26 PM, Christian K?nig >> wrote: >>> >>> On 02.07.2012 17:39, j.glisse at gmail.com wrote: From: Jerome Glisse GPU reset need to be exclusive, one happening at a time. For this add a rw semaphore so that any path that trigger GPU activities have to take the semaphore as a reader thus allowing concurency. The GPU reset path take the semaphore as a writer ensuring that no concurrent reset take place. Signed-off-by: Jerome Glisse >>> >>> NAK, that isn't as bad as the cs mutex was but still to complicated. It's >>> just too far up in the call stack, e.g. it tries to catch ioctl >>> operations, >>> instead of catching the underlying hardware operation which is caused by >>> the >>> ioctl/ttm/etc... >>> >>> Why not just take the ring look as I suggested? >>> >>> >> No we can't use ring lock, we need to protect suspend/resume path and >> we need an exclusive lock for that so we need a reset mutex at the >> very least. But instead of having a reset mutex i prefer using a rw >> lock so that we can stop ioctl until a reset goes through an let >> pending ioctl take proper action. Think about multiple context trying >> to reset GPU ... >> >> Really this is the best option, the rw locking wont induce any lock >> contention execept in gpu reset case which is anyway bad news. > > Why? That makes no sense to me. Well I don't want to prevent lock > contention, but understand why we should add locking at the ioctl level. > That violates locking rule number one "lock data instead of code" (or in our > case "lock hardware access instead of code path") and it really is the > reason why we ended up with the cs_mutex protecting practically everything. > > Multiple context trying to reset the GPU should be pretty fine, current it > would just reset the GPU twice, but in the future asic_reset should be much > more fine grained and only reset the parts of the GPU which really needs an > reset. > > Cheers, > Christian. No multiple reset is not fine, try it your self and you will see all kind of oops (strongly advise you to sync you hd before stress testing that). Yes we need to protect code path because suspend/resume code path is special one it touch many data in the device structure. GPU reset is a rare event or should be a rare event. I stress it we need at very least a mutex to protect gpu reset and i will stand on that position because there is no other way around. Using rw lock have a bonus of allowing proper handling of gpu reset failure and that what the patch i sent to linus fix tree is about, so to make drm next merge properly while preserving proper behavior in gpu reset failure the rw semaphore is the best option. Cheers, Jerome
Re: [PATCH] drm/radeon: fix rare segfault
On Mon, Jul 2, 2012 at 12:40 PM, wrote: > From: Jerome Glisse > > In gem idle/busy ioctl the radeon object was derefenced after > drm_gem_object_unreference_unlocked which in case the object > have been destroyed lead to use of a possibly free pointer with > possibly wrong data. > > Signed-off-by: Jerome Glisse Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/radeon/radeon_gem.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c > b/drivers/gpu/drm/radeon/radeon_gem.c > index 74176c5..c8838fc 100644 > --- a/drivers/gpu/drm/radeon/radeon_gem.c > +++ b/drivers/gpu/drm/radeon/radeon_gem.c > @@ -325,6 +325,7 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void > *data, > int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > { > + struct radeon_device *rdev = dev->dev_private; > struct drm_radeon_gem_busy *args = data; > struct drm_gem_object *gobj; > struct radeon_bo *robj; > @@ -350,13 +351,14 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void > *data, > break; > } > drm_gem_object_unreference_unlocked(gobj); > - r = radeon_gem_handle_lockup(robj->rdev, r); > + r = radeon_gem_handle_lockup(rdev, r); > return r; > } > > int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > { > + struct radeon_device *rdev = dev->dev_private; > struct drm_radeon_gem_wait_idle *args = data; > struct drm_gem_object *gobj; > struct radeon_bo *robj; > @@ -369,10 +371,10 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, > void *data, > robj = gem_to_radeon_bo(gobj); > r = radeon_bo_wait(robj, NULL, false); > /* callback hw specific functions if any */ > - if (robj->rdev->asic->ioctl_wait_idle) > - robj->rdev->asic->ioctl_wait_idle(robj->rdev, robj); > + if (rdev->asic->ioctl_wait_idle) > + robj->rdev->asic->ioctl_wait_idle(rdev, robj); > drm_gem_object_unreference_unlocked(gobj); > - r = radeon_gem_handle_lockup(robj->rdev, r); > + r = radeon_gem_handle_lockup(rdev, r); > return r; > } > > -- > 1.7.10.2 > > ___ > 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
[Bug 28622] radeon video lockup
https://bugzilla.kernel.org/show_bug.cgi?id=28622 --- Comment #11 from Alex Deucher 2012-07-02 20:21:02 --- Is this still an issue with a more recent kernel (3.x)? -- 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.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 33309] [855GM] GPU freeze due to overlay hang
https://bugs.freedesktop.org/show_bug.cgi?id=33309 --- Comment #28 from stefan 2012-07-02 13:13:59 PDT --- Hi, are there any news on this issue? The 3.4 and 3.5-rc series seem stable wrt this issue, but unfortunately something broke resume from s2ram badly, the backlight stays off and the machine does not respond even to SysRq and I need to do a hard power-off. Cheers, Stefan. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH, RFC] i.MX DRM support
On 02.07.2012 12:05, Sascha Hauer wrote: > On Thu, Jun 14, 2012 at 03:43:22PM +0200, Sascha Hauer wrote: >> Hi All, >> >> The following is the state-of-the-art i.MX IPU (Image Processing Unit) >> DRM support. >> >> This code is around for quite some time now and has been posted several >> times with different APIs, first with plain old framebuffer support, now >> DRM, first platform device binding, now devicetree. Unfortunately there's >> quite much code needed to get something useful out of the IPU, so these >> patches haven't received a lot of attention from people not involved in >> i.MX. I think we have now come to a point where this code needs more public >> exposure and where it's easier to talk in incremental changes instead of >> blobs. Therefore I request this to go to staging for some cycles. > > Dave, Greg, > > Comments to this one? I addressed the comments I received so far and am > about to respin this series. Is it ok to put this to staging? If yes, > should I move the whole stuff into drivers/staging/ or should it stay > in drivers/gpu/drm with just a Kconfig dependency on STAGING? We are very interested in this, so +1 from my side for this. Many thanks and best regards Dirk Btw.: Based on http://git.pengutronix.de/?p=imx/linux-2.6.git;a=shortlog;h=refs/heads/work/gpu/imx-drm-ipu-complete with [1] below I fixed some compiler warnings. I have some additional warnings drivers/gpu/drm/drm_edid.c: In function 'drm_find_cea_extension': drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds drivers/gpu/drm/drm_edid.c: In function 'drm_detect_hdmi_monitor': drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds drivers/gpu/drm/drm_edid.c: In function 'drm_detect_monitor_audio': drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds drivers/gpu/drm/drm_edid.c: In function 'drm_edid_to_eld': drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds drivers/gpu/drm/drm_edid.c: In function 'drm_add_edid_modes': drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds but these seems to be compiler specific and not related to Sacha's patches. [1] From: Dirk Behme Subject: [PATCH 1/2] DRM i.MX: ldb: Fix compiler warnings Fix the compiler warnings drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_encoder_mode_fixup': drivers/gpu/drm/imx/imx-ldb.c:118: warning: unused variable 'imx_ldb_ch' drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_encoder_prepare': drivers/gpu/drm/imx/imx-ldb.c:134: warning: format '%ld' expects type 'long int', but argument 6 has type 'int' Signed-off-by: Dirk Behme --- drivers/gpu/drm/imx/imx-ldb.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: freescale-linux-2.6-imx.git/drivers/gpu/drm/imx/imx-ldb.c === --- freescale-linux-2.6-imx.git.orig/drivers/gpu/drm/imx/imx-ldb.c +++ freescale-linux-2.6-imx.git/drivers/gpu/drm/imx/imx-ldb.c @@ -115,9 +115,9 @@ static bool imx_ldb_encoder_mode_fixup(s struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) { +/* struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder); -/* adjusted_mode->clock = clk_round_rate(imx_ldb_ch->clk_pll, adjusted_mode->clock * 1000) / 1000; */ @@ -131,7 +131,7 @@ static void imx_ldb_encoder_prepare(stru int ret; struct drm_display_mode *mode = &encoder->crtc->mode; - dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__, + dev_dbg(ldb->dev, "%s: now: %ld want: %d\n", __func__, clk_get_rate(imx_ldb_ch->clk_pll), mode->clock * 1000 * 7); clk_set_rate(imx_ldb_ch->clk_pll, mode->clock * 1000 * 7);
[Bug 51652] New: [6550D SUMO] problems with secondar monitor on VGA, causing GPU lockups
https://bugs.freedesktop.org/show_bug.cgi?id=51652 Bug #: 51652 Summary: [6550D SUMO] problems with secondar monitor on VGA, causing GPU lockups Classification: Unclassified Product: DRI Version: XOrg CVS Platform: x86-64 (AMD64) OS/Version: Linux (All) Status: NEW Severity: critical Priority: medium Component: DRM/Radeon AssignedTo: dri-devel at lists.freedesktop.org ReportedBy: d.okias at gmail.com Created attachment 63701 --> https://bugs.freedesktop.org/attachment.cgi?id=63701 dmesg_VGAonBootConnected.txt 1) when is computer booted with VGA connected, ends with repeatly restarting GPU 2) when is monitor connected on-fly in running Xorg, it seems fine (except small artifacts, i guess EXA, on screen mostly on top of DVI and bottom VGA trigerred by switching KDE virtual desktops). No errors in dmesg 3) when is DPMS activated and monitors suspends, back to online is returned only DVI 4) randomly I found: [drm:radeon_dp_link_train_cr] *ERROR* clock recovery reached max voltage [drm:radeon_dp_link_train_cr] *ERROR* clock recovery failed 5) out-topic, exist even only on DVI: when is enabled OGL KWIN and OpenGL video render, color of rendered context goes to white (every frame is whitier, until it's #ff) No HDMI or DP conneted to system. DVI-D: LG IPS235 1920x1080 VGA: ASUS VW224 1680x1050 Gentoo ~amd64 Kernel,libdrm,mesa,ddx: git, KMS I'm able to test patches against anything. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
Re: Bogus video resolution in Linux 3.5-rc4
On 6/26/12 3:21 AM, Takashi Iwai wrote: From: Takashi Iwai Subject: [PATCH] drm: edid: Don't add inferred modes with higher resolution When a monitor EDID doesn't give the preferred bit, driver assumes that the mode with the higest resolution and rate is the preferred mode. Meanwhile the recent changes for allowing more modes in the GFT/CVT ranges give actually more modes, and some modes may be over the native size. Thus such a mode would be picked up as the preferred mode although it's no native resolution. For avoiding such a problem, this patch limits the addition of inferred modes by checking not to be greater than other modes. Also, it checks the duplicated mode entry at the same time. This is a little aggressive on CRTs, but whatever, better than what's there. Reviewed-by: Adam Jackson - ajax ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: add an exclusive lock for GPU reset v2
From: Jerome Glisse GPU reset need to be exclusive, one happening at a time. For this add a rw semaphore so that any path that trigger GPU activities have to take the semaphore as a reader thus allowing concurency. The GPU reset path take the semaphore as a writer ensuring that no concurrent reset take place. v2: init rw semaphore Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/radeon.h|1 + drivers/gpu/drm/radeon/radeon_cs.c |5 + drivers/gpu/drm/radeon/radeon_device.c |3 +++ drivers/gpu/drm/radeon/radeon_gem.c|8 4 files changed, 17 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 77b4519b..29d6986 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1446,6 +1446,7 @@ struct radeon_device { struct device *dev; struct drm_device *ddev; struct pci_dev *pdev; + struct rw_semaphore exclusive_lock; /* ASIC */ union radeon_asic_configconfig; enum radeon_family family; diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index f1b7527..7ee6491 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct radeon_cs_parser parser; int r; + down_read(&rdev->exclusive_lock); if (!rdev->accel_working) { + up_read(&rdev->exclusive_lock); return -EBUSY; } /* initialize parser */ @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r) { DRM_ERROR("Failed to initialize parser !\n"); radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r != -ERESTARTSYS) DRM_ERROR("Failed to parse relocation %d!\n", r); radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) } out: radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index f654ba8..254fdb4 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -734,6 +734,7 @@ int radeon_device_init(struct radeon_device *rdev, mutex_init(&rdev->gem.mutex); mutex_init(&rdev->pm.mutex); init_rwsem(&rdev->pm.mclk_lock); + init_rwsem(&rdev->exclusive_lock); init_waitqueue_head(&rdev->irq.vblank_queue); init_waitqueue_head(&rdev->irq.idle_queue); r = radeon_gem_init(rdev); @@ -988,6 +989,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) int r; int resched; + down_write(&rdev->exclusive_lock); radeon_save_bios_scratch_regs(rdev); /* block TTM */ resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev); @@ -1007,6 +1009,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) dev_info(rdev->dev, "GPU reset failed\n"); } + up_write(&rdev->exclusive_lock); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index d9b0809..b0be9c4 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data, uint32_t handle; int r; + down_read(&rdev->exclusive_lock); /* create a gem object to contain this object in */ args->size = roundup(args->size, PAGE_SIZE); r = radeon_gem_object_create(rdev, args->size, args->alignment, args->initial_domain, false, false, &gobj); if (r) { + up_read(&rdev->exclusive_lock); r = radeon_gem_handle_lockup(rdev, r); return r; } @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data, /* drop reference from allocate - handle holds it now */ drm_gem_object_unreference_unlocked(gobj); if (r) { + up_read(&rdev->exclusive_lock); r = radeon_gem_handle_lockup(rdev, r); r
[PATCH 01/10] drm/radeon: document radeon_device.c (v2)
On 29.06.2012 18:50, alexdeucher at gmail.com wrote: > From: Alex Deucher > > Adds documentation to most of the functions in > radeon_device.c > > v2: split out general descriptions as per Christian's > comments. > > Signed-off-by: Alex Deucher For the whole series: Reviewed-by: Christian K?nig > --- > drivers/gpu/drm/radeon/radeon_device.c | 313 > +++- > 1 files changed, 310 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_device.c > b/drivers/gpu/drm/radeon/radeon_device.c > index f654ba8..926d7e8 100644 > --- a/drivers/gpu/drm/radeon/radeon_device.c > +++ b/drivers/gpu/drm/radeon/radeon_device.c > @@ -96,8 +96,12 @@ static const char radeon_family_name[][16] = { > "LAST", > }; > > -/* > - * Clear GPU surface registers. > +/** > + * radeon_surface_init - Clear GPU surface registers. > + * > + * @rdev: radeon_device pointer > + * > + * Clear GPU surface registers (r1xx-r5xx). >*/ > void radeon_surface_init(struct radeon_device *rdev) > { > @@ -119,6 +123,13 @@ void radeon_surface_init(struct radeon_device *rdev) > /* >* GPU scratch registers helpers function. >*/ > +/** > + * radeon_scratch_init - Init scratch register driver information. > + * > + * @rdev: radeon_device pointer > + * > + * Init CP scratch register driver information (r1xx-r5xx) > + */ > void radeon_scratch_init(struct radeon_device *rdev) > { > int i; > @@ -136,6 +147,15 @@ void radeon_scratch_init(struct radeon_device *rdev) > } > } > > +/** > + * radeon_scratch_get - Allocate a scratch register > + * > + * @rdev: radeon_device pointer > + * @reg: scratch register mmio offset > + * > + * Allocate a CP scratch register for use by the driver (all asics). > + * Returns 0 on success or -EINVAL on failure. > + */ > int radeon_scratch_get(struct radeon_device *rdev, uint32_t *reg) > { > int i; > @@ -150,6 +170,14 @@ int radeon_scratch_get(struct radeon_device *rdev, > uint32_t *reg) > return -EINVAL; > } > > +/** > + * radeon_scratch_free - Free a scratch register > + * > + * @rdev: radeon_device pointer > + * @reg: scratch register mmio offset > + * > + * Free a CP scratch register allocated for use by the driver (all asics) > + */ > void radeon_scratch_free(struct radeon_device *rdev, uint32_t reg) > { > int i; > @@ -162,6 +190,20 @@ void radeon_scratch_free(struct radeon_device *rdev, > uint32_t reg) > } > } > > +/* > + * radeon_wb_*() > + * Writeback is the the method by which the the GPU updates special pages > + * in memory with the status of certain GPU events (fences, ring pointers, > + * etc.). > + */ > + > +/** > + * radeon_wb_disable - Disable Writeback > + * > + * @rdev: radeon_device pointer > + * > + * Disables Writeback (all asics). Used for suspend. > + */ > void radeon_wb_disable(struct radeon_device *rdev) > { > int r; > @@ -177,6 +219,14 @@ void radeon_wb_disable(struct radeon_device *rdev) > rdev->wb.enabled = false; > } > > +/** > + * radeon_wb_fini - Disable Writeback and free memory > + * > + * @rdev: radeon_device pointer > + * > + * Disables Writeback and frees the Writeback memory (all asics). > + * Used at driver shutdown. > + */ > void radeon_wb_fini(struct radeon_device *rdev) > { > radeon_wb_disable(rdev); > @@ -187,6 +237,15 @@ void radeon_wb_fini(struct radeon_device *rdev) > } > } > > +/** > + * radeon_wb_init- Init Writeback driver info and allocate memory > + * > + * @rdev: radeon_device pointer > + * > + * Disables Writeback and frees the Writeback memory (all asics). > + * Used at driver startup. > + * Returns 0 on success or an -error on failure. > + */ > int radeon_wb_init(struct radeon_device *rdev) > { > int r; > @@ -355,6 +414,15 @@ void radeon_gtt_location(struct radeon_device *rdev, > struct radeon_mc *mc) > /* >* GPU helpers function. >*/ > +/** > + * radeon_card_posted - check if the hw has already been initialized > + * > + * @rdev: radeon_device pointer > + * > + * Check if the asic has been initialized (all asics). > + * Used at driver startup. > + * Returns true if initialized or false if not. > + */ > bool radeon_card_posted(struct radeon_device *rdev) > { > uint32_t reg; > @@ -404,6 +472,14 @@ bool radeon_card_posted(struct radeon_device *rdev) > > } > > +/** > + * radeon_update_bandwidth_info - update display bandwidth params > + * > + * @rdev: radeon_device pointer > + * > + * Used when sclk/mclk are switched or display modes are set. > + * params are used to calculate display watermarks (all asics) > + */ > void radeon_update_bandwidth_info(struct radeon_device *rdev) > { > fixed20_12 a; > @@ -424,6 +500,15 @@ void radeon_update_bandwidth_info(struct radeon_device > *rdev) > } > } > > +/** > + * radeon_boot_test_post_card - check and possibly initialize the hw > + * > + * @rdev: radeon_device pointer > + * > + * Check if the a
[PATCH] drm/radeon: add an exclusive lock for GPU reset
On Mon, Jul 2, 2012 at 12:26 PM, Christian K?nig wrote: > On 02.07.2012 17:39, j.glisse at gmail.com wrote: >> >> From: Jerome Glisse >> >> GPU reset need to be exclusive, one happening at a time. For this >> add a rw semaphore so that any path that trigger GPU activities >> have to take the semaphore as a reader thus allowing concurency. >> >> The GPU reset path take the semaphore as a writer ensuring that >> no concurrent reset take place. >> >> Signed-off-by: Jerome Glisse > > NAK, that isn't as bad as the cs mutex was but still to complicated. It's > just too far up in the call stack, e.g. it tries to catch ioctl operations, > instead of catching the underlying hardware operation which is caused by the > ioctl/ttm/etc... > > Why not just take the ring look as I suggested? > > No we can't use ring lock, we need to protect suspend/resume path and we need an exclusive lock for that so we need a reset mutex at the very least. But instead of having a reset mutex i prefer using a rw lock so that we can stop ioctl until a reset goes through an let pending ioctl take proper action. Think about multiple context trying to reset GPU ... Really this is the best option, the rw locking wont induce any lock contention execept in gpu reset case which is anyway bad news. Jerome
[PATCH] drm/radeon: fix rare segfault
From: Jerome Glisse In gem idle/busy ioctl the radeon object was derefenced after drm_gem_object_unreference_unlocked which in case the object have been destroyed lead to use of a possibly free pointer with possibly wrong data. Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/radeon_gem.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index 74176c5..c8838fc 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -325,6 +325,7 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void *data, int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) { + struct radeon_device *rdev = dev->dev_private; struct drm_radeon_gem_busy *args = data; struct drm_gem_object *gobj; struct radeon_bo *robj; @@ -350,13 +351,14 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, break; } drm_gem_object_unreference_unlocked(gobj); - r = radeon_gem_handle_lockup(robj->rdev, r); + r = radeon_gem_handle_lockup(rdev, r); return r; } int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) { + struct radeon_device *rdev = dev->dev_private; struct drm_radeon_gem_wait_idle *args = data; struct drm_gem_object *gobj; struct radeon_bo *robj; @@ -369,10 +371,10 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data, robj = gem_to_radeon_bo(gobj); r = radeon_bo_wait(robj, NULL, false); /* callback hw specific functions if any */ - if (robj->rdev->asic->ioctl_wait_idle) - robj->rdev->asic->ioctl_wait_idle(robj->rdev, robj); + if (rdev->asic->ioctl_wait_idle) + robj->rdev->asic->ioctl_wait_idle(rdev, robj); drm_gem_object_unreference_unlocked(gobj); - r = radeon_gem_handle_lockup(robj->rdev, r); + r = radeon_gem_handle_lockup(rdev, r); return r; } -- 1.7.10.2
[PATCH] drm/radeon: add an exclusive lock for GPU reset
On Mon, Jul 2, 2012 at 11:39 AM, wrote: > From: Jerome Glisse > > GPU reset need to be exclusive, one happening at a time. For this > add a rw semaphore so that any path that trigger GPU activities > have to take the semaphore as a reader thus allowing concurency. > > The GPU reset path take the semaphore as a writer ensuring that > no concurrent reset take place. > > Signed-off-by: Jerome Glisse Wrong patch sorry resending > --- > ?drivers/gpu/drm/radeon/radeon.h ? ? ? ?| ? ?1 + > ?drivers/gpu/drm/radeon/radeon_cs.c ? ? | ? ?5 + > ?drivers/gpu/drm/radeon/radeon_device.c | ? ?2 ++ > ?drivers/gpu/drm/radeon/radeon_gem.c ? ?| ? ?7 +++ > ?4 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index 77b4519b..29d6986 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -1446,6 +1446,7 @@ struct radeon_device { > ? ? ? ?struct device ? ? ? ? ? ? ? ? ? *dev; > ? ? ? ?struct drm_device ? ? ? ? ? ? ? *ddev; > ? ? ? ?struct pci_dev ? ? ? ? ? ? ? ? ?*pdev; > + ? ? ? struct rw_semaphore ? ? ? ? ? ? exclusive_lock; > ? ? ? ?/* ASIC */ > ? ? ? ?union radeon_asic_config ? ? ? ?config; > ? ? ? ?enum radeon_family ? ? ? ? ? ? ?family; > diff --git a/drivers/gpu/drm/radeon/radeon_cs.c > b/drivers/gpu/drm/radeon/radeon_cs.c > index f1b7527..7ee6491 100644 > --- a/drivers/gpu/drm/radeon/radeon_cs.c > +++ b/drivers/gpu/drm/radeon/radeon_cs.c > @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > ? ? ? ?struct radeon_cs_parser parser; > ? ? ? ?int r; > > + ? ? ? down_read(&rdev->exclusive_lock); > ? ? ? ?if (!rdev->accel_working) { > + ? ? ? ? ? ? ? up_read(&rdev->exclusive_lock); > ? ? ? ? ? ? ? ?return -EBUSY; > ? ? ? ?} > ? ? ? ?/* initialize parser */ > @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > ? ? ? ?if (r) { > ? ? ? ? ? ? ? ?DRM_ERROR("Failed to initialize parser !\n"); > ? ? ? ? ? ? ? ?radeon_cs_parser_fini(&parser, r); > + ? ? ? ? ? ? ? up_read(&rdev->exclusive_lock); > ? ? ? ? ? ? ? ?r = radeon_cs_handle_lockup(rdev, r); > ? ? ? ? ? ? ? ?return r; > ? ? ? ?} > @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > ? ? ? ? ? ? ? ?if (r != -ERESTARTSYS) > ? ? ? ? ? ? ? ? ? ? ? ?DRM_ERROR("Failed to parse relocation %d!\n", r); > ? ? ? ? ? ? ? ?radeon_cs_parser_fini(&parser, r); > + ? ? ? ? ? ? ? up_read(&rdev->exclusive_lock); > ? ? ? ? ? ? ? ?r = radeon_cs_handle_lockup(rdev, r); > ? ? ? ? ? ? ? ?return r; > ? ? ? ?} > @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > ? ? ? ?} > ?out: > ? ? ? ?radeon_cs_parser_fini(&parser, r); > + ? ? ? up_read(&rdev->exclusive_lock); > ? ? ? ?r = radeon_cs_handle_lockup(rdev, r); > ? ? ? ?return r; > ?} > diff --git a/drivers/gpu/drm/radeon/radeon_device.c > b/drivers/gpu/drm/radeon/radeon_device.c > index f654ba8..c8fdb40 100644 > --- a/drivers/gpu/drm/radeon/radeon_device.c > +++ b/drivers/gpu/drm/radeon/radeon_device.c > @@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) > ? ? ? ?int r; > ? ? ? ?int resched; > > + ? ? ? down_write(&rdev->exclusive_lock); > ? ? ? ?radeon_save_bios_scratch_regs(rdev); > ? ? ? ?/* block TTM */ > ? ? ? ?resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev); > @@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) > ? ? ? ? ? ? ? ?dev_info(rdev->dev, "GPU reset failed\n"); > ? ? ? ?} > > + ? ? ? up_write(&rdev->exclusive_lock); > ? ? ? ?return r; > ?} > > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c > b/drivers/gpu/drm/radeon/radeon_gem.c > index d9b0809..f99db63 100644 > --- a/drivers/gpu/drm/radeon/radeon_gem.c > +++ b/drivers/gpu/drm/radeon/radeon_gem.c > @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, > void *data, > ? ? ? ?uint32_t handle; > ? ? ? ?int r; > > + ? ? ? down_read(&rdev->exclusive_lock); > ? ? ? ?/* create a gem object to contain this object in */ > ? ? ? ?args->size = roundup(args->size, PAGE_SIZE); > ? ? ? ?r = radeon_gem_object_create(rdev, args->size, args->alignment, > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?args->initial_domain, false, > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?false, &gobj); > ? ? ? ?if (r) { > + ? ? ? ? ? ? ? up_read(&rdev->exclusive_lock); > ? ? ? ? ? ? ? ?r = radeon_gem_handle_lockup(rdev, r); > ? ? ? ? ? ? ? ?return r; > ? ? ? ?} > @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, > void *data, > ? ? ? ?/* drop reference from allocate - handle holds it now */ > ? ? ? ?drm_gem_object_unreference_unlocked(gobj); > ? ? ? ?if (r) { > + ? ? ? ? ? ? ? up_read(&rdev->exclusive_lock); > ? ? ? ? ? ? ? ?r = radeon_gem_handle_lockup(rdev, r); > ? ? ? ? ? ? ? ?return r; > ? ? ? ?} > ? ? ? ?args->handle = handle; > + ? ? ? up_read(&rdev->exclusive_lock); > ? ? ? ?return 0; > ?} > > @@ -240,6 +244,7 @@
[PATCH 1/3] drm/radeon: move ring locking out of reset path
On Mon, Jul 2, 2012 at 11:58 AM, Christian K?nig wrote: > On 02.07.2012 17:41, Jerome Glisse wrote: >> >> On Fri, Jun 29, 2012 at 12:15 PM, Michel D?nzer >> wrote: >>> >>> On Fre, 2012-06-29 at 17:18 +0200, Christian K?nig wrote: On 29.06.2012 17:09, Michel D?nzer wrote: > > On Fre, 2012-06-29 at 16:45 +0200, Christian K?nig wrote: >> >> Hold the ring lock the whole time the reset is in progress, >> otherwise another process can submit new jobs. > > Sounds good, but doesn't this create other paths (e.g. initialization, > resume) where the ring is being accessed without holding the lock? > Isn't > that a problem? Thought about that also. For init I'm pretty sure that no application can submit commands before we are done, otherwise we are doomed anyway. For resume I'm not really sure, but I think that applications are resumed after the hardware driver had a chance of doing so. >>> >>> I hope you're right... but if it's not too much trouble, it might be >>> better to be safe than sorry and take the lock for those paths as well. >>> >>> >> NAK this is the wrong way to solve the issue, we need a global lock on >> all path that can trigger gpu activities. Previously it was the cs >> mutex, but i haven't thought about it too much when it got removed. So >> to fix the situation i am sending a patch with rw semaphore. > > So what I'm missing? What else can trigger GPU activity when not the rings? > > I'm currently working on ring-partial resets and also resets where you only > skip over a single faulty IB instead of flushing the whole ring. And my > current idea for that to work is that we hold the ring lock while we do > suspend, ring_save, asic_reset, resume and ring_restore. > > Christian. > I should add that gpu_reset should be an heavy reset, and if you want to only reset one ring and see if gpu can continue without heavy reset then you should do it as a special ring reset that doesn't reset mc and some other block but only the affected ring (and i am assuming that hw behave properly here). If that light weight ring reset doesn't work than let the heavy reset kicks in. So yes your light weight per ring reset would only need to take the ring lock but now need to change the ring lock usage we have now. Cheers, Jerome
[PATCH, RFC] i.MX DRM support
On Thu, Jun 14, 2012 at 03:43:22PM +0200, Sascha Hauer wrote: > Hi All, > > The following is the state-of-the-art i.MX IPU (Image Processing Unit) > DRM support. > > This code is around for quite some time now and has been posted several > times with different APIs, first with plain old framebuffer support, now > DRM, first platform device binding, now devicetree. Unfortunately there's > quite much code needed to get something useful out of the IPU, so these > patches haven't received a lot of attention from people not involved in > i.MX. I think we have now come to a point where this code needs more public > exposure and where it's easier to talk in incremental changes instead of > blobs. Therefore I request this to go to staging for some cycles. Dave, Greg, Comments to this one? I addressed the comments I received so far and am about to respin this series. Is it ok to put this to staging? If yes, should I move the whole stuff into drivers/staging/ or should it stay in drivers/gpu/drm with just a Kconfig dependency on STAGING? Thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH 1/3] drm/radeon: move ring locking out of reset path
On Mon, Jul 2, 2012 at 11:58 AM, Christian K?nig wrote: > On 02.07.2012 17:41, Jerome Glisse wrote: >> >> On Fri, Jun 29, 2012 at 12:15 PM, Michel D?nzer >> wrote: >>> >>> On Fre, 2012-06-29 at 17:18 +0200, Christian K?nig wrote: On 29.06.2012 17:09, Michel D?nzer wrote: > > On Fre, 2012-06-29 at 16:45 +0200, Christian K?nig wrote: >> >> Hold the ring lock the whole time the reset is in progress, >> otherwise another process can submit new jobs. > > Sounds good, but doesn't this create other paths (e.g. initialization, > resume) where the ring is being accessed without holding the lock? > Isn't > that a problem? Thought about that also. For init I'm pretty sure that no application can submit commands before we are done, otherwise we are doomed anyway. For resume I'm not really sure, but I think that applications are resumed after the hardware driver had a chance of doing so. >>> >>> I hope you're right... but if it's not too much trouble, it might be >>> better to be safe than sorry and take the lock for those paths as well. >>> >>> >> NAK this is the wrong way to solve the issue, we need a global lock on >> all path that can trigger gpu activities. Previously it was the cs >> mutex, but i haven't thought about it too much when it got removed. So >> to fix the situation i am sending a patch with rw semaphore. > > So what I'm missing? What else can trigger GPU activity when not the rings? > > I'm currently working on ring-partial resets and also resets where you only > skip over a single faulty IB instead of flushing the whole ring. And my > current idea for that to work is that we hold the ring lock while we do > suspend, ring_save, asic_reset, resume and ring_restore. > > Christian. > I just sent a patch, the idea is that you want gpu reset to be an exclusive operation like gpu init, or gpu resume. So by taking rw semaphore you allow the gpu reset to be exclusive and so you know nobody can trigger gpu activies while still allowing concurrency in case no gpu reset is on going. Cheers, Jerome
gma500 suspend to ram fails (3.4)
On Mon, 2 Jul 2012 07:06:59 +0900 Mattia Dongili wrote: > On Thu, Jun 21, 2012 at 06:19:25AM +0900, Mattia Dongili wrote: > > On Tue, Jun 19, 2012 at 07:56:52PM +0900, Mattia Dongili wrote: > > > On Mon, Jun 18, 2012 at 10:04:00PM +0200, Patrik Jakobsson wrote: > > > > On Sun, Jun 17, 2012 at 12:03 PM, Mattia Dongili > > > > wrote: > > ... > > > > If possible, add drm.debug=7 to your boot line and send a dmesg > > > > of 3.5-rc3. > > > > > > attached the dmesg on rc3 with drm.debug=7. > > > In addition I have a black screen and 3 modprobe processes stuck > > > that udev tried to kill for a bit... then the console went blank > > > and I can't tell if it's still trying: > > > > > > 359 ?R 11:08 /sbin/modprobe -b > > > pci:v8086d8108sv104Dsd905Fbc03sc00i00 > > > 361 ?D 0:00 /sbin/modprobe -b > > > pci:v8086d811Bsv104Dsd905Fbc04sc03i00 > > > 422 ?D 0:00 /sbin/modprobe -b > > > pci:v8086d8119sv104Dsd905Fbc06sc01i00 > > > > for the record, here below is where the screen flickers a bit and > > booting gets stuck until udev starts trying to kill the modprobe > > calls. > > I gave 3.5-rc4 a try, no significant change but by comparing 3.4 and > 3.5 logs I have this difference that may mean something: > > [0.00] Linux version 3.5.0-rc4... > [5.712020] gma500 :00:02.0: Backlight lvds set brightness > 74367436 > > while with 3.4 > > [5.525296] gma500 :00:02.0: Backlight lvds set brightness > 74407440 > > a did put some printks here and there and psb_driver_init seems to > never return from gma_backlight_init. Is it ok if you just comment out gma_backlight_init (at which point it should just skip backlight handling and leave the firmware configured one) Alan
[PATCH 1/3] drm/radeon: move ring locking out of reset path
On Fri, Jun 29, 2012 at 12:15 PM, Michel D?nzer wrote: > On Fre, 2012-06-29 at 17:18 +0200, Christian K?nig wrote: >> On 29.06.2012 17:09, Michel D?nzer wrote: >> > On Fre, 2012-06-29 at 16:45 +0200, Christian K?nig wrote: >> >> Hold the ring lock the whole time the reset is in progress, >> >> otherwise another process can submit new jobs. >> > Sounds good, but doesn't this create other paths (e.g. initialization, >> > resume) where the ring is being accessed without holding the lock? Isn't >> > that a problem? >> >> Thought about that also. >> >> For init I'm pretty sure that no application can submit commands before >> we are done, otherwise we are doomed anyway. >> >> For resume I'm not really sure, but I think that applications are >> resumed after the hardware driver had a chance of doing so. > > I hope you're right... but if it's not too much trouble, it might be > better to be safe than sorry and take the lock for those paths as well. > > NAK this is the wrong way to solve the issue, we need a global lock on all path that can trigger gpu activities. Previously it was the cs mutex, but i haven't thought about it too much when it got removed. So to fix the situation i am sending a patch with rw semaphore. Cheers, Jerome
[PATCH] drm/radeon: add an exclusive lock for GPU reset
From: Jerome Glisse GPU reset need to be exclusive, one happening at a time. For this add a rw semaphore so that any path that trigger GPU activities have to take the semaphore as a reader thus allowing concurency. The GPU reset path take the semaphore as a writer ensuring that no concurrent reset take place. Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/radeon.h|1 + drivers/gpu/drm/radeon/radeon_cs.c |5 + drivers/gpu/drm/radeon/radeon_device.c |2 ++ drivers/gpu/drm/radeon/radeon_gem.c|7 +++ 4 files changed, 15 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 77b4519b..29d6986 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1446,6 +1446,7 @@ struct radeon_device { struct device *dev; struct drm_device *ddev; struct pci_dev *pdev; + struct rw_semaphore exclusive_lock; /* ASIC */ union radeon_asic_configconfig; enum radeon_family family; diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index f1b7527..7ee6491 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct radeon_cs_parser parser; int r; + down_read(&rdev->exclusive_lock); if (!rdev->accel_working) { + up_read(&rdev->exclusive_lock); return -EBUSY; } /* initialize parser */ @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r) { DRM_ERROR("Failed to initialize parser !\n"); radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r != -ERESTARTSYS) DRM_ERROR("Failed to parse relocation %d!\n", r); radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) } out: radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index f654ba8..c8fdb40 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) int r; int resched; + down_write(&rdev->exclusive_lock); radeon_save_bios_scratch_regs(rdev); /* block TTM */ resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev); @@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) dev_info(rdev->dev, "GPU reset failed\n"); } + up_write(&rdev->exclusive_lock); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index d9b0809..f99db63 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data, uint32_t handle; int r; + down_read(&rdev->exclusive_lock); /* create a gem object to contain this object in */ args->size = roundup(args->size, PAGE_SIZE); r = radeon_gem_object_create(rdev, args->size, args->alignment, args->initial_domain, false, false, &gobj); if (r) { + up_read(&rdev->exclusive_lock); r = radeon_gem_handle_lockup(rdev, r); return r; } @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data, /* drop reference from allocate - handle holds it now */ drm_gem_object_unreference_unlocked(gobj); if (r) { + up_read(&rdev->exclusive_lock); r = radeon_gem_handle_lockup(rdev, r); return r; } args->handle = handle; + up_read(&rdev->exclusive_lock); return 0; } @@ -240,6 +244,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, void *data, { /* transition the BO to a domain - * just validate the BO into a certain domain */ + struct radeon_device *rdev = dev->dev_private; struct drm_radeon_gem_set_dom
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #8 from Eugene St Leger 2012-07-02 11:04:54 PDT --- A summary of all of the patches follows. r200 essential patches (1st 3 patches) for gnome shell: "Essential patch to disable texture formats that are reported unrenderable elsewhere in driver." "Essential patch to allow 2048 pixel blits." "Essential patch to reapply dirtied texenv registers." suggested minor fixes patch (4th patch): "Unessential fixes/enhancements patch." proposed r200 blit patch (5th patch) - unknown importance: "Proposed blit register dirtying patch." untested but probably essential r100/radeon patch (7th patch) for gnome shell: "Untested but probably essential patch to allow 2048 pixel blits on r100." optional unrecommended patches to supplement blit patches already mentioned above (6th & 8th patches): "Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on r200." "Optional untested patch to warn (once) when a blit with 2048 pixel dimension occurs on r100." -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #7 from Eugene St Leger 2012-07-02 10:51:10 PDT --- Created attachment 63718 --> https://bugs.freedesktop.org/attachment.cgi?id=63718 Optional untested patch to warn (once) when a blit with 2048 pixel dimension occurs on r100. If 2048 pixel blits cause graphical glitches/problems on r100, this patch can be used to provide a single warning. This patch is untested. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #6 from Eugene St Leger 2012-07-02 10:49:07 PDT --- Created attachment 63717 --> https://bugs.freedesktop.org/attachment.cgi?id=63717 Untested but probably essential patch to allow 2048 pixel blits on r100. Without this patch, gnome shell is expected to crash. This patch is untested. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #5 from Eugene St Leger 2012-07-02 10:45:01 UTC --- Created attachment 63716 --> https://bugs.freedesktop.org/attachment.cgi?id=63716 Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on r200. If 2048 pixel blits cause graphical glitches/problems on r200, this patch can be used to provide a single warning. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #5 from Eugene St Leger 2012-07-02 10:45:01 UTC --- Created attachment 63716 --> https://bugs.freedesktop.org/attachment.cgi?id=63716 Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on r200. If 2048 pixel blits cause graphical glitches/problems on r200, this patch can be used to provide a single warning. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #4 from Eugene St Leger 2012-07-02 10:40:51 PDT --- Created attachment 63715 --> https://bugs.freedesktop.org/attachment.cgi?id=63715 Proposed blit register dirtying patch. It appears bliting dirties some registers without notifying. This proposed patch notifies of register dirtying. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #3 from Eugene St Leger 2012-07-02 10:37:47 PDT --- Created attachment 63714 --> https://bugs.freedesktop.org/attachment.cgi?id=63714 Unessential fixes/enhancements patch. Minor spelling fixes & enhancements. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #2 from Eugene St Leger 2012-07-02 10:34:41 PDT --- Created attachment 63713 --> https://bugs.freedesktop.org/attachment.cgi?id=63713 Essential patch to reapply dirtied texenv registers. Without this patch, colour corruption happens with xv. xf86-video-ati textured xv dirties texenv registers and r200 DRI does not reapply them. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 51658] r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 --- Comment #1 from Eugene St Leger 2012-07-02 10:29:44 PDT --- Created attachment 63712 --> https://bugs.freedesktop.org/attachment.cgi?id=63712 Essential patch to allow 2048 pixel blits. Without this patch, gnome shell crashes. 2048 pixel blits are used. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: add an exclusive lock for GPU reset
On Mon, Jul 2, 2012 at 1:05 PM, Christian König wrote: > On 02.07.2012 18:41, Jerome Glisse wrote: >> >> On Mon, Jul 2, 2012 at 12:26 PM, Christian König >> wrote: >>> >>> On 02.07.2012 17:39, j.gli...@gmail.com wrote: From: Jerome Glisse GPU reset need to be exclusive, one happening at a time. For this add a rw semaphore so that any path that trigger GPU activities have to take the semaphore as a reader thus allowing concurency. The GPU reset path take the semaphore as a writer ensuring that no concurrent reset take place. Signed-off-by: Jerome Glisse >>> >>> NAK, that isn't as bad as the cs mutex was but still to complicated. It's >>> just too far up in the call stack, e.g. it tries to catch ioctl >>> operations, >>> instead of catching the underlying hardware operation which is caused by >>> the >>> ioctl/ttm/etc... >>> >>> Why not just take the ring look as I suggested? >>> >>> >> No we can't use ring lock, we need to protect suspend/resume path and >> we need an exclusive lock for that so we need a reset mutex at the >> very least. But instead of having a reset mutex i prefer using a rw >> lock so that we can stop ioctl until a reset goes through an let >> pending ioctl take proper action. Think about multiple context trying >> to reset GPU ... >> >> Really this is the best option, the rw locking wont induce any lock >> contention execept in gpu reset case which is anyway bad news. > > Why? That makes no sense to me. Well I don't want to prevent lock > contention, but understand why we should add locking at the ioctl level. > That violates locking rule number one "lock data instead of code" (or in our > case "lock hardware access instead of code path") and it really is the > reason why we ended up with the cs_mutex protecting practically everything. > > Multiple context trying to reset the GPU should be pretty fine, current it > would just reset the GPU twice, but in the future asic_reset should be much > more fine grained and only reset the parts of the GPU which really needs an > reset. > > Cheers, > Christian. No multiple reset is not fine, try it your self and you will see all kind of oops (strongly advise you to sync you hd before stress testing that). Yes we need to protect code path because suspend/resume code path is special one it touch many data in the device structure. GPU reset is a rare event or should be a rare event. I stress it we need at very least a mutex to protect gpu reset and i will stand on that position because there is no other way around. Using rw lock have a bonus of allowing proper handling of gpu reset failure and that what the patch i sent to linus fix tree is about, so to make drm next merge properly while preserving proper behavior in gpu reset failure the rw semaphore is the best option. Cheers, Jerome ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 51658] New: r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
https://bugs.freedesktop.org/show_bug.cgi?id=51658 Bug #: 51658 Summary: r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3 Classification: Unclassified Product: Mesa Version: 8.0 Platform: All OS/Version: All Status: NEW Severity: major Priority: medium Component: Drivers/DRI/r200 AssignedTo: dri-devel@lists.freedesktop.org ReportedBy: gri...@yahoo.com Created attachment 63711 --> https://bugs.freedesktop.org/attachment.cgi?id=63711 Essential patch to disable texture formats that are reported unrenderable elsewhere in driver. Without r200-tex-format-fix.patch, gnome shell crashes. Texture formats are selected by radeonChooseTexFormat but reported as unrenderable by radeonIsFormatRenderable. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: add an exclusive lock for GPU reset
On 02.07.2012 18:41, Jerome Glisse wrote: On Mon, Jul 2, 2012 at 12:26 PM, Christian König wrote: On 02.07.2012 17:39, j.gli...@gmail.com wrote: From: Jerome Glisse GPU reset need to be exclusive, one happening at a time. For this add a rw semaphore so that any path that trigger GPU activities have to take the semaphore as a reader thus allowing concurency. The GPU reset path take the semaphore as a writer ensuring that no concurrent reset take place. Signed-off-by: Jerome Glisse NAK, that isn't as bad as the cs mutex was but still to complicated. It's just too far up in the call stack, e.g. it tries to catch ioctl operations, instead of catching the underlying hardware operation which is caused by the ioctl/ttm/etc... Why not just take the ring look as I suggested? No we can't use ring lock, we need to protect suspend/resume path and we need an exclusive lock for that so we need a reset mutex at the very least. But instead of having a reset mutex i prefer using a rw lock so that we can stop ioctl until a reset goes through an let pending ioctl take proper action. Think about multiple context trying to reset GPU ... Really this is the best option, the rw locking wont induce any lock contention execept in gpu reset case which is anyway bad news. Why? That makes no sense to me. Well I don't want to prevent lock contention, but understand why we should add locking at the ioctl level. That violates locking rule number one "lock data instead of code" (or in our case "lock hardware access instead of code path") and it really is the reason why we ended up with the cs_mutex protecting practically everything. Multiple context trying to reset the GPU should be pretty fine, current it would just reset the GPU twice, but in the future asic_reset should be much more fine grained and only reset the parts of the GPU which really needs an reset. Cheers, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: add an exclusive lock for GPU reset v2
From: Jerome Glisse GPU reset need to be exclusive, one happening at a time. For this add a rw semaphore so that any path that trigger GPU activities have to take the semaphore as a reader thus allowing concurency. The GPU reset path take the semaphore as a writer ensuring that no concurrent reset take place. v2: init rw semaphore Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/radeon.h|1 + drivers/gpu/drm/radeon/radeon_cs.c |5 + drivers/gpu/drm/radeon/radeon_device.c |3 +++ drivers/gpu/drm/radeon/radeon_gem.c|8 4 files changed, 17 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 77b4519b..29d6986 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1446,6 +1446,7 @@ struct radeon_device { struct device *dev; struct drm_device *ddev; struct pci_dev *pdev; + struct rw_semaphore exclusive_lock; /* ASIC */ union radeon_asic_configconfig; enum radeon_family family; diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index f1b7527..7ee6491 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct radeon_cs_parser parser; int r; + down_read(&rdev->exclusive_lock); if (!rdev->accel_working) { + up_read(&rdev->exclusive_lock); return -EBUSY; } /* initialize parser */ @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r) { DRM_ERROR("Failed to initialize parser !\n"); radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r != -ERESTARTSYS) DRM_ERROR("Failed to parse relocation %d!\n", r); radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) } out: radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index f654ba8..254fdb4 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -734,6 +734,7 @@ int radeon_device_init(struct radeon_device *rdev, mutex_init(&rdev->gem.mutex); mutex_init(&rdev->pm.mutex); init_rwsem(&rdev->pm.mclk_lock); + init_rwsem(&rdev->exclusive_lock); init_waitqueue_head(&rdev->irq.vblank_queue); init_waitqueue_head(&rdev->irq.idle_queue); r = radeon_gem_init(rdev); @@ -988,6 +989,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) int r; int resched; + down_write(&rdev->exclusive_lock); radeon_save_bios_scratch_regs(rdev); /* block TTM */ resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev); @@ -1007,6 +1009,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) dev_info(rdev->dev, "GPU reset failed\n"); } + up_write(&rdev->exclusive_lock); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index d9b0809..b0be9c4 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data, uint32_t handle; int r; + down_read(&rdev->exclusive_lock); /* create a gem object to contain this object in */ args->size = roundup(args->size, PAGE_SIZE); r = radeon_gem_object_create(rdev, args->size, args->alignment, args->initial_domain, false, false, &gobj); if (r) { + up_read(&rdev->exclusive_lock); r = radeon_gem_handle_lockup(rdev, r); return r; } @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data, /* drop reference from allocate - handle holds it now */ drm_gem_object_unreference_unlocked(gobj); if (r) { + up_read(&rdev->exclusive_lock); r = radeon_gem_handle_lockup(rdev, r);
[PATCH] drm/radeon: fix rare segfault
From: Jerome Glisse In gem idle/busy ioctl the radeon object was derefenced after drm_gem_object_unreference_unlocked which in case the object have been destroyed lead to use of a possibly free pointer with possibly wrong data. Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/radeon_gem.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index 74176c5..c8838fc 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -325,6 +325,7 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void *data, int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) { + struct radeon_device *rdev = dev->dev_private; struct drm_radeon_gem_busy *args = data; struct drm_gem_object *gobj; struct radeon_bo *robj; @@ -350,13 +351,14 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void *data, break; } drm_gem_object_unreference_unlocked(gobj); - r = radeon_gem_handle_lockup(robj->rdev, r); + r = radeon_gem_handle_lockup(rdev, r); return r; } int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) { + struct radeon_device *rdev = dev->dev_private; struct drm_radeon_gem_wait_idle *args = data; struct drm_gem_object *gobj; struct radeon_bo *robj; @@ -369,10 +371,10 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data, robj = gem_to_radeon_bo(gobj); r = radeon_bo_wait(robj, NULL, false); /* callback hw specific functions if any */ - if (robj->rdev->asic->ioctl_wait_idle) - robj->rdev->asic->ioctl_wait_idle(robj->rdev, robj); + if (rdev->asic->ioctl_wait_idle) + robj->rdev->asic->ioctl_wait_idle(rdev, robj); drm_gem_object_unreference_unlocked(gobj); - r = radeon_gem_handle_lockup(robj->rdev, r); + r = radeon_gem_handle_lockup(rdev, r); return r; } -- 1.7.10.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: add an exclusive lock for GPU reset
On Mon, Jul 2, 2012 at 12:26 PM, Christian König wrote: > On 02.07.2012 17:39, j.gli...@gmail.com wrote: >> >> From: Jerome Glisse >> >> GPU reset need to be exclusive, one happening at a time. For this >> add a rw semaphore so that any path that trigger GPU activities >> have to take the semaphore as a reader thus allowing concurency. >> >> The GPU reset path take the semaphore as a writer ensuring that >> no concurrent reset take place. >> >> Signed-off-by: Jerome Glisse > > NAK, that isn't as bad as the cs mutex was but still to complicated. It's > just too far up in the call stack, e.g. it tries to catch ioctl operations, > instead of catching the underlying hardware operation which is caused by the > ioctl/ttm/etc... > > Why not just take the ring look as I suggested? > > No we can't use ring lock, we need to protect suspend/resume path and we need an exclusive lock for that so we need a reset mutex at the very least. But instead of having a reset mutex i prefer using a rw lock so that we can stop ioctl until a reset goes through an let pending ioctl take proper action. Think about multiple context trying to reset GPU ... Really this is the best option, the rw locking wont induce any lock contention execept in gpu reset case which is anyway bad news. Jerome ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: add an exclusive lock for GPU reset
On 02.07.2012 17:39, j.gli...@gmail.com wrote: From: Jerome Glisse GPU reset need to be exclusive, one happening at a time. For this add a rw semaphore so that any path that trigger GPU activities have to take the semaphore as a reader thus allowing concurency. The GPU reset path take the semaphore as a writer ensuring that no concurrent reset take place. Signed-off-by: Jerome Glisse NAK, that isn't as bad as the cs mutex was but still to complicated. It's just too far up in the call stack, e.g. it tries to catch ioctl operations, instead of catching the underlying hardware operation which is caused by the ioctl/ttm/etc... Why not just take the ring look as I suggested? Christian. --- drivers/gpu/drm/radeon/radeon.h|1 + drivers/gpu/drm/radeon/radeon_cs.c |5 + drivers/gpu/drm/radeon/radeon_device.c |2 ++ drivers/gpu/drm/radeon/radeon_gem.c|7 +++ 4 files changed, 15 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 77b4519b..29d6986 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1446,6 +1446,7 @@ struct radeon_device { struct device *dev; struct drm_device *ddev; struct pci_dev *pdev; + struct rw_semaphore exclusive_lock; /* ASIC */ union radeon_asic_configconfig; enum radeon_family family; diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index f1b7527..7ee6491 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct radeon_cs_parser parser; int r; + down_read(&rdev->exclusive_lock); if (!rdev->accel_working) { + up_read(&rdev->exclusive_lock); return -EBUSY; } /* initialize parser */ @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r) { DRM_ERROR("Failed to initialize parser !\n"); radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r != -ERESTARTSYS) DRM_ERROR("Failed to parse relocation %d!\n", r); radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) } out: radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index f654ba8..c8fdb40 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) int r; int resched; + down_write(&rdev->exclusive_lock); radeon_save_bios_scratch_regs(rdev); /* block TTM */ resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev); @@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) dev_info(rdev->dev, "GPU reset failed\n"); } + up_write(&rdev->exclusive_lock); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index d9b0809..f99db63 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data, uint32_t handle; int r; + down_read(&rdev->exclusive_lock); /* create a gem object to contain this object in */ args->size = roundup(args->size, PAGE_SIZE); r = radeon_gem_object_create(rdev, args->size, args->alignment, args->initial_domain, false, false, &gobj); if (r) { + up_read(&rdev->exclusive_lock); r = radeon_gem_handle_lockup(rdev, r); return r; } @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data, /* drop reference from allocate - handle holds it now */ drm_gem_object_unreference_unlocked(gobj); if (r) { + up_read(&rdev->exclusive_lock); r = radeon_gem_handle_lockup(rdev, r); return r; } args->handle
Re: [PATCH] drm/radeon: add an exclusive lock for GPU reset
On Mon, Jul 2, 2012 at 11:39 AM, wrote: > From: Jerome Glisse > > GPU reset need to be exclusive, one happening at a time. For this > add a rw semaphore so that any path that trigger GPU activities > have to take the semaphore as a reader thus allowing concurency. > > The GPU reset path take the semaphore as a writer ensuring that > no concurrent reset take place. > > Signed-off-by: Jerome Glisse Wrong patch sorry resending > --- > drivers/gpu/drm/radeon/radeon.h | 1 + > drivers/gpu/drm/radeon/radeon_cs.c | 5 + > drivers/gpu/drm/radeon/radeon_device.c | 2 ++ > drivers/gpu/drm/radeon/radeon_gem.c | 7 +++ > 4 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index 77b4519b..29d6986 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -1446,6 +1446,7 @@ struct radeon_device { > struct device *dev; > struct drm_device *ddev; > struct pci_dev *pdev; > + struct rw_semaphore exclusive_lock; > /* ASIC */ > union radeon_asic_config config; > enum radeon_family family; > diff --git a/drivers/gpu/drm/radeon/radeon_cs.c > b/drivers/gpu/drm/radeon/radeon_cs.c > index f1b7527..7ee6491 100644 > --- a/drivers/gpu/drm/radeon/radeon_cs.c > +++ b/drivers/gpu/drm/radeon/radeon_cs.c > @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > struct radeon_cs_parser parser; > int r; > > + down_read(&rdev->exclusive_lock); > if (!rdev->accel_working) { > + up_read(&rdev->exclusive_lock); > return -EBUSY; > } > /* initialize parser */ > @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > if (r) { > DRM_ERROR("Failed to initialize parser !\n"); > radeon_cs_parser_fini(&parser, r); > + up_read(&rdev->exclusive_lock); > r = radeon_cs_handle_lockup(rdev, r); > return r; > } > @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > if (r != -ERESTARTSYS) > DRM_ERROR("Failed to parse relocation %d!\n", r); > radeon_cs_parser_fini(&parser, r); > + up_read(&rdev->exclusive_lock); > r = radeon_cs_handle_lockup(rdev, r); > return r; > } > @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > } > out: > radeon_cs_parser_fini(&parser, r); > + up_read(&rdev->exclusive_lock); > r = radeon_cs_handle_lockup(rdev, r); > return r; > } > diff --git a/drivers/gpu/drm/radeon/radeon_device.c > b/drivers/gpu/drm/radeon/radeon_device.c > index f654ba8..c8fdb40 100644 > --- a/drivers/gpu/drm/radeon/radeon_device.c > +++ b/drivers/gpu/drm/radeon/radeon_device.c > @@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) > int r; > int resched; > > + down_write(&rdev->exclusive_lock); > radeon_save_bios_scratch_regs(rdev); > /* block TTM */ > resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev); > @@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) > dev_info(rdev->dev, "GPU reset failed\n"); > } > > + up_write(&rdev->exclusive_lock); > return r; > } > > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c > b/drivers/gpu/drm/radeon/radeon_gem.c > index d9b0809..f99db63 100644 > --- a/drivers/gpu/drm/radeon/radeon_gem.c > +++ b/drivers/gpu/drm/radeon/radeon_gem.c > @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, > void *data, > uint32_t handle; > int r; > > + down_read(&rdev->exclusive_lock); > /* create a gem object to contain this object in */ > args->size = roundup(args->size, PAGE_SIZE); > r = radeon_gem_object_create(rdev, args->size, args->alignment, > args->initial_domain, false, > false, &gobj); > if (r) { > + up_read(&rdev->exclusive_lock); > r = radeon_gem_handle_lockup(rdev, r); > return r; > } > @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, > void *data, > /* drop reference from allocate - handle holds it now */ > drm_gem_object_unreference_unlocked(gobj); > if (r) { > + up_read(&rdev->exclusive_lock); > r = radeon_gem_handle_lockup(rdev, r); > return r; > } > args->handle = handle; > + up_read(&rdev->exclusive_lock); > return 0; > } > > @@ -240,6 +244,7 @@
Re: [PATCH 1/3] drm/radeon: move ring locking out of reset path
On Mon, Jul 2, 2012 at 11:58 AM, Christian König wrote: > On 02.07.2012 17:41, Jerome Glisse wrote: >> >> On Fri, Jun 29, 2012 at 12:15 PM, Michel Dänzer >> wrote: >>> >>> On Fre, 2012-06-29 at 17:18 +0200, Christian König wrote: On 29.06.2012 17:09, Michel Dänzer wrote: > > On Fre, 2012-06-29 at 16:45 +0200, Christian König wrote: >> >> Hold the ring lock the whole time the reset is in progress, >> otherwise another process can submit new jobs. > > Sounds good, but doesn't this create other paths (e.g. initialization, > resume) where the ring is being accessed without holding the lock? > Isn't > that a problem? Thought about that also. For init I'm pretty sure that no application can submit commands before we are done, otherwise we are doomed anyway. For resume I'm not really sure, but I think that applications are resumed after the hardware driver had a chance of doing so. >>> >>> I hope you're right... but if it's not too much trouble, it might be >>> better to be safe than sorry and take the lock for those paths as well. >>> >>> >> NAK this is the wrong way to solve the issue, we need a global lock on >> all path that can trigger gpu activities. Previously it was the cs >> mutex, but i haven't thought about it too much when it got removed. So >> to fix the situation i am sending a patch with rw semaphore. > > So what I'm missing? What else can trigger GPU activity when not the rings? > > I'm currently working on ring-partial resets and also resets where you only > skip over a single faulty IB instead of flushing the whole ring. And my > current idea for that to work is that we hold the ring lock while we do > suspend, ring_save, asic_reset, resume and ring_restore. > > Christian. > I should add that gpu_reset should be an heavy reset, and if you want to only reset one ring and see if gpu can continue without heavy reset then you should do it as a special ring reset that doesn't reset mc and some other block but only the affected ring (and i am assuming that hw behave properly here). If that light weight ring reset doesn't work than let the heavy reset kicks in. So yes your light weight per ring reset would only need to take the ring lock but now need to change the ring lock usage we have now. Cheers, Jerome ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2] implicit drm synchronization wip with dma-buf
Well, V2 time! I was hinted to look at ttm_execbuf_util, and it does indeed contain some nice code. My goal was to extend dma-buf in a generic way now, some elements from v1 are remaining, most notably a dma-buf used for syncing. However it is expected now that dma-buf syncing will work in a very specific way now, slightly more strict than in v1. Instead of each buffer having their own dma-buf I put in 1 per command submission. This submission hasn't been run-time tested yet, but I expect the api to go something like this. Intended to be used like this: list_init(&head); add_list_tail(&validate1->entry, &head); add_list_tail(&validate2->entry, &head); add_list_tail(&validate3->entry, &head); r = dmabufmgr_eu_reserve_buffers(&head); if (r) return r; // add waits on cpu or gpu list_for_each_entry(validate, ...) { if (!validate->sync_buf) continue; // Check attachments to see if we already imported sync_buf // somewhere, if not attach to it. // waiting until cur_seq - dmabuf->sync_val >= 0 on either cpu // or as command submitted to gpu // sync_buf itself is a dma-buf, so it should be trivial // TODO: sync_buf should NEVER be validated, add is_sync_buf to dma_buf? // If this step fails: dmabufmgr_eu_backoff_reservation // else: // dmabufmgr_eu_fence_buffer_objects(our_own_sync_buf, // hwchannel * max(minhwalign, 4), ++counter[hwchannel]); // XXX: Do we still require a minimum alignment? I set up 16 for nouveau, // but this is no longer needed in this design since it only matters // for writes for which nouveau would already control the offset. } // Some time after execbuffer was executed, doesn't have to be right away but before // getting in the danger of our own counter wrapping around: // grab dmabufmgr.lru_lock, and cleanup by unreffing sync_buf when // sync_buf == ownbuf, sync_ofs == ownofs, and sync_val == saved_counter // In the meantime someone else or even us might have reserved this dma_buf // again, which is why all those checks are needed before unreffing. diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 5aa2d70..86e7598 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -10,7 +10,7 @@ obj-$(CONFIG_CMA) += dma-contiguous.o obj-y += power/ obj-$(CONFIG_HAS_DMA) += dma-mapping.o obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o -obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o +obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o dma-buf-mgr.o dma-buf-mgr-eu.o obj-$(CONFIG_ISA) += isa.o obj-$(CONFIG_FW_LOADER)+= firmware_class.o obj-$(CONFIG_NUMA) += node.o diff --git a/drivers/base/dma-buf-mgr-eu.c b/drivers/base/dma-buf-mgr-eu.c new file mode 100644 index 000..27ebc68 --- /dev/null +++ b/drivers/base/dma-buf-mgr-eu.c @@ -0,0 +1,170 @@ +/* + * Copyright (C) 2012 Canonical Ltd + * + * Based on ttm_bo.c which bears the following copyright notice, + * but is dual licensed: + * + * Copyright (c) 2006-2009 VMware, Inc., Palo Alto, CA., USA + * All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sub license, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice (including the + * next paragraph) shall be included in all copies or substantial portions + * of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE + * USE OR OTHER DEALINGS IN THE SOFTWARE. + * + **/ + +#include +#include +#include + +static void dmabufmgr_eu_backoff_reservation_locked(struct list_head *list) +{ + struct dmabufmgr_validate *entry; + + list_for_each_entry(entry, list, head) { + struct dma_buf *bo = entry->bo; + if (!entry->reserved) + continue; + + entry->reserved = false; + atomic_set(&bo->reserved, 0); + wake_up_all(&bo->event_queue); + if (entry->sync_buf) + dma_buf_put(entry->sync_buf); + entry->sync_buf =
Re: [PATCH 1/3] drm/radeon: move ring locking out of reset path
On Mon, Jul 2, 2012 at 11:58 AM, Christian König wrote: > On 02.07.2012 17:41, Jerome Glisse wrote: >> >> On Fri, Jun 29, 2012 at 12:15 PM, Michel Dänzer >> wrote: >>> >>> On Fre, 2012-06-29 at 17:18 +0200, Christian König wrote: On 29.06.2012 17:09, Michel Dänzer wrote: > > On Fre, 2012-06-29 at 16:45 +0200, Christian König wrote: >> >> Hold the ring lock the whole time the reset is in progress, >> otherwise another process can submit new jobs. > > Sounds good, but doesn't this create other paths (e.g. initialization, > resume) where the ring is being accessed without holding the lock? > Isn't > that a problem? Thought about that also. For init I'm pretty sure that no application can submit commands before we are done, otherwise we are doomed anyway. For resume I'm not really sure, but I think that applications are resumed after the hardware driver had a chance of doing so. >>> >>> I hope you're right... but if it's not too much trouble, it might be >>> better to be safe than sorry and take the lock for those paths as well. >>> >>> >> NAK this is the wrong way to solve the issue, we need a global lock on >> all path that can trigger gpu activities. Previously it was the cs >> mutex, but i haven't thought about it too much when it got removed. So >> to fix the situation i am sending a patch with rw semaphore. > > So what I'm missing? What else can trigger GPU activity when not the rings? > > I'm currently working on ring-partial resets and also resets where you only > skip over a single faulty IB instead of flushing the whole ring. And my > current idea for that to work is that we hold the ring lock while we do > suspend, ring_save, asic_reset, resume and ring_restore. > > Christian. > I just sent a patch, the idea is that you want gpu reset to be an exclusive operation like gpu init, or gpu resume. So by taking rw semaphore you allow the gpu reset to be exclusive and so you know nobody can trigger gpu activies while still allowing concurrency in case no gpu reset is on going. Cheers, Jerome ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/radeon: move ring locking out of reset path
On 02.07.2012 17:41, Jerome Glisse wrote: On Fri, Jun 29, 2012 at 12:15 PM, Michel Dänzer wrote: On Fre, 2012-06-29 at 17:18 +0200, Christian König wrote: On 29.06.2012 17:09, Michel Dänzer wrote: On Fre, 2012-06-29 at 16:45 +0200, Christian König wrote: Hold the ring lock the whole time the reset is in progress, otherwise another process can submit new jobs. Sounds good, but doesn't this create other paths (e.g. initialization, resume) where the ring is being accessed without holding the lock? Isn't that a problem? Thought about that also. For init I'm pretty sure that no application can submit commands before we are done, otherwise we are doomed anyway. For resume I'm not really sure, but I think that applications are resumed after the hardware driver had a chance of doing so. I hope you're right... but if it's not too much trouble, it might be better to be safe than sorry and take the lock for those paths as well. NAK this is the wrong way to solve the issue, we need a global lock on all path that can trigger gpu activities. Previously it was the cs mutex, but i haven't thought about it too much when it got removed. So to fix the situation i am sending a patch with rw semaphore. So what I'm missing? What else can trigger GPU activity when not the rings? I'm currently working on ring-partial resets and also resets where you only skip over a single faulty IB instead of flushing the whole ring. And my current idea for that to work is that we hold the ring lock while we do suspend, ring_save, asic_reset, resume and ring_restore. Christian. Cheers, Jerome ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH, RFC] i.MX DRM support
On 02.07.2012 12:05, Sascha Hauer wrote: On Thu, Jun 14, 2012 at 03:43:22PM +0200, Sascha Hauer wrote: Hi All, The following is the state-of-the-art i.MX IPU (Image Processing Unit) DRM support. This code is around for quite some time now and has been posted several times with different APIs, first with plain old framebuffer support, now DRM, first platform device binding, now devicetree. Unfortunately there's quite much code needed to get something useful out of the IPU, so these patches haven't received a lot of attention from people not involved in i.MX. I think we have now come to a point where this code needs more public exposure and where it's easier to talk in incremental changes instead of blobs. Therefore I request this to go to staging for some cycles. Dave, Greg, Comments to this one? I addressed the comments I received so far and am about to respin this series. Is it ok to put this to staging? If yes, should I move the whole stuff into drivers/staging/ or should it stay in drivers/gpu/drm with just a Kconfig dependency on STAGING? We are very interested in this, so +1 from my side for this. Many thanks and best regards Dirk Btw.: Based on http://git.pengutronix.de/?p=imx/linux-2.6.git;a=shortlog;h=refs/heads/work/gpu/imx-drm-ipu-complete with [1] below I fixed some compiler warnings. I have some additional warnings drivers/gpu/drm/drm_edid.c: In function 'drm_find_cea_extension': drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds drivers/gpu/drm/drm_edid.c: In function 'drm_detect_hdmi_monitor': drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds drivers/gpu/drm/drm_edid.c: In function 'drm_detect_monitor_audio': drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds drivers/gpu/drm/drm_edid.c: In function 'drm_edid_to_eld': drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds drivers/gpu/drm/drm_edid.c: In function 'drm_add_edid_modes': drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds drivers/gpu/drm/drm_edid.c:1466: warning: array subscript is above array bounds but these seems to be compiler specific and not related to Sacha's patches. [1] From: Dirk Behme Subject: [PATCH 1/2] DRM i.MX: ldb: Fix compiler warnings Fix the compiler warnings drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_encoder_mode_fixup': drivers/gpu/drm/imx/imx-ldb.c:118: warning: unused variable 'imx_ldb_ch' drivers/gpu/drm/imx/imx-ldb.c: In function 'imx_ldb_encoder_prepare': drivers/gpu/drm/imx/imx-ldb.c:134: warning: format '%ld' expects type 'long int', but argument 6 has type 'int' Signed-off-by: Dirk Behme --- drivers/gpu/drm/imx/imx-ldb.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: freescale-linux-2.6-imx.git/drivers/gpu/drm/imx/imx-ldb.c === --- freescale-linux-2.6-imx.git.orig/drivers/gpu/drm/imx/imx-ldb.c +++ freescale-linux-2.6-imx.git/drivers/gpu/drm/imx/imx-ldb.c @@ -115,9 +115,9 @@ static bool imx_ldb_encoder_mode_fixup(s struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) { +/* struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder); -/* adjusted_mode->clock = clk_round_rate(imx_ldb_ch->clk_pll, adjusted_mode->clock * 1000) / 1000; */ @@ -131,7 +131,7 @@ static void imx_ldb_encoder_prepare(stru int ret; struct drm_display_mode *mode = &encoder->crtc->mode; - dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__, + dev_dbg(ldb->dev, "%s: now: %ld want: %d\n", __func__, clk_get_rate(imx_ldb_ch->clk_pll), mode->clock * 1000 * 7); clk_set_rate(imx_ldb_ch->clk_pll, mode->clock * 1000 * 7); ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: gma500 suspend to ram fails (3.4)
On Thu, Jun 21, 2012 at 06:19:25AM +0900, Mattia Dongili wrote: > On Tue, Jun 19, 2012 at 07:56:52PM +0900, Mattia Dongili wrote: > > On Mon, Jun 18, 2012 at 10:04:00PM +0200, Patrik Jakobsson wrote: > > > On Sun, Jun 17, 2012 at 12:03 PM, Mattia Dongili > > > wrote: > ... > > > If possible, add drm.debug=7 to your boot line and send a dmesg of > > > 3.5-rc3. > > > > attached the dmesg on rc3 with drm.debug=7. > > In addition I have a black screen and 3 modprobe processes stuck that > > udev tried to kill for a bit... then the console went blank and I can't > > tell if it's still trying: > > > > 359 ?R 11:08 /sbin/modprobe -b > > pci:v8086d8108sv104Dsd905Fbc03sc00i00 > > 361 ?D 0:00 /sbin/modprobe -b > > pci:v8086d811Bsv104Dsd905Fbc04sc03i00 > > 422 ?D 0:00 /sbin/modprobe -b > > pci:v8086d8119sv104Dsd905Fbc06sc01i00 > > for the record, here below is where the screen flickers a bit and > booting gets stuck until udev starts trying to kill the modprobe calls. I gave 3.5-rc4 a try, no significant change but by comparing 3.4 and 3.5 logs I have this difference that may mean something: [0.00] Linux version 3.5.0-rc4... [5.712020] gma500 :00:02.0: Backlight lvds set brightness 74367436 while with 3.4 [5.525296] gma500 :00:02.0: Backlight lvds set brightness 74407440 a did put some printks here and there and psb_driver_init seems to never return from gma_backlight_init. -- mattia :wq! ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: add an exclusive lock for GPU reset
From: Jerome Glisse GPU reset need to be exclusive, one happening at a time. For this add a rw semaphore so that any path that trigger GPU activities have to take the semaphore as a reader thus allowing concurency. The GPU reset path take the semaphore as a writer ensuring that no concurrent reset take place. Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/radeon.h|1 + drivers/gpu/drm/radeon/radeon_cs.c |5 + drivers/gpu/drm/radeon/radeon_device.c |2 ++ drivers/gpu/drm/radeon/radeon_gem.c|7 +++ 4 files changed, 15 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 77b4519b..29d6986 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1446,6 +1446,7 @@ struct radeon_device { struct device *dev; struct drm_device *ddev; struct pci_dev *pdev; + struct rw_semaphore exclusive_lock; /* ASIC */ union radeon_asic_configconfig; enum radeon_family family; diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index f1b7527..7ee6491 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct radeon_cs_parser parser; int r; + down_read(&rdev->exclusive_lock); if (!rdev->accel_working) { + up_read(&rdev->exclusive_lock); return -EBUSY; } /* initialize parser */ @@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r) { DRM_ERROR("Failed to initialize parser !\n"); radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } @@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) if (r != -ERESTARTSYS) DRM_ERROR("Failed to parse relocation %d!\n", r); radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } @@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) } out: radeon_cs_parser_fini(&parser, r); + up_read(&rdev->exclusive_lock); r = radeon_cs_handle_lockup(rdev, r); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index f654ba8..c8fdb40 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -988,6 +988,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) int r; int resched; + down_write(&rdev->exclusive_lock); radeon_save_bios_scratch_regs(rdev); /* block TTM */ resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev); @@ -1007,6 +1008,7 @@ int radeon_gpu_reset(struct radeon_device *rdev) dev_info(rdev->dev, "GPU reset failed\n"); } + up_write(&rdev->exclusive_lock); return r; } diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index d9b0809..f99db63 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data, uint32_t handle; int r; + down_read(&rdev->exclusive_lock); /* create a gem object to contain this object in */ args->size = roundup(args->size, PAGE_SIZE); r = radeon_gem_object_create(rdev, args->size, args->alignment, args->initial_domain, false, false, &gobj); if (r) { + up_read(&rdev->exclusive_lock); r = radeon_gem_handle_lockup(rdev, r); return r; } @@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data, /* drop reference from allocate - handle holds it now */ drm_gem_object_unreference_unlocked(gobj); if (r) { + up_read(&rdev->exclusive_lock); r = radeon_gem_handle_lockup(rdev, r); return r; } args->handle = handle; + up_read(&rdev->exclusive_lock); return 0; } @@ -240,6 +244,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, void *data, { /* transition the BO to a domain - * just validate the BO into a certain domain */ + struct radeon_device *rdev = dev->dev_private; struct drm_radeon_gem_s
Re: [PATCH 1/3] drm/radeon: move ring locking out of reset path
On Fri, Jun 29, 2012 at 12:15 PM, Michel Dänzer wrote: > On Fre, 2012-06-29 at 17:18 +0200, Christian König wrote: >> On 29.06.2012 17:09, Michel Dänzer wrote: >> > On Fre, 2012-06-29 at 16:45 +0200, Christian König wrote: >> >> Hold the ring lock the whole time the reset is in progress, >> >> otherwise another process can submit new jobs. >> > Sounds good, but doesn't this create other paths (e.g. initialization, >> > resume) where the ring is being accessed without holding the lock? Isn't >> > that a problem? >> >> Thought about that also. >> >> For init I'm pretty sure that no application can submit commands before >> we are done, otherwise we are doomed anyway. >> >> For resume I'm not really sure, but I think that applications are >> resumed after the hardware driver had a chance of doing so. > > I hope you're right... but if it's not too much trouble, it might be > better to be safe than sorry and take the lock for those paths as well. > > NAK this is the wrong way to solve the issue, we need a global lock on all path that can trigger gpu activities. Previously it was the cs mutex, but i haven't thought about it too much when it got removed. So to fix the situation i am sending a patch with rw semaphore. Cheers, Jerome ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] DRM: Add DRM kms/fb cma helper
This patchset introduces a set of helper function for implementing the KMS framebuffer layer for drivers which use the drm gem CMA helper function. Signed-off-by: Lars-Peter Clausen --- Note: This patch depends on Sascha's "DRM: add drm gem CMA helper" patch Changes since v2: * Adapt to changes in the GEM CMA helper * Add basic buffer size checking in drm_fb_cma_create Changes since v1: * Some spelling fixes * Add missing kfree in drm_fb_cma_alloc error path * Add multi-plane support --- drivers/gpu/drm/Kconfig | 10 + drivers/gpu/drm/Makefile|1 + drivers/gpu/drm/drm_fb_cma_helper.c | 393 +++ include/drm/drm_fb_cma_helper.h | 27 +++ 4 files changed, 431 insertions(+) create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c create mode 100644 include/drm/drm_fb_cma_helper.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 41bbd95..e511c9a 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -41,6 +41,16 @@ config DRM_GEM_CMA_HELPER help Choose this if you need the GEM CMA helper functions +config DRM_KMS_CMA_HELPER + tristate + select DRM_GEM_CMA_HELPER + select DRM_KMS_HELPER + select FB_SYS_FILLRECT + select FB_SYS_COPYAREA + select FB_SYS_IMAGEBLIT + help + Choose this if you need the KMS cma helper functions + config DRM_TDFX tristate "3dfx Banshee/Voodoo3+" depends on DRM && PCI diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 6e9e948..5dcb1a5 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o +drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c new file mode 100644 index 000..9042233 --- /dev/null +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -0,0 +1,393 @@ +/* + * drm kms/fb cma (contiguous memory allocator) helper functions + * + * Copyright (C) 2012 Analog Device Inc. + * Author: Lars-Peter Clausen + * + * Based on udl_fbdev.c + * Copyright (C) 2012 Red Hat + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +struct drm_fb_cma { + struct drm_framebuffer fb; + struct drm_gem_cma_object *obj[4]; +}; + +struct drm_fbdev_cma { + struct drm_fb_helperfb_helper; + struct drm_fb_cma *fb; +}; + +static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper) +{ + return container_of(helper, struct drm_fbdev_cma, fb_helper); +} + +static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb) +{ + return container_of(fb, struct drm_fb_cma, fb); +} + +static void drm_fb_cma_destroy(struct drm_framebuffer *fb) +{ + struct drm_fb_cma *fb_cma = to_fb_cma(fb); + int i; + + for (i = 0; i < 4; i++) { + if (fb_cma->obj[i]) + drm_gem_object_unreference_unlocked(&fb_cma->obj[i]->base); + } + + drm_framebuffer_cleanup(fb); + kfree(fb_cma); +} + +static int drm_fb_cma_create_handle(struct drm_framebuffer *fb, + struct drm_file *file_priv, unsigned int *handle) +{ + struct drm_fb_cma *fb_cma = to_fb_cma(fb); + + return drm_gem_handle_create(file_priv, + &fb_cma->obj[0]->base, handle); +} + +static struct drm_framebuffer_funcs drm_fb_cma_funcs = { + .destroy= drm_fb_cma_destroy, + .create_handle = drm_fb_cma_create_handle, +}; + +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, + struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_object **obj, + unsigned int num_planes) +{ + struct drm_fb_cma *fb_cma; + int ret; + int i; + + fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL); + if (!fb_cma) + return ERR_PTR(-ENOMEM); + + ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs); + if (ret) { + dev_err(dev->dev, "Failed to initalize framebuffer: %d\n", ret); + kfree(fb_cma); + return ERR_PTR(ret); + } + + drm_he
gma500 suspend to ram fails (3.4)
On Thu, Jun 21, 2012 at 06:19:25AM +0900, Mattia Dongili wrote: > On Tue, Jun 19, 2012 at 07:56:52PM +0900, Mattia Dongili wrote: > > On Mon, Jun 18, 2012 at 10:04:00PM +0200, Patrik Jakobsson wrote: > > > On Sun, Jun 17, 2012 at 12:03 PM, Mattia Dongili > > > wrote: > ... > > > If possible, add drm.debug=7 to your boot line and send a dmesg of > > > 3.5-rc3. > > > > attached the dmesg on rc3 with drm.debug=7. > > In addition I have a black screen and 3 modprobe processes stuck that > > udev tried to kill for a bit... then the console went blank and I can't > > tell if it's still trying: > > > > 359 ?R 11:08 /sbin/modprobe -b > > pci:v8086d8108sv104Dsd905Fbc03sc00i00 > > 361 ?D 0:00 /sbin/modprobe -b > > pci:v8086d811Bsv104Dsd905Fbc04sc03i00 > > 422 ?D 0:00 /sbin/modprobe -b > > pci:v8086d8119sv104Dsd905Fbc06sc01i00 > > for the record, here below is where the screen flickers a bit and > booting gets stuck until udev starts trying to kill the modprobe calls. I gave 3.5-rc4 a try, no significant change but by comparing 3.4 and 3.5 logs I have this difference that may mean something: [0.00] Linux version 3.5.0-rc4... [5.712020] gma500 :00:02.0: Backlight lvds set brightness 74367436 while with 3.4 [5.525296] gma500 :00:02.0: Backlight lvds set brightness 74407440 a did put some printks here and there and psb_driver_init seems to never return from gma_backlight_init. -- mattia :wq!
Re: [PATCH] staging: drm/omap: add rotation properties
On Fri, Jun 29, 2012 at 07:17:23AM -0500, Rob Clark wrote: > On Fri, Jun 29, 2012 at 5:46 AM, Tomi Valkeinen wrote: > > On Wed, 2012-06-27 at 09:06 -0500, Rob Clark wrote: > >> From: Rob Clark > >> > >> Use tiled buffers for rotated/reflected scanout, with CRTC and plane > >> properties as the interface for userspace to configure rotation. > >> > >> Signed-off-by: Rob Clark > > > >> +/* this should probably be in drm-core to standardize amongst drivers */ > >> +#define DRM_ROTATE_0 0 > >> +#define DRM_ROTATE_90 1 > >> +#define DRM_ROTATE_180 2 > >> +#define DRM_ROTATE_270 3 > >> +#define DRM_REFLECT_X 4 > >> +#define DRM_REFLECT_Y 5 > > > > Are both reflect X and Y needed? You can get all the possible > > orientations with just one of the reflects. > > > > And I think the word "mirror" represents nicely what the reflect does, > > i.e. if you look at the mirror, the image you see is flipped > > horizontally. > > fwiw these values are aligned with what is used in userspace xrandr.. > an earlier version of this patch used just 3 bits, which where aligned > with what the omap hw uses and can describe all possible combinations > of mirroring and isomorphic rotation (x-invert, y-invert, and > xy-flip). But the advantage of the xrandr approach is you can more > easily leave out bits for rotation/mirroring modes that your hw does > not support.. for example if some hw supports only certain rotations > or does not support mirror/reflect. When I hear "mirror" I always think of it as the happening after rotation, but that's not how xrandr does things. I suppose "reflection" has more or less the same connotation for me, but since it's already used by xrandr, I know I have to do the necessary mental gymnastics. If you think about it in terms of matrix multiplication, xrandr does things like this: x' = Rot * Ref * x, whereas for me the more natural order (for whatever reason) would be x' = Ref * Rot * x, where x is the source coordinates, and x' the destination coordinates. IIRC in dss mirroring was specified in the post-rotation way, and the direction of the rotation was also opposite to xrandr (cw in dss, ccw in xrandr). So FWIW, my vote goes to "reflect" if we want to match xrandr semantics. Also I agree on the number of bits, six is better than three since it allows you to describe the hw capabilities. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 28622] radeon video lockup
https://bugzilla.kernel.org/show_bug.cgi?id=28622 Alan changed: What|Removed |Added CC||a...@lxorguk.ukuu.org.uk Kernel Version|2.6.37 |2.6.39 -- 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.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 51652] New: [6550D SUMO] problems with secondar monitor on VGA, causing GPU lockups
https://bugs.freedesktop.org/show_bug.cgi?id=51652 Bug #: 51652 Summary: [6550D SUMO] problems with secondar monitor on VGA, causing GPU lockups Classification: Unclassified Product: DRI Version: XOrg CVS Platform: x86-64 (AMD64) OS/Version: Linux (All) Status: NEW Severity: critical Priority: medium Component: DRM/Radeon AssignedTo: dri-devel@lists.freedesktop.org ReportedBy: d.ok...@gmail.com Created attachment 63701 --> https://bugs.freedesktop.org/attachment.cgi?id=63701 dmesg_VGAonBootConnected.txt 1) when is computer booted with VGA connected, ends with repeatly restarting GPU 2) when is monitor connected on-fly in running Xorg, it seems fine (except small artifacts, i guess EXA, on screen mostly on top of DVI and bottom VGA trigerred by switching KDE virtual desktops). No errors in dmesg 3) when is DPMS activated and monitors suspends, back to online is returned only DVI 4) randomly I found: [drm:radeon_dp_link_train_cr] *ERROR* clock recovery reached max voltage [drm:radeon_dp_link_train_cr] *ERROR* clock recovery failed 5) out-topic, exist even only on DVI: when is enabled OGL KWIN and OpenGL video render, color of rendered context goes to white (every frame is whitier, until it's #ff) No HDMI or DP conneted to system. DVI-D: LG IPS235 1920x1080 VGA: ASUS VW224 1680x1050 Gentoo ~amd64 Kernel,libdrm,mesa,ddx: git, KMS I'm able to test patches against anything. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/10] drm/radeon: document radeon_device.c (v2)
On 29.06.2012 18:50, alexdeuc...@gmail.com wrote: From: Alex Deucher Adds documentation to most of the functions in radeon_device.c v2: split out general descriptions as per Christian's comments. Signed-off-by: Alex Deucher For the whole series: Reviewed-by: Christian König --- drivers/gpu/drm/radeon/radeon_device.c | 313 +++- 1 files changed, 310 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index f654ba8..926d7e8 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -96,8 +96,12 @@ static const char radeon_family_name[][16] = { "LAST", }; -/* - * Clear GPU surface registers. +/** + * radeon_surface_init - Clear GPU surface registers. + * + * @rdev: radeon_device pointer + * + * Clear GPU surface registers (r1xx-r5xx). */ void radeon_surface_init(struct radeon_device *rdev) { @@ -119,6 +123,13 @@ void radeon_surface_init(struct radeon_device *rdev) /* * GPU scratch registers helpers function. */ +/** + * radeon_scratch_init - Init scratch register driver information. + * + * @rdev: radeon_device pointer + * + * Init CP scratch register driver information (r1xx-r5xx) + */ void radeon_scratch_init(struct radeon_device *rdev) { int i; @@ -136,6 +147,15 @@ void radeon_scratch_init(struct radeon_device *rdev) } } +/** + * radeon_scratch_get - Allocate a scratch register + * + * @rdev: radeon_device pointer + * @reg: scratch register mmio offset + * + * Allocate a CP scratch register for use by the driver (all asics). + * Returns 0 on success or -EINVAL on failure. + */ int radeon_scratch_get(struct radeon_device *rdev, uint32_t *reg) { int i; @@ -150,6 +170,14 @@ int radeon_scratch_get(struct radeon_device *rdev, uint32_t *reg) return -EINVAL; } +/** + * radeon_scratch_free - Free a scratch register + * + * @rdev: radeon_device pointer + * @reg: scratch register mmio offset + * + * Free a CP scratch register allocated for use by the driver (all asics) + */ void radeon_scratch_free(struct radeon_device *rdev, uint32_t reg) { int i; @@ -162,6 +190,20 @@ void radeon_scratch_free(struct radeon_device *rdev, uint32_t reg) } } +/* + * radeon_wb_*() + * Writeback is the the method by which the the GPU updates special pages + * in memory with the status of certain GPU events (fences, ring pointers, + * etc.). + */ + +/** + * radeon_wb_disable - Disable Writeback + * + * @rdev: radeon_device pointer + * + * Disables Writeback (all asics). Used for suspend. + */ void radeon_wb_disable(struct radeon_device *rdev) { int r; @@ -177,6 +219,14 @@ void radeon_wb_disable(struct radeon_device *rdev) rdev->wb.enabled = false; } +/** + * radeon_wb_fini - Disable Writeback and free memory + * + * @rdev: radeon_device pointer + * + * Disables Writeback and frees the Writeback memory (all asics). + * Used at driver shutdown. + */ void radeon_wb_fini(struct radeon_device *rdev) { radeon_wb_disable(rdev); @@ -187,6 +237,15 @@ void radeon_wb_fini(struct radeon_device *rdev) } } +/** + * radeon_wb_init- Init Writeback driver info and allocate memory + * + * @rdev: radeon_device pointer + * + * Disables Writeback and frees the Writeback memory (all asics). + * Used at driver startup. + * Returns 0 on success or an -error on failure. + */ int radeon_wb_init(struct radeon_device *rdev) { int r; @@ -355,6 +414,15 @@ void radeon_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc) /* * GPU helpers function. */ +/** + * radeon_card_posted - check if the hw has already been initialized + * + * @rdev: radeon_device pointer + * + * Check if the asic has been initialized (all asics). + * Used at driver startup. + * Returns true if initialized or false if not. + */ bool radeon_card_posted(struct radeon_device *rdev) { uint32_t reg; @@ -404,6 +472,14 @@ bool radeon_card_posted(struct radeon_device *rdev) } +/** + * radeon_update_bandwidth_info - update display bandwidth params + * + * @rdev: radeon_device pointer + * + * Used when sclk/mclk are switched or display modes are set. + * params are used to calculate display watermarks (all asics) + */ void radeon_update_bandwidth_info(struct radeon_device *rdev) { fixed20_12 a; @@ -424,6 +500,15 @@ void radeon_update_bandwidth_info(struct radeon_device *rdev) } } +/** + * radeon_boot_test_post_card - check and possibly initialize the hw + * + * @rdev: radeon_device pointer + * + * Check if the asic is initialized and if not, attempt to initialize + * it (all asics). + * Returns true if initialized or false if not. + */ bool radeon_boot_test_post_card(struct radeon_device *rdev) { if (radeon_card_posted(rdev)) @@ -442,6 +527,16 @@ bool radeon_boot_test_post_card(struct radeon_device *rdev
Re: gma500 suspend to ram fails (3.4)
On Mon, 2 Jul 2012 07:06:59 +0900 Mattia Dongili wrote: > On Thu, Jun 21, 2012 at 06:19:25AM +0900, Mattia Dongili wrote: > > On Tue, Jun 19, 2012 at 07:56:52PM +0900, Mattia Dongili wrote: > > > On Mon, Jun 18, 2012 at 10:04:00PM +0200, Patrik Jakobsson wrote: > > > > On Sun, Jun 17, 2012 at 12:03 PM, Mattia Dongili > > > > wrote: > > ... > > > > If possible, add drm.debug=7 to your boot line and send a dmesg > > > > of 3.5-rc3. > > > > > > attached the dmesg on rc3 with drm.debug=7. > > > In addition I have a black screen and 3 modprobe processes stuck > > > that udev tried to kill for a bit... then the console went blank > > > and I can't tell if it's still trying: > > > > > > 359 ?R 11:08 /sbin/modprobe -b > > > pci:v8086d8108sv104Dsd905Fbc03sc00i00 > > > 361 ?D 0:00 /sbin/modprobe -b > > > pci:v8086d811Bsv104Dsd905Fbc04sc03i00 > > > 422 ?D 0:00 /sbin/modprobe -b > > > pci:v8086d8119sv104Dsd905Fbc06sc01i00 > > > > for the record, here below is where the screen flickers a bit and > > booting gets stuck until udev starts trying to kill the modprobe > > calls. > > I gave 3.5-rc4 a try, no significant change but by comparing 3.4 and > 3.5 logs I have this difference that may mean something: > > [0.00] Linux version 3.5.0-rc4... > [5.712020] gma500 :00:02.0: Backlight lvds set brightness > 74367436 > > while with 3.4 > > [5.525296] gma500 :00:02.0: Backlight lvds set brightness > 74407440 > > a did put some printks here and there and psb_driver_init seems to > never return from gma_backlight_init. Is it ok if you just comment out gma_backlight_init (at which point it should just skip backlight handling and leave the firmware configured one) Alan ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH, RFC] i.MX DRM support
On Thu, Jun 14, 2012 at 03:43:22PM +0200, Sascha Hauer wrote: > Hi All, > > The following is the state-of-the-art i.MX IPU (Image Processing Unit) > DRM support. > > This code is around for quite some time now and has been posted several > times with different APIs, first with plain old framebuffer support, now > DRM, first platform device binding, now devicetree. Unfortunately there's > quite much code needed to get something useful out of the IPU, so these > patches haven't received a lot of attention from people not involved in > i.MX. I think we have now come to a point where this code needs more public > exposure and where it's easier to talk in incremental changes instead of > blobs. Therefore I request this to go to staging for some cycles. Dave, Greg, Comments to this one? I addressed the comments I received so far and am about to respin this series. Is it ok to put this to staging? If yes, should I move the whole stuff into drivers/staging/ or should it stay in drivers/gpu/drm with just a Kconfig dependency on STAGING? Thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel