[PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Hans, On Thursday 02 August 2012 09:08:18 Hans Verkuil wrote: > On Thu August 2 2012 08:56:43 R?mi Denis-Courmont wrote: > > Le jeudi 2 ao?t 2012 09:35:58 Hans Verkuil, vous avez ?crit : > > > On Wed August 1 2012 22:49:57 R?mi Denis-Courmont wrote: > > > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at > > > > > runtime ? > > > > > > > > Does CREATE_BUFS always work while already streaming has already > > > > started? If it depends on the driver, it's kinda helpless. > > > > > > Yes, it does. It's one of the reasons it exists in the first place. But > > > there are currently only a handful of drivers that implement it. I hope > > > that as more and more drivers are converted to vb2 that the availability > > > of create_bufs will increase. > > > > That's contradictory. If most drivers do not support it, then it won't > > work during streaming. > > IF create_bufs is implemented in the driver, THEN you can use it during > streaming. I.e., it will never return EBUSY as an error due to the fact > that streaming is in progress. > > Obviously it won't work if the driver didn't implement it in the first > place. > > > > > What's the guaranteed minimum buffer count? It seems in any case, MMAP > > > > has a hard limit of 32 buffers (at least videobuf2 has), though one > > > > might argue this should be more than enough. > > > > > > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 > > > core. Although drivers may force a lower maximum if they want. I have no > > > idea whether there are drivers that do that. There probably are. > > > > The smallest of the maxima of all drivers. > > I've no idea. Most will probably abide by the 32 maximum, but without > analyzing all drivers I can't guarantee it. > > > > The minimum is usually between 1 and 3, depending on hardware > > > limitations. > > > > And that's clearly insufficient without memory copy to userspace buffers. > > > > It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for > > REQBUFS+USERBUF then. > > Just to put your mind at rest: USERPTR mode will *not* disappear or be > deprecated in any way. It's been there for a long time, it's in heavy use, > it's easy to use and it will not be turned into a second class citizen, > because it isn't. Just because there is a new dmabuf mode available doesn't > mean that everything should be done as a mmap+dmabuf thing. I disagree with this. Not everything should obviously be done with MMAP + DMABUF, but for buffer sharing between devices, we should encourage application developers to use DMABUF instead of USERPTR. -- Regards, Laurent Pinchart
[PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi R?mi, On Thursday 02 August 2012 09:56:43 R?mi Denis-Courmont wrote: > Le jeudi 2 ao?t 2012 09:35:58 Hans Verkuil, vous avez ?crit : > > On Wed August 1 2012 22:49:57 R?mi Denis-Courmont wrote: > > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at > > > > runtime ? > > > > > > Does CREATE_BUFS always work while already streaming has already > > > started? > > > If it depends on the driver, it's kinda helpless. > > > > Yes, it does. It's one of the reasons it exists in the first place. But > > there are currently only a handful of drivers that implement it. I hope > > that as more and more drivers are converted to vb2 that the availability > > of create_bufs will increase. > > That's contradictory. If most drivers do not support it, then it won't work > during streaming. > > > > What's the guaranteed minimum buffer count? It seems in any case, MMAP > > > has a hard limit of 32 buffers (at least videobuf2 has), though one > > > might argue this should be more than enough. > > > > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. > > Although drivers may force a lower maximum if they want. I have no idea > > whether there are drivers that do that. There probably are. > > The smallest of the maxima of all drivers. > > > The minimum is usually between 1 and 3, depending on hardware limitations. > > And that's clearly insufficient without memory copy to userspace buffers. That's the minimum number of buffers *required* by the hardware. You can add up to 32 buffers, I'm not aware of any driver that would prevent that. > It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for > REQBUFS+USERBUF then. -- Regards, Laurent Pinchart
[Bug 51787] performance regression with llvm shader compiler in ut2004
https://bugs.freedesktop.org/show_bug.cgi?id=51787 --- Comment #3 from Andy Furniss 2012-08-02 22:32:43 UTC --- (In reply to comment #2) > It might not help in fixing this, but I found that the framerate is much more > consistent if I load all cpu cores with something while playing ut2004. > Normally, the framerate counter is green, but flashes into yellow and purple, > which makes movements very choppy. When something is running in the > background, > the framerate counter stays green, and all movements are smooth. Maybe cpufreq is causing this here's what I get on the demo on an HD4890 + 4x3.4GHz phenom II. R600_LLVM=0 ut2004 as-convoy?spectatoronly=1?numbots=8?quickstart=1?attractcam=1 -benchmark -seconds=120 -nosound cpufreq ondemand - 30.027578 / 76.497787 / 162.457611 fps cpufreq set to performance 37.627441 / 93.523949 / 197.051376 fps > > With the llvm compiler the game is unplayable either way. I see the stalling in the demo benchmark. 2.030273 / 70.589706 / 196.047073 fps -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 51787] performance regression with llvm shader compiler in ut2004
https://bugs.freedesktop.org/show_bug.cgi?id=51787 --- Comment #2 from almos 2012-08-02 22:04:09 UTC --- It might not help in fixing this, but I found that the framerate is much more consistent if I load all cpu cores with something while playing ut2004. Normally, the framerate counter is green, but flashes into yellow and purple, which makes movements very choppy. When something is running in the background, the framerate counter stays green, and all movements are smooth. With the llvm compiler the game is unplayable either way. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events
On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui wrote: > On ?, 2012-08-02 at 15:46 +0200, Luca Tettamanti wrote: >> On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: >> > On ?, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: >> > > AMD ACPI interface may overload the standard event >> > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such >> > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the >> > > userspace because the user did not press the mode switch key (the >> > > spurious keypress confuses the DE which usually changes the >> > > display configuration and messes up a dual-screen setup). >> > > This patch gives the radeon driver the chance to examine the event and >> > > block the keypress if the event is an "AMD event". >> > > >> > > Signed-off-by: Luca Tettamanti >> > > --- >> > > Any comment from ACPI front? >> > > >> > it looks good to me. >> > But I'm wondering if we can use the following code for ACPI part, which >> > looks cleaner. >> > I know this may change the behavior of other events, but in theory, we >> > should not send any input event if we know something wrong in kernel. >> > >> > what do you think? >> >> I like it, it's cleaner. >> I've split the patch in two pieces (one for video, the other for >> radeon) and adopted your suggestion. >> > Great. > Acked-by: Zhang Rui > > hmm, who should take these two patches? I'm happy to take the patches. > and which tree the second patch is based on? I've got a tree with all the radeon ACPI patches on the acpi_patches branches of my git tree: git://people.freedesktop.org/~agd5f/linux Alex > > thanks, > rui >
[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11
https://bugs.freedesktop.org/show_bug.cgi?id=45018 --- Comment #86 from Alexandre Demers 2012-08-03 03:00:00 UTC --- (In reply to comment #85) > I found a demo that has the issue, in the demos repository for mesa within the > src/demo folder the program 'reflect'. After I start it up and press 's' to > see the stencil buffer the white plan blinks continuously. Applying the patch > 'fixes to wait on the bo and to free the va after the kernel' removes the > blinking, as does disabling va through the variable > ws->info.r600_virtual_address. > > The other issue with the kernel reporting a va conflict is going to be a > little > harder to reproduce because it appears to be caused by a race condition. > > I'll still look for other demos that have the issue. Yes, I understand it can be hard to track for you Jerome. Well for the va issue, on my side, it is as simple as logging in KDE or Gnome 3. Before logging in, there is no va error in dmesg. Once I'm in, there are usually 3 or sometimes 6 errors (they are written in block of 3, so I suspect it tries a first time and for some reason it fails and try again second time). I also experience the issue when watching some movies. With Anthony's patch, va issues are gone and I watched a couple of shows yesterday without any problem. Before the patch, it would blink and get corrupted after about 16 minutes and then crash. So, Anthony has put a finger on something. However, I also run piglit tests and some other applications like RendererFeatTest64 (which is an application released before Amnesia went out to test OpenGL performances if I recall recorrectly). With Anthony's patch, I'm still able to lock the display everytime (if I play music at the same time, it will continue to play but I won't be able to change terminal even if sometimes my mouse pointer can still be moved). RendererFeatTest64 will always lock at the same test, but it is not the same for piglit tests (even if it happens often at the same or near the same). I'm installing a freshly compiled kernel 3.5.0 with both Alex and your patches (by the way, they can't be applied on latest drm-next branch) and I'll tell you if I'm still experiencing the lockups. I'll also try Anthony's test to see if I get the same results (blinking without his patch, OK with 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
[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11
https://bugs.freedesktop.org/show_bug.cgi?id=45018 --- Comment #85 from Anthony Waters 2012-08-03 02:07:06 UTC --- I found a demo that has the issue, in the demos repository for mesa within the src/demo folder the program 'reflect'. After I start it up and press 's' to see the stencil buffer the white plan blinks continuously. Applying the patch 'fixes to wait on the bo and to free the va after the kernel' removes the blinking, as does disabling va through the variable ws->info.r600_virtual_address. The other issue with the kernel reporting a va conflict is going to be a little harder to reproduce because it appears to be caused by a race condition. I'll still look for other demos that have the issue. -- 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/RFC] drm/radeon: ACPI: veto the keypress on ATIF events
On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui wrote: > On 四, 2012-08-02 at 15:46 +0200, Luca Tettamanti wrote: >> On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: >> > On 三, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: >> > > AMD ACPI interface may overload the standard event >> > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such >> > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the >> > > userspace because the user did not press the mode switch key (the >> > > spurious keypress confuses the DE which usually changes the >> > > display configuration and messes up a dual-screen setup). >> > > This patch gives the radeon driver the chance to examine the event and >> > > block the keypress if the event is an "AMD event". >> > > >> > > Signed-off-by: Luca Tettamanti >> > > --- >> > > Any comment from ACPI front? >> > > >> > it looks good to me. >> > But I'm wondering if we can use the following code for ACPI part, which >> > looks cleaner. >> > I know this may change the behavior of other events, but in theory, we >> > should not send any input event if we know something wrong in kernel. >> > >> > what do you think? >> >> I like it, it's cleaner. >> I've split the patch in two pieces (one for video, the other for >> radeon) and adopted your suggestion. >> > Great. > Acked-by: Zhang Rui > > hmm, who should take these two patches? I'm happy to take the patches. > and which tree the second patch is based on? I've got a tree with all the radeon ACPI patches on the acpi_patches branches of my git tree: git://people.freedesktop.org/~agd5f/linux Alex > > thanks, > rui > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
drm/nouveau: crash regression in 3.5
On Thu, Aug 02, 2012 at 01:26:55PM +0200, Ortwin Gl?ck wrote: > I have managed to turn the crash into a WARN_ON, by adding this to the > begin of nouveau_software_vblank(): > > if (!psw) { > WARN_ON(1); > return; > } Yes, I know about it, but this change fixes only a symptom. We should not get into this code at all without enabling vblank. > And I have also managed to load the module manually instead by udev. So > I am happy to attach a full log of what's going on here. See also my > added printk's starting with XXX that mark some interesting points in > the initialization. > > This should give you enough information to track down the cause of the > problem. To my non-expert eyes it looks like "noaccel" prevents > registration of NVOBJ_ENGINE_SW or at least delays it too much. Yes, that's what I wrote in my last patch - with noaccel it's not registered, which leads to NULL pointer derefence. I'm currently out of ideas why my patch does not work. But I have some ideas how to debug it. Can you come to nouveau IRC channel at freenode and do some additional tests? My nick is "joi" and I'm available on IRC between 6pm and 11pm CEST. Marcin
[PATCH] drm/radeon: add new AMD ACPI header and update relevant code
On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher wrote: > I admit I'm not really an ACPI expert, but thinking about this more, > I'm wondering if maybe we should just send the appropriate brightness > change, switch display, etc. event to userspace rather than handling > it directly in the radeon driver, then let userspace callback down via > the bl interface, etc. With backlight for example, does handling it > in the kernel driver as per your patch prevent userspace from seeing > the brightness up/down event? Wouldn't that break things like OSD > brightness displays and such? No, the event is sent to userspace by the standard ACPI video driver, it works as before. Changing brightness usually goes like this: 1) user presses a hotkey 2) a notification is generated (0x86 or 0x87) 3) video.ko handles the notification and calls into ACPI to change the level (_BCM) and firmware does its magic 4) a key press (brightness up/down) is sent to userspace With ATIF step 3 does not actually change the brightness, it just send out another event (VIDEO_PROBE, or one of the device specific ones) so we need to take care of that too. The rest of the process, including the delivery of the key presses, stays the same. Luca
[PATCHv7 03/15] v4l: vb2: add support for shared buffer (dma_buf)
Hi Laurent, Hi Dima, On 06/27/2012 10:40 PM, Laurent Pinchart wrote: > Hi Dima, > > On Tuesday 26 June 2012 13:53:34 Dima Zavin wrote: >> On Tue, Jun 26, 2012 at 2:40 AM, Hans Verkuil wrote: >>> On Tue 26 June 2012 11:11:06 Laurent Pinchart wrote: On Tuesday 26 June 2012 10:40:44 Tomasz Stanislawski wrote: > Hi Dima Zavin, > Thank you for the patch and for a ping remainder :). > > You are right. The unmap is missing in __vb2_queue_cancel. > I will apply your fix into next version of V4L2 support for dmabuf. > > Please refer to some comments below. > > On 06/20/2012 08:12 AM, Dima Zavin wrote: >> Tomasz, >> >> I've encountered an issue with this patch when userspace does several >> stream_on/stream_off cycles. When the user tries to qbuf a buffer >> after doing stream_off, we trigger the "dmabuf already pinned" >> warning since we didn't unmap the buffer as dqbuf was never called. >> >> The below patch adds calls to unmap in queue_cancel, but my feeling >> is that we probably should be calling detach too (i.e. put_dmabuf). According to the V4L2 specification, the "VIDIOC_STREAMOFF ioctl, apart of aborting or finishing any DMA in progress, unlocks any user pointer buffers locked in physical memory, and it removes all buffers from the incoming and outgoing queues". >>> >>> Correct. And what that means in practice is that after a streamoff all >>> buffers are returned to the state they had just before STREAMON was >>> called. >> >> That can't be right. The buffers had to have been returned to the >> state just *after REQBUFS*, not just *before STREAMON*. You need to >> re-enqueue buffers before calling STREAMON. I assume that's what you >> meant? > > Your interpretation is correct. > So now we should decide what should be changed: the spec or vb2 ? Bringing the queue state back to *after REQBUFS* may make the next (STREAMON + QBUFs) very costly operations. Reattaching and mapping a DMABUF might trigger DMA allocation and *will* trigger creation of IOMMU mappings. In case of a user pointer, calling next get_user_pages may cause numerous fault events and will *create* new IOMMU mapping. Is there any need to do such a cleanup if the destruction of buffers and all caches can be explicitly executed by REQBUFS(count = 0) ? Maybe it would be easier to change the spec by removing "apart of ... in physical memory" part? STREAMOFF should mean stopping streaming, and that resources are no longer used. DMABUFs are unmapped but unmapping does not mean releasing. The exporter may keep the resource in its caches. Currently, vb2 does not follow the policy from the spec. The put_userptr ops is called on: - VIDIOC_REBUFS - VIDIOC_CREATE_BUFS - vb2_queue_release() which is usually called on close() syscall The put_userptr is not called and streamoff therefore the user pages are locked after STREAMOFF. BTW. What does 'buffer locked' mean? Does it mean that a buffer is pinned or referenced by a driver/HW? Regards, Tomasz Stanislawski >>> So after STREAMOFF you can immediately queue all buffers again with QBUF >>> and call STREAMON to restart streaming. No mmap or other operations >>> should be required. This behavior must be kept. >>> >>> VIDIOC_REQBUFS() or a close() are the only two operations that will >>> actually free the buffers completely. >>> >>> In practice, a STREAMOFF is either followed by a STREAMON at a later time, >>> or almost immediately followed by REQBUFS or close() to tear down the >>> buffers. So I don't think the buffers should be detached at streamoff. >> >> I agree. I was leaning this way which is why I left it out of my patch >> and wanted to hear your guys' opinion as you are much more familiar >> with the intended behavior than I am. >> >> Thanks! > > You're welcome. Thank you for reporting the problem and providing a patch. >
[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11
https://bugs.freedesktop.org/show_bug.cgi?id=45018 --- Comment #84 from Anthony Waters 2012-08-03 01:28:25 UTC --- I randomly saw it when I was playing a game of Warcraft 3, the terrain textures would blink. I'll check the piglit tests and mesa demos to see if I can reproduce the issue with 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
[PATCH] drm/radeon: add new AMD ACPI header and update relevant code
On Thu, Aug 2, 2012 at 12:33 PM, Alex Deucher wrote: > On Thu, Aug 2, 2012 at 12:31 PM, Luca Tettamanti > wrote: >> On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher >> wrote: >>> I admit I'm not really an ACPI expert, but thinking about this more, >>> I'm wondering if maybe we should just send the appropriate brightness >>> change, switch display, etc. event to userspace rather than handling >>> it directly in the radeon driver, then let userspace callback down via >>> the bl interface, etc. With backlight for example, does handling it >>> in the kernel driver as per your patch prevent userspace from seeing >>> the brightness up/down event? Wouldn't that break things like OSD >>> brightness displays and such? >> >> No, the event is sent to userspace by the standard ACPI video driver, >> it works as before. >> Changing brightness usually goes like this: >> 1) user presses a hotkey >> 2) a notification is generated (0x86 or 0x87) >> 3) video.ko handles the notification and calls into ACPI to change the >> level (_BCM) and firmware does its magic >> 4) a key press (brightness up/down) is sent to userspace >> >> With ATIF step 3 does not actually change the brightness, it just send >> out another event (VIDEO_PROBE, or one of the device specific ones) so >> we need to take care of that too. The rest of the process, including >> the delivery of the key presses, stays the same. Updated tree with fixes to a few existing patches and improved support for ATPX and initial support for ATCS: http://cgit.freedesktop.org/~agd5f/linux/?h=acpi_patches Alex
[RFC 0/3] solving omapdrm/omapdss layering issues
On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote: > On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen > wrote: > > On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: > >> On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen > >> wrote: > > > >> > I guess the fact is that DRM concepts do not really match the OMAP DSS > >> > hardware, and we'll have to use whatever gives us least problems. > >> > >> Actually, I think it does map fairly well to the hardware.. at least > >> more so than to omapdss ;-) > > > > Hm, I'm not sure I understand, omapdss concepts map directly to the > > hardware. > > I think it is mainly exposing the encoder and panel as two separate > entities.. which seems to be what Archit is working on I still don't follow =) They are separate entities. Omapdss models the HW quite directly, I think. It doesn't expose everything, though, as the output drivers (dsi.c, dpi.c etc) are used via the panel drivers. > in case of something like DVI bridge from DPI, this seems pretty > straightforward.. only the connector needs to know about DDC stuff, > which i2c to use and that sort of thing. So at kms level we would > have (for example) an omap_dpi_encoder which would be the same for DPI > panel (connector) or DPI->DVI bridge. For HDMI I'm still looking > through the code to see how this would work. Honestly I've looked > less at this part of code and encoder related registers in the TRM, > compared to the ovl/mgr parts, but at least from the 'DSS overview' > picture in the TRM it seems to make sense ;-) > > KMS even exposes the idea that certain crtcs can connect to only > certain encoders. Or that you could you could have certain connectors > switched between encoders. For example if you had a hw w/ DPI out, > and some mux to switch that back and forth between a DPI lcd panel and > a DPI->DVI bridge. (Ok, I'm not aware of any board that actually does > this, but it is in theory possible.) So we could expose possible > video chain topologies to userspace in this way. OMAP3 SDP board has such a setup, with manual switch to select between LCD and DVI. > The other thing is that we don't need to propagate timings from the > panel up to the mgr at the dss level.. kms is already handling this > for us. In my latest version, which I haven't pushed, I removed the > 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'. You're thinking only about simple DPI cases. Consider this example, with a DSI-to-DP bridge chip. What we have is the following flow of data: DISPC -> DSI -> DSI-2-DP -> DP monitor The timings you are thinking about are in the DISPC, but here they are only one part of the link. And here the DISPC timings are not actually the timings what the user is interested about. The user wants his timings to be between DSI-2-DP chip and the DP monitor. Timings programmed to DISPC are not the same. The timings for DISPC come from the DSI driver, and they may be very different than the user's timings. With DSI video mode, the DISPC timings would have some resemblance to the user's timings, mainly the time to send one line would be the same. With DSI cmd mode, the DISPC timings would be something totally different, most likely with 0 blank times and as fast pixel clock as possible. What omapdss does currently is that you set the user's timings to the right side of the chain, which propagate back to DSS. This allows the DSI-2-DP bridge use DSI timings that work optimally for the bridge, and DSI driver will use DISPC timings that work optimally for it. And it's not only about timings above, but also other settings related to the busses between the components. Clock dividers, polarities, stuff like that. > I think the problem was there were some cases, like ovl updates before > setting the mgr, where the user_info_dirty flag would be cleared but > the registers not updated. This is what I meant by sequence of Hmm, I'd appreciate more info about this if you can give. Sounds like a bug, that shouldn't happen. So you mean that you have an ovl, with no manager set. And you change the settings of the ovl before assigning it to a mgr? That's something I haven't really tested, so it could bug, true. > operations at KMS level confusing omapdss. This should be fixable > with some debugging. Although getting rid of the state tracking at > omapdss level altogether was a much simpler solution, and is the one I > prefer :-) Yes, I don't prefer the state tracking either (we didn't have it in earlier versions of omapdss), but I still don't see an option to it if we want to support all the stuff we have. Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120802/ca1d188c/attachment.pgp>
Re: [PATCH v2] of: Add videomode helper
On 07/05/2012 08:51 AM, Rob Herring wrote: > On 07/04/2012 02:56 AM, Sascha Hauer wrote: >> This patch adds a helper function for parsing videomodes from the devicetree. >> The videomode can be either converted to a struct drm_display_mode or a >> struct fb_videomode. >> diff --git a/Documentation/devicetree/bindings/video/displaymode >> b/Documentation/devicetree/bindings/video/displaymode >> +Example: >> + >> + display@0 { >> + /* 1920x1080p24 */ >> + clock = <5200>; > > Should this use the clock binding? You probably need both constraints > and clock binding though. I don't think the clock binding is appropriate here. This binding specifies a desired video mode, and should specify just the expected clock frequency for that mode. Perhaps any tolerance imposed by the specification used to calculate the mode timing could be specified too, but the allowed tolerance (for a mode to be recognized by the dispaly) is more driven by the receiving device than the transmitting device. The clock bindings should come into play in the display controller that sends a signal in that display mode. Something like: mode: hd1080p { clock = <5200>; xres = <1920>; yres = <1080>; }; display-controller { pixel-clock-source = <&clk ...>; // common clock bindings here default-mode = <&mode>; > Often you don't know the frequency up front and/or have limited control > of the frequency (i.e. integer dividers). Then you have to adjust the > margins to get the desired refresh rate. To do that, you need to know > the ranges of values a panel can support. Perhaps you just assume you > can increase the right-margin and lower-margins as I think you will hit > pixel clock frequency max before any limit on margins. I think this is more usually dealt with in HW design and matching components. The PLL/... feeding the display controller is going to have some known specifications that imply which pixel clocks it can generate. HW designers will pick a panel that accepts a clock the display controller can generate. The driver for the display controller will just generate as near of a pixel clock as it can to the rate specified in the mode definition. I believe it'd be unusual for a display driver to start fiddling with front-back porch (or margin) widths to munge the timing; that kind of thing probably worked OK with analog displays, but with digital displays where each pixel is clocked even in the margins, I think that would cause more problems than it solved. Similarly for external displays, the display controller will just pick the nearest clock it can. If it can't generate a close enough clock, it will just refuse to set the mode. In fact, a display controller driver would typically validate this when the set of legal modes was enumerated when initially detecting the display, so user-space typically wouldn't even request invalid modes. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] of: Add videomode helper
On 07/04/2012 01:56 AM, Sascha Hauer wrote: > This patch adds a helper function for parsing videomodes from the devicetree. > The videomode can be either converted to a struct drm_display_mode or a > struct fb_videomode. > diff --git a/Documentation/devicetree/bindings/video/displaymode > b/Documentation/devicetree/bindings/video/displaymode > +Required properties: > + - xres, yres: Display resolution > + - left-margin, right-margin, hsync-len: Horizontal Display timing parameters > + in pixels > + upper-margin, lower-margin, vsync-len: Vertical display timing parameters > in > + lines Perhaps bike-shedding, but... For the margin property names, wouldn't it be better to use terminology more commonly used for video timings rather than Linux FB naming. In other words naming like: hactive, hsync-len, hfront-porch, hback-porch? > + - clock: displayclock in Hz > + > +Optional properties: > + - width-mm, height-mm: Display dimensions in mm > + - hsync-active-high (bool): Hsync pulse is active high > + - vsync-active-high (bool): Vsync pulse is active high > + - interlaced (bool): This is an interlaced mode > + - doublescan (bool): This is a doublescan mode > + > +There are different ways of describing a display mode. The devicetree > representation > +corresponds to the one used by the Linux Framebuffer framework described > here in > +Documentation/fb/framebuffer.txt. This representation has been chosen > because it's > +the only format which does not allow for inconsistent parameters.Unlike the > Framebuffer > +framework the devicetree has the clock in Hz instead of ps. As Rob mentioned, I think there shouldn't be any mention of Linux FB here. > + > +Example: > + > + display@0 { This node appears to describe a video mode, not a display, hence the node name seems wrong. Many displays can support multiple different video modes. As mentioned elsewhere, properties like display width/height are properties of the display not the mode. So, I might expect something more like the following (various overhead properties like reg/#address-cells etc. elided for simplicity): disp: display { width-mm = <...>; height-mm = <...>; modes { mode@0 { /* 1920x1080p24 */ clock = <5200>; xres = <1920>; yres = <1080>; left-margin = <25>; right-margin = <25>; hsync-len = <25>; lower-margin = <2>; upper-margin = <2>; vsync-len = <2>; hsync-active-high; }; mode@1 { }; }; }; display-connector { display = <&disp>; // interface-specific properties such as pixel format here }; Finally, have you considered just using an EDID instead of creating something custom? I know that creating an EDID is harder than writing a few simple properties into a DT, but EDIDs have the following advantages: a) They're already standardized and very common. b) They allow other information such as a display's HDMI audio capabilities to be represented, which can then feed into an ALSA driver. c) The few LCD panel datasheets I've seen actually quote their specification as an EDID already, so deriving the EDID may actually be easy. d) People familiar with displays are almost certainly familiar with EDID's mode representation. There are many ways of representing display modes (sync position vs. porch widths, htotal specified rather than specifying all the components and hence htotal being calculated etc.). Not everyone will be familiar with all representations. Conversion errors are less likely if the target is EDID's familiar format. e) You'll end up with exactly the same data as if you have a DDC-based external monitor rather than an internal panel, so you end up getting to a common path in display handling code much more quickly. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit : > On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote: > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at > > > runtime ? > > > > Does CREATE_BUFS always work while already streaming has already started? > > If it depends on the driver, it's kinda helpless. > > Yes, it does. It's one of the reasons it exists in the first place. But > there are currently only a handful of drivers that implement it. I hope > that as more and more drivers are converted to vb2 that the availability > of create_bufs will increase. That's contradictory. If most drivers do not support it, then it won't work during streaming. > > What's the guaranteed minimum buffer count? It seems in any case, MMAP > > has a hard limit of 32 buffers (at least videobuf2 has), though one > > might argue this should be more than enough. > > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. > Although drivers may force a lower maximum if they want. I have no idea > whether there are drivers that do that. There probably are. The smallest of the maxima of all drivers. > The minimum is usually between 1 and 3, depending on hardware limitations. And that's clearly insufficient without memory copy to userspace buffers. It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for REQBUFS+USERBUF then. -- Rémi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: ignore disconnected <-> unknown status changes
On an AOpen i915GMm-hfs the hotplug events generated by transitions between connector_status_unknown and connector_status_disconnected cause screen distortions. The attached patch cures the problem by disabling the generation of hotplug events in those cases. That should be safe for everybody as the only relevant changes are those from / to connector_status_connected. cu, Knut >From f631128c46f916eb58bbdabf867248a04a0d2fc5 Mon Sep 17 00:00:00 2001 From: Knut Petersen Date: Thu, 2 Aug 2012 08:52:04 +0200 Subject: [PATCH] drm: ignore disconnected <-> unknown status changes Only generate a hotplug event if the status changed to / from connector_status_connected. On some hardware the connector status is oscillating between disconnected and unknown, and the hotplug events mirroring these transitions cause screen distortions. As the only reasonable action on such a status change is to ignore it, it also is safe not to genereate a hotplug event at all. Needed for / tested on AOpen i915GMm-hfs mobo. Signed-off-by: Knut Petersen --- drivers/gpu/drm/drm_crtc_helper.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 3252e70..fb6160b 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -954,7 +954,11 @@ static void output_poll_execute(struct work_struct *work) connector->base.id, drm_get_connector_name(connector), old_status, connector->status); - if (old_status != connector->status) + /* changes are only relevant if previous or + current status is connector_status_connected */ + if (old_status != connector->status && + (old_status == connector_status_connected || + connector->status == connector_status_connected)) changed = true; } -- 1.7.7 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Massive power regression going 3.4->3.5
On 2012-08-01 03:06, Chris Wilson wrote: On Wed, 01 Aug 2012 10:38:36 +0100, James Bottomley wrote: On Wed, 2012-08-01 at 09:58 +0100, Chris Wilson wrote: > On Wed, 01 Aug 2012 09:45:04 +0100, James Bottomley wrote: > > On Wed, 2012-08-01 at 09:16 +0100, Chris Wilson wrote: > > > On Wed, 01 Aug 2012 09:06:12 +0100, James Bottomley wrote: > > > > I got the attached to apply and it doesn't really improve the idle power > > > > much (12.5W). > > > > > > That's good to know. Next step is to try overriding i915.semaphores. > > > Can you please test with i915.semaphores=0 and i915.semaphores=1? > > > > There's not much point doing i915_semaphores=1 since that's the default > > on gen 6 hardware, but i915_semaphores=0 recovers and idle power of > > ~6.5W > > It is only the default if iommu is off, and changing the default > was one of the side-effects of the patch you bisected. > > Can you please login to the desktop, let it idle, record > /sys/kernel/debug/dri/0/i915_cur_delayinfo and .../i915_drpc_info. > Then trace-cmd record -e i915 sleep 10s, OK, what is trace-cmd? It looks similar to perf tools ... is that it? Yes, it is roughly equivalent and you should be able to achieve the same with perf trace - except I haven't done it before so I don't have quick advice on how to drive it. :) -Chris Should be something like: perf record -f -g -e i915:* -a ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events
On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: > On ?, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: > > AMD ACPI interface may overload the standard event > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > > userspace because the user did not press the mode switch key (the > > spurious keypress confuses the DE which usually changes the > > display configuration and messes up a dual-screen setup). > > This patch gives the radeon driver the chance to examine the event and > > block the keypress if the event is an "AMD event". > > > > Signed-off-by: Luca Tettamanti > > --- > > Any comment from ACPI front? > > > it looks good to me. > But I'm wondering if we can use the following code for ACPI part, which > looks cleaner. > I know this may change the behavior of other events, but in theory, we > should not send any input event if we know something wrong in kernel. > > what do you think? I like it, it's cleaner. I've split the patch in two pieces (one for video, the other for radeon) and adopted your suggestion. BTW, I'm leaving for vacation in a few hours, I'll be offline till mid August. Luca -- next part -- >From acce30c96b90d1bc550e82a9e7f19226fa194d5e Mon Sep 17 00:00:00 2001 From: Luca Tettamanti Date: Thu, 2 Aug 2012 15:30:27 +0200 Subject: [PATCH 1/2] ACPI video: allow events handlers to veto the keypress The standard video events may be overloaded for device specific purposes. For example AMD ACPI interface overloads ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the userspace because the user did not press the mode switch key (the spurious keypress confuses the DE which usually changes the display configuration and messes up a dual-screen setup). This patch gives the handlers the chance to examine the event and block the keypress if the event is device specific. v2: refactor as suggested by Zhang Rui Signed-off-by: Luca Tettamanti --- drivers/acpi/video.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1e0a9e1..d75642a 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1448,8 +1448,7 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) case ACPI_VIDEO_NOTIFY_SWITCH: /* User requested a switch, * most likely via hotkey. */ acpi_bus_generate_proc_event(device, event, 0); - if (!acpi_notifier_call_chain(device, event, 0)) - keycode = KEY_SWITCHVIDEOMODE; + keycode = KEY_SWITCHVIDEOMODE; break; case ACPI_VIDEO_NOTIFY_PROBE: /* User plugged in or removed a video @@ -1479,8 +1478,9 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) break; } - if (event != ACPI_VIDEO_NOTIFY_SWITCH) - acpi_notifier_call_chain(device, event, 0); + if (acpi_notifier_call_chain(device, event, 0)) + /* Something vetoed the keypress. */ + keycode = 0; if (keycode) { input_report_key(input, keycode, 1); -- 1.7.10.4 -- next part -- >From def5119d8f617eef0fac2cd35d7bb18c176ff8f6 Mon Sep 17 00:00:00 2001 From: Luca Tettamanti Date: Thu, 2 Aug 2012 15:33:03 +0200 Subject: [PATCH 2/2] drm/radeon: block the keypress on ATIF events The AMD ACPI interface may use ACPI_VIDEO_NOTIFY_PROBE to signal SBIOS requests; block the keypress in this case since the user did not actually press the mode switch key. Signed-off-by: Luca Tettamanti --- drivers/gpu/drm/radeon/radeon_acpi.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 96de08d..ee0d29e 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev, } /* TODO: check other events */ - return NOTIFY_OK; + /* We've handled the event, stop the notifier chain. The ACPI interface +* overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to +* userspace if the event was generated only to signal a SBIOS +* request. +*/ + return NOTIFY_BAD; } /* Call all ACPI methods here */ -- 1.7.10.4
[Bug 51787] performance regression with llvm shader compiler in ut2004
https://bugs.freedesktop.org/show_bug.cgi?id=51787 --- Comment #3 from Andy Furniss 2012-08-02 22:32:43 UTC --- (In reply to comment #2) > It might not help in fixing this, but I found that the framerate is much more > consistent if I load all cpu cores with something while playing ut2004. > Normally, the framerate counter is green, but flashes into yellow and purple, > which makes movements very choppy. When something is running in the > background, > the framerate counter stays green, and all movements are smooth. Maybe cpufreq is causing this here's what I get on the demo on an HD4890 + 4x3.4GHz phenom II. R600_LLVM=0 ut2004 as-convoy?spectatoronly=1?numbots=8?quickstart=1?attractcam=1 -benchmark -seconds=120 -nosound cpufreq ondemand - 30.027578 / 76.497787 / 162.457611 fps cpufreq set to performance 37.627441 / 93.523949 / 197.051376 fps > > With the llvm compiler the game is unplayable either way. I see the stalling in the demo benchmark. 2.030273 / 70.589706 / 196.047073 fps -- 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 51787] performance regression with llvm shader compiler in ut2004
https://bugs.freedesktop.org/show_bug.cgi?id=51787 --- Comment #2 from almos 2012-08-02 22:04:09 UTC --- It might not help in fixing this, but I found that the framerate is much more consistent if I load all cpu cores with something while playing ut2004. Normally, the framerate counter is green, but flashes into yellow and purple, which makes movements very choppy. When something is running in the background, the framerate counter stays green, and all movements are smooth. With the llvm compiler the game is unplayable either way. -- 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: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Hans, On Thursday 02 August 2012 09:08:18 Hans Verkuil wrote: > On Thu August 2 2012 08:56:43 Rémi Denis-Courmont wrote: > > Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit : > > > On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote: > > > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at > > > > > runtime ? > > > > > > > > Does CREATE_BUFS always work while already streaming has already > > > > started? If it depends on the driver, it's kinda helpless. > > > > > > Yes, it does. It's one of the reasons it exists in the first place. But > > > there are currently only a handful of drivers that implement it. I hope > > > that as more and more drivers are converted to vb2 that the availability > > > of create_bufs will increase. > > > > That's contradictory. If most drivers do not support it, then it won't > > work during streaming. > > IF create_bufs is implemented in the driver, THEN you can use it during > streaming. I.e., it will never return EBUSY as an error due to the fact > that streaming is in progress. > > Obviously it won't work if the driver didn't implement it in the first > place. > > > > > What's the guaranteed minimum buffer count? It seems in any case, MMAP > > > > has a hard limit of 32 buffers (at least videobuf2 has), though one > > > > might argue this should be more than enough. > > > > > > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 > > > core. Although drivers may force a lower maximum if they want. I have no > > > idea whether there are drivers that do that. There probably are. > > > > The smallest of the maxima of all drivers. > > I've no idea. Most will probably abide by the 32 maximum, but without > analyzing all drivers I can't guarantee it. > > > > The minimum is usually between 1 and 3, depending on hardware > > > limitations. > > > > And that's clearly insufficient without memory copy to userspace buffers. > > > > It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for > > REQBUFS+USERBUF then. > > Just to put your mind at rest: USERPTR mode will *not* disappear or be > deprecated in any way. It's been there for a long time, it's in heavy use, > it's easy to use and it will not be turned into a second class citizen, > because it isn't. Just because there is a new dmabuf mode available doesn't > mean that everything should be done as a mmap+dmabuf thing. I disagree with this. Not everything should obviously be done with MMAP + DMABUF, but for buffer sharing between devices, we should encourage application developers to use DMABUF instead of USERPTR. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Rémi, On Thursday 02 August 2012 09:56:43 Rémi Denis-Courmont wrote: > Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit : > > On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote: > > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at > > > > runtime ? > > > > > > Does CREATE_BUFS always work while already streaming has already > > > started? > > > If it depends on the driver, it's kinda helpless. > > > > Yes, it does. It's one of the reasons it exists in the first place. But > > there are currently only a handful of drivers that implement it. I hope > > that as more and more drivers are converted to vb2 that the availability > > of create_bufs will increase. > > That's contradictory. If most drivers do not support it, then it won't work > during streaming. > > > > What's the guaranteed minimum buffer count? It seems in any case, MMAP > > > has a hard limit of 32 buffers (at least videobuf2 has), though one > > > might argue this should be more than enough. > > > > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. > > Although drivers may force a lower maximum if they want. I have no idea > > whether there are drivers that do that. There probably are. > > The smallest of the maxima of all drivers. > > > The minimum is usually between 1 and 3, depending on hardware limitations. > > And that's clearly insufficient without memory copy to userspace buffers. That's the minimum number of buffers *required* by the hardware. You can add up to 32 buffers, I'm not aware of any driver that would prevent that. > It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for > REQBUFS+USERBUF then. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V3 11/16] drm/radeon: Make radeon card usable for Loongson.
Hi, Lucas, sorry for so long a delay because I have a holiday for one month. I found Loongson-3 must turn on SWIOTLB if the system memory has addresses above 4G. As I know, there are two ways to get a DMA addr, the first way is use dma_alloc_coherent(), and the other one is use map_page()/map_sg() on an exsisting address. The first way can make sure DMA addr below 4G, but the second way cannot (unless the memory is so little that all the address space is below 4G). Take SATA driver as an example, during initialization an 'ata_device' struct is allocated and its address is probably above 4G (because this is first used by CPU, not for DMA). 'ata_device' has a member 'id' and 'id' will be use for DMA in such a call path: ata_host_register() --> ata_scsi_add_hosts() --> async_port_probe() --> ata_port_probe() --> ata_bus_probe() --> ata_dev_read_id() --> ata_exec_internal() --> ata_exec_internal_sg() --> ata_qc_issue() --> ata_sg_setup() --> dma_map_sg() Here, dma_map_sg() will get a DMA addr above 4G, then SATA init failed. In OHCI driver, there are similar situations. P.S.: With recently drm changes, I found radeon with SWIOTLB can already work after suspend/resume, so my next version of Loongson patches will not modify radeon_ttm.c. On Fri, Jun 22, 2012 at 1:59 PM, Huacai Chen wrote: > On Fri, Jun 22, 2012 at 1:25 PM, Lucas Stach wrote: >> Hello Huacai, >> >> Am Freitag, den 22.06.2012, 11:01 +0800 schrieb Huacai Chen: >>> 1, Handle io prot correctly for MIPS. >>> 2, Define SAREA_MAX as the size of one page. >>> 3, Don't use swiotlb on Loongson machines (Loonson need swioitlb, but >>>when use swiotlb, GPU reset occurs at resume from suspend). >>> >> I still think this is wrong. You say Loongson needs SWIOTLB, but when >> it's actually used you ignore it in the radeon driver code. >> >> I looked up why you are using SWIOTLB and I don't agree with you that it >> is needed. SWIOTLB just gives you bounce pages for DMA memory above >> DMA32 and therefore papers over your >4GB DMA platform bug in some >> cases, while hurting performance. >> >> Please fix your DMA platform code so that region DMA is an alias for >> region DMA32. It should allow you to drop all those ugly workarounds. >> > Hmm, you are probably right, I think I should have a discuss with the > original author of this part of code. > >>> Signed-off-by: Huacai Chen >>> Signed-off-by: Hongliang Tao >>> Signed-off-by: Hua Yan >>> Reviewed-by: Michel D?nzer >>> Reviewed-by: Alex Deucher >>> Reviewed-by: Lucas Stach >>> Reviewed-by: j.glisse >> >> You should probably only stick this tag on your patches after the people >> you are naming explicitly gave their r-b for a specific version of a >> patch. >> >> Thanks, >> Lucas >>> Cc: dri-devel at lists.freedesktop.org >>> --- >>> drivers/gpu/drm/drm_vm.c|2 +- >>> drivers/gpu/drm/radeon/radeon_ttm.c |6 +++--- >>> drivers/gpu/drm/ttm/ttm_bo_util.c |2 +- >>> include/drm/drm_sarea.h |2 ++ >>> 4 files changed, 7 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c >>> index 961ee08..3f06166 100644 >>> --- a/drivers/gpu/drm/drm_vm.c >>> +++ b/drivers/gpu/drm/drm_vm.c >>> @@ -62,7 +62,7 @@ static pgprot_t drm_io_prot(uint32_t map_type, struct >>> vm_area_struct *vma) >>> tmp = pgprot_writecombine(tmp); >>> else >>> tmp = pgprot_noncached(tmp); >>> -#elif defined(__sparc__) || defined(__arm__) >>> +#elif defined(__sparc__) || defined(__arm__) || defined(__mips__) >>> tmp = pgprot_noncached(tmp); >>> #endif >>> return tmp; >>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c >>> b/drivers/gpu/drm/radeon/radeon_ttm.c >>> index c94a225..f49bdd1 100644 >>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c >>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c >>> @@ -630,7 +630,7 @@ static int radeon_ttm_tt_populate(struct ttm_tt *ttm) >>> } >>> #endif >>> >>> -#ifdef CONFIG_SWIOTLB >>> +#if defined(CONFIG_SWIOTLB) && !defined(CONFIG_CPU_LOONGSON3) >>> if (swiotlb_nr_tbl()) { >>> return ttm_dma_populate(>t->ttm, rdev->dev); >>> } >>> @@ -676,7 +676,7 @@ static void radeon_ttm_tt_unpopulate(struct ttm_tt *ttm) >>> } >>> #endif >>> >>> -#ifdef CONFIG_SWIOTLB >>> +#if defined(CONFIG_SWIOTLB) && !defined(CONFIG_CPU_LOONGSON3) >>> if (swiotlb_nr_tbl()) { >>> ttm_dma_unpopulate(>t->ttm, rdev->dev); >>> return; >>> @@ -906,7 +906,7 @@ static int radeon_ttm_debugfs_init(struct radeon_device >>> *rdev) >>> radeon_mem_types_list[i].show = &ttm_page_alloc_debugfs; >>> radeon_mem_types_list[i].driver_features = 0; >>> radeon_mem_types_list[i++].data = NULL; >>> -#ifdef CONFIG_SWIOTLB >>> +#if defined(CONFIG_SWIOTLB) && !defined(CONFIG_CPU_LOONGSON3) >>> if (swiotlb_nr_tbl()) { >>> sprintf(radeon_mem_types_names[i], "ttm_dma_page_pool"); >>> radeon_mem_types_l
Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code
On Thu, Aug 2, 2012 at 12:33 PM, Alex Deucher wrote: > On Thu, Aug 2, 2012 at 12:31 PM, Luca Tettamanti wrote: >> On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher wrote: >>> I admit I'm not really an ACPI expert, but thinking about this more, >>> I'm wondering if maybe we should just send the appropriate brightness >>> change, switch display, etc. event to userspace rather than handling >>> it directly in the radeon driver, then let userspace callback down via >>> the bl interface, etc. With backlight for example, does handling it >>> in the kernel driver as per your patch prevent userspace from seeing >>> the brightness up/down event? Wouldn't that break things like OSD >>> brightness displays and such? >> >> No, the event is sent to userspace by the standard ACPI video driver, >> it works as before. >> Changing brightness usually goes like this: >> 1) user presses a hotkey >> 2) a notification is generated (0x86 or 0x87) >> 3) video.ko handles the notification and calls into ACPI to change the >> level (_BCM) and firmware does its magic >> 4) a key press (brightness up/down) is sent to userspace >> >> With ATIF step 3 does not actually change the brightness, it just send >> out another event (VIDEO_PROBE, or one of the device specific ones) so >> we need to take care of that too. The rest of the process, including >> the delivery of the key presses, stays the same. Updated tree with fixes to a few existing patches and improved support for ATPX and initial support for ATCS: http://cgit.freedesktop.org/~agd5f/linux/?h=acpi_patches Alex ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] of: Add videomode helper
On 07/05/2012 08:51 AM, Rob Herring wrote: > On 07/04/2012 02:56 AM, Sascha Hauer wrote: >> This patch adds a helper function for parsing videomodes from the devicetree. >> The videomode can be either converted to a struct drm_display_mode or a >> struct fb_videomode. >> diff --git a/Documentation/devicetree/bindings/video/displaymode >> b/Documentation/devicetree/bindings/video/displaymode >> +Example: >> + >> + display at 0 { >> + /* 1920x1080p24 */ >> + clock = <5200>; > > Should this use the clock binding? You probably need both constraints > and clock binding though. I don't think the clock binding is appropriate here. This binding specifies a desired video mode, and should specify just the expected clock frequency for that mode. Perhaps any tolerance imposed by the specification used to calculate the mode timing could be specified too, but the allowed tolerance (for a mode to be recognized by the dispaly) is more driven by the receiving device than the transmitting device. The clock bindings should come into play in the display controller that sends a signal in that display mode. Something like: mode: hd1080p { clock = <5200>; xres = <1920>; yres = <1080>; }; display-controller { pixel-clock-source = <&clk ...>; // common clock bindings here default-mode = <&mode>; > Often you don't know the frequency up front and/or have limited control > of the frequency (i.e. integer dividers). Then you have to adjust the > margins to get the desired refresh rate. To do that, you need to know > the ranges of values a panel can support. Perhaps you just assume you > can increase the right-margin and lower-margins as I think you will hit > pixel clock frequency max before any limit on margins. I think this is more usually dealt with in HW design and matching components. The PLL/... feeding the display controller is going to have some known specifications that imply which pixel clocks it can generate. HW designers will pick a panel that accepts a clock the display controller can generate. The driver for the display controller will just generate as near of a pixel clock as it can to the rate specified in the mode definition. I believe it'd be unusual for a display driver to start fiddling with front-back porch (or margin) widths to munge the timing; that kind of thing probably worked OK with analog displays, but with digital displays where each pixel is clocked even in the margins, I think that would cause more problems than it solved. Similarly for external displays, the display controller will just pick the nearest clock it can. If it can't generate a close enough clock, it will just refuse to set the mode. In fact, a display controller driver would typically validate this when the set of legal modes was enumerated when initially detecting the display, so user-space typically wouldn't even request invalid modes.
[PATCH v2] of: Add videomode helper
On 07/04/2012 01:56 AM, Sascha Hauer wrote: > This patch adds a helper function for parsing videomodes from the devicetree. > The videomode can be either converted to a struct drm_display_mode or a > struct fb_videomode. > diff --git a/Documentation/devicetree/bindings/video/displaymode > b/Documentation/devicetree/bindings/video/displaymode > +Required properties: > + - xres, yres: Display resolution > + - left-margin, right-margin, hsync-len: Horizontal Display timing parameters > + in pixels > + upper-margin, lower-margin, vsync-len: Vertical display timing parameters > in > + lines Perhaps bike-shedding, but... For the margin property names, wouldn't it be better to use terminology more commonly used for video timings rather than Linux FB naming. In other words naming like: hactive, hsync-len, hfront-porch, hback-porch? > + - clock: displayclock in Hz > + > +Optional properties: > + - width-mm, height-mm: Display dimensions in mm > + - hsync-active-high (bool): Hsync pulse is active high > + - vsync-active-high (bool): Vsync pulse is active high > + - interlaced (bool): This is an interlaced mode > + - doublescan (bool): This is a doublescan mode > + > +There are different ways of describing a display mode. The devicetree > representation > +corresponds to the one used by the Linux Framebuffer framework described > here in > +Documentation/fb/framebuffer.txt. This representation has been chosen > because it's > +the only format which does not allow for inconsistent parameters.Unlike the > Framebuffer > +framework the devicetree has the clock in Hz instead of ps. As Rob mentioned, I think there shouldn't be any mention of Linux FB here. > + > +Example: > + > + display at 0 { This node appears to describe a video mode, not a display, hence the node name seems wrong. Many displays can support multiple different video modes. As mentioned elsewhere, properties like display width/height are properties of the display not the mode. So, I might expect something more like the following (various overhead properties like reg/#address-cells etc. elided for simplicity): disp: display { width-mm = <...>; height-mm = <...>; modes { mode at 0 { /* 1920x1080p24 */ clock = <5200>; xres = <1920>; yres = <1080>; left-margin = <25>; right-margin = <25>; hsync-len = <25>; lower-margin = <2>; upper-margin = <2>; vsync-len = <2>; hsync-active-high; }; mode at 1 { }; }; }; display-connector { display = <&disp>; // interface-specific properties such as pixel format here }; Finally, have you considered just using an EDID instead of creating something custom? I know that creating an EDID is harder than writing a few simple properties into a DT, but EDIDs have the following advantages: a) They're already standardized and very common. b) They allow other information such as a display's HDMI audio capabilities to be represented, which can then feed into an ALSA driver. c) The few LCD panel datasheets I've seen actually quote their specification as an EDID already, so deriving the EDID may actually be easy. d) People familiar with displays are almost certainly familiar with EDID's mode representation. There are many ways of representing display modes (sync position vs. porch widths, htotal specified rather than specifying all the components and hence htotal being calculated etc.). Not everyone will be familiar with all representations. Conversion errors are less likely if the target is EDID's familiar format. e) You'll end up with exactly the same data as if you have a DDC-based external monitor rather than an internal panel, so you end up getting to a common path in display handling code much more quickly.
drm/nouveau: crash regression in 3.5
I have managed to turn the crash into a WARN_ON, by adding this to the begin of nouveau_software_vblank(): if (!psw) { WARN_ON(1); return; } And I have also managed to load the module manually instead by udev. So I am happy to attach a full log of what's going on here. See also my added printk's starting with XXX that mark some interesting points in the initialization. This should give you enough information to track down the cause of the problem. To my non-expert eyes it looks like "noaccel" prevents registration of NVOBJ_ENGINE_SW or at least delays it too much. Thanks, Ortwin -- next part -- Aug 2 13:03:22 localhost kernel: Linux version 3.5.0 (root at ortwin-hp) (gcc version 4.5.3 (Gentoo 4.5.3-r2 p1.1, pie-0.4.7) ) #5 SMP PREEMPT Thu Aug 2 13:01:46 CEST 2012 Aug 2 13:03:22 localhost kernel: Command line: root=/dev/sda5 rootfstype=ext4 pciehp_force=1 nouveau.modeset=1 nouveau.noaccel=1 netconsole= at 10.11.1.234/eth0, at 10.11.1.19/00:17:f2:c7:5f:06 drm.debug=15 debug nox Aug 2 13:03:22 localhost kernel: e820: BIOS-provided physical RAM map: Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0x-0x0009fbff] usable Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0x0009fc00-0x0009] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0x000e-0x000f] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0x0010-0xbefc1fff] usable Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xbefc2000-0xbf6c1fff] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xbf6c2000-0xbf7c1fff] ACPI NVS Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xbf7c2000-0xbf7fefff] ACPI data Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xbf7ff000-0xbf7f] usable Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xbf80-0xbfff] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xe000-0xefff] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xfec0-0xfec00fff] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xfed1-0xfed13fff] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xfed19000-0xfed19fff] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xfed1b000-0xfed1] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xfee0-0xfee00fff] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0xffd0-0x] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0x0001-0x0001fbff] usable Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0x0001fc00-0x0001] reserved Aug 2 13:03:22 localhost kernel: BIOS-e820: [mem 0x0002-0x00023bff] usable Aug 2 13:03:22 localhost kernel: NX (Execute Disable) protection: active Aug 2 13:03:22 localhost kernel: DMI 2.6 present. Aug 2 13:03:22 localhost kernel: DMI: Hewlett-Packard HP EliteBook 8540w/1521, BIOS 68CVD Ver. F.0E 11/25/2010 Aug 2 13:03:22 localhost kernel: e820: update [mem 0x-0x] usable ==> reserved Aug 2 13:03:22 localhost kernel: e820: remove [mem 0x000a-0x000f] usable Aug 2 13:03:22 localhost kernel: No AGP bridge found Aug 2 13:03:22 localhost kernel: e820: last_pfn = 0x23c000 max_arch_pfn = 0x4 Aug 2 13:03:22 localhost start-stop-daemon: pam_unix(start-stop-daemon:session): session opened for user nobody by (uid=0) Aug 2 13:03:22 localhost kernel: MTRR default type: uncachable Aug 2 13:03:22 localhost kernel: MTRR fixed ranges enabled: Aug 2 13:03:22 localhost kernel: 0-9 write-back Aug 2 13:03:22 localhost kernel: A-B uncachable Aug 2 13:03:22 localhost kernel: C-F write-protect Aug 2 13:03:22 localhost kernel: MTRR variable ranges enabled: Aug 2 13:03:22 localhost kernel: 0 base 0FFC0 mask FFFC0 write-protect Aug 2 13:03:22 localhost kernel: 1 base 0 mask F8000 write-back Aug 2 13:03:22 localhost kernel: 2 base 08000 mask FC000 write-back Aug 2 13:03:22 localhost kernel: 3 base 1 mask F write-back Aug 2 13:03:22 localhost kernel: 4 base 2 mask FC000 write-back Aug 2 13:03:22 localhost kernel: 5 base 23C00 mask FFC00 uncachable Aug 2 13:03:22 localhost kernel: 6 disabled Aug 2 13:03:22 localhost kernel: 7 disabled Aug 2 13:03:22 localhost kernel: x86 PAT enabled: cpu 0, old 0x7040600070406, new 0x7010600070106 Aug 2 13:03:22 localhost kernel: e820: last_pfn = 0xbf800 max_arch_pfn = 0x4 Aug 2 13:03:22 localhost kernel: initial memory mapped: [mem 0x-0x1fff] Aug 2 13:03:22 localhost kernel: Base memory trampoline at [88099000] 99000 si
[PATCH] drm/radeon: add new AMD ACPI header and update relevant code
On Thu, Aug 2, 2012 at 12:31 PM, Luca Tettamanti wrote: > On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher wrote: >> I admit I'm not really an ACPI expert, but thinking about this more, >> I'm wondering if maybe we should just send the appropriate brightness >> change, switch display, etc. event to userspace rather than handling >> it directly in the radeon driver, then let userspace callback down via >> the bl interface, etc. With backlight for example, does handling it >> in the kernel driver as per your patch prevent userspace from seeing >> the brightness up/down event? Wouldn't that break things like OSD >> brightness displays and such? > > No, the event is sent to userspace by the standard ACPI video driver, > it works as before. > Changing brightness usually goes like this: > 1) user presses a hotkey > 2) a notification is generated (0x86 or 0x87) > 3) video.ko handles the notification and calls into ACPI to change the > level (_BCM) and firmware does its magic > 4) a key press (brightness up/down) is sent to userspace > > With ATIF step 3 does not actually change the brightness, it just send > out another event (VIDEO_PROBE, or one of the device specific ones) so > we need to take care of that too. The rest of the process, including > the delivery of the key presses, stays the same. Excellent! thanks for clarifying. Alex
[PATCH] drm/radeon: fix virtual memory locking in case of reset
On Thu, Aug 2, 2012 at 12:05 PM, wrote: > From: Jerome Glisse > > Lock/unlock mutex in proper order to avoid deadlock in case > of GPU reset triggered from VM code path. > > Cc: stable at vger.kernel.org [3.5] > Signed-off-by: Jerome Glisse Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/radeon/radeon_gart.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_gart.c > b/drivers/gpu/drm/radeon/radeon_gart.c > index b372005..7eabb59 100644 > --- a/drivers/gpu/drm/radeon/radeon_gart.c > +++ b/drivers/gpu/drm/radeon/radeon_gart.c > @@ -508,14 +508,19 @@ static void radeon_vm_unbind_locked(struct > radeon_device *rdev, > while (vm->fence) { > int r; > r = radeon_fence_wait(vm->fence, false); > - if (r) > + if (r) { > DRM_ERROR("error while waiting for fence: %d\n", r); > + } > if (r == -EDEADLK) { > + /* release mutex and lock in right order */ > mutex_unlock(&rdev->vm_manager.lock); > + mutex_unlock(&vm->mutex); > r = radeon_gpu_reset(rdev); > mutex_lock(&rdev->vm_manager.lock); > - if (!r) > + mutex_lock(&vm->mutex); > + if (!r) { > continue; > + } > } > break; > } > @@ -551,7 +556,9 @@ void radeon_vm_manager_fini(struct radeon_device *rdev) > mutex_lock(&rdev->vm_manager.lock); > /* unbind all active vm */ > list_for_each_entry_safe(vm, tmp, &rdev->vm_manager.lru_vm, list) { > + mutex_lock(&vm->mutex); > radeon_vm_unbind_locked(rdev, vm); > + mutex_unlock(&vm->mutex); > } > rdev->vm_manager.funcs->fini(rdev); > mutex_unlock(&rdev->vm_manager.lock); > -- > 1.7.10.4 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: fix virtual memory locking in case of reset
From: Jerome Glisse Lock/unlock mutex in proper order to avoid deadlock in case of GPU reset triggered from VM code path. Cc: stable at vger.kernel.org [3.5] Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/radeon_gart.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index b372005..7eabb59 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -508,14 +508,19 @@ static void radeon_vm_unbind_locked(struct radeon_device *rdev, while (vm->fence) { int r; r = radeon_fence_wait(vm->fence, false); - if (r) + if (r) { DRM_ERROR("error while waiting for fence: %d\n", r); + } if (r == -EDEADLK) { + /* release mutex and lock in right order */ mutex_unlock(&rdev->vm_manager.lock); + mutex_unlock(&vm->mutex); r = radeon_gpu_reset(rdev); mutex_lock(&rdev->vm_manager.lock); - if (!r) + mutex_lock(&vm->mutex); + if (!r) { continue; + } } break; } @@ -551,7 +556,9 @@ void radeon_vm_manager_fini(struct radeon_device *rdev) mutex_lock(&rdev->vm_manager.lock); /* unbind all active vm */ list_for_each_entry_safe(vm, tmp, &rdev->vm_manager.lru_vm, list) { + mutex_lock(&vm->mutex); radeon_vm_unbind_locked(rdev, vm); + mutex_unlock(&vm->mutex); } rdev->vm_manager.funcs->fini(rdev); mutex_unlock(&rdev->vm_manager.lock); -- 1.7.10.4
[PATCH] drm/radeon: add new AMD ACPI header and update relevant code
I admit I'm not really an ACPI expert, but thinking about this more, I'm wondering if maybe we should just send the appropriate brightness change, switch display, etc. event to userspace rather than handling it directly in the radeon driver, then let userspace callback down via the bl interface, etc. With backlight for example, does handling it in the kernel driver as per your patch prevent userspace from seeing the brightness up/down event? Wouldn't that break things like OSD brightness displays and such? Alex On Wed, Aug 1, 2012 at 9:56 AM, Alex Deucher wrote: > On Wed, Aug 1, 2012 at 4:57 AM, Luca Tettamanti > wrote: >> On Tue, Jul 31, 2012 at 05:33:16PM -0400, Alex Deucher wrote: >>> Patches look good. I picked them up and combined them with may >>> patches plus a few other small fixes. They are available here: >>> http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches >>> Let me know what you think. >> >> Looks ok, I lost one fix along the road though, I'm attaching the patch. > > Thanks, I rolled it into your original patch. New tree: > http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches > > Alex > >> >> Luca
[RFC 0/3] solving omapdrm/omapdss layering issues
On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: > On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen > wrote: > > I guess the fact is that DRM concepts do not really match the OMAP DSS > > hardware, and we'll have to use whatever gives us least problems. > > Actually, I think it does map fairly well to the hardware.. at least > more so than to omapdss ;-) Hm, I'm not sure I understand, omapdss concepts map directly to the hardware. > The one area that kms mismatches a bit is decoupling of ovl from mgr > that we have in our hw.. I've partially solved that a while back w/ What do you mean with that? Ovls and mgrs are one entity in KMS? Didn't the drm_plane stuff separate these? > For enabling/disabling an output (manager+encoder), this is relatively > infrequent, so it can afford to block to avoid races. (Like userspace > enabling and then rapidly disabling an output part way through the > enable.) But enabling/disabling an overlay, or adjusting position or > scanout address must not block. And ideally, if possible, switching > an overlay between two managers should not block. Adjusting the position or address of the buffer are simple, they can be easily done without any blocking. But ovl enable/disable and switching an ovl to another mgr do (possibly) take multiple vsyncs (and in the switch case, vsyncs of two separate outputs). So if those do not block, we'll need to handle them as a state machine and try to avoid races etc. It'll be "interesting". However, we can sometimes do those operations immediately. So I think we should have these conditional fast-paths in the code, and do them in non-blocking manner when possible. > I'll look again, but as far as I remember that at least wasn't > addressing the performance issues from making overlay enable/disable Right, it wasn't addressing those issues. But your branch doesn't really address those issues either, as it doesn't handle the problems related to ovl enable/disable. > synchronous. And fixing that would, I expect, trigger the same > problems that I already spent a few days debugging before switching > over to handle apply in omapdrm. I was under the impression that the apply mechanism, the caching and setting of the configs, was the major issue you had. But you're hinting that the actual problem is related to ovl enable/disable? I haven't tried fixing the ovl enable/disable, as I didn't know it's an issue for omapdrm. Or are they both as big issues? Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120802/7f7e4611/attachment-0001.pgp>
Re: drm/nouveau: crash regression in 3.5
On Thu, Aug 02, 2012 at 01:26:55PM +0200, Ortwin Glück wrote: > I have managed to turn the crash into a WARN_ON, by adding this to the > begin of nouveau_software_vblank(): > > if (!psw) { > WARN_ON(1); > return; > } Yes, I know about it, but this change fixes only a symptom. We should not get into this code at all without enabling vblank. > And I have also managed to load the module manually instead by udev. So > I am happy to attach a full log of what's going on here. See also my > added printk's starting with XXX that mark some interesting points in > the initialization. > > This should give you enough information to track down the cause of the > problem. To my non-expert eyes it looks like "noaccel" prevents > registration of NVOBJ_ENGINE_SW or at least delays it too much. Yes, that's what I wrote in my last patch - with noaccel it's not registered, which leads to NULL pointer derefence. I'm currently out of ideas why my patch does not work. But I have some ideas how to debug it. Can you come to nouveau IRC channel at freenode and do some additional tests? My nick is "joi" and I'm available on IRC between 6pm and 11pm CEST. Marcin ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Le jeudi 2 ao?t 2012 09:35:58 Hans Verkuil, vous avez ?crit : > On Wed August 1 2012 22:49:57 R?mi Denis-Courmont wrote: > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at > > > runtime ? > > > > Does CREATE_BUFS always work while already streaming has already started? > > If it depends on the driver, it's kinda helpless. > > Yes, it does. It's one of the reasons it exists in the first place. But > there are currently only a handful of drivers that implement it. I hope > that as more and more drivers are converted to vb2 that the availability > of create_bufs will increase. That's contradictory. If most drivers do not support it, then it won't work during streaming. > > What's the guaranteed minimum buffer count? It seems in any case, MMAP > > has a hard limit of 32 buffers (at least videobuf2 has), though one > > might argue this should be more than enough. > > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. > Although drivers may force a lower maximum if they want. I have no idea > whether there are drivers that do that. There probably are. The smallest of the maxima of all drivers. > The minimum is usually between 1 and 3, depending on hardware limitations. And that's clearly insufficient without memory copy to userspace buffers. It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for REQBUFS+USERBUF then. -- R?mi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis
Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code
On Thu, Aug 2, 2012 at 12:31 PM, Luca Tettamanti wrote: > On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher wrote: >> I admit I'm not really an ACPI expert, but thinking about this more, >> I'm wondering if maybe we should just send the appropriate brightness >> change, switch display, etc. event to userspace rather than handling >> it directly in the radeon driver, then let userspace callback down via >> the bl interface, etc. With backlight for example, does handling it >> in the kernel driver as per your patch prevent userspace from seeing >> the brightness up/down event? Wouldn't that break things like OSD >> brightness displays and such? > > No, the event is sent to userspace by the standard ACPI video driver, > it works as before. > Changing brightness usually goes like this: > 1) user presses a hotkey > 2) a notification is generated (0x86 or 0x87) > 3) video.ko handles the notification and calls into ACPI to change the > level (_BCM) and firmware does its magic > 4) a key press (brightness up/down) is sent to userspace > > With ATIF step 3 does not actually change the brightness, it just send > out another event (VIDEO_PROBE, or one of the device specific ones) so > we need to take care of that too. The rest of the process, including > the delivery of the key presses, stays the same. Excellent! thanks for clarifying. Alex ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code
On Thu, Aug 2, 2012 at 5:03 PM, Alex Deucher wrote: > I admit I'm not really an ACPI expert, but thinking about this more, > I'm wondering if maybe we should just send the appropriate brightness > change, switch display, etc. event to userspace rather than handling > it directly in the radeon driver, then let userspace callback down via > the bl interface, etc. With backlight for example, does handling it > in the kernel driver as per your patch prevent userspace from seeing > the brightness up/down event? Wouldn't that break things like OSD > brightness displays and such? No, the event is sent to userspace by the standard ACPI video driver, it works as before. Changing brightness usually goes like this: 1) user presses a hotkey 2) a notification is generated (0x86 or 0x87) 3) video.ko handles the notification and calls into ACPI to change the level (_BCM) and firmware does its magic 4) a key press (brightness up/down) is sent to userspace With ATIF step 3 does not actually change the brightness, it just send out another event (VIDEO_PROBE, or one of the device specific ones) so we need to take care of that too. The rest of the process, including the delivery of the key presses, stays the same. Luca ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv7 03/15] v4l: vb2: add support for shared buffer (dma_buf)
Hi Laurent, Hi Dima, On 06/27/2012 10:40 PM, Laurent Pinchart wrote: > Hi Dima, > > On Tuesday 26 June 2012 13:53:34 Dima Zavin wrote: >> On Tue, Jun 26, 2012 at 2:40 AM, Hans Verkuil wrote: >>> On Tue 26 June 2012 11:11:06 Laurent Pinchart wrote: On Tuesday 26 June 2012 10:40:44 Tomasz Stanislawski wrote: > Hi Dima Zavin, > Thank you for the patch and for a ping remainder :). > > You are right. The unmap is missing in __vb2_queue_cancel. > I will apply your fix into next version of V4L2 support for dmabuf. > > Please refer to some comments below. > > On 06/20/2012 08:12 AM, Dima Zavin wrote: >> Tomasz, >> >> I've encountered an issue with this patch when userspace does several >> stream_on/stream_off cycles. When the user tries to qbuf a buffer >> after doing stream_off, we trigger the "dmabuf already pinned" >> warning since we didn't unmap the buffer as dqbuf was never called. >> >> The below patch adds calls to unmap in queue_cancel, but my feeling >> is that we probably should be calling detach too (i.e. put_dmabuf). According to the V4L2 specification, the "VIDIOC_STREAMOFF ioctl, apart of aborting or finishing any DMA in progress, unlocks any user pointer buffers locked in physical memory, and it removes all buffers from the incoming and outgoing queues". >>> >>> Correct. And what that means in practice is that after a streamoff all >>> buffers are returned to the state they had just before STREAMON was >>> called. >> >> That can't be right. The buffers had to have been returned to the >> state just *after REQBUFS*, not just *before STREAMON*. You need to >> re-enqueue buffers before calling STREAMON. I assume that's what you >> meant? > > Your interpretation is correct. > So now we should decide what should be changed: the spec or vb2 ? Bringing the queue state back to *after REQBUFS* may make the next (STREAMON + QBUFs) very costly operations. Reattaching and mapping a DMABUF might trigger DMA allocation and *will* trigger creation of IOMMU mappings. In case of a user pointer, calling next get_user_pages may cause numerous fault events and will *create* new IOMMU mapping. Is there any need to do such a cleanup if the destruction of buffers and all caches can be explicitly executed by REQBUFS(count = 0) ? Maybe it would be easier to change the spec by removing "apart of ... in physical memory" part? STREAMOFF should mean stopping streaming, and that resources are no longer used. DMABUFs are unmapped but unmapping does not mean releasing. The exporter may keep the resource in its caches. Currently, vb2 does not follow the policy from the spec. The put_userptr ops is called on: - VIDIOC_REBUFS - VIDIOC_CREATE_BUFS - vb2_queue_release() which is usually called on close() syscall The put_userptr is not called and streamoff therefore the user pages are locked after STREAMOFF. BTW. What does 'buffer locked' mean? Does it mean that a buffer is pinned or referenced by a driver/HW? Regards, Tomasz Stanislawski >>> So after STREAMOFF you can immediately queue all buffers again with QBUF >>> and call STREAMON to restart streaming. No mmap or other operations >>> should be required. This behavior must be kept. >>> >>> VIDIOC_REQBUFS() or a close() are the only two operations that will >>> actually free the buffers completely. >>> >>> In practice, a STREAMOFF is either followed by a STREAMON at a later time, >>> or almost immediately followed by REQBUFS or close() to tear down the >>> buffers. So I don't think the buffers should be detached at streamoff. >> >> I agree. I was leaning this way which is why I left it out of my patch >> and wanted to hear your guys' opinion as you are much more familiar >> with the intended behavior than I am. >> >> Thanks! > > You're welcome. Thank you for reporting the problem and providing a patch. > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: fix virtual memory locking in case of reset
On Thu, Aug 2, 2012 at 12:05 PM, wrote: > From: Jerome Glisse > > Lock/unlock mutex in proper order to avoid deadlock in case > of GPU reset triggered from VM code path. > > Cc: sta...@vger.kernel.org [3.5] > Signed-off-by: Jerome Glisse Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/radeon/radeon_gart.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_gart.c > b/drivers/gpu/drm/radeon/radeon_gart.c > index b372005..7eabb59 100644 > --- a/drivers/gpu/drm/radeon/radeon_gart.c > +++ b/drivers/gpu/drm/radeon/radeon_gart.c > @@ -508,14 +508,19 @@ static void radeon_vm_unbind_locked(struct > radeon_device *rdev, > while (vm->fence) { > int r; > r = radeon_fence_wait(vm->fence, false); > - if (r) > + if (r) { > DRM_ERROR("error while waiting for fence: %d\n", r); > + } > if (r == -EDEADLK) { > + /* release mutex and lock in right order */ > mutex_unlock(&rdev->vm_manager.lock); > + mutex_unlock(&vm->mutex); > r = radeon_gpu_reset(rdev); > mutex_lock(&rdev->vm_manager.lock); > - if (!r) > + mutex_lock(&vm->mutex); > + if (!r) { > continue; > + } > } > break; > } > @@ -551,7 +556,9 @@ void radeon_vm_manager_fini(struct radeon_device *rdev) > mutex_lock(&rdev->vm_manager.lock); > /* unbind all active vm */ > list_for_each_entry_safe(vm, tmp, &rdev->vm_manager.lru_vm, list) { > + mutex_lock(&vm->mutex); > radeon_vm_unbind_locked(rdev, vm); > + mutex_unlock(&vm->mutex); > } > rdev->vm_manager.funcs->fini(rdev); > mutex_unlock(&rdev->vm_manager.lock); > -- > 1.7.10.4 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: ignore disconnected <-> unknown status changes
On an AOpen i915GMm-hfs the hotplug events generated by transitions between connector_status_unknown and connector_status_disconnected cause screen distortions. The attached patch cures the problem by disabling the generation of hotplug events in those cases. That should be safe for everybody as the only relevant changes are those from / to connector_status_connected. cu, Knut -- next part -- A non-text attachment was scrubbed... Name: 0001-drm-ignore-disconnected-unknown-status-changes.patch Type: text/x-patch Size: 1539 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120802/eb35539b/attachment-0001.bin>
[RFC 0/3] solving omapdrm/omapdss layering issues
On Thu, Aug 2, 2012 at 8:15 AM, Tomi Valkeinen wrote: > On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote: >> On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen >> wrote: >> > On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: >> >> On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen >> >> wrote: >> > >> >> > I guess the fact is that DRM concepts do not really match the OMAP DSS >> >> > hardware, and we'll have to use whatever gives us least problems. >> >> >> >> Actually, I think it does map fairly well to the hardware.. at least >> >> more so than to omapdss ;-) >> > >> > Hm, I'm not sure I understand, omapdss concepts map directly to the >> > hardware. >> >> I think it is mainly exposing the encoder and panel as two separate >> entities.. which seems to be what Archit is working on > > I still don't follow =) They are separate entities. Omapdss models the > HW quite directly, I think. It doesn't expose everything, though, as the > output drivers (dsi.c, dpi.c etc) are used via the panel drivers. right.. so we just need to expose the output drivers as separate entities, and let omapdrm propagate information such as timings between them >> in case of something like DVI bridge from DPI, this seems pretty >> straightforward.. only the connector needs to know about DDC stuff, >> which i2c to use and that sort of thing. So at kms level we would >> have (for example) an omap_dpi_encoder which would be the same for DPI >> panel (connector) or DPI->DVI bridge. For HDMI I'm still looking >> through the code to see how this would work. Honestly I've looked >> less at this part of code and encoder related registers in the TRM, >> compared to the ovl/mgr parts, but at least from the 'DSS overview' >> picture in the TRM it seems to make sense ;-) >> >> KMS even exposes the idea that certain crtcs can connect to only >> certain encoders. Or that you could you could have certain connectors >> switched between encoders. For example if you had a hw w/ DPI out, >> and some mux to switch that back and forth between a DPI lcd panel and >> a DPI->DVI bridge. (Ok, I'm not aware of any board that actually does >> this, but it is in theory possible.) So we could expose possible >> video chain topologies to userspace in this way. > > OMAP3 SDP board has such a setup, with manual switch to select between > LCD and DVI. ahh, good to know.. so I'm not crazy for wanting to expose this possibility to userspace >> The other thing is that we don't need to propagate timings from the >> panel up to the mgr at the dss level.. kms is already handling this >> for us. In my latest version, which I haven't pushed, I removed the >> 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'. > > You're thinking only about simple DPI cases. Consider this example, with > a DSI-to-DP bridge chip. What we have is the following flow of data: > > DISPC -> DSI -> DSI-2-DP -> DP monitor > > The timings you are thinking about are in the DISPC, but here they are > only one part of the link. And here the DISPC timings are not actually > the timings what the user is interested about. The user wants his > timings to be between DSI-2-DP chip and the DP monitor. > > Timings programmed to DISPC are not the same. The timings for DISPC come > from the DSI driver, and they may be very different than the user's > timings. With DSI video mode, the DISPC timings would have some > resemblance to the user's timings, mainly the time to send one line > would be the same. With DSI cmd mode, the DISPC timings would be > something totally different, most likely with 0 blank times and as fast > pixel clock as possible. hmm, well kms already has a concept of adjusted_timings, which could perhaps be used here to propagate the timings between crtc->encoder.. although the order is probably backwards from what we want (it comes from the crtc to the encoder.. and if I understand properly we want it the other way and actually possibly from the connector). But that isn't to say that internally in omapdrm the crtc couldn't get the adjusted timings from the connector. So I still think the parameter flow doesn't need to be 'under the hood' in omapdss. And fwiw, the adjusted_timings stuff is handled by drm_crtc_helper fxns, so if the way the core kms handles it isn't what we want, we can just plug in our own fxn instead of using drm_crtc_helper_set_mode(), so that isn't really a big problem. > What omapdss does currently is that you set the user's timings to the > right side of the chain, which propagate back to DSS. This allows the > DSI-2-DP bridge use DSI timings that work optimally for the bridge, and > DSI driver will use DISPC timings that work optimally for it. > > And it's not only about timings above, but also other settings related > to the busses between the components. Clock dividers, polarities, stuff > like that. I expect we could handle other settings in the same way as the timings. >> I think the problem was there were some cases, like ovl updates befo
[PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Thu August 2 2012 08:56:43 R?mi Denis-Courmont wrote: > Le jeudi 2 ao?t 2012 09:35:58 Hans Verkuil, vous avez ?crit : > > On Wed August 1 2012 22:49:57 R?mi Denis-Courmont wrote: > > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at > > > > runtime ? > > > > > > Does CREATE_BUFS always work while already streaming has already started? > > > If it depends on the driver, it's kinda helpless. > > > > Yes, it does. It's one of the reasons it exists in the first place. But > > there are currently only a handful of drivers that implement it. I hope > > that as more and more drivers are converted to vb2 that the availability > > of create_bufs will increase. > > That's contradictory. If most drivers do not support it, then it won't work > during streaming. IF create_bufs is implemented in the driver, THEN you can use it during streaming. I.e., it will never return EBUSY as an error due to the fact that streaming is in progress. Obviously it won't work if the driver didn't implement it in the first place. > > > > What's the guaranteed minimum buffer count? It seems in any case, MMAP > > > has a hard limit of 32 buffers (at least videobuf2 has), though one > > > might argue this should be more than enough. > > > > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. > > Although drivers may force a lower maximum if they want. I have no idea > > whether there are drivers that do that. There probably are. > > The smallest of the maxima of all drivers. I've no idea. Most will probably abide by the 32 maximum, but without analyzing all drivers I can't guarantee it. > > The minimum is usually between 1 and 3, depending on hardware limitations. > > And that's clearly insufficient without memory copy to userspace buffers. > > It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for > REQBUFS+USERBUF then. Just to put your mind at rest: USERPTR mode will *not* disappear or be deprecated in any way. It's been there for a long time, it's in heavy use, it's easy to use and it will not be turned into a second class citizen, because it isn't. Just because there is a new dmabuf mode available doesn't mean that everything should be done as a mmap+dmabuf thing. Regards, Hans
[PATCH] drm/radeon: fix virtual memory locking in case of reset
From: Jerome Glisse Lock/unlock mutex in proper order to avoid deadlock in case of GPU reset triggered from VM code path. Cc: sta...@vger.kernel.org [3.5] Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/radeon_gart.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index b372005..7eabb59 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -508,14 +508,19 @@ static void radeon_vm_unbind_locked(struct radeon_device *rdev, while (vm->fence) { int r; r = radeon_fence_wait(vm->fence, false); - if (r) + if (r) { DRM_ERROR("error while waiting for fence: %d\n", r); + } if (r == -EDEADLK) { + /* release mutex and lock in right order */ mutex_unlock(&rdev->vm_manager.lock); + mutex_unlock(&vm->mutex); r = radeon_gpu_reset(rdev); mutex_lock(&rdev->vm_manager.lock); - if (!r) + mutex_lock(&vm->mutex); + if (!r) { continue; + } } break; } @@ -551,7 +556,9 @@ void radeon_vm_manager_fini(struct radeon_device *rdev) mutex_lock(&rdev->vm_manager.lock); /* unbind all active vm */ list_for_each_entry_safe(vm, tmp, &rdev->vm_manager.lru_vm, list) { + mutex_lock(&vm->mutex); radeon_vm_unbind_locked(rdev, vm); + mutex_unlock(&vm->mutex); } rdev->vm_manager.funcs->fini(rdev); mutex_unlock(&rdev->vm_manager.lock); -- 1.7.10.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events
On ?, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: > AMD ACPI interface may overload the standard event > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > userspace because the user did not press the mode switch key (the > spurious keypress confuses the DE which usually changes the > display configuration and messes up a dual-screen setup). > This patch gives the radeon driver the chance to examine the event and > block the keypress if the event is an "AMD event". > > Signed-off-by: Luca Tettamanti > --- > Any comment from ACPI front? > it looks good to me. But I'm wondering if we can use the following code for ACPI part, which looks cleaner. I know this may change the behavior of other events, but in theory, we should not send any input event if we know something wrong in kernel. what do you think? diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1e0a9e1..8ad1f53 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1448,8 +1448,7 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) case ACPI_VIDEO_NOTIFY_SWITCH: /* User requested a switch, * most likely via hotkey. */ acpi_bus_generate_proc_event(device, event, 0); - if (!acpi_notifier_call_chain(device, event, 0)) - keycode = KEY_SWITCHVIDEOMODE; + keycode = KEY_SWITCHVIDEOMODE; break; case ACPI_VIDEO_NOTIFY_PROBE: /* User plugged in or removed a video @@ -1479,8 +1478,8 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) break; } - if (event != ACPI_VIDEO_NOTIFY_SWITCH) - acpi_notifier_call_chain(device, event, 0); + if (acpi_notifier_call_chain(device, event, 0)) + keycode = 0; if (keycode) { input_report_key(input, keycode, 1); > drivers/acpi/video.c | 10 -- > drivers/gpu/drm/radeon/radeon_acpi.c |7 ++- > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c > index 1e0a9e1..a8592c4 100644 > --- a/drivers/acpi/video.c > +++ b/drivers/acpi/video.c > @@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device > *device, u32 event) > acpi_video_device_enumerate(video); > acpi_video_device_rebind(video); > acpi_bus_generate_proc_event(device, event, 0); > - keycode = KEY_SWITCHVIDEOMODE; > + /* This event is also overloaded by AMD ACPI interface, don't > + * send the key press if the event has been handled elsewhere > + * (e.g. radeon DRM driver). > + */ > + if (!acpi_notifier_call_chain(device, event, 0)) > + keycode = KEY_SWITCHVIDEOMODE; > break; > > case ACPI_VIDEO_NOTIFY_CYCLE: /* Cycle Display output hotkey pressed. > */ > @@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device > *device, u32 event) > break; > } > > - if (event != ACPI_VIDEO_NOTIFY_SWITCH) > + if (event != ACPI_VIDEO_NOTIFY_SWITCH && > + event != ACPI_VIDEO_NOTIFY_PROBE) > acpi_notifier_call_chain(device, event, 0); > > if (keycode) { > diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c > b/drivers/gpu/drm/radeon/radeon_acpi.c > index 96de08d..ee0d29e 100644 > --- a/drivers/gpu/drm/radeon/radeon_acpi.c > +++ b/drivers/gpu/drm/radeon/radeon_acpi.c > @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev, > } > /* TODO: check other events */ > > - return NOTIFY_OK; > + /* We've handled the event, stop the notifier chain. The ACPI interface > + * overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to > + * userspace if the event was generated only to signal a SBIOS > + * request. > + */ > + return NOTIFY_BAD; > } > > /* Call all ACPI methods here */
[PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Wed August 1 2012 22:49:57 R?mi Denis-Courmont wrote: > Le mercredi 1 ao?t 2012 14:35:03 Laurent Pinchart, vous avez ?crit : > > > But in general, the V4L element in the pipeline does not know how fast > > > the downstream element(s) will consume the buffers. Thus it has to copy > > > from the MMAP buffers into anonymous user memory pending processing. > > > Then any dequeued buffer can be requeued as soon as possible. In theory, > > > it might also be that, even though the latency is known, the number of > > > required buffers exceeds the maximum MMAP buffers count of the V4L > > > device. Either way, user space ends up doing memory copy from MMAP to > > > custom buffers. > > > > > > This problem does not exist with USERBUF - the V4L2 element can simply > > > allocate a new buffer for each dequeued buffer. > > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ? > > Does CREATE_BUFS always work while already streaming has already started? If > it depends on the driver, it's kinda helpless. Yes, it does. It's one of the reasons it exists in the first place. But there are currently only a handful of drivers that implement it. I hope that as more and more drivers are converted to vb2 that the availability of create_bufs will increase. > What's the guaranteed minimum buffer count? It seems in any case, MMAP has a > hard limit of 32 buffers (at least videobuf2 has), though one might argue > this > should be more than enough. Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. Although drivers may force a lower maximum if they want. I have no idea whether there are drivers that do that. There probably are. The minimum is usually between 1 and 3, depending on hardware limitations. Regards, Hans
Massive power regression going 3.4->3.5
On Wed, 2012-08-01 at 22:08 -0700, bwidawsk wrote: > On 2012-08-01 03:06, Chris Wilson wrote: > > On Wed, 01 Aug 2012 10:38:36 +0100, James Bottomley > > wrote: > >> On Wed, 2012-08-01 at 09:58 +0100, Chris Wilson wrote: > >> > On Wed, 01 Aug 2012 09:45:04 +0100, James Bottomley > >> wrote: > >> > > On Wed, 2012-08-01 at 09:16 +0100, Chris Wilson wrote: > >> > > > On Wed, 01 Aug 2012 09:06:12 +0100, James Bottomley > >> wrote: > >> > > > > I got the attached to apply and it doesn't really improve > >> the idle power > >> > > > > much (12.5W). > >> > > > > >> > > > That's good to know. Next step is to try overriding > >> i915.semaphores. > >> > > > Can you please test with i915.semaphores=0 and > >> i915.semaphores=1? > >> > > > >> > > There's not much point doing i915_semaphores=1 since that's the > >> default > >> > > on gen 6 hardware, but i915_semaphores=0 recovers and idle power > >> of > >> > > ~6.5W > >> > > >> > It is only the default if iommu is off, and changing the default > >> > was one of the side-effects of the patch you bisected. > >> > > >> > Can you please login to the desktop, let it idle, record > >> > /sys/kernel/debug/dri/0/i915_cur_delayinfo and .../i915_drpc_info. > >> > Then trace-cmd record -e i915 sleep 10s, > >> > >> OK, what is trace-cmd? It looks similar to perf tools ... is that > >> it? > > > > Yes, it is roughly equivalent and you should be able to achieve the > > same > > with perf trace - except I haven't done it before so I don't have > > quick > > advice on how to drive it. :) > > -Chris > > Should be something like: > perf record -f -g -e i915:* -a I already sent the output of trace-cmd ... is that enough? James
Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code
I admit I'm not really an ACPI expert, but thinking about this more, I'm wondering if maybe we should just send the appropriate brightness change, switch display, etc. event to userspace rather than handling it directly in the radeon driver, then let userspace callback down via the bl interface, etc. With backlight for example, does handling it in the kernel driver as per your patch prevent userspace from seeing the brightness up/down event? Wouldn't that break things like OSD brightness displays and such? Alex On Wed, Aug 1, 2012 at 9:56 AM, Alex Deucher wrote: > On Wed, Aug 1, 2012 at 4:57 AM, Luca Tettamanti wrote: >> On Tue, Jul 31, 2012 at 05:33:16PM -0400, Alex Deucher wrote: >>> Patches look good. I picked them up and combined them with may >>> patches plus a few other small fixes. They are available here: >>> http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches >>> Let me know what you think. >> >> Looks ok, I lost one fix along the road though, I'm attaching the patch. > > Thanks, I rolled it into your original patch. New tree: > http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches > > Alex > >> >> Luca ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC 0/3] solving omapdrm/omapdss layering issues
On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen wrote: > On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: >> On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen >> wrote: > >> > I guess the fact is that DRM concepts do not really match the OMAP DSS >> > hardware, and we'll have to use whatever gives us least problems. >> >> Actually, I think it does map fairly well to the hardware.. at least >> more so than to omapdss ;-) > > Hm, I'm not sure I understand, omapdss concepts map directly to the > hardware. I think it is mainly exposing the encoder and panel as two separate entities.. which seems to be what Archit is working on in case of something like DVI bridge from DPI, this seems pretty straightforward.. only the connector needs to know about DDC stuff, which i2c to use and that sort of thing. So at kms level we would have (for example) an omap_dpi_encoder which would be the same for DPI panel (connector) or DPI->DVI bridge. For HDMI I'm still looking through the code to see how this would work. Honestly I've looked less at this part of code and encoder related registers in the TRM, compared to the ovl/mgr parts, but at least from the 'DSS overview' picture in the TRM it seems to make sense ;-) KMS even exposes the idea that certain crtcs can connect to only certain encoders. Or that you could you could have certain connectors switched between encoders. For example if you had a hw w/ DPI out, and some mux to switch that back and forth between a DPI lcd panel and a DPI->DVI bridge. (Ok, I'm not aware of any board that actually does this, but it is in theory possible.) So we could expose possible video chain topologies to userspace in this way. The other thing is that we don't need to propagate timings from the panel up to the mgr at the dss level.. kms is already handling this for us. In my latest version, which I haven't pushed, I removed the 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'. >> The one area that kms mismatches a bit is decoupling of ovl from mgr >> that we have in our hw.. I've partially solved that a while back w/ > > What do you mean with that? Ovls and mgrs are one entity in KMS? Didn't > the drm_plane stuff separate these? yes and no.. it is in our omapdrm implementation, because each crtc has it's own private plane assigned. Basically the purpose is that we can't break the interface to existing KMS userspace, which expects a CRTC to include the dma scanout engine. But it means at the moment we can't re-use these planes from crtc's that are not in use. I have some ideas about how to expose this to userspace in a backwards compatible way, so a userspace that is aware of this can re-use planes from crtcs that are not in use. There is at least one other SoC platform (STE, IIRC?) that has similar flexibility in hw, so I think this is a worthwhile thing to do.. but just haven't gotten to it yet. >> For enabling/disabling an output (manager+encoder), this is relatively >> infrequent, so it can afford to block to avoid races. (Like userspace >> enabling and then rapidly disabling an output part way through the >> enable.) But enabling/disabling an overlay, or adjusting position or >> scanout address must not block. And ideally, if possible, switching >> an overlay between two managers should not block. > > Adjusting the position or address of the buffer are simple, they can be > easily done without any blocking. > > But ovl enable/disable and switching an ovl to another mgr do (possibly) > take multiple vsyncs (and in the switch case, vsyncs of two separate > outputs). So if those do not block, we'll need to handle them as a state > machine and try to avoid races etc. It'll be "interesting". ok, I see the problem. Really the one thing I'm not handling properly is disconnecting a plane from one crtc and connecting to another. The disconnect should synchronize on the outgoing crtc's vblank/GO, and connect on the incoming crtc. But this is not a frequent operation, so I think the easy solution here is to block on the connect-to-new-crtc if the disconnect is still pending. I'd prefer this to introducing intermediate states. A simple enable/disable without changing crtc does not need to block. If usespace disables and then re-enables before the vblank, we just apply whatever is the most recent state at the vblank. Meaning enable->disable->enable, the middle disable might just get skipped. This is fine, and actually desirable. > However, we can sometimes do those operations immediately. So I think we > should have these conditional fast-paths in the code, and do them in > non-blocking manner when possible. > >> I'll look again, but as far as I remember that at least wasn't >> addressing the performance issues from making overlay enable/disable > > Right, it wasn't addressing those issues. But your branch doesn't really > address those issues either, as it doesn't handle the problems related > to ovl enable/disable. > >> synchronous. And fixing that would
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Thu, Aug 2, 2012 at 8:15 AM, Tomi Valkeinen wrote: > On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote: >> On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen wrote: >> > On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: >> >> On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen >> >> wrote: >> > >> >> > I guess the fact is that DRM concepts do not really match the OMAP DSS >> >> > hardware, and we'll have to use whatever gives us least problems. >> >> >> >> Actually, I think it does map fairly well to the hardware.. at least >> >> more so than to omapdss ;-) >> > >> > Hm, I'm not sure I understand, omapdss concepts map directly to the >> > hardware. >> >> I think it is mainly exposing the encoder and panel as two separate >> entities.. which seems to be what Archit is working on > > I still don't follow =) They are separate entities. Omapdss models the > HW quite directly, I think. It doesn't expose everything, though, as the > output drivers (dsi.c, dpi.c etc) are used via the panel drivers. right.. so we just need to expose the output drivers as separate entities, and let omapdrm propagate information such as timings between them >> in case of something like DVI bridge from DPI, this seems pretty >> straightforward.. only the connector needs to know about DDC stuff, >> which i2c to use and that sort of thing. So at kms level we would >> have (for example) an omap_dpi_encoder which would be the same for DPI >> panel (connector) or DPI->DVI bridge. For HDMI I'm still looking >> through the code to see how this would work. Honestly I've looked >> less at this part of code and encoder related registers in the TRM, >> compared to the ovl/mgr parts, but at least from the 'DSS overview' >> picture in the TRM it seems to make sense ;-) >> >> KMS even exposes the idea that certain crtcs can connect to only >> certain encoders. Or that you could you could have certain connectors >> switched between encoders. For example if you had a hw w/ DPI out, >> and some mux to switch that back and forth between a DPI lcd panel and >> a DPI->DVI bridge. (Ok, I'm not aware of any board that actually does >> this, but it is in theory possible.) So we could expose possible >> video chain topologies to userspace in this way. > > OMAP3 SDP board has such a setup, with manual switch to select between > LCD and DVI. ahh, good to know.. so I'm not crazy for wanting to expose this possibility to userspace >> The other thing is that we don't need to propagate timings from the >> panel up to the mgr at the dss level.. kms is already handling this >> for us. In my latest version, which I haven't pushed, I removed the >> 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'. > > You're thinking only about simple DPI cases. Consider this example, with > a DSI-to-DP bridge chip. What we have is the following flow of data: > > DISPC -> DSI -> DSI-2-DP -> DP monitor > > The timings you are thinking about are in the DISPC, but here they are > only one part of the link. And here the DISPC timings are not actually > the timings what the user is interested about. The user wants his > timings to be between DSI-2-DP chip and the DP monitor. > > Timings programmed to DISPC are not the same. The timings for DISPC come > from the DSI driver, and they may be very different than the user's > timings. With DSI video mode, the DISPC timings would have some > resemblance to the user's timings, mainly the time to send one line > would be the same. With DSI cmd mode, the DISPC timings would be > something totally different, most likely with 0 blank times and as fast > pixel clock as possible. hmm, well kms already has a concept of adjusted_timings, which could perhaps be used here to propagate the timings between crtc->encoder.. although the order is probably backwards from what we want (it comes from the crtc to the encoder.. and if I understand properly we want it the other way and actually possibly from the connector). But that isn't to say that internally in omapdrm the crtc couldn't get the adjusted timings from the connector. So I still think the parameter flow doesn't need to be 'under the hood' in omapdss. And fwiw, the adjusted_timings stuff is handled by drm_crtc_helper fxns, so if the way the core kms handles it isn't what we want, we can just plug in our own fxn instead of using drm_crtc_helper_set_mode(), so that isn't really a big problem. > What omapdss does currently is that you set the user's timings to the > right side of the chain, which propagate back to DSS. This allows the > DSI-2-DP bridge use DSI timings that work optimally for the bridge, and > DSI driver will use DISPC timings that work optimally for it. > > And it's not only about timings above, but also other settings related > to the busses between the components. Clock dividers, polarities, stuff > like that. I expect we could handle other settings in the same way as the timings. >> I think the problem was there were some cases, like ovl updates before >
Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events
On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: > On 三, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: > > AMD ACPI interface may overload the standard event > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > > userspace because the user did not press the mode switch key (the > > spurious keypress confuses the DE which usually changes the > > display configuration and messes up a dual-screen setup). > > This patch gives the radeon driver the chance to examine the event and > > block the keypress if the event is an "AMD event". > > > > Signed-off-by: Luca Tettamanti > > --- > > Any comment from ACPI front? > > > it looks good to me. > But I'm wondering if we can use the following code for ACPI part, which > looks cleaner. > I know this may change the behavior of other events, but in theory, we > should not send any input event if we know something wrong in kernel. > > what do you think? I like it, it's cleaner. I've split the patch in two pieces (one for video, the other for radeon) and adopted your suggestion. BTW, I'm leaving for vacation in a few hours, I'll be offline till mid August. Luca >From acce30c96b90d1bc550e82a9e7f19226fa194d5e Mon Sep 17 00:00:00 2001 From: Luca Tettamanti Date: Thu, 2 Aug 2012 15:30:27 +0200 Subject: [PATCH 1/2] ACPI video: allow events handlers to veto the keypress The standard video events may be overloaded for device specific purposes. For example AMD ACPI interface overloads ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the userspace because the user did not press the mode switch key (the spurious keypress confuses the DE which usually changes the display configuration and messes up a dual-screen setup). This patch gives the handlers the chance to examine the event and block the keypress if the event is device specific. v2: refactor as suggested by Zhang Rui Signed-off-by: Luca Tettamanti --- drivers/acpi/video.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1e0a9e1..d75642a 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1448,8 +1448,7 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) case ACPI_VIDEO_NOTIFY_SWITCH: /* User requested a switch, * most likely via hotkey. */ acpi_bus_generate_proc_event(device, event, 0); - if (!acpi_notifier_call_chain(device, event, 0)) - keycode = KEY_SWITCHVIDEOMODE; + keycode = KEY_SWITCHVIDEOMODE; break; case ACPI_VIDEO_NOTIFY_PROBE: /* User plugged in or removed a video @@ -1479,8 +1478,9 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) break; } - if (event != ACPI_VIDEO_NOTIFY_SWITCH) - acpi_notifier_call_chain(device, event, 0); + if (acpi_notifier_call_chain(device, event, 0)) + /* Something vetoed the keypress. */ + keycode = 0; if (keycode) { input_report_key(input, keycode, 1); -- 1.7.10.4 >From def5119d8f617eef0fac2cd35d7bb18c176ff8f6 Mon Sep 17 00:00:00 2001 From: Luca Tettamanti Date: Thu, 2 Aug 2012 15:33:03 +0200 Subject: [PATCH 2/2] drm/radeon: block the keypress on ATIF events The AMD ACPI interface may use ACPI_VIDEO_NOTIFY_PROBE to signal SBIOS requests; block the keypress in this case since the user did not actually press the mode switch key. Signed-off-by: Luca Tettamanti --- drivers/gpu/drm/radeon/radeon_acpi.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 96de08d..ee0d29e 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev, } /* TODO: check other events */ - return NOTIFY_OK; + /* We've handled the event, stop the notifier chain. The ACPI interface +* overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to +* userspace if the event was generated only to signal a SBIOS +* request. +*/ + return NOTIFY_BAD; } /* Call all ACPI methods here */ -- 1.7.10.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote: > On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen wrote: > > On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: > >> On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen > >> wrote: > > > >> > I guess the fact is that DRM concepts do not really match the OMAP DSS > >> > hardware, and we'll have to use whatever gives us least problems. > >> > >> Actually, I think it does map fairly well to the hardware.. at least > >> more so than to omapdss ;-) > > > > Hm, I'm not sure I understand, omapdss concepts map directly to the > > hardware. > > I think it is mainly exposing the encoder and panel as two separate > entities.. which seems to be what Archit is working on I still don't follow =) They are separate entities. Omapdss models the HW quite directly, I think. It doesn't expose everything, though, as the output drivers (dsi.c, dpi.c etc) are used via the panel drivers. > in case of something like DVI bridge from DPI, this seems pretty > straightforward.. only the connector needs to know about DDC stuff, > which i2c to use and that sort of thing. So at kms level we would > have (for example) an omap_dpi_encoder which would be the same for DPI > panel (connector) or DPI->DVI bridge. For HDMI I'm still looking > through the code to see how this would work. Honestly I've looked > less at this part of code and encoder related registers in the TRM, > compared to the ovl/mgr parts, but at least from the 'DSS overview' > picture in the TRM it seems to make sense ;-) > > KMS even exposes the idea that certain crtcs can connect to only > certain encoders. Or that you could you could have certain connectors > switched between encoders. For example if you had a hw w/ DPI out, > and some mux to switch that back and forth between a DPI lcd panel and > a DPI->DVI bridge. (Ok, I'm not aware of any board that actually does > this, but it is in theory possible.) So we could expose possible > video chain topologies to userspace in this way. OMAP3 SDP board has such a setup, with manual switch to select between LCD and DVI. > The other thing is that we don't need to propagate timings from the > panel up to the mgr at the dss level.. kms is already handling this > for us. In my latest version, which I haven't pushed, I removed the > 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'. You're thinking only about simple DPI cases. Consider this example, with a DSI-to-DP bridge chip. What we have is the following flow of data: DISPC -> DSI -> DSI-2-DP -> DP monitor The timings you are thinking about are in the DISPC, but here they are only one part of the link. And here the DISPC timings are not actually the timings what the user is interested about. The user wants his timings to be between DSI-2-DP chip and the DP monitor. Timings programmed to DISPC are not the same. The timings for DISPC come from the DSI driver, and they may be very different than the user's timings. With DSI video mode, the DISPC timings would have some resemblance to the user's timings, mainly the time to send one line would be the same. With DSI cmd mode, the DISPC timings would be something totally different, most likely with 0 blank times and as fast pixel clock as possible. What omapdss does currently is that you set the user's timings to the right side of the chain, which propagate back to DSS. This allows the DSI-2-DP bridge use DSI timings that work optimally for the bridge, and DSI driver will use DISPC timings that work optimally for it. And it's not only about timings above, but also other settings related to the busses between the components. Clock dividers, polarities, stuff like that. > I think the problem was there were some cases, like ovl updates before > setting the mgr, where the user_info_dirty flag would be cleared but > the registers not updated. This is what I meant by sequence of Hmm, I'd appreciate more info about this if you can give. Sounds like a bug, that shouldn't happen. So you mean that you have an ovl, with no manager set. And you change the settings of the ovl before assigning it to a mgr? That's something I haven't really tested, so it could bug, true. > operations at KMS level confusing omapdss. This should be fixable > with some debugging. Although getting rid of the state tracking at > omapdss level altogether was a much simpler solution, and is the one I > prefer :-) Yes, I don't prefer the state tracking either (we didn't have it in earlier versions of omapdss), but I still don't see an option to it if we want to support all the stuff we have. Tomi signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen wrote: > On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: >> On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen wrote: > >> > I guess the fact is that DRM concepts do not really match the OMAP DSS >> > hardware, and we'll have to use whatever gives us least problems. >> >> Actually, I think it does map fairly well to the hardware.. at least >> more so than to omapdss ;-) > > Hm, I'm not sure I understand, omapdss concepts map directly to the > hardware. I think it is mainly exposing the encoder and panel as two separate entities.. which seems to be what Archit is working on in case of something like DVI bridge from DPI, this seems pretty straightforward.. only the connector needs to know about DDC stuff, which i2c to use and that sort of thing. So at kms level we would have (for example) an omap_dpi_encoder which would be the same for DPI panel (connector) or DPI->DVI bridge. For HDMI I'm still looking through the code to see how this would work. Honestly I've looked less at this part of code and encoder related registers in the TRM, compared to the ovl/mgr parts, but at least from the 'DSS overview' picture in the TRM it seems to make sense ;-) KMS even exposes the idea that certain crtcs can connect to only certain encoders. Or that you could you could have certain connectors switched between encoders. For example if you had a hw w/ DPI out, and some mux to switch that back and forth between a DPI lcd panel and a DPI->DVI bridge. (Ok, I'm not aware of any board that actually does this, but it is in theory possible.) So we could expose possible video chain topologies to userspace in this way. The other thing is that we don't need to propagate timings from the panel up to the mgr at the dss level.. kms is already handling this for us. In my latest version, which I haven't pushed, I removed the 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'. >> The one area that kms mismatches a bit is decoupling of ovl from mgr >> that we have in our hw.. I've partially solved that a while back w/ > > What do you mean with that? Ovls and mgrs are one entity in KMS? Didn't > the drm_plane stuff separate these? yes and no.. it is in our omapdrm implementation, because each crtc has it's own private plane assigned. Basically the purpose is that we can't break the interface to existing KMS userspace, which expects a CRTC to include the dma scanout engine. But it means at the moment we can't re-use these planes from crtc's that are not in use. I have some ideas about how to expose this to userspace in a backwards compatible way, so a userspace that is aware of this can re-use planes from crtcs that are not in use. There is at least one other SoC platform (STE, IIRC?) that has similar flexibility in hw, so I think this is a worthwhile thing to do.. but just haven't gotten to it yet. >> For enabling/disabling an output (manager+encoder), this is relatively >> infrequent, so it can afford to block to avoid races. (Like userspace >> enabling and then rapidly disabling an output part way through the >> enable.) But enabling/disabling an overlay, or adjusting position or >> scanout address must not block. And ideally, if possible, switching >> an overlay between two managers should not block. > > Adjusting the position or address of the buffer are simple, they can be > easily done without any blocking. > > But ovl enable/disable and switching an ovl to another mgr do (possibly) > take multiple vsyncs (and in the switch case, vsyncs of two separate > outputs). So if those do not block, we'll need to handle them as a state > machine and try to avoid races etc. It'll be "interesting". ok, I see the problem. Really the one thing I'm not handling properly is disconnecting a plane from one crtc and connecting to another. The disconnect should synchronize on the outgoing crtc's vblank/GO, and connect on the incoming crtc. But this is not a frequent operation, so I think the easy solution here is to block on the connect-to-new-crtc if the disconnect is still pending. I'd prefer this to introducing intermediate states. A simple enable/disable without changing crtc does not need to block. If usespace disables and then re-enables before the vblank, we just apply whatever is the most recent state at the vblank. Meaning enable->disable->enable, the middle disable might just get skipped. This is fine, and actually desirable. > However, we can sometimes do those operations immediately. So I think we > should have these conditional fast-paths in the code, and do them in > non-blocking manner when possible. > >> I'll look again, but as far as I remember that at least wasn't >> addressing the performance issues from making overlay enable/disable > > Right, it wasn't addressing those issues. But your branch doesn't really > address those issues either, as it doesn't handle the problems related > to ovl enable/disable. > >> synchronous. And fixing that would, I
Re: Massive power regression going 3.4->3.5
On 2012-08-02 00:20, James Bottomley wrote: On Wed, 2012-08-01 at 22:08 -0700, bwidawsk wrote: On 2012-08-01 03:06, Chris Wilson wrote: > On Wed, 01 Aug 2012 10:38:36 +0100, James Bottomley > wrote: >> On Wed, 2012-08-01 at 09:58 +0100, Chris Wilson wrote: >> > On Wed, 01 Aug 2012 09:45:04 +0100, James Bottomley >> wrote: >> > > On Wed, 2012-08-01 at 09:16 +0100, Chris Wilson wrote: >> > > > On Wed, 01 Aug 2012 09:06:12 +0100, James Bottomley >> wrote: >> > > > > I got the attached to apply and it doesn't really improve >> the idle power >> > > > > much (12.5W). >> > > > >> > > > That's good to know. Next step is to try overriding >> i915.semaphores. >> > > > Can you please test with i915.semaphores=0 and >> i915.semaphores=1? >> > > >> > > There's not much point doing i915_semaphores=1 since that's the >> default >> > > on gen 6 hardware, but i915_semaphores=0 recovers and idle power >> of >> > > ~6.5W >> > >> > It is only the default if iommu is off, and changing the default >> > was one of the side-effects of the patch you bisected. >> > >> > Can you please login to the desktop, let it idle, record >> > /sys/kernel/debug/dri/0/i915_cur_delayinfo and .../i915_drpc_info. >> > Then trace-cmd record -e i915 sleep 10s, >> >> OK, what is trace-cmd? It looks similar to perf tools ... is that >> it? > > Yes, it is roughly equivalent and you should be able to achieve the > same > with perf trace - except I haven't done it before so I don't have > quick > advice on how to drive it. :) > -Chris Should be something like: perf record -f -g -e i915:* -a I already sent the output of trace-cmd ... is that enough? James Yes, should do. Have we already eliminated the obvious? GPU semaphores will give time back to the GPU clients normally waiting on such things, X, XFCE, whatever else. It'd probably be handy to begin investigating what those guys are doing with their new extra time. Chris, this is what I was getting at on IRC the other day. What do you think? ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Massive power regression going 3.4->3.5
On 2012-08-02 00:20, James Bottomley wrote: > On Wed, 2012-08-01 at 22:08 -0700, bwidawsk wrote: >> On 2012-08-01 03:06, Chris Wilson wrote: >> > On Wed, 01 Aug 2012 10:38:36 +0100, James Bottomley >> > wrote: >> >> On Wed, 2012-08-01 at 09:58 +0100, Chris Wilson wrote: >> >> > On Wed, 01 Aug 2012 09:45:04 +0100, James Bottomley >> >> wrote: >> >> > > On Wed, 2012-08-01 at 09:16 +0100, Chris Wilson wrote: >> >> > > > On Wed, 01 Aug 2012 09:06:12 +0100, James Bottomley >> >> wrote: >> >> > > > > I got the attached to apply and it doesn't really improve >> >> the idle power >> >> > > > > much (12.5W). >> >> > > > >> >> > > > That's good to know. Next step is to try overriding >> >> i915.semaphores. >> >> > > > Can you please test with i915.semaphores=0 and >> >> i915.semaphores=1? >> >> > > >> >> > > There's not much point doing i915_semaphores=1 since that's >> the >> >> default >> >> > > on gen 6 hardware, but i915_semaphores=0 recovers and idle >> power >> >> of >> >> > > ~6.5W >> >> > >> >> > It is only the default if iommu is off, and changing the >> default >> >> > was one of the side-effects of the patch you bisected. >> >> > >> >> > Can you please login to the desktop, let it idle, record >> >> > /sys/kernel/debug/dri/0/i915_cur_delayinfo and >> .../i915_drpc_info. >> >> > Then trace-cmd record -e i915 sleep 10s, >> >> >> >> OK, what is trace-cmd? It looks similar to perf tools ... is >> that >> >> it? >> > >> > Yes, it is roughly equivalent and you should be able to achieve >> the >> > same >> > with perf trace - except I haven't done it before so I don't have >> > quick >> > advice on how to drive it. :) >> > -Chris >> >> Should be something like: >> perf record -f -g -e i915:* -a > > I already sent the output of trace-cmd ... is that enough? > > James Yes, should do. Have we already eliminated the obvious? GPU semaphores will give time back to the GPU clients normally waiting on such things, X, XFCE, whatever else. It'd probably be handy to begin investigating what those guys are doing with their new extra time. Chris, this is what I was getting at on IRC the other day. What do you think?
[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11
https://bugs.freedesktop.org/show_bug.cgi?id=45018 --- Comment #83 from Jerome Glisse 2012-08-02 03:46:43 UTC --- How do you trigger the va issue ? piglit ? I was not able to reproduce. It's kind of painful to debug in the dark. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11
https://bugs.freedesktop.org/show_bug.cgi?id=45018 --- Comment #82 from Alexandre Demers 2012-08-02 01:02:42 UTC --- (In reply to comment #80) > I tried both patches, the one from comment 76 and the one from comment 70, > neither fixed the issue with the rendering regression or the va conflict. Same here, I was rebuilding my kernel from scratch just in case. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
Re: Massive power regression going 3.4->3.5
On Wed, 2012-08-01 at 22:08 -0700, bwidawsk wrote: > On 2012-08-01 03:06, Chris Wilson wrote: > > On Wed, 01 Aug 2012 10:38:36 +0100, James Bottomley > > wrote: > >> On Wed, 2012-08-01 at 09:58 +0100, Chris Wilson wrote: > >> > On Wed, 01 Aug 2012 09:45:04 +0100, James Bottomley > >> wrote: > >> > > On Wed, 2012-08-01 at 09:16 +0100, Chris Wilson wrote: > >> > > > On Wed, 01 Aug 2012 09:06:12 +0100, James Bottomley > >> wrote: > >> > > > > I got the attached to apply and it doesn't really improve > >> the idle power > >> > > > > much (12.5W). > >> > > > > >> > > > That's good to know. Next step is to try overriding > >> i915.semaphores. > >> > > > Can you please test with i915.semaphores=0 and > >> i915.semaphores=1? > >> > > > >> > > There's not much point doing i915_semaphores=1 since that's the > >> default > >> > > on gen 6 hardware, but i915_semaphores=0 recovers and idle power > >> of > >> > > ~6.5W > >> > > >> > It is only the default if iommu is off, and changing the default > >> > was one of the side-effects of the patch you bisected. > >> > > >> > Can you please login to the desktop, let it idle, record > >> > /sys/kernel/debug/dri/0/i915_cur_delayinfo and .../i915_drpc_info. > >> > Then trace-cmd record -e i915 sleep 10s, > >> > >> OK, what is trace-cmd? It looks similar to perf tools ... is that > >> it? > > > > Yes, it is roughly equivalent and you should be able to achieve the > > same > > with perf trace - except I haven't done it before so I don't have > > quick > > advice on how to drive it. :) > > -Chris > > Should be something like: > perf record -f -g -e i915:* -a I already sent the output of trace-cmd ... is that enough? James ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: > On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen wrote: > > I guess the fact is that DRM concepts do not really match the OMAP DSS > > hardware, and we'll have to use whatever gives us least problems. > > Actually, I think it does map fairly well to the hardware.. at least > more so than to omapdss ;-) Hm, I'm not sure I understand, omapdss concepts map directly to the hardware. > The one area that kms mismatches a bit is decoupling of ovl from mgr > that we have in our hw.. I've partially solved that a while back w/ What do you mean with that? Ovls and mgrs are one entity in KMS? Didn't the drm_plane stuff separate these? > For enabling/disabling an output (manager+encoder), this is relatively > infrequent, so it can afford to block to avoid races. (Like userspace > enabling and then rapidly disabling an output part way through the > enable.) But enabling/disabling an overlay, or adjusting position or > scanout address must not block. And ideally, if possible, switching > an overlay between two managers should not block. Adjusting the position or address of the buffer are simple, they can be easily done without any blocking. But ovl enable/disable and switching an ovl to another mgr do (possibly) take multiple vsyncs (and in the switch case, vsyncs of two separate outputs). So if those do not block, we'll need to handle them as a state machine and try to avoid races etc. It'll be "interesting". However, we can sometimes do those operations immediately. So I think we should have these conditional fast-paths in the code, and do them in non-blocking manner when possible. > I'll look again, but as far as I remember that at least wasn't > addressing the performance issues from making overlay enable/disable Right, it wasn't addressing those issues. But your branch doesn't really address those issues either, as it doesn't handle the problems related to ovl enable/disable. > synchronous. And fixing that would, I expect, trigger the same > problems that I already spent a few days debugging before switching > over to handle apply in omapdrm. I was under the impression that the apply mechanism, the caching and setting of the configs, was the major issue you had. But you're hinting that the actual problem is related to ovl enable/disable? I haven't tried fixing the ovl enable/disable, as I didn't know it's an issue for omapdrm. Or are they both as big issues? Tomi signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Thu August 2 2012 08:56:43 Rémi Denis-Courmont wrote: > Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit : > > On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote: > > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at > > > > runtime ? > > > > > > Does CREATE_BUFS always work while already streaming has already started? > > > If it depends on the driver, it's kinda helpless. > > > > Yes, it does. It's one of the reasons it exists in the first place. But > > there are currently only a handful of drivers that implement it. I hope > > that as more and more drivers are converted to vb2 that the availability > > of create_bufs will increase. > > That's contradictory. If most drivers do not support it, then it won't work > during streaming. IF create_bufs is implemented in the driver, THEN you can use it during streaming. I.e., it will never return EBUSY as an error due to the fact that streaming is in progress. Obviously it won't work if the driver didn't implement it in the first place. > > > > What's the guaranteed minimum buffer count? It seems in any case, MMAP > > > has a hard limit of 32 buffers (at least videobuf2 has), though one > > > might argue this should be more than enough. > > > > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. > > Although drivers may force a lower maximum if they want. I have no idea > > whether there are drivers that do that. There probably are. > > The smallest of the maxima of all drivers. I've no idea. Most will probably abide by the 32 maximum, but without analyzing all drivers I can't guarantee it. > > The minimum is usually between 1 and 3, depending on hardware limitations. > > And that's clearly insufficient without memory copy to userspace buffers. > > It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for > REQBUFS+USERBUF then. Just to put your mind at rest: USERPTR mode will *not* disappear or be deprecated in any way. It's been there for a long time, it's in heavy use, it's easy to use and it will not be turned into a second class citizen, because it isn't. Just because there is a new dmabuf mode available doesn't mean that everything should be done as a mmap+dmabuf thing. Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11
https://bugs.freedesktop.org/show_bug.cgi?id=45018 --- Comment #81 from Anthony Waters 2012-08-02 00:47:06 UTC --- Created attachment 65051 --> https://bugs.freedesktop.org/attachment.cgi?id=65051 fixes to wait on the bo and to free the va after the kernel These are the changes I made to make it work in mesa, the first change, inserting radeon_bo_wait was so that the va wouldn't be immediately reallocated for a different va while the GPU was still using it causing the rendering regression. The second change was to move the freeing of the va in mesa after the kernel was freed so that the kernel's list would be updated before mesa's list. Hopefully this provides more insight to the issue/cause -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote: > Le mercredi 1 août 2012 14:35:03 Laurent Pinchart, vous avez écrit : > > > But in general, the V4L element in the pipeline does not know how fast > > > the downstream element(s) will consume the buffers. Thus it has to copy > > > from the MMAP buffers into anonymous user memory pending processing. > > > Then any dequeued buffer can be requeued as soon as possible. In theory, > > > it might also be that, even though the latency is known, the number of > > > required buffers exceeds the maximum MMAP buffers count of the V4L > > > device. Either way, user space ends up doing memory copy from MMAP to > > > custom buffers. > > > > > > This problem does not exist with USERBUF - the V4L2 element can simply > > > allocate a new buffer for each dequeued buffer. > > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ? > > Does CREATE_BUFS always work while already streaming has already started? If > it depends on the driver, it's kinda helpless. Yes, it does. It's one of the reasons it exists in the first place. But there are currently only a handful of drivers that implement it. I hope that as more and more drivers are converted to vb2 that the availability of create_bufs will increase. > What's the guaranteed minimum buffer count? It seems in any case, MMAP has a > hard limit of 32 buffers (at least videobuf2 has), though one might argue > this > should be more than enough. Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. Although drivers may force a lower maximum if they want. I have no idea whether there are drivers that do that. There probably are. The minimum is usually between 1 and 3, depending on hardware limitations. Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V3 11/16] drm/radeon: Make radeon card usable for Loongson.
Hi, Lucas, sorry for so long a delay because I have a holiday for one month. I found Loongson-3 must turn on SWIOTLB if the system memory has addresses above 4G. As I know, there are two ways to get a DMA addr, the first way is use dma_alloc_coherent(), and the other one is use map_page()/map_sg() on an exsisting address. The first way can make sure DMA addr below 4G, but the second way cannot (unless the memory is so little that all the address space is below 4G). Take SATA driver as an example, during initialization an 'ata_device' struct is allocated and its address is probably above 4G (because this is first used by CPU, not for DMA). 'ata_device' has a member 'id' and 'id' will be use for DMA in such a call path: ata_host_register() --> ata_scsi_add_hosts() --> async_port_probe() --> ata_port_probe() --> ata_bus_probe() --> ata_dev_read_id() --> ata_exec_internal() --> ata_exec_internal_sg() --> ata_qc_issue() --> ata_sg_setup() --> dma_map_sg() Here, dma_map_sg() will get a DMA addr above 4G, then SATA init failed. In OHCI driver, there are similar situations. P.S.: With recently drm changes, I found radeon with SWIOTLB can already work after suspend/resume, so my next version of Loongson patches will not modify radeon_ttm.c. On Fri, Jun 22, 2012 at 1:59 PM, Huacai Chen wrote: > On Fri, Jun 22, 2012 at 1:25 PM, Lucas Stach wrote: >> Hello Huacai, >> >> Am Freitag, den 22.06.2012, 11:01 +0800 schrieb Huacai Chen: >>> 1, Handle io prot correctly for MIPS. >>> 2, Define SAREA_MAX as the size of one page. >>> 3, Don't use swiotlb on Loongson machines (Loonson need swioitlb, but >>>when use swiotlb, GPU reset occurs at resume from suspend). >>> >> I still think this is wrong. You say Loongson needs SWIOTLB, but when >> it's actually used you ignore it in the radeon driver code. >> >> I looked up why you are using SWIOTLB and I don't agree with you that it >> is needed. SWIOTLB just gives you bounce pages for DMA memory above >> DMA32 and therefore papers over your >4GB DMA platform bug in some >> cases, while hurting performance. >> >> Please fix your DMA platform code so that region DMA is an alias for >> region DMA32. It should allow you to drop all those ugly workarounds. >> > Hmm, you are probably right, I think I should have a discuss with the > original author of this part of code. > >>> Signed-off-by: Huacai Chen >>> Signed-off-by: Hongliang Tao >>> Signed-off-by: Hua Yan >>> Reviewed-by: Michel Dänzer >>> Reviewed-by: Alex Deucher >>> Reviewed-by: Lucas Stach >>> Reviewed-by: j.glisse >> >> You should probably only stick this tag on your patches after the people >> you are naming explicitly gave their r-b for a specific version of a >> patch. >> >> Thanks, >> Lucas >>> Cc: dri-devel@lists.freedesktop.org >>> --- >>> drivers/gpu/drm/drm_vm.c|2 +- >>> drivers/gpu/drm/radeon/radeon_ttm.c |6 +++--- >>> drivers/gpu/drm/ttm/ttm_bo_util.c |2 +- >>> include/drm/drm_sarea.h |2 ++ >>> 4 files changed, 7 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c >>> index 961ee08..3f06166 100644 >>> --- a/drivers/gpu/drm/drm_vm.c >>> +++ b/drivers/gpu/drm/drm_vm.c >>> @@ -62,7 +62,7 @@ static pgprot_t drm_io_prot(uint32_t map_type, struct >>> vm_area_struct *vma) >>> tmp = pgprot_writecombine(tmp); >>> else >>> tmp = pgprot_noncached(tmp); >>> -#elif defined(__sparc__) || defined(__arm__) >>> +#elif defined(__sparc__) || defined(__arm__) || defined(__mips__) >>> tmp = pgprot_noncached(tmp); >>> #endif >>> return tmp; >>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c >>> b/drivers/gpu/drm/radeon/radeon_ttm.c >>> index c94a225..f49bdd1 100644 >>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c >>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c >>> @@ -630,7 +630,7 @@ static int radeon_ttm_tt_populate(struct ttm_tt *ttm) >>> } >>> #endif >>> >>> -#ifdef CONFIG_SWIOTLB >>> +#if defined(CONFIG_SWIOTLB) && !defined(CONFIG_CPU_LOONGSON3) >>> if (swiotlb_nr_tbl()) { >>> return ttm_dma_populate(>t->ttm, rdev->dev); >>> } >>> @@ -676,7 +676,7 @@ static void radeon_ttm_tt_unpopulate(struct ttm_tt *ttm) >>> } >>> #endif >>> >>> -#ifdef CONFIG_SWIOTLB >>> +#if defined(CONFIG_SWIOTLB) && !defined(CONFIG_CPU_LOONGSON3) >>> if (swiotlb_nr_tbl()) { >>> ttm_dma_unpopulate(>t->ttm, rdev->dev); >>> return; >>> @@ -906,7 +906,7 @@ static int radeon_ttm_debugfs_init(struct radeon_device >>> *rdev) >>> radeon_mem_types_list[i].show = &ttm_page_alloc_debugfs; >>> radeon_mem_types_list[i].driver_features = 0; >>> radeon_mem_types_list[i++].data = NULL; >>> -#ifdef CONFIG_SWIOTLB >>> +#if defined(CONFIG_SWIOTLB) && !defined(CONFIG_CPU_LOONGSON3) >>> if (swiotlb_nr_tbl()) { >>> sprintf(radeon_mem_types_names[i], "ttm_dma_page_pool"); >>> radeon_mem_types_list
[Bug 45018] [bisected] rendering regression since added support for virtual address space on cayman v11
https://bugs.freedesktop.org/show_bug.cgi?id=45018 --- Comment #80 from Anthony Waters 2012-08-02 00:41:23 UTC --- I tried both patches, the one from comment 76 and the one from comment 70, neither fixed the issue with the rendering regression or the va conflict. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.