[Bug 66963] Rv6xx dpm problems
https://bugs.freedesktop.org/show_bug.cgi?id=66963 --- Comment #208 from Kajzer --- (In reply to comment #207) > Well, for me nothing changed on RV635. Computer boots perfectly fine, but > video still crashes on random state dpm changes, sometimes in 10 minutes, > sometimes in 10 hours after boot. But always when there is unsaved work on > the screen. Hm, that's interesting, I'm using the same driver RV635, card is HD 3650 Had the system up for 3 days and had no crash, had to reboot for other reasons. Watched many videos (XBMC, mostly HD movies) , played few games. Working fine really, except like I said, I had black screens on boot few times, when that happens the very next reboot is success. Maybe there are other things in play here, really can't tell. btw I'm running OpenSuse 13.1 x64 , stock mesa 9.2 , kernel 3.14.0-5 Can you reproduce it every time ? If yes what do you do exactly ? I would like to try that. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/68ef3149/attachment.html>
[PATCH] drm/mm: Don't WARN if drm_mm_reserve_node
Jesse's BIOS fb reconstruction code actually relies on the -ENOSPC return value to detect overlapping framebuffers (which the bios uses always when lighting up more than one screen). All this fanciness happens in intel_alloc_plane_obj in intel_display.c. Since no one else uses this we can savely remove the WARN without repercursions. Reported-by: Ben Widawsky Cc: Ben Widawsky Cc: Jesse Barnes Cc: Dave Airlie Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_mm.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index a2d45b748f86..e4dfd5c3b15e 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -192,8 +192,6 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node) return 0; } - WARN(1, "no hole found for node 0x%lx + 0x%lx\n", -node->start, node->size); return -ENOSPC; } EXPORT_SYMBOL(drm_mm_reserve_node); -- 1.8.5.2
[PATCH] drm/mm: Don't WARN if drm_mm_reserve_node
On Mon, 7 Apr 2014 23:25:20 +0200 Daniel Vetter wrote: > Jesse's BIOS fb reconstruction code actually relies on the -ENOSPC > return value to detect overlapping framebuffers (which the bios uses > always when lighting up more than one screen). All this fanciness > happens in intel_alloc_plane_obj in intel_display.c. > > Since no one else uses this we can savely remove the WARN without > repercursions. > > Reported-by: Ben Widawsky > Cc: Ben Widawsky > Cc: Jesse Barnes > Cc: Dave Airlie > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_mm.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c > index a2d45b748f86..e4dfd5c3b15e 100644 > --- a/drivers/gpu/drm/drm_mm.c > +++ b/drivers/gpu/drm/drm_mm.c > @@ -192,8 +192,6 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct > drm_mm_node *node) > return 0; > } > > - WARN(1, "no hole found for node 0x%lx + 0x%lx\n", > - node->start, node->size); > return -ENOSPC; > } > EXPORT_SYMBOL(drm_mm_reserve_node); Yeah thanks, pushing this has been on my list for weeks now...
[RFCv3 12/14] drm: Specify cursor plane at CRTC initialization
On Mon, Apr 7, 2014 at 7:23 PM, Rob Clark wrote: > On Mon, Apr 7, 2014 at 6:05 AM, Thierry Reding > wrote: >> On Fri, Mar 28, 2014 at 10:04:09PM +0100, Daniel Vetter wrote: >>> On Tue, Mar 18, 2014 at 05:22:57PM -0700, Matt Roper wrote: >>> > Add cursor plane as a parameter to drm_crtc_init() and update all >>> > existing DRM drivers to use a helper-provided primary plane. Passing >>> > NULL for this parameter indicates that there is no hardware cursor >>> > supported by the driver and no cursor plane should be provided to >>> > userspace. >>> > >>> > Signed-off-by: Matt Roper >>> >>> Ok, cursor planes. I've poked around in this a lot and unfortunately I >>> don't think we can achieve nirvana :( >> [...] >>> - For the pixel format I think it's ok to always assume RGBA. >> >> How so? We do have hardware on Tegra that doesn't support RGBA cursors >> and if at all possible I'd very much like to support that as well. New >> generations can do RGBA but still support the old pixel format. Having >> the option of supporting all formats would be nice. >> >> There was some discussion about implementing a bunch of plane properties >> so I think having one to enumerate a set of pixel formats wouldn't be >> such a big deal. > > No need for properties to expose formats, we already have that in > getplane ioctl. But would be nice to support more than just RGBA. This is _just_ for the compat cursor plane setup helper. Of course we can add whatever insane cursor formats exist out there as fourcc codes and then drivers can add them for their custom-created cursor planes. This is similar to the current primary plane helper code which only exposes 32bit XRGB atm. If you want to take full advantage of this you need to do a bit of work in your driver ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Intel-gfx] [PATCH] drm/i915: support address only i2c-over-aux transactions
On Mon, Apr 7, 2014 at 4:35 PM, Alex Deucher wrote: > On Mon, Apr 7, 2014 at 5:37 AM, Jani Nikula wrote: >> To support bare address requests used by the drm dp helpers. >> >> Signed-off-by: Jani Nikula >> >> --- >> >> Hi Alex, similar to Thierry's patch for i915. >> > > Looks good to me. > > Reviewed-by: Alex Deucher > > Do you want me to add this to the patch set? Afaict we've done an unconditional msg_bytes = send_bytes + 4; in the i915 driver before the conversion to the dp aux helper, which is why I've slapped an r-b onto your patch without asking for a i915 adjustement. Otoh we have a pile of bugs for dp dongles still. Imo my preferred approach would be to get the helper + radeon stuff in and then hawk the i915 patch to a bunch of bug reporter and see whether it sticks. Ofc if we already know that we need it it would be best to merge it in one pull together with all your other patches. Jani? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 01/13] drm/mgag200: Remove unnecessary NULL check in bo_unref
On Mon, Apr 7, 2014 at 5:19 PM, Ian Romanick wrote: > On 04/05/2014 02:44 AM, Daniel Vetter wrote: >> ttm_bo_unref unconditionally calls kref_put on it's argument, so the >> thing can't be NULL without already causing Oopses. > > Doesn't this mean the NULL check is in the wrong place (rather than the > NULL check should be removed)? Afaics chasing callchains it's a bug to call this with NULL pointer and no one really should be doing it. Like David Herrmann said it's sometimes useful if unref/free functions automatically cope with NULL, but ttm buffers don't seem to be of this kind. So consistency with other ttm drivers seems better, same with all the gem_free_object callbacks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 08/13] drm/ast: Remove dead code from cbr_scan2
On Mon, Apr 7, 2014 at 5:28 PM, Ian Romanick wrote: > On 04/05/2014 02:44 AM, Daniel Vetter wrote: >> The outer if already checks for data != 0, so it can't really be >> 0. Hence remove it. >> >> Now I don't have specs or anything for this beast, so I have no >> idea whether this was actually intended or whether the logic >> should be different. At least the code still seems to be doing >> something useful. >> >> Spotted by coverity. >> >> Cc: Dave Airlie >> Signed-off-by: Daniel Vetter >> --- >> drivers/gpu/drm/ast/ast_post.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c >> index 977cfb35837a..6263116054b6 100644 >> --- a/drivers/gpu/drm/ast/ast_post.c >> +++ b/drivers/gpu/drm/ast/ast_post.c >> @@ -572,8 +572,6 @@ static u32 cbr_scan2(struct ast_private *ast) >> for (loop = 0; loop < CBR_PASSNUM2; loop++) { >> if ((data = cbr_test2(ast)) != 0) { >> data2 &= data; >> - if (!data) >> - return 0; > > That feels like a typo... was that supposed to be 'if (!data2)'? Yeah this one really needs a close look, since I have no idea what's actually intended behaviour. The patch just removes the dead code as it is now, and the double-loop still makes some sense imo after this change. But I really don't know the spec for this hw. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 74539] [r600g] Memory leak when playing WoW with RV790
https://bugs.freedesktop.org/show_bug.cgi?id=74539 --- Comment #25 from Chris Rankin --- Created attachment 97051 --> https://bugs.freedesktop.org/attachment.cgi?id=97051&action=edit dmesg output from 3.13.9 This memory leak is still present with 3.13.9 and HEAD 4ccff1499c956b51f18710c7308cbce883f64cd9. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/d5d56cf5/attachment.html>
[PATCH] imx-drm: imx-drm-core: Fix imx_drm_encoder_get_mux_id
On Mon, Apr 07, 2014 at 10:22:36AM +0200, Philipp Zabel wrote: > The decoder mux id is equal to the port id of the encoder's input port > that is connected to the given crtc, not to the endpoint id (which is > arbitrary and usually zero). > > Signed-off-by: Philipp Zabel It fixes a color corruption issue on my dual display case (LVDS + HDMI). Tested-by: Shawn Guo Shawn > --- > drivers/staging/imx-drm/imx-drm-core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/imx-drm/imx-drm-core.c > b/drivers/staging/imx-drm/imx-drm-core.c > index 4144a75..bc7f8bd 100644 > --- a/drivers/staging/imx-drm/imx-drm-core.c > +++ b/drivers/staging/imx-drm/imx-drm-core.c > @@ -517,7 +517,7 @@ int imx_drm_encoder_get_mux_id(struct device_node *node, > of_node_put(port); > if (port == imx_crtc->port) { > ret = of_graph_parse_endpoint(ep, &endpoint); > - return ret ? ret : endpoint.id; > + return ret ? ret : endpoint.port; > } > } while (ep); > > -- > 1.9.1 >
[PATCH v5 00/11] imx-drm dt bindings
On Mon, Apr 07, 2014 at 12:05:35PM +0200, Philipp Zabel wrote: > > Did you get any chance to reproduce this dual display issue? Now it > > shows on mainline kernel. > > I have not yet reproduced, but I've sent a patch today ("imx-drm: > imx-drm-core: Fix imx_drm_encoder_get_mux_id") that might fix this. Yes, it fixes my issue. > > > And I see another HDMI regression with my testing on mainline kernel. I > > can have my HDMI work at 1920x1080 with v3.14 kernel, but it can only > > probes 1024x768 with the mainline today. The Xorg.0.log are attached > > below. The hardware and user space are same, so I guess this is another > > issue introduced by the recently kernel driver changes? > > Hmm, not sure. Have you updated to the i2c-ddc-bus phandle property as > described in Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt? That's indeed the cause of my problem. Thanks, Philipp. Shawn
[PATCH v5 00/11] imx-drm dt bindings
On Mon, Apr 07, 2014 at 10:09:27AM +0100, Russell King - ARM Linux wrote: > So no EDID. Did you update the "ddc" property to "ddc-i2c-bus" ? Ah, right. This is exactly the cause. Thanks, Russell. Shawn
[Bug 66963] Rv6xx dpm problems
https://bugs.freedesktop.org/show_bug.cgi?id=66963 --- Comment #207 from lockheed --- Well, for me nothing changed on RV635. Computer boots perfectly fine, but video still crashes on random state dpm changes, sometimes in 10 minutes, sometimes in 10 hours after boot. But always when there is unsaved work on the screen. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/68ec22d5/attachment.html>
[Bug 66963] Rv6xx dpm problems
https://bugs.freedesktop.org/show_bug.cgi?id=66963 --- Comment #206 from Kajzer --- With kernel 3.14 things are much better, sometimes it doesn't boot but that's very rarely, once booted everything works fine, had no crashes at all. The only problem for me is that I cannot run any game from steam except two (Penumbra Overture and Half Life) , those games that wouldn't start are working with catalyst. I tried latest mesa and x11, makes no difference. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/2a430f4b/attachment.html>
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 --- Comment #22 from Alex Deucher --- (In reply to comment #19) > (In reply to comment #17) > > > > What do you mean by 4 streams? Is that the same as audio pins? > > AFAIK this is the maximum amount of different streams played back at once. > Before the patch I just attached, ALSA always used the "stream numbers" > starting from the last one, which caused problems with assumed 8 streams > because Christian's card only had 4 streams (his card had a single output > only, though; alsa-info: http://sprunge.us/dbiB ). > > So in addition to the patch I just attached I think we need to correct the > stream count, _IF_ the 6/7-output cards only have 4 streams as well. But if > those cards support 6/7 streams as well, all is OK. Kaveri:4 streams, 7 endpoints Kabini:2 streams, 3 endpoints Bonaire/Hawaii:6 streams, 7 endpoints Trinity/Richland: 4 streams, 6 endpoints Oland: 2 streams, 2 endpoints Tahiti/Pitcairn/Verde: 6 streams, 6 endpoints Endpoints would be equivalent to pins as I understand it. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/e996c122/attachment.html>
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 --- Comment #21 from Anssi Hannula --- (In reply to comment #19) > maximum amount of different streams played back at once. Well streams can be inactive as well, so this was not entirely accurate... See Section 2.2 of HDA spec for what a stream is in this context. What I'm after is the correct value for HDA spec 3.3.2, register offset 00h, bits 12-15 (Number of Output Streams Supported (OSS)), which is 0x00 (no audio output streams) on the AMD cards for some reason. For Christian's card the correct value seems to be 4, but I wonder if the same is true for 6/7-pin cards as well or if they have it at least 6/7 so there are streams for every pin available. If (streams >= pins) is true always, I think there is no need to add any other fixes. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/8bd2a89f/attachment-0001.html>
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 --- Comment #20 from Anssi Hannula --- Sorry, and by "outputs" I mean "pins". -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/b912535e/attachment-0001.html>
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 Anssi Hannula changed: What|Removed |Added CC||anssi at mageia.org --- Comment #19 from Anssi Hannula --- (In reply to comment #17) > (In reply to comment #16) > > It seems to be > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/ > > ?id=7546abfb8e1f9933b549f05898377e9444ee4cb2 > > + the fact streams are assigned in reverse order. > > > > I'll have a patch for reversing the assignment shortly. > > > > Alex, is the limit 4 streams on 7-output cards as well, or is it just these > > 1-output cards only? > > What do you mean by 4 streams? Is that the same as audio pins? AFAIK this is the maximum amount of different streams played back at once. Before the patch I just attached, ALSA always used the "stream numbers" starting from the last one, which caused problems with assumed 8 streams because Christian's card only had 4 streams (his card had a single output only, though; alsa-info: http://sprunge.us/dbiB ). So in addition to the patch I just attached I think we need to correct the stream count, _IF_ the 6/7-output cards only have 4 streams as well. But if those cards support 6/7 streams as well, all is OK. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/459f1d1b/attachment.html>
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 --- Comment #18 from Anssi Hannula --- Created attachment 97049 --> https://bugs.freedesktop.org/attachment.cgi?id=97049&action=edit ALSA: hda - Do not assign streams in reverse order Please try this one. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/3c030c5a/attachment.html>
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 --- Comment #17 from Alex Deucher --- (In reply to comment #16) > It seems to be > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/ > ?id=7546abfb8e1f9933b549f05898377e9444ee4cb2 > + the fact streams are assigned in reverse order. > > I'll have a patch for reversing the assignment shortly. > > Alex, is the limit 4 streams on 7-output cards as well, or is it just these > 1-output cards only? What do you mean by 4 streams? Is that the same as audio pins? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/9d1b7980/attachment.html>
[Bug 71461] monitor doesn't get detected after boot or disconnection
https://bugzilla.kernel.org/show_bug.cgi?id=71461 --- Comment #18 from Alex Deucher --- (In reply to Tom Yan from comment #16) > I tried to remove the DisplayPort-specific code in > radeon_connector_hotplug() of radeon_connector.c and everything seems to > work like a charm. What is the code supposed to do actually? > It's required to re-establish the DP link if you physically disconnect and reconnect an active display since DP requires link training while other digital links do not, otherwise you'd end up with a blank display after disconnecting and reconnecting a DP display. It sounds like may have a problem with the hpd interrupts on your board. > Though, btw, it seems that only turning off my DP monitor in X would make it > "dead". Doing that in console (or switch to console before turning it on) > does not. > How are you turning it off in X vs console? Are you physically powering the monitor off or triggering dpms (e.g., xset dpms force off)? (In reply to Tom Yan from comment #17) > One thing to add, unplugging it physically in X still makes it blank. It > happens no matter the code is removed or not. That code is required to re-establish the link. > > Also, HDMI doesn't work all time yet, seems that if the TV is powered off > and it goes into DPMS after sometime (in console), the display still gone > completely. Are you physically powering off the TV or do you mean dpms? -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 --- Comment #16 from Anssi Hannula --- It seems to be https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=7546abfb8e1f9933b549f05898377e9444ee4cb2 + the fact streams are assigned in reverse order. I'll have a patch for reversing the assignment shortly. Alex, is the limit 4 streams on 7-output cards as well, or is it just these 1-output cards only? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/6377f175/attachment.html>
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 --- Comment #15 from bgunteriv at gmail.com --- (In reply to comment #14) > Does reverting 832eafaf34ff7d0348fe701e417900c6cf1f5656 help? no, this did not help either. just to make sure i did this correctly. i got the git from here --> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git and then i ran: git revert 832eafaf34ff7d0348fe701e417900c6cf1f5656 it said that it did it... after updating the kernel, i still saw that message about: drm:module has bad taint, not creating trace events -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/2bc40304/attachment.html>
[PATCH 07/13] drm/via: Remove unecessary NULL check
On Mon, Apr 7, 2014 at 5:51 PM, David Herrmann wrote: > On Sat, Apr 5, 2014 at 11:44 AM, Daniel Vetter > wrote: >> The context_dtor callback is only called once we've successfully loaded >> the driver, which means dev->dev_private is set up. The check is hence >> pointless. >> >> Also dev->dev_private is deref already above, so compilers are free >> to elide it anyway. > > Are you sure compilers can assume "*ptr" implies "ptr != NULL"? I > doubt that and depending on CONFIG_DEFAULT_MMAP_MIN_ADDR I think you > can even build user-space that can successfully mmap(MAP_FIXED) at > address 0. Anyhow, I guess no-one cares besides me, so patch looks > good :) Yeah, my understand has been that every time you deref a pointer somewhere the compiler is allowed to presume that the pointer isn't NULL. Which makes mmap(MAP_FIXED) at address NULL such a dangerous thing and iirc there's been patches floating around to severely restrict that to make exploiting such bugs much harder. Iirc it's only emulators like dosemu who really need to be able to map something at NULL. Since if gcc drops the NULL check the last line of defense (namely Oopsing on the NULL deref) can be disabled by userspace. The usual exploit is to put a real data structure at NULL and use that (thorugh vtables if possible) to take over the kernel. I'm not always entirely sure on what the precise rules are really in detail, but since coverity screamed at me about this here I've figured coverity is probably right ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 71461] monitor doesn't get detected after boot or disconnection
https://bugzilla.kernel.org/show_bug.cgi?id=71461 --- Comment #17 from Tom Yan --- One thing to add, unplugging it physically in X still makes it blank. It happens no matter the code is removed or not. Also, HDMI doesn't work all time yet, seems that if the TV is powered off and it goes into DPMS after sometime (in console), the display still gone completely. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 42960] Display does not work when resuming from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=42960 --- Comment #30 from D --- (In reply to comment #26) > *** Bug 73882 has been marked as a duplicate of this bug. *** For some reason, if I use it both (s3_bios,s3_mode) as kernel parameter *and* as parameter for pm-resume, it works. Otherwise it doesn't. I had already tested the parameters only for pm-suspend without as much success before. May also be that 3.14 has improved things, though. Another system fixed. Anyway,... thanks a lot! :) -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/d684ac95/attachment.html>
[Bug 71461] monitor doesn't get detected after boot or disconnection
https://bugzilla.kernel.org/show_bug.cgi?id=71461 --- Comment #16 from Tom Yan --- I tried to remove the DisplayPort-specific code in radeon_connector_hotplug() of radeon_connector.c and everything seems to work like a charm. What is the code supposed to do actually? Though, btw, it seems that only turning off my DP monitor in X would make it "dead". Doing that in console (or switch to console before turning it on) does not. I am with kernel 3.13.8 now. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH] drm/omap: fix plane rotation
On Mon, Apr 7, 2014 at 5:41 PM, Grazvydas Ignotas wrote: > Gra?vydas > > > On Mon, Apr 7, 2014 at 8:17 PM, Rob Clark wrote: >> On Mon, Apr 7, 2014 at 6:32 AM, Grazvydas Ignotas >> wrote: >>> On Sun, Apr 6, 2014 at 12:45 AM, Rob Clark wrote: On Sat, Apr 5, 2014 at 2:33 PM, Grazvydas Ignotas wrote: > Plane rotation with omapdrm is currently broken. > It seems omap_plane_mode_set() expects width and height in screen > coordinates, so pass it like that. > > Cc: Rob Clark > Signed-off-by: Grazvydas Ignotas > --- > drivers/gpu/drm/omapdrm/omap_plane.c |8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c > b/drivers/gpu/drm/omapdrm/omap_plane.c > index 370580c..5611f15 100644 > --- a/drivers/gpu/drm/omapdrm/omap_plane.c > +++ b/drivers/gpu/drm/omapdrm/omap_plane.c > @@ -253,6 +253,14 @@ static int omap_plane_update(struct drm_plane *plane, > > drm_framebuffer_reference(fb); > > + /* omap_plane_mode_set() takes adjusted src */ > + switch (omap_plane->win.rotation & 0xf) { > + case BIT(DRM_ROTATE_90): > + case BIT(DRM_ROTATE_270): > + swap(src_w, src_h); > + break; > + } > + hmm, I think that the better thing would be to do this in omap_framebuffer_update_scanout(). In fact we do already swap src_w/src_h there. Only we don't swap the width/height parameters we pass down to omapdss. Not quite sure if something changed in omapdss with regards to rotation_type, but keeping it with the rest of the rotation related maths in _update_scanout() seems cleaner. >>> >>> But then it has to know somehow if apply was triggered from >>> omap_crtc_mode_set() or omap_plane_update(). The problem is >>> omap_crtc_mode_set() passes rotated width/height (and crtc rotation >>> works correctly), but omap_plane_update() uses unrotated width/height >>> (so plane rotation is currently broken). >> >> >> Something still seems a bit suspicious.. drm core is not swapping >> width/height for crtc (other than for validating the configuration... >> which might also be needed for planes). I guess it is getting >> already-rotated width/height from userspace. So probably we want to >> do the same for planes, so it might be a good idea to move >> crtc->invert_dimensions into plane. > > OK I guess I said it wrong.. > The display I have is 1080x1920 portrait panel. > For omap_crtc_mode_set(), hdisplay is always 1080 and vdisplay, is > 1920, regardless of rotation. That is passed over to > omap_plane_mode_set() and everything works. ahh, ok, that makes much more sense.. (sorry, it's been a long time since I looked at rotation) Reviewed-by: Rob Clark > But in omap_plane_update(), src_w and src_h are passed instead, which > correspond to framebuffer size and not display? At least > drmModeSetPlane(), where those values originate, seems to expect > framebuffer dimensions, but passing 1920, 1080 there (with 90 deg. > rotation set) results in corruption on screen caused by bad base > address for hardware overlay without my patch. > > > Gra?vydas
[Bug 70706] Regression in fbconfig
https://bugs.freedesktop.org/show_bug.cgi?id=70706 ajax at nwnk dot net changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #22 from ajax at nwnk dot net --- (In reply to comment #20) > (In reply to comment #19) > > Might be worth testing these two patches [1] [2] > > > > [1] http://patchwork.freedesktop.org/patch/20458/ > > [2] http://patchwork.freedesktop.org/patch/20464/ > > Those patches fixed it for me. Both of these patches are now merged to Mesa, closing. -- You are receiving this mail because: You are on the CC list for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/ccb8f37f/attachment.html>
[PATCH 00/13] coverity
Hi On Sat, Apr 5, 2014 at 11:44 AM, Daniel Vetter wrote: > Hi all, > > Rainy w/e here and I got a bit bored, so looked at coverity again. I've closed > all outstanding issues in drivers/gpu now either as false positives or fixed > in > this series expect for vmwgfx/ttm stuff (not enough clue) and one insane > savage > init issue (no desire to wake dragons today). All patches besides "drm/ast: Remove dead code from cbr_scan2" are: Reviewed-by: David Herrmann However, a few notes: - ttm_bo_unref() sets "bo = NULL;", so the following condition is always true. Your commit-message argues with an unconditional kref_unref(). I think that's misleading (but still true) as that does not change the fact that bo is always NULL afterwards. - I am a big fan of "if (!obj) return;" in destructors. It simplifies error-paths considerably and we can call them despite objects being already cleared. But that's just personal style, and I don't deal much with TTM anyway. But generally, I think we shouldn't remove these checks blindly. - The AST patch should be reviewed by someone who knows that hw. The code is obviously wrong, but that doesn't mean we cannot fix it properly. Thanks for the cleanups, the coverity reports have been pending here for months.. David
[PATCH 07/13] drm/via: Remove unecessary NULL check
Hi On Sat, Apr 5, 2014 at 11:44 AM, Daniel Vetter wrote: > The context_dtor callback is only called once we've successfully loaded > the driver, which means dev->dev_private is set up. The check is hence > pointless. > > Also dev->dev_private is deref already above, so compilers are free > to elide it anyway. Are you sure compilers can assume "*ptr" implies "ptr != NULL"? I doubt that and depending on CONFIG_DEFAULT_MMAP_MIN_ADDR I think you can even build user-space that can successfully mmap(MAP_FIXED) at address 0. Anyhow, I guess no-one cares besides me, so patch looks good :) Thanks David
[PATCH v2 1/7] drm/exynos: add super device support
On 07.04.2014 17:16, Inki Dae wrote: > Hi Andrzej, > > 2014-04-07 23:18 GMT+09:00 Andrzej Hajda : >> Hi Inki and Tomasz, >> >> On 04/06/2014 05:15 AM, Inki Dae wrote: >> >> (...) >>> The code creating the list of components to wait for >>> (exynos_drm_add_components()) doesn't seem to consider which sub-drivers are >>> actually enabled in kernel config. >>> >>> Are you sure? >>> >>> exynos_drm_add_components() will try to attach components *added to >>> component_lists. And these components will be added by only >>> corresponding sub drivers to the component_lists and >>> master->components. >>> >>> So in this case, if users disabled HDMI support then a commponent >>> object for HDMI sub driver doesn't exists in the component_lists and >>> master->components. This means that a component object for HDMI sub >>> driver *cannot be attached to master object*. >>> >>> As a result, component_bind_add() will ignor component_bind call for >>> HDMI sub driver so HDMI driver will not be bounded. The only >>> components added by sub drivers will be bound, component->ops->bind(). >>> >>> For more understanding, it seems like you need to look into below codes, >>> >>> static int exynos_drm_add_components(...) >>> { >>> ... >>> for (i == 0;; i++) { >>> ... >>> node = of_parse_phandle(np, "ports", i); >>> ... >>> ret = component_master_add_child(m, compare_of, node); >>> ... >>> } >>> } >> >> Hmm, In case HDMI driver is not present, HDMI device is not probed and >> HDMI component is not added, so component_master_add_child returns >> an error when it tries to add hdmi component and master is never brought >> up, am I correct? >> > > Ok, after that, > > ret = component_master_add(,..) > if (ret < 0) > DRM_DEBUG_KMS("re-tried by last sub driver probed later.\n"); > > As you can see above, if component_master_add() is failed, return 0, > *not error*. And then component_add() called by other kms drivers, > late probing of hdmi or fimd probing - if there is any kms driver > enabled - will try to bring up master again by calling > try_to_bring_up_master() from beginning. > > And if component_list is empty then it means that there is no any kms > driver enabled. > > Do you still think in that case, exynos drm initialization will be failed? There will be no HDMI driver to call component_add(), because HDMI sub driver will be disabled in kernel config and any previous component_master_add_child() for the phandle pointing to HDMI node will wail, because such component will never be registered. The complete failure path is as follows: exynos_drm_platform_probe() component_master_add() try_to_bring_up_master() exynos_drm_add_components() // phandle to HDMI node component_master_add_child() = -ENXIO = -ENXIO = 0 // but no call to master->ops->bind(master->dev); = 0 = 0 // but Exynos DRM platform is not registered yet Now if any other late-probed sub-driver comes, the sequence will be as follows (let's take FIMD as an example): fimd_probe() component_add() try_to_bring_up_masters() try_to_bring_up_master() exynos_drm_add_components() // phandle to HDMI node component_master_add_child() = -ENXIO = -ENXIO = 0 // but no call to // master->ops->bind(master->dev); = 0 = 0 = 0 // but Exynos DRM platform still not registered And the same for any late-probed driver, unless HDMI drivers loads, but there is no HDMI driver, because it is disabled in kernel config and not compiled in. Best regards, Tomasz
[RFC 0/3] TTM priority queue logic
On Mon, 07 Apr 2014 14:25:28 +0200 Thomas Hellstrom wrote: > Hi, Lauri. > > On 04/04/2014 03:52 PM, Lauri Kasanen wrote: > > Hi list, Thomas, > > > > I'd like to know if this is going in the right direction. > > This looks fine with me. > > However, if possible I'd like the drivers to enable both alloc_threshold > and priority queue on a per-memory-type basis. > > That would mean no new arguments (use_pqueue, alloc_threshold) in > ttm_bo_device_init(). Instead, set default values in > ttm_bo_init_mm(), and let the driver change them in the init_mem_type() > callback. > > Do you think that would work? Thanks for the review. Alloc_threshold was removed, and the current patch (in drm-next) replaced it with a placement flag. So now that logic is in the driver. Making the pqueue a per-type option would certainly work, I'll edit it to do so. I don't see why it would improve GTT or SYSTEM given their perf characteristics, but I suppose future devices can always do weird things. - Lauri
[Bug 75992] Display freezes & corruption with an r7 260x on 3.14-rc6
https://bugs.freedesktop.org/show_bug.cgi?id=75992 --- Comment #20 from nicolas.laplante at gmail.com --- Just to add my 2c to this issue: if I set radeon.dpm=0 on the command line, the card defaults to the "profile" power method. If I try to: echo "high" > /sys/class/drm/card0/device/power_profile the screen goes black and I have to shut down using the power button (Ctrl+Alt+Del doesn't even work). This happens both while on the desktop or in another vt. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/39973576/attachment-0001.html>
[PATCH 2/3] drm/edid: Check for user aspect ratio input
In case user has specified an input for aspect ratio through the property, then the user space value for PAR would take preference over the value from CEA mode list. Signed-off-by: Vandana Kannan Cc: dri-devel at lists.freedesktop.org --- drivers/gpu/drm/drm_edid.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b8d6c51..62680e7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3630,8 +3630,13 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; - /* Populate picture aspect ratio from CEA mode list */ - if (frame->video_code > 0) + /* Populate picture aspect ratio from either CEA mode list or +* user input + */ + if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 || + mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) + frame->picture_aspect = mode->picture_aspect_ratio; + else if (frame->video_code > 0) frame->picture_aspect = drm_get_cea_aspect_ratio( frame->video_code); -- 1.9.1
[PATCH 1/3] drm/crtc: Add property for aspect ratio
Added a property to enable user space to set aspect ratio. This patch contains declaration of the property and code to create the property. Signed-off-by: Vandana Kannan Cc: dri-devel at lists.freedesktop.org --- drivers/gpu/drm/drm_crtc.c | 31 +++ include/drm/drm_crtc.h | 2 ++ 2 files changed, 33 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d8b7099..6cd34ad 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -139,6 +139,12 @@ static const struct drm_prop_enum_list drm_scaling_mode_enum_list[] = { DRM_MODE_SCALE_ASPECT, "Full aspect" }, }; +static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = { + { HDMI_PICTURE_ASPECT_NONE, "Automatic" }, + { HDMI_PICTURE_ASPECT_4_3, "4:3" }, + { HDMI_PICTURE_ASPECT_16_9, "16:9" }, +}; + /* * Non-global properties, but "required" for certain connectors. */ @@ -1334,6 +1340,31 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev) EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); /** + * drm_mode_create_aspect_ratio_property - create aspect ratio property + * @dev: DRM device + * + * Called by a driver the first time it's needed, must be attached to desired + * connectors. + */ +int drm_mode_create_aspect_ratio_property(struct drm_device *dev) +{ + struct drm_property *aspect_ratio; + + if (dev->mode_config.aspect_ratio_property) + return 0; + + aspect_ratio = + drm_property_create_enum(dev, 0, "aspect ratio", + drm_aspect_ratio_enum_list, + ARRAY_SIZE(drm_aspect_ratio_enum_list)); + + dev->mode_config.aspect_ratio_property = aspect_ratio; + + return 0; +} +EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); + +/** * drm_mode_create_dirty_property - create dirty property * @dev: DRM device * diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c061bb3..99bb6ed 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -797,6 +797,7 @@ struct drm_mode_config { /* Optional properties */ struct drm_property *scaling_mode_property; + struct drm_property *aspect_ratio_property; struct drm_property *dirty_info_property; /* dumb ioctl parameters */ @@ -966,6 +967,7 @@ extern int drm_mode_create_dvi_i_properties(struct drm_device *dev); extern int drm_mode_create_tv_properties(struct drm_device *dev, int num_formats, char *formats[]); extern int drm_mode_create_scaling_mode_property(struct drm_device *dev); +extern int drm_mode_create_aspect_ratio_property(struct drm_device *dev); extern int drm_mode_create_dirty_info_property(struct drm_device *dev); extern const char *drm_get_encoder_name(const struct drm_encoder *encoder); -- 1.9.1
[RFC 0/3] TTM priority queue logic
On 04/07/2014 04:39 PM, Lauri Kasanen wrote: > On Mon, 07 Apr 2014 14:25:28 +0200 > Thomas Hellstrom wrote: > >> Hi, Lauri. >> >> On 04/04/2014 03:52 PM, Lauri Kasanen wrote: >>> Hi list, Thomas, >>> >>> I'd like to know if this is going in the right direction. >> This looks fine with me. >> >> However, if possible I'd like the drivers to enable both alloc_threshold >> and priority queue on a per-memory-type basis. >> >> That would mean no new arguments (use_pqueue, alloc_threshold) in >> ttm_bo_device_init(). Instead, set default values in >> ttm_bo_init_mm(), and let the driver change them in the init_mem_type() >> callback. >> >> Do you think that would work? > Thanks for the review. > > Alloc_threshold was removed, and the current patch (in drm-next) > replaced it with a placement flag. So now that logic is in the driver. > > Making the pqueue a per-type option would certainly work, I'll edit it > to do so. I don't see why it would improve GTT or SYSTEM given their > perf characteristics, but I suppose future devices can always do weird > things. Yes. You don't have to explicitly check for VRAM, and drivers can also set up other fixed memory types, like a pre-allocated chunk of GTT. Thanks, Thomas > - Lauri
[PATCH v2 1/7] drm/exynos: add super device support
Hi Andrzej, On 07.04.2014 16:18, Andrzej Hajda wrote: > Hi Inki and Tomasz, > > On 04/06/2014 05:15 AM, Inki Dae wrote: > > (...) >> The code creating the list of components to wait for >> (exynos_drm_add_components()) doesn't seem to consider which sub-drivers are >> actually enabled in kernel config. >> >> Are you sure? >> >> exynos_drm_add_components() will try to attach components *added to >> component_lists. And these components will be added by only >> corresponding sub drivers to the component_lists and >> master->components. >> >> So in this case, if users disabled HDMI support then a commponent >> object for HDMI sub driver doesn't exists in the component_lists and >> master->components. This means that a component object for HDMI sub >> driver *cannot be attached to master object*. >> >> As a result, component_bind_add() will ignor component_bind call for >> HDMI sub driver so HDMI driver will not be bounded. The only >> components added by sub drivers will be bound, component->ops->bind(). >> >> For more understanding, it seems like you need to look into below codes, >> >> static int exynos_drm_add_components(...) >> { >> ... >> for (i == 0;; i++) { >> ... >> node = of_parse_phandle(np, "ports", i); >> ... >> ret = component_master_add_child(m, compare_of, node); >> ... >> } >> } > > Hmm, In case HDMI driver is not present, HDMI device is not probed and > HDMI component is not added, so component_master_add_child returns > an error when it tries to add hdmi component and master is never brought > up, am I correct? > This is my thought here as well. >> >> >> And below codes, >> >> int component_master_add_child(...) >> { >> list_for_each_entry(c, &component_list, node) { >> if (c->master) >> continue; >> >> if (compare(...)) { >> component_attach_master(master, c); >> ... >> } >> } >> } >> >> And below codes, >> >> static void component_attach_master(master, c) >> { >> c->master = master; >> list_add_tail(&c->master_node, &master->comonents); >> } >> >> >> As you can see above, the only components added to component_list can >> be attached to master. And the important thing is that components can >> be added by sub drivers to the component_list. >> >> And below codes that actually tries to bind each sub drivers, >> >> int component_bind_add(...) >> { >> >> list_for_each_entry(c, &master->components, master_node) { >> ret = component_bind(c, master, data); >> ... >> } >> ... >> } >> >> The hdmi driver users disabled doesn't exist to master->components list. >> How Exynos DRM cannot be initialized? >> >> Of course, there may be my missing point, but I think I see them >> correctly. Anyway I will test them tomorrow. >> >> Thanks, >> Inki Dae >> > register, which is completely wrong. Users should be able to select which > drivers should be compiled into their kernels. So users are be able to select drivers they want to use, and will be compiled correctly. So no, the only thing users can disable is each sub driver, not core module. > 3) Such approach leads to complete integration of all Exynos DRM drivers, > without possibility of loading some sub-drivers as modules. I know that > current driver design doesn't support it either, but if this series is No, current drm driver *must also be built* as one integrated single drm driver without super device approach. >>> >>> As I wrote, I know that current design before this patch isn't modular >>> either, but this is not my concern here. See below. >>> >>> So the super device approach *has no any effect on existing design*, and what the super device approch tries to do is to resolve the probe order issue to sub drivers *without some codes specific to Exynos drm*. >>> >>> My concern is that the supernode design is actually carving such broken >>> non-modular design in stone. Remember that we are currently heading towards >>> a fully multi-platform kernel where it is critical for such subsystems to be >>> modular, because the same zImage is going to be running on completely >>> different machines. >>> >>> > claimed to improve things, it should really do so. > > 4) Exactly the same can be achieved without changing the DT bindings at > all. > In fact even without adding any new single property or node to DT. We > discussed this with Andrzej and Marek today and came to a solution in > which > just by adding a little bit of code to Exynos DRM subdrivers, you could > guarantee correct registration of Exynos DRM platform and also get rid of > #ifdeffery in exynos_drm_drv.c. Andrzej will send a
[Bug 77142] Garbled text in Gtk2 applications
https://bugs.freedesktop.org/show_bug.cgi?id=77142 eric changed: What|Removed |Added Attachment #97046|text/plain |image/png mime type|| -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/523bd4d7/attachment.html>
[PATCH v2 1/7] drm/exynos: add super device support
Hi Inki and Tomasz, On 04/06/2014 05:15 AM, Inki Dae wrote: (...) > The code creating the list of components to wait for > (exynos_drm_add_components()) doesn't seem to consider which sub-drivers are > actually enabled in kernel config. > > Are you sure? > > exynos_drm_add_components() will try to attach components *added to > component_lists. And these components will be added by only > corresponding sub drivers to the component_lists and > master->components. > > So in this case, if users disabled HDMI support then a commponent > object for HDMI sub driver doesn't exists in the component_lists and > master->components. This means that a component object for HDMI sub > driver *cannot be attached to master object*. > > As a result, component_bind_add() will ignor component_bind call for > HDMI sub driver so HDMI driver will not be bounded. The only > components added by sub drivers will be bound, component->ops->bind(). > > For more understanding, it seems like you need to look into below codes, > > static int exynos_drm_add_components(...) > { > ... > for (i == 0;; i++) { > ... > node = of_parse_phandle(np, "ports", i); > ... > ret = component_master_add_child(m, compare_of, node); > ... > } > } Hmm, In case HDMI driver is not present, HDMI device is not probed and HDMI component is not added, so component_master_add_child returns an error when it tries to add hdmi component and master is never brought up, am I correct? > > > And below codes, > > int component_master_add_child(...) > { > list_for_each_entry(c, &component_list, node) { > if (c->master) > continue; > > if (compare(...)) { > component_attach_master(master, c); > ... > } > } > } > > And below codes, > > static void component_attach_master(master, c) > { > c->master = master; > list_add_tail(&c->master_node, &master->comonents); > } > > > As you can see above, the only components added to component_list can > be attached to master. And the important thing is that components can > be added by sub drivers to the component_list. > > And below codes that actually tries to bind each sub drivers, > > int component_bind_add(...) > { > > list_for_each_entry(c, &master->components, master_node) { >ret = component_bind(c, master, data); >... > } > ... > } > > The hdmi driver users disabled doesn't exist to master->components list. > How Exynos DRM cannot be initialized? > > Of course, there may be my missing point, but I think I see them > correctly. Anyway I will test them tomorrow. > > Thanks, > Inki Dae > register, which is completely wrong. Users should be able to select which drivers should be compiled into their kernels. >>> >>> So users are be able to select drivers they want to use, and will be >>> compiled correctly. So no, the only thing users can disable is each >>> sub driver, not core module. >>> 3) Such approach leads to complete integration of all Exynos DRM drivers, without possibility of loading some sub-drivers as modules. I know that current driver design doesn't support it either, but if this series is >>> >>> No, current drm driver *must also be built* as one integrated single >>> drm driver without super device approach. >> >> As I wrote, I know that current design before this patch isn't modular >> either, but this is not my concern here. See below. >> >> >>> So the super device approach >>> *has no any effect on existing design*, and what the super device >>> approch tries to do is to resolve the probe order issue to sub drivers >>> *without some codes specific to Exynos drm*. >> >> My concern is that the supernode design is actually carving such broken >> non-modular design in stone. Remember that we are currently heading towards >> a fully multi-platform kernel where it is critical for such subsystems to be >> modular, because the same zImage is going to be running on completely >> different machines. >> >> claimed to improve things, it should really do so. 4) Exactly the same can be achieved without changing the DT bindings at all. In fact even without adding any new single property or node to DT. We discussed this with Andrzej and Marek today and came to a solution in which just by adding a little bit of code to Exynos DRM subdrivers, you could guarantee correct registration of Exynos DRM platform and also get rid of #ifdeffery in exynos_drm_drv.c. Andrzej will send an RFC after the weekend. >>> >>> I'm not sure but I had implemented below prototype codes for that, see >>> the below link, >>> >>> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?h=exynos-bridge-test&id=2860ad02d736e300034
[Bug 77142] New: Garbled text in Gtk2 applications
https://bugs.freedesktop.org/show_bug.cgi?id=77142 Priority: medium Bug ID: 77142 Assignee: dri-devel at lists.freedesktop.org Summary: Garbled text in Gtk2 applications Severity: normal Classification: Unclassified OS: Linux (All) Reporter: e.durmin at gmx.net Hardware: x86 (IA32) Status: NEW Version: 10.1 Component: Drivers/Gallium/r300 Product: Mesa Created attachment 97046 --> https://bugs.freedesktop.org/attachment.cgi?id=97046&action=edit A small snapshot from Firefox showing garbled text I'm seeing this issue on my computer with the Radeon-9500-AGP (r300) using the opensource drivers since the beginning of this year. That means that this issue may have started since version mesa-10.0.0. I'm using mesa-10.1.0 now. I've tested a Radeon-3650-AGP (r600) on the same computer for a few days and noticed the same issue. The garbled text is sometimes noticeable in the UI of the application like menus, but it is always noticeable when a lot of text is shown, for example in Firefox showing a webpage with a lot of text. In Firefox: The garbled text is not shown on all lines: some lines will have garbled text, while most other lines will show the text correct. Sometimes the text is so severily garbled that it is not readable anymore. Selecting the line with the garbled text with the mouse (as if doing a copy & paste action) will change (redraw?) the garbled text and usually correcting the garbled letters. The text is not garbled in Qt/KDE applications. This issue disappeared when in xorg.conf EXAPixmaps or RenderAccel is disabled (both are enabled by default). After doing this "fix" text will not be garbled anymore, but scrolling in Firefox (and possible other applications) is much slower/less smooth. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/49b853cd/attachment.html>
[PATCH] drm/radeon: fix audio pin counts for DCE6+
There is actually quite a bit of variance based on the asic. Signed-off-by: Alex Deucher Cc: stable at vger.kernel.org --- drivers/gpu/drm/radeon/dce6_afmt.c | 14 ++ drivers/gpu/drm/radeon/radeon.h| 5 - 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/radeon/dce6_afmt.c b/drivers/gpu/drm/radeon/dce6_afmt.c index 8595449..38766aa 100644 --- a/drivers/gpu/drm/radeon/dce6_afmt.c +++ b/drivers/gpu/drm/radeon/dce6_afmt.c @@ -309,11 +309,17 @@ int dce6_audio_init(struct radeon_device *rdev) rdev->audio.enabled = true; - if (ASIC_IS_DCE8(rdev)) - rdev->audio.num_pins = 6; - else if (ASIC_IS_DCE61(rdev)) + if (ASIC_IS_DCE81(rdev)) /* KV: 4 streams, 7 endpoints */ + rdev->audio.num_pins = 7; + else if (ASIC_IS_DCE83(rdev)) /* KB: 2 streams, 3 endpoints */ rdev->audio.num_pins = 4; - else + else if (ASIC_IS_DCE8(rdev)) /* BN/HW: 6 streams, 7 endpoints */ + rdev->audio.num_pins = 7; + else if (ASIC_IS_DCE61(rdev)) /* TN: 4 streams, 6 endpoints */ + rdev->audio.num_pins = 6; + else if (ASIC_IS_DCE64(rdev)) /* OL: 2 streams, 2 endpoints */ + rdev->audio.num_pins = 2; + else /* SI: 6 streams, 6 endpoints */ rdev->audio.num_pins = 6; for (i = 0; i < rdev->audio.num_pins; i++) { diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 7a07ec8..f0fc2c8 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -736,7 +736,7 @@ union radeon_irq_stat_regs { struct cik_irq_stat_regs cik; }; -#define RADEON_MAX_HPD_PINS 6 +#define RADEON_MAX_HPD_PINS 7 #define RADEON_MAX_CRTCS 6 #define RADEON_MAX_AFMT_BLOCKS 7 @@ -2628,6 +2628,9 @@ void r100_pll_errata_after_index(struct radeon_device *rdev); #define ASIC_IS_DCE64(rdev) ((rdev->family == CHIP_OLAND)) #define ASIC_IS_NODCE(rdev) ((rdev->family == CHIP_HAINAN)) #define ASIC_IS_DCE8(rdev) ((rdev->family >= CHIP_BONAIRE)) +#define ASIC_IS_DCE81(rdev) ((rdev->family == CHIP_KAVERI)) +#define ASIC_IS_DCE82(rdev) ((rdev->family == CHIP_BONAIRE)) +#define ASIC_IS_DCE83(rdev) ((rdev->family == CHIP_KABINI)) #define ASIC_IS_LOMBOK(rdev) ((rdev->ddev->pdev->device == 0x6849) || \ (rdev->ddev->pdev->device == 0x6850) || \ -- 1.8.3.1
[RFCv3 12/14] drm: Specify cursor plane at CRTC initialization
On Mon, Apr 7, 2014 at 4:03 PM, Daniel Vetter wrote: > On Mon, Apr 7, 2014 at 7:23 PM, Rob Clark wrote: >> On Mon, Apr 7, 2014 at 6:05 AM, Thierry Reding >> wrote: >>> On Fri, Mar 28, 2014 at 10:04:09PM +0100, Daniel Vetter wrote: On Tue, Mar 18, 2014 at 05:22:57PM -0700, Matt Roper wrote: > Add cursor plane as a parameter to drm_crtc_init() and update all > existing DRM drivers to use a helper-provided primary plane. Passing > NULL for this parameter indicates that there is no hardware cursor > supported by the driver and no cursor plane should be provided to > userspace. > > Signed-off-by: Matt Roper Ok, cursor planes. I've poked around in this a lot and unfortunately I don't think we can achieve nirvana :( >>> [...] - For the pixel format I think it's ok to always assume RGBA. >>> >>> How so? We do have hardware on Tegra that doesn't support RGBA cursors >>> and if at all possible I'd very much like to support that as well. New >>> generations can do RGBA but still support the old pixel format. Having >>> the option of supporting all formats would be nice. >>> >>> There was some discussion about implementing a bunch of plane properties >>> so I think having one to enumerate a set of pixel formats wouldn't be >>> such a big deal. >> >> No need for properties to expose formats, we already have that in >> getplane ioctl. But would be nice to support more than just RGBA. > > This is _just_ for the compat cursor plane setup helper. Of course we > can add whatever insane cursor formats exist out there as fourcc codes > and then drivers can add them for their custom-created cursor planes. > This is similar to the current primary plane helper code which only > exposes 32bit XRGB atm. If you want to take full advantage of this you > need to do a bit of work in your driver ;-) oh, heh.. should have read the whole thread first :-P yeah, for compat helpers, only ARGB makes sense BR, -R > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 2/4] drm/dp/i2c: send bare addresses to properly reset i2c connections (v3)
On Mon, Apr 07, 2014 at 09:44:06AM -0400, Alex Deucher wrote: > On Mon, Apr 7, 2014 at 3:49 AM, Thierry Reding > wrote: > > On Fri, Apr 04, 2014 at 03:58:37PM -0400, Alex Deucher wrote: > >> We need bare address packets at the start and end of > >> each i2c over aux transaction to properly reset the connection > >> between transactions. This mirrors what the existing dp i2c > >> over aux algo currently does. > >> > >> This fixes EDID fetches on certain monitors especially with > >> dp bridges. > >> > >> v2: update as per Ville's comments > >> - Set buffer to NULL for zero sized packets > >> - abort the entre transaction if one of the messages fails > >> v3: drop leftover debugging code > >> > >> Signed-off-by: Alex Deucher > >> Cc: Ville Syrj?l? > >> Cc: Jani Nikula > >> Cc: Thierry Reding > >> Reviewed-by: Ville Syrj?l? > >> --- > >> drivers/gpu/drm/drm_dp_helper.c | 52 > >> +++-- > >> 1 file changed, 29 insertions(+), 23 deletions(-) > > > > Can we please document that zero-sized messages specify address-only > > transactions? Perhaps it would also be useful to mention that these can > > only happen for I2C-over-AUX messages. > > Can do. I don't know of any uses for bare address packets with > regular aux off hand, but there may be cases I'm not familiar with. > Does anyone know of any use for a bare address packet with regular > aux? > > > > >> diff --git a/drivers/gpu/drm/drm_dp_helper.c > >> b/drivers/gpu/drm/drm_dp_helper.c > >> index 74724aa..dfe4cf4 100644 > >> --- a/drivers/gpu/drm/drm_dp_helper.c > >> +++ b/drivers/gpu/drm/drm_dp_helper.c > >> @@ -664,12 +664,23 @@ static int drm_dp_i2c_xfer(struct i2c_adapter > >> *adapter, struct i2c_msg *msgs, > >> int num) > >> { > >> struct drm_dp_aux *aux = adapter->algo_data; > >> - unsigned int i, j; > >> + unsigned int m, b; > > > > I don't see why these would need to be changed. i and j are perfectly > > fine loop variable names. > > It was easier for me to follow the code since the variables matched > the objects they were iterating, but I can change them back if you'd > prefer. I think there's enough context in the surrounding code to make it obvious what they are used for. I also think that keeping the names makes the diff easier to follow. But if you feel really strongly about keeping m and b I can live with it. > >> - for (i = 0; i < num; i++) { > >> - struct drm_dp_aux_msg msg; > >> - int err; > >> + memset(&msg, 0, sizeof(msg)); > >> > >> + for (m = 0; m < num; m++) { > >> + msg.address = msgs[m].addr; > >> + msg.request = (msgs[m].flags & I2C_M_RD) ? > >> + DP_AUX_I2C_READ : > >> + DP_AUX_I2C_WRITE; > >> + msg.request |= DP_AUX_I2C_MOT; > >> + msg.buffer = NULL; > >> + msg.size = 0; > >> + err = drm_dp_i2c_do_msg(aux, &msg); > >> + if (err < 0) > >> + break; > > > > This seems somewhat brittle to me. Even though I notice that patch 3/4 > > updates a comment that documents these assumptions, I don't see a reason > > for these assumptions in the first place. > > We already assume that in drm_dp_i2c_do_msg() for the retry loop. Ugh... you're right. > > I'd prefer if we simply provided the complete message rather than rely > > on drivers not to touch anything but the reply field. > > Are you suggesting we re-write drm_dp_i2c_do_msg() or move the retry > logic into drm_dp_i2c_xfer()? Do you mind if we do that in a follow > up patch so we can keep it separate from the addition of the bare > address packets? No, looking at the code again, I don't see how we can do much better without sacrificing much of the clarity of the code. But perhaps this could be documented somewhere else than the I2C helpers. Perhaps the struct drm_dp_aux comment should be updated, which would cause the DRM docbook to automatically pick up that change as well. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/aa052f13/attachment-0001.sig>
[Bug 77009] 24P playback video signal loss with latest DRI patches
https://bugs.freedesktop.org/show_bug.cgi?id=77009 --- Comment #21 from Garrett --- Created attachment 97043 --> https://bugs.freedesktop.org/attachment.cgi?id=97043&action=edit 368 and others dmesg >Garrett, please stop trying to merge the patches. Since the initial change is >>already upsteram we are going to need to fix it in a separate patch anyway. Sorry. New to this FOSS stuff. OE is a complete OS download with local code not pulling from mainline kernels (from what I can tell). It was tricky for me to patch it. I am trying to be transparent so others can reproduce my results easily. >My fix limited the reference divider so that "post_div * ref_div <= 100". >Could >you try to raise the 100 to see at which point your TV says the signal >isn't >valid any more? >With a limit of 500 you should be at the same point as before, so I suggest to >>try values of 200, 300, 400 first. Then if 200 works but 300 doesn't (for >>example) try 250, and so on... Thanks for the tips. OK- I built many. 368 is the closest I got. I have to head to the office. I uploaded all dmesg, tests. What I don't understand is the pll numbers go up and down, despite having the higher max vals. This concerns me.. Hope it helps. I assume you do not want to run all pc's at max. Else tolerances at the hardware/silica/chipset level may result in similar results to my sys. But at least now I can tune my sys. :) 368 is much higher than 100. LMK if you need more. TIA. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/6642275c/attachment-0001.html>
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 --- Comment #14 from Alex Deucher --- Does reverting 832eafaf34ff7d0348fe701e417900c6cf1f5656 help? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/15687069/attachment.html>
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 --- Comment #13 from bgunteriv at gmail.com --- Created attachment 97042 --> https://bugs.freedesktop.org/attachment.cgi?id=97042&action=edit dmesg - kernel 3.14 w/ patch -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/cb58ec57/attachment.html>
[PATCH 2/3] drm/edid: Check for user aspect ratio input
In case user has specified an input for aspect ratio through the property, then the user space value for PAR would take preference over the value from CEA mode list. Signed-off-by: Vandana Kannan Cc: dri-devel at lists.freedesktop.org --- drivers/gpu/drm/drm_edid.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b8d6c51..62680e7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3630,8 +3630,13 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; - /* Populate picture aspect ratio from CEA mode list */ - if (frame->video_code > 0) + /* Populate picture aspect ratio from either CEA mode list or +* user input + */ + if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 || + mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) + frame->picture_aspect = mode->picture_aspect_ratio; + else if (frame->video_code > 0) frame->picture_aspect = drm_get_cea_aspect_ratio( frame->video_code); -- 1.9.1
[PATCH 1/3] drm/crtc: Add property for aspect ratio
Added a property to enable user space to set aspect ratio. This patch contains declaration of the property and code to create the property. Signed-off-by: Vandana Kannan Cc: dri-devel at lists.freedesktop.org --- drivers/gpu/drm/drm_crtc.c | 31 +++ include/drm/drm_crtc.h | 2 ++ 2 files changed, 33 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d8b7099..6cd34ad 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -139,6 +139,12 @@ static const struct drm_prop_enum_list drm_scaling_mode_enum_list[] = { DRM_MODE_SCALE_ASPECT, "Full aspect" }, }; +static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = { + { HDMI_PICTURE_ASPECT_NONE, "Automatic" }, + { HDMI_PICTURE_ASPECT_4_3, "4:3" }, + { HDMI_PICTURE_ASPECT_16_9, "16:9" }, +}; + /* * Non-global properties, but "required" for certain connectors. */ @@ -1334,6 +1340,31 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev) EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); /** + * drm_mode_create_aspect_ratio_property - create aspect ratio property + * @dev: DRM device + * + * Called by a driver the first time it's needed, must be attached to desired + * connectors. + */ +int drm_mode_create_aspect_ratio_property(struct drm_device *dev) +{ + struct drm_property *aspect_ratio; + + if (dev->mode_config.aspect_ratio_property) + return 0; + + aspect_ratio = + drm_property_create_enum(dev, 0, "aspect ratio", + drm_aspect_ratio_enum_list, + ARRAY_SIZE(drm_aspect_ratio_enum_list)); + + dev->mode_config.aspect_ratio_property = aspect_ratio; + + return 0; +} +EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); + +/** * drm_mode_create_dirty_property - create dirty property * @dev: DRM device * diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c061bb3..99bb6ed 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -797,6 +797,7 @@ struct drm_mode_config { /* Optional properties */ struct drm_property *scaling_mode_property; + struct drm_property *aspect_ratio_property; struct drm_property *dirty_info_property; /* dumb ioctl parameters */ @@ -966,6 +967,7 @@ extern int drm_mode_create_dvi_i_properties(struct drm_device *dev); extern int drm_mode_create_tv_properties(struct drm_device *dev, int num_formats, char *formats[]); extern int drm_mode_create_scaling_mode_property(struct drm_device *dev); +extern int drm_mode_create_aspect_ratio_property(struct drm_device *dev); extern int drm_mode_create_dirty_info_property(struct drm_device *dev); extern const char *drm_get_encoder_name(const struct drm_encoder *encoder); -- 1.9.1
[Bug 77002] hdmi audio problems with 3.14
https://bugs.freedesktop.org/show_bug.cgi?id=77002 --- Comment #12 from bgunteriv at gmail.com --- (In reply to comment #11) > Created attachment 97008 [details] [review] > workaround > > Does that attached kernel patch fix the issue? no this did not help. also, now when it boots up, I get the message: drm:module has bad taint, not creating trace events i'm attaching new dmesg. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/256734dd/attachment.html>
[PATCH 1/3] drm/crtc: Add property for aspect ratio
On Apr-07-2014 3:33 PM, Kannan, Vandana wrote: > Added a property to enable user space to set aspect ratio. > This patch contains declaration of the property and code to create the > property. > > Signed-off-by: Vandana Kannan > Cc: dri-devel at lists.freedesktop.org > --- This patch series is being submitted as discussed in http://lists.freedesktop.org/archives/dri-devel/2014-April/056593.html -Vandana > drivers/gpu/drm/drm_crtc.c | 31 +++ > include/drm/drm_crtc.h | 2 ++ > 2 files changed, 33 insertions(+) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index d8b7099..6cd34ad 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -139,6 +139,12 @@ static const struct drm_prop_enum_list > drm_scaling_mode_enum_list[] = > { DRM_MODE_SCALE_ASPECT, "Full aspect" }, > }; > > +static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = { > + { HDMI_PICTURE_ASPECT_NONE, "Automatic" }, > + { HDMI_PICTURE_ASPECT_4_3, "4:3" }, > + { HDMI_PICTURE_ASPECT_16_9, "16:9" }, > +}; > + > /* > * Non-global properties, but "required" for certain connectors. > */ > @@ -1334,6 +1340,31 @@ int drm_mode_create_scaling_mode_property(struct > drm_device *dev) > EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > > /** > + * drm_mode_create_aspect_ratio_property - create aspect ratio property > + * @dev: DRM device > + * > + * Called by a driver the first time it's needed, must be attached to desired > + * connectors. > + */ > +int drm_mode_create_aspect_ratio_property(struct drm_device *dev) > +{ > + struct drm_property *aspect_ratio; > + > + if (dev->mode_config.aspect_ratio_property) > + return 0; > + > + aspect_ratio = > + drm_property_create_enum(dev, 0, "aspect ratio", > + drm_aspect_ratio_enum_list, > + ARRAY_SIZE(drm_aspect_ratio_enum_list)); > + > + dev->mode_config.aspect_ratio_property = aspect_ratio; > + > + return 0; > +} > +EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); > + > +/** > * drm_mode_create_dirty_property - create dirty property > * @dev: DRM device > * > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index c061bb3..99bb6ed 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -797,6 +797,7 @@ struct drm_mode_config { > > /* Optional properties */ > struct drm_property *scaling_mode_property; > + struct drm_property *aspect_ratio_property; > struct drm_property *dirty_info_property; > > /* dumb ioctl parameters */ > @@ -966,6 +967,7 @@ extern int drm_mode_create_dvi_i_properties(struct > drm_device *dev); > extern int drm_mode_create_tv_properties(struct drm_device *dev, int > num_formats, >char *formats[]); > extern int drm_mode_create_scaling_mode_property(struct drm_device *dev); > +extern int drm_mode_create_aspect_ratio_property(struct drm_device *dev); > extern int drm_mode_create_dirty_info_property(struct drm_device *dev); > extern const char *drm_get_encoder_name(const struct drm_encoder *encoder); > >
[PATCH v12][ 12/12] ARM: imx_v6_v7_defconfig: Add more drm drivers.
The DRM_PANEL_SIMPLE is needed by the eukrea mbimxsd51's displays. Signed-off-by: Denis Carikli --- ChangeLog v10->v11: - New patch, splitting it would be overkill. --- arch/arm/configs/imx_v6_v7_defconfig |2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig index 09e9743..0316926 100644 --- a/arch/arm/configs/imx_v6_v7_defconfig +++ b/arch/arm/configs/imx_v6_v7_defconfig @@ -183,6 +183,7 @@ CONFIG_V4L_MEM2MEM_DRIVERS=y CONFIG_VIDEO_CODA=y CONFIG_SOC_CAMERA_OV2640=y CONFIG_DRM=y +CONFIG_DRM_PANEL_SIMPLE=y CONFIG_BACKLIGHT_LCD_SUPPORT=y CONFIG_LCD_CLASS_DEVICE=y CONFIG_LCD_L4F00242T03=y @@ -245,6 +246,7 @@ CONFIG_DRM_IMX_TVE=y CONFIG_DRM_IMX_LDB=y CONFIG_DRM_IMX_IPUV3_CORE=y CONFIG_DRM_IMX_IPUV3=y +CONFIG_DRM_IMX_HDMI=y CONFIG_COMMON_CLK_DEBUG=y # CONFIG_IOMMU_SUPPORT is not set CONFIG_PWM=y -- 1.7.9.5
[PATCH v12][ 11/12] ARM: dts: mbimx51sd: Add CMO-QVGA backlight support.
Signed-off-by: Denis Carikli --- ChangeLog v9->v11: - Now uses the drm-panel instead of the display-timings. ChangeLog v8->v9: - Removed the Cc. They are now set in git-send-email directly. - The backlight is now on at boot. ChangeLog v6->v7: - Shrinked even more the Cc list. ChangeLog v5->v6: - Reordered the Cc list. ChangeLog v3->v5: - Updated to the new GPIO defines. ChangeLog v2->v3: - Splitted out from the patch that added support for the cpuimx51/mbimxsd51 boards. - This patch now only adds backlight support. - Added some interested people in the Cc list, and removed some people that might be annoyed by the receiving of that patch which is unrelated to their subsystem. --- .../imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts index 7c280f8..4a3f805 100644 --- a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts +++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts @@ -17,9 +17,19 @@ model = "Eukrea MBIMXSD51 with the CMO-QVGA Display"; compatible = "eukrea,mbimxsd51-baseboard-cmo-qvga", "eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51"; + backlight: backlight { + compatible = "gpio-backlight"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_backlight_1>; + gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>; + default-brightness-level = <1>; + default-on; + }; + panel: panel { compatible = "eukrea,mbimxsd51-cmo-qvga", "simple-panel"; power-supply = <®_lcd_3v3>; + backlight = <&backlight>; }; reg_lcd_3v3: lcd-en { -- 1.7.9.5
[PATCH v12][ 10/12] ARM: dts: mbimx51sd: Add display support.
The CMO-QVGA, DVI-SVGA and DVI-VGA are added. Signed-off-by: Denis Carikli --- ChangeLog v10->v11: - Now uses the drm-panel instead of the display-timings. This is to get regulator support, which is lacking in the imx-drm driver when using the display-timings. ChangeLog v9->v10: - Rebased - Now enables the cmo-qvga regulator at boot. ChangeLog v8->v9: - Removed the Cc. They are now set in git-send-email directly. - updated pixelclk-active after the following patch: "imx-drm: Match ipu_di_signal_cfg's clk_pol with its description." ChangeLog v7->v8: - Rebased the patch: added the now required imx-drm node. - Adapted the svga clock-frequency value in order to still be able to display an image after the following commit: "imx-drm: ipu-v3: more inteligent DI clock selection" ChangeLog v6->v7: - Shrinked even more the Cc list. - Since the pingrp headers were removed, the references to it where replaced by the actual pins. - Added the targets to arch/arm/boot/dts/Makefile ChangeLog v5->v6: - Reordered the Cc list. ChangeLog v3->v5: - Updated to new GPIO defines. - Updated to new licenses checkpatch requirements. - one whitespace cleanup. ChangeLog v2->v3: - Splitted out from the patch that added support for the cpuimx51/mbimxsd51 boards. - This patch now only adds display support. - Added some interested people in the Cc list, and removed some people that might be annoyed by the receiving of that patch which is unrelated to their subsystem. - rebased and reworked the dts displays addition. - Also rebased and reworked the fsl,pins usage. --- arch/arm/boot/dts/Makefile |3 ++ .../imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts | 41 .../imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts | 28 +++ .../imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts | 28 +++ .../boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts | 49 5 files changed, 149 insertions(+) create mode 100644 arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts create mode 100644 arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts create mode 100644 arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 2145af6..4ac8aeb 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -166,6 +166,9 @@ dtb-$(CONFIG_ARCH_MXC) += \ imx51-apf51dev.dtb \ imx51-babbage.dtb \ imx51-eukrea-mbimxsd51-baseboard.dtb \ + imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dtb \ + imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dtb \ + imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dtb \ imx53-ard.dtb \ imx53-m53evk.dtb \ imx53-mba53.dtb \ diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts new file mode 100644 index 000..7c280f8 --- /dev/null +++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts @@ -0,0 +1,41 @@ +/* + * Copyright 2013 Eukr?a Electromatique + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "imx51-eukrea-mbimxsd51-baseboard.dts" + +/ { + model = "Eukrea MBIMXSD51 with the CMO-QVGA Display"; + compatible = "eukrea,mbimxsd51-baseboard-cmo-qvga", "eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51"; + + panel: panel { + compatible = "eukrea,mbimxsd51-cmo-qvga", "simple-panel"; + power-supply = <®_lcd_3v3>; + }; + + reg_lcd_3v3: lcd-en { + compatible = "regulator-fixed"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_reg_lcd_3v3>; + regulator-name = "lcd-3v3"; + regulator-min-microvolt = <330>; + regulator-max-microvolt = <330>; + gpio = <&gpio3 13 GPIO_ACTIVE_HIGH>; + enable-active-high; + regulator-boot-on; + }; +}; + +&display { + status = "okay"; + fsl,panel = <&panel>; +}; diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts new file mode 100644 index 000..323ebf4 --- /dev/null +++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts @@ -0,0 +1,28 @@ +/* + * Copyright 2013 Eukr?a Electromatique + * + * This program is free software; you can redistribute it and/or + * modify
[PATCH v12][ 09/12] drm/panel: Add Eukrea mbimxsd51 displays.
Signed-off-by: Denis Carikli --- ChangeLog v11->v12: - Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names ChangeLog v10->v11: - New patch. --- .../bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt |7 ++ .../bindings/panel/eukrea,mbimxsd51-dvi-svga.txt |7 ++ .../bindings/panel/eukrea,mbimxsd51-dvi-vga.txt|7 ++ drivers/gpu/drm/panel/panel-simple.c | 81 4 files changed, 102 insertions(+) create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt new file mode 100644 index 000..03679d0 --- /dev/null +++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt @@ -0,0 +1,7 @@ +Eukrea CMO-QVGA (320x240 pixels) TFT LCD panel + +Required properties: +- compatible: should be "eukrea,mbimxsd51-cmo-qvga" + +This binding is compatible with the simple-panel binding, which is specified +in simple-panel.txt in this directory. diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt new file mode 100644 index 000..f408c9a --- /dev/null +++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt @@ -0,0 +1,7 @@ +Eukrea DVI-SVGA (800x600 pixels) DVI output. + +Required properties: +- compatible: should be "eukrea,mbimxsd51-dvi-svga" + +This binding is compatible with the simple-panel binding, which is specified +in simple-panel.txt in this directory. diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt new file mode 100644 index 000..8ea90da --- /dev/null +++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt @@ -0,0 +1,7 @@ +Eukrea DVI-VGA (640x480 pixels) DVI output. + +Required properties: +- compatible: should be "eukrea,mbimxsd51-dvi-vga" + +This binding is compatible with the simple-panel binding, which is specified +in simple-panel.txt in this directory. diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 309f29e..45797d8 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -328,6 +328,78 @@ static const struct panel_desc chunghwa_claa101wb01 = { }, }; +static const struct drm_display_mode eukrea_mbimxsd51_cmoqvga_mode = { + .clock = 6500, + .hdisplay = 320, + .hsync_start = 320 + 38, + .hsync_end = 320 + 38 + 20, + .htotal = 320 + 38 + 20 + 30, + .vdisplay = 240, + .vsync_start = 240 + 15, + .vsync_end = 240 + 15 + 4, + .vtotal = 240 + 15 + 4 + 3, + .vrefresh = 60, + .pol_flags = DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE | +DRM_MODE_FLAG_POL_DE_LOW, +}; + +static const struct panel_desc eukrea_mbimxsd51_cmoqvga = { + .modes = &eukrea_mbimxsd51_cmoqvga_mode, + .num_modes = 1, + .size = { + .width = 73, + .height = 56, + }, +}; + +static const struct drm_display_mode eukrea_mbimxsd51_dvisvga_mode = { + .clock = 44333, + .hdisplay = 800, + .hsync_start = 800 + 112, + .hsync_end = 800 + 112 + 32, + .htotal = 800 + 112 + 32 + 80, + .vdisplay = 600, + .vsync_start = 600 + 3, + .vsync_end = 600 + 3 + 17, + .vtotal = 600 + 3 + 17 + 4, + .vrefresh = 60, + .pol_flags = DRM_MODE_FLAG_POL_PIXDATA_POSEDGE | +DRM_MODE_FLAG_POL_DE_HIGH, +}; + +static const struct panel_desc eukrea_mbimxsd51_dvisvga = { + .modes = &eukrea_mbimxsd51_dvisvga_mode, + .num_modes = 1, + .size = { + .width = 0, + .height = 0, + }, +}; + +static const struct drm_display_mode eukrea_mbimxsd51_dvivga_mode = { + .clock = 23750, + .hdisplay = 640, + .hsync_start = 640 + 80, + .hsync_end = 640 + 80 + 16, + .htotal = 640 + 80 + 16 + 64, + .vdisplay = 480, + .vsync_start = 480 + 3, + .vsync_end = 480 + 3 + 13, + .vtotal = 480 + 3 + 13 + 4, + .vrefresh = 60, + .pol_flags = DRM_MODE_FLAG_POL_PIXDATA_POSEDGE | +DRM_MODE_FLAG_POL_DE_HIGH, +}; + +static const struct panel_desc eukrea_mbimxsd51_dvivga = { + .modes = &eukrea_mbimxsd51_dvivga_mode, + .num_modes = 1, + .size = { + .width = 0, + .height = 0, + }, +}; + static const struct drm_display_mode lg_lp129qe_mode = { .clock = 285250, .hdisplay = 2560, @@ -380,6 +452,15 @@ static const struct of_device_i
[PATCH v12][ 08/12] imx-drm: Use drm_display_mode timings flags.
The previous hardware behaviour was kept if the flags are not set. Signed-off-by: Denis Carikli --- ChangeLog v11->v12: - Rebased: It now uses the following new flags defines names: CLK_POL, ENABLE_POL - The inversions in ipuv3-crtc.c are now fixed. - ipuv3-crtc.c was still using mode->private_flags from the previous versions of this patchset, that's now fixed. ChangeLog v10->v11: - This patch was splitted-out and adapted from: "Prepare imx-drm for extra display-timings retrival." - The display-timings dt specific part was removed. - The flags names were changed to use the DRM ones from: "drm: drm_display_mode: add signal polarity flags" --- drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h |6 -- drivers/staging/imx-drm/ipu-v3/ipu-di.c |7 ++- drivers/staging/imx-drm/ipuv3-crtc.c| 20 ++-- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h index eba8893..c934394 100644 --- a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h +++ b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h @@ -29,9 +29,11 @@ enum ipuv3_type { #define CLK_POL_NEGEDGE0 #define CLK_POL_POSEDGE1 +#define CLK_POL_PRESERVE 2 #define ENABLE_POL_LOW 0 #define ENABLE_POL_HIGH1 +#define ENABLE_POL_PRESERVE2 /* * Bitfield of Display Interface signal polarities. @@ -43,10 +45,10 @@ struct ipu_di_signal_cfg { unsigned clksel_en:1; unsigned clkidle_en:1; unsigned data_pol:1;/* true = inverted */ - unsigned clk_pol:1; - unsigned enable_pol:1; unsigned Hsync_pol:1; /* true = active high */ unsigned Vsync_pol:1; + u8 clk_pol; + u8 enable_pol; u16 width; u16 height; diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-di.c b/drivers/staging/imx-drm/ipu-v3/ipu-di.c index 0ce3f52..c00b0ba 100644 --- a/drivers/staging/imx-drm/ipu-v3/ipu-di.c +++ b/drivers/staging/imx-drm/ipu-v3/ipu-di.c @@ -597,6 +597,8 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig) if (sig->clk_pol == CLK_POL_POSEDGE) di_gen |= DI_GEN_POLARITY_DISP_CLK; + else if (sig->clk_pol == CLK_POL_NEGEDGE) + di_gen &= ~DI_GEN_POLARITY_DISP_CLK; ipu_di_write(di, di_gen, DI_GENERAL); @@ -604,10 +606,13 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig) DI_SYNC_AS_GEN); reg = ipu_di_read(di, DI_POL); - reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15); + reg &= ~(DI_POL_DRDY_DATA_POLARITY); if (sig->enable_pol == ENABLE_POL_HIGH) reg |= DI_POL_DRDY_POLARITY_15; + else if (sig->enable_pol == ENABLE_POL_LOW) + reg &= ~DI_POL_DRDY_POLARITY_15; + if (sig->data_pol) reg |= DI_POL_DRDY_DATA_POLARITY; diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c index 9ba089c..b59b745 100644 --- a/drivers/staging/imx-drm/ipuv3-crtc.c +++ b/drivers/staging/imx-drm/ipuv3-crtc.c @@ -157,8 +157,24 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc, if (mode->flags & DRM_MODE_FLAG_PVSYNC) sig_cfg.Vsync_pol = 1; - sig_cfg.enable_pol = ENABLE_POL_HIGH; - sig_cfg.clk_pol = CLK_POL_NEGEDGE; + if (mode->pol_flags & DRM_MODE_FLAG_POL_PIXDATA_POSEDGE) + sig_cfg.clk_pol = CLK_POL_POSEDGE; + else if (mode->pol_flags & DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE) + sig_cfg.clk_pol = CLK_POL_NEGEDGE; + else if (mode->pol_flags & DRM_MODE_FLAG_POL_PIXDATA_PRESERVE) + sig_cfg.clk_pol = CLK_POL_PRESERVE; + else + sig_cfg.clk_pol = CLK_POL_NEGEDGE; + + if (mode->pol_flags & DRM_MODE_FLAG_POL_DE_HIGH) + sig_cfg.enable_pol = ENABLE_POL_HIGH; + else if (mode->pol_flags & DRM_MODE_FLAG_POL_DE_LOW) + sig_cfg.enable_pol = ENABLE_POL_LOW; + else if (mode->pol_flags & DRM_MODE_FLAG_POL_DE_PRESERVE) + sig_cfg.enable_pol = ENABLE_POL_PRESERVE; + else + sig_cfg.enable_pol = ENABLE_POL_HIGH; + sig_cfg.width = mode->hdisplay; sig_cfg.height = mode->vdisplay; sig_cfg.pixel_fmt = out_pixel_fmt; -- 1.7.9.5
[PATCH v12][ 07/12] drm: drm_display_mode: add signal polarity flags
We need a way to pass signal polarity informations between DRM panels, and the display drivers. To do that, a pol_flags field was added to drm_display_mode. Signed-off-by: Denis Carikli --- ChangeLog v11->v12: - Rebased: This patch now applies against drm_modes.h - Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names ChangeLog v10->v11: - Since the imx-drm won't be able to retrive its regulators from the device tree when using display-timings nodes, and that I was told that the drm simple-panel driver already supported that, I then, instead, added what was lacking to make the eukrea displays work with the drm-simple-panel driver. That required a way to get back the display polarity informations from the imx-drm driver without affecting userspace. --- include/drm/drm_modes.h |8 1 file changed, 8 insertions(+) diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 2dbbf99..b3789e2 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -93,6 +93,13 @@ enum drm_mode_status { #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE BIT(1) +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE BIT(2) +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3) +#define DRM_MODE_FLAG_POL_DE_LOW BIT(4) +#define DRM_MODE_FLAG_POL_DE_HIGH BIT(5) +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) + struct drm_display_mode { /* Header */ struct list_head head; @@ -144,6 +151,7 @@ struct drm_display_mode { int vrefresh; /* in Hz */ int hsync; /* in kHz */ enum hdmi_picture_aspect picture_aspect_ratio; + unsigned int pol_flags; }; /* mode specified on the command line */ -- 1.7.9.5
[PATCH v12][ 06/12] ARM: dts: imx5*, imx6*: correct display-timings nodes.
The imx-drm driver can't use the de-active and pixelclk-active display-timings properties yet. Instead the data-enable and the pixel data clock polarity are hardcoded in the imx-drm driver. So theses properties are now set to keep the same behaviour when imx-drm will start using them. Signed-off-by: Denis Carikli --- ChangeLog v9->v10: - New patch that was splitted out of: "staging imx-drm: Use de-active and pixelclk-active display-timings." --- arch/arm/boot/dts/imx51-babbage.dts |2 ++ arch/arm/boot/dts/imx53-m53evk.dts|2 ++ arch/arm/boot/dts/imx53-tx53-x03x.dts |2 +- arch/arm/boot/dts/imx6qdl-gw53xx.dtsi |2 ++ arch/arm/boot/dts/imx6qdl-gw54xx.dtsi |2 ++ arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi |2 ++ arch/arm/boot/dts/imx6qdl-sabreauto.dtsi |2 ++ arch/arm/boot/dts/imx6qdl-sabrelite.dtsi |2 ++ arch/arm/boot/dts/imx6qdl-sabresd.dtsi|2 ++ 9 files changed, 17 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts index 2dda06b..91ef454 100644 --- a/arch/arm/boot/dts/imx51-babbage.dts +++ b/arch/arm/boot/dts/imx51-babbage.dts @@ -38,6 +38,8 @@ vfront-porch = <7>; hsync-len = <60>; vsync-len = <10>; + de-active = <1>; + pixelclk-active = <0>; }; }; diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts index 4b036b4..d03ced7 100644 --- a/arch/arm/boot/dts/imx53-m53evk.dts +++ b/arch/arm/boot/dts/imx53-m53evk.dts @@ -41,6 +41,8 @@ vfront-porch = <9>; vsync-len = <3>; vsync-active = <1>; + de-active = <1>; + pixelclk-active = <0>; }; }; }; diff --git a/arch/arm/boot/dts/imx53-tx53-x03x.dts b/arch/arm/boot/dts/imx53-tx53-x03x.dts index 0217dde3..4092a81 100644 --- a/arch/arm/boot/dts/imx53-tx53-x03x.dts +++ b/arch/arm/boot/dts/imx53-tx53-x03x.dts @@ -93,7 +93,7 @@ hsync-active = <0>; vsync-active = <0>; de-active = <1>; - pixelclk-active = <1>; + pixelclk-active = <0>; }; ET0500 { diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi index c8e5ae0..43f48f2 100644 --- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi +++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi @@ -494,6 +494,8 @@ vfront-porch = <7>; hsync-len = <60>; vsync-len = <10>; + de-active = <1>; + pixelclk-active = <0>; }; }; }; diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi index 2795dfc..59ecfd1 100644 --- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi +++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi @@ -516,6 +516,8 @@ vfront-porch = <7>; hsync-len = <60>; vsync-len = <10>; + de-active = <1>; + pixelclk-active = <0>; }; }; }; diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi index 99be301..e9419a2 100644 --- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi +++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi @@ -349,6 +349,8 @@ vfront-porch = <7>; hsync-len = <60>; vsync-len = <10>; + de-active = <1>; + pixelclk-active = <0>; }; }; }; diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi index 009abd6..230bbc6 100644 --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi @@ -405,6 +405,8 @@ vfront-porch = <7>; hsync-len = <60>; vsync-len = <10>; + de-active = <1>; + pixelclk-active = <0>; }; }; }; diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/imx6qdl-sabr
[PATCH v12][ 05/12] imx-drm: use defines for clock polarity settings
Signed-off-by: Denis Carikli --- ChangeLog 11->v12: - Improved the define names to match the hardware: ENABLE_POL is not a clock signal but instead an enable signal. ChangeLog v9->v10: - New patch which was splitted out from: "staging: imx-drm: Use de-active and pixelclk-active display-timings.". - Fixes many issues in "staging: imx-drm: Use de-active and pixelclk-active display-timings.": - More clear meaning of the polarity settings. - The SET_CLK_POL and SET_DE_POL masks are not needed anymore. --- drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h |8 +++- drivers/staging/imx-drm/ipu-v3/ipu-di.c |4 ++-- drivers/staging/imx-drm/ipuv3-crtc.c|4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h index c4d14ea..eba8893 100644 --- a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h +++ b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h @@ -27,6 +27,12 @@ enum ipuv3_type { #define IPU_PIX_FMT_GBR24 v4l2_fourcc('G', 'B', 'R', '3') +#define CLK_POL_NEGEDGE0 +#define CLK_POL_POSEDGE1 + +#define ENABLE_POL_LOW 0 +#define ENABLE_POL_HIGH1 + /* * Bitfield of Display Interface signal polarities. */ @@ -37,7 +43,7 @@ struct ipu_di_signal_cfg { unsigned clksel_en:1; unsigned clkidle_en:1; unsigned data_pol:1;/* true = inverted */ - unsigned clk_pol:1; /* true = rising edge */ + unsigned clk_pol:1; unsigned enable_pol:1; unsigned Hsync_pol:1; /* true = active high */ unsigned Vsync_pol:1; diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-di.c b/drivers/staging/imx-drm/ipu-v3/ipu-di.c index 849b3e1..0ce3f52 100644 --- a/drivers/staging/imx-drm/ipu-v3/ipu-di.c +++ b/drivers/staging/imx-drm/ipu-v3/ipu-di.c @@ -595,7 +595,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig) } } - if (sig->clk_pol) + if (sig->clk_pol == CLK_POL_POSEDGE) di_gen |= DI_GEN_POLARITY_DISP_CLK; ipu_di_write(di, di_gen, DI_GENERAL); @@ -606,7 +606,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig) reg = ipu_di_read(di, DI_POL); reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15); - if (sig->enable_pol) + if (sig->enable_pol == ENABLE_POL_HIGH) reg |= DI_POL_DRDY_POLARITY_15; if (sig->data_pol) reg |= DI_POL_DRDY_DATA_POLARITY; diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c index f2c9cd0..9ba089c 100644 --- a/drivers/staging/imx-drm/ipuv3-crtc.c +++ b/drivers/staging/imx-drm/ipuv3-crtc.c @@ -157,8 +157,8 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc, if (mode->flags & DRM_MODE_FLAG_PVSYNC) sig_cfg.Vsync_pol = 1; - sig_cfg.enable_pol = 1; - sig_cfg.clk_pol = 0; + sig_cfg.enable_pol = ENABLE_POL_HIGH; + sig_cfg.clk_pol = CLK_POL_NEGEDGE; sig_cfg.width = mode->hdisplay; sig_cfg.height = mode->vdisplay; sig_cfg.pixel_fmt = out_pixel_fmt; -- 1.7.9.5
[PATCH v12][ 04/12] imx-drm: Match ipu_di_signal_cfg's clk_pol with its description.
According to the datasheet, setting the di0_polarity_disp_clk field in the GENERAL di register sets the output clock polarity to active high. Signed-off-by: Denis Carikli --- ChangeLog v9->v10: - New patch that is now needed by the "staging: imx-drm: Use de-active and pixelclk-active" patch. --- drivers/staging/imx-drm/ipu-v3/ipu-di.c |2 +- drivers/staging/imx-drm/ipuv3-crtc.c|2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-di.c b/drivers/staging/imx-drm/ipu-v3/ipu-di.c index 82a9eba..849b3e1 100644 --- a/drivers/staging/imx-drm/ipu-v3/ipu-di.c +++ b/drivers/staging/imx-drm/ipu-v3/ipu-di.c @@ -595,7 +595,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig) } } - if (!sig->clk_pol) + if (sig->clk_pol) di_gen |= DI_GEN_POLARITY_DISP_CLK; ipu_di_write(di, di_gen, DI_GENERAL); diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c index c48f640..f2c9cd0 100644 --- a/drivers/staging/imx-drm/ipuv3-crtc.c +++ b/drivers/staging/imx-drm/ipuv3-crtc.c @@ -158,7 +158,7 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc, sig_cfg.Vsync_pol = 1; sig_cfg.enable_pol = 1; - sig_cfg.clk_pol = 1; + sig_cfg.clk_pol = 0; sig_cfg.width = mode->hdisplay; sig_cfg.height = mode->vdisplay; sig_cfg.pixel_fmt = out_pixel_fmt; -- 1.7.9.5
[PATCH v12][ 03/12] imx-drm: Correct BGR666 and the board's dts that use them.
The current BGR666 is not consistent with the other color mapings like BGR24. BGR666 should be in the same byte order than BGR24. Signed-off-by: Denis Carikli Acked-by: Philipp Zabel --- ChangeLog v9->v10: - Rebased. - Added Philipp Zabel's Ack. - Included Lothar Wa?mann's suggestion about imx-ldb.c. - Shortened the patch title ChangeLog v8->v9: - Removed the Cc. They are now set in git-send-email directly. ChangeLog v7->v8: - Shrinked even more the Cc list. ChangeLog v6->v7: - Shrinked even more the Cc list. ChangeLog v5->v6: - Remove people not concerned by this patch from the Cc list. - Added a better explanation of the change. ChangeLog v5: - New patch. --- arch/arm/boot/dts/imx51-apf51dev.dts|2 +- arch/arm/boot/dts/imx53-m53evk.dts |2 +- drivers/staging/imx-drm/imx-ldb.c |4 ++-- drivers/staging/imx-drm/ipu-v3/ipu-dc.c |4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm/boot/dts/imx51-apf51dev.dts b/arch/arm/boot/dts/imx51-apf51dev.dts index c5a9a24..7b3851d 100644 --- a/arch/arm/boot/dts/imx51-apf51dev.dts +++ b/arch/arm/boot/dts/imx51-apf51dev.dts @@ -18,7 +18,7 @@ display at di1 { compatible = "fsl,imx-parallel-display"; - interface-pix-fmt = "bgr666"; + interface-pix-fmt = "rgb666"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_ipu_disp1>; diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts index d5d146a..4b036b4 100644 --- a/arch/arm/boot/dts/imx53-m53evk.dts +++ b/arch/arm/boot/dts/imx53-m53evk.dts @@ -24,7 +24,7 @@ soc { display1: display at di1 { compatible = "fsl,imx-parallel-display"; - interface-pix-fmt = "bgr666"; + interface-pix-fmt = "rgb666"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_ipu_disp1>; diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c index fe4c1ef..9141293 100644 --- a/drivers/staging/imx-drm/imx-ldb.c +++ b/drivers/staging/imx-drm/imx-ldb.c @@ -188,11 +188,11 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder) switch (imx_ldb_ch->chno) { case 0: pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ? - V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666; + V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_RGB666; break; case 1: pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ? - V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666; + V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_RGB666; break; default: dev_err(ldb->dev, "unable to config di%d panel format\n", diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c index 6f9abe8..154d293 100644 --- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c +++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c @@ -397,9 +397,9 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev, /* bgr666 */ ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666); - ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 5, 0xfc); /* blue */ + ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 17, 0xfc); /* blue */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /* green */ - ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */ + ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */ /* bgr24 */ ipu_dc_map_clear(priv, IPU_DC_MAP_BGR24); -- 1.7.9.5
[PATCH v12][ 02/12] imx-drm: Add RGB666 support for parallel display.
Signed-off-by: Denis Carikli Acked-by: Philipp Zabel --- ChangeLog v8->v9: - Rebased. - Added Philipp Zabel's ack. - Shortened the patch title. ChangeLog v8->v9: - Removed the Cc. They are now set in git-send-email directly. - Rebased. ChangeLog v7->v8: - Shrinked even more the Cc list. ChangeLog v6->v7: - Shrinked even more the Cc list. ChangeLog v5->v6: - Remove people not concerned by this patch from the Cc list. ChangeLog v3->v5: - Use the correct RGB order. ChangeLog v2->v3: - Added some interested people in the Cc list. - Removed the commit message long desciption that was just a copy of the short description. - Rebased the patch. - Fixed a copy-paste error in the ipu_dc_map_clear parameter. --- .../bindings/staging/imx-drm/fsl-imx-drm.txt |3 ++- drivers/staging/imx-drm/ipu-v3/ipu-dc.c|9 + drivers/staging/imx-drm/parallel-display.c |2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt index 3be5ce7..83137ef 100644 --- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt +++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt @@ -60,7 +60,8 @@ Required properties: - compatible: Should be "fsl,imx-parallel-display" Optional properties: - interface_pix_fmt: How this display is connected to the - display interface. Currently supported types: "rgb24", "rgb565", "bgr666" + display interface. Currently supported types: "rgb24", "rgb565", "bgr666", + "rgb666" - edid: verbatim EDID data block describing attached display. - ddc: phandle describing the i2c bus handling the display data channel diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c index d5de8bb..6f9abe8 100644 --- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c +++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c @@ -92,6 +92,7 @@ enum ipu_dc_map { IPU_DC_MAP_GBR24, /* TVEv2 */ IPU_DC_MAP_BGR666, IPU_DC_MAP_BGR24, + IPU_DC_MAP_RGB666, }; struct ipu_dc { @@ -155,6 +156,8 @@ static int ipu_pixfmt_to_map(u32 fmt) return IPU_DC_MAP_BGR666; case V4L2_PIX_FMT_BGR24: return IPU_DC_MAP_BGR24; + case V4L2_PIX_FMT_RGB666: + return IPU_DC_MAP_RGB666; default: return -EINVAL; } @@ -404,6 +407,12 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev, ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */ + /* rgb666 */ + ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666); + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */ + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */ + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */ + return 0; } diff --git a/drivers/staging/imx-drm/parallel-display.c b/drivers/staging/imx-drm/parallel-display.c index c60b6c6..01b7ce5 100644 --- a/drivers/staging/imx-drm/parallel-display.c +++ b/drivers/staging/imx-drm/parallel-display.c @@ -219,6 +219,8 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB565; else if (!strcmp(fmt, "bgr666")) imxpd->interface_pix_fmt = V4L2_PIX_FMT_BGR666; + else if (!strcmp(fmt, "rgb666")) + imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB666; } panel_node = of_parse_phandle(np, "fsl,panel", 0); -- 1.7.9.5
[PATCH v12][ 01/12] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.
That new macro is needed by the imx_drm staging driver for supporting the QVGA display of the eukrea-cpuimx51 board. Signed-off-by: Denis Carikli Acked-by: Mauro Carvalho Chehab Acked-by: Laurent Pinchart Acked-by: Philipp Zabel --- ChangeLog v9->v10: - Rebased on top of: "211e7f2 [media] DocBook media: drop the old incorrect packed RGB table" - Added Philipp Zabel's Ack. ChangeLog v8->v9: - Removed the Cc. They are now set in git-send-email directly. ChangeLog v7->v8: - Added Mauro Carvalho Chehab back to the list of Cc ChangeLog v6->v7: - Shrinked even more the Cc list. ChangeLog v5->v6: - Remove people not concerned by this patch from the Cc list. ChangeLog v3->v4: - Added Laurent Pinchart's Ack. ChangeLog v2->v3: - Added some interested people in the Cc list. - Added Mauro Carvalho Chehab's Ack. - Added documentation. --- .../DocBook/media/v4l/pixfmt-packed-rgb.xml| 39 include/uapi/linux/videodev2.h |1 + 2 files changed, 40 insertions(+) diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index e1c4f8b..88a7fe1 100644 --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml @@ -279,6 +279,45 @@ colorspace V4L2_COLORSPACE_SRGB. + + V4L2_PIX_FMT_RGB666 + 'RGBH' + + r5 + r4 + r3 + r2 + r1 + r0 + g5 + g4 + + g3 + g2 + g1 + g0 + b5 + b4 + b3 + b2 + + b1 + b0 + + + + + + + + + + + + + + + V4L2_PIX_FMT_BGR24 'BGR3' diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index ea468ee..d5d818a 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -299,6 +299,7 @@ struct v4l2_pix_format { #define V4L2_PIX_FMT_RGB555X v4l2_fourcc('R', 'G', 'B', 'Q') /* 16 RGB-5-5-5 BE */ #define V4L2_PIX_FMT_RGB565X v4l2_fourcc('R', 'G', 'B', 'R') /* 16 RGB-5-6-5 BE */ #define V4L2_PIX_FMT_BGR666 v4l2_fourcc('B', 'G', 'R', 'H') /* 18 BGR-6-6-6 */ +#define V4L2_PIX_FMT_RGB666 v4l2_fourcc('R', 'G', 'B', 'H') /* 18 RGB-6-6-6 */ #define V4L2_PIX_FMT_BGR24 v4l2_fourcc('B', 'G', 'R', '3') /* 24 BGR-8-8-8 */ #define V4L2_PIX_FMT_RGB24 v4l2_fourcc('R', 'G', 'B', '3') /* 24 RGB-8-8-8 */ #define V4L2_PIX_FMT_BGR32 v4l2_fourcc('B', 'G', 'R', '4') /* 32 BGR-8-8-8-8 */ -- 1.7.9.5
[PATCH v5 00/11] imx-drm dt bindings
Am Montag, den 07.04.2014, 12:05 +0200 schrieb Philipp Zabel: > Hi Shawn, > > Am Montag, den 07.04.2014, 12:23 +0800 schrieb Shawn Guo: > > On Tue, Mar 11, 2014 at 11:46:11AM +0800, Shawn Guo wrote: > > > I just came across a couple problems when testing the series on > > > my imx6dl-sabresd board in dual display case - HDMI + LVDS. I tested it > > > using Russell's branch below, which I believe has all the pieces put > > > together. > > > > > > git://ftp.arm.linux.org.uk/~rmk/linux-arm.git imx-drm-staging > > > > > > - When I enable HDMI and LVDS support in both kernel build and device > > > tree, HDMI seems working fine but LVDS color is corrupted quite badly. > > > > Philipp, > > > > Did you get any chance to reproduce this dual display issue? Now it > > shows on mainline kernel. > > I have not yet reproduced, but I've sent a patch today ("imx-drm: > imx-drm-core: Fix imx_drm_encoder_get_mux_id") that might fix this. Tested on imx6q-phytec-pbab01 with LVDS panel and DVI monitor. > > And I see another HDMI regression with my testing on mainline kernel. I > > can have my HDMI work at 1920x1080 with v3.14 kernel, but it can only > > probes 1024x768 with the mainline today. The Xorg.0.log are attached > > below. The hardware and user space are same, so I guess this is another > > issue introduced by the recently kernel driver changes? > > Hmm, not sure. Have you updated to the i2c-ddc-bus phandle property as > described in Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt? regards Philipp
[RFC 0/3] TTM priority queue logic
Hi, Lauri. On 04/04/2014 03:52 PM, Lauri Kasanen wrote: > Hi list, Thomas, > > I'd like to know if this is going in the right direction. This looks fine with me. However, if possible I'd like the drivers to enable both alloc_threshold and priority queue on a per-memory-type basis. That would mean no new arguments (use_pqueue, alloc_threshold) in ttm_bo_device_init(). Instead, set default values in ttm_bo_init_mm(), and let the driver change them in the init_mem_type() callback. Do you think that would work? /Thomas
[PATCH] drm/omap: fix plane rotation
On Sun, Apr 6, 2014 at 12:45 AM, Rob Clark wrote: > On Sat, Apr 5, 2014 at 2:33 PM, Grazvydas Ignotas > wrote: >> Plane rotation with omapdrm is currently broken. >> It seems omap_plane_mode_set() expects width and height in screen >> coordinates, so pass it like that. >> >> Cc: Rob Clark >> Signed-off-by: Grazvydas Ignotas >> --- >> drivers/gpu/drm/omapdrm/omap_plane.c |8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c >> b/drivers/gpu/drm/omapdrm/omap_plane.c >> index 370580c..5611f15 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_plane.c >> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c >> @@ -253,6 +253,14 @@ static int omap_plane_update(struct drm_plane *plane, >> >> drm_framebuffer_reference(fb); >> >> + /* omap_plane_mode_set() takes adjusted src */ >> + switch (omap_plane->win.rotation & 0xf) { >> + case BIT(DRM_ROTATE_90): >> + case BIT(DRM_ROTATE_270): >> + swap(src_w, src_h); >> + break; >> + } >> + > > hmm, I think that the better thing would be to do this in > omap_framebuffer_update_scanout(). In fact we do already swap > src_w/src_h there. Only we don't swap the width/height parameters we > pass down to omapdss. Not quite sure if something changed in omapdss > with regards to rotation_type, but keeping it with the rest of the > rotation related maths in _update_scanout() seems cleaner. But then it has to know somehow if apply was triggered from omap_crtc_mode_set() or omap_plane_update(). The problem is omap_crtc_mode_set() passes rotated width/height (and crtc rotation works correctly), but omap_plane_update() uses unrotated width/height (so plane rotation is currently broken). Gra?vydas
[RFCv3 12/14] drm: Specify cursor plane at CRTC initialization
On Mon, Apr 7, 2014 at 6:05 AM, Thierry Reding wrote: > On Fri, Mar 28, 2014 at 10:04:09PM +0100, Daniel Vetter wrote: >> On Tue, Mar 18, 2014 at 05:22:57PM -0700, Matt Roper wrote: >> > Add cursor plane as a parameter to drm_crtc_init() and update all >> > existing DRM drivers to use a helper-provided primary plane. Passing >> > NULL for this parameter indicates that there is no hardware cursor >> > supported by the driver and no cursor plane should be provided to >> > userspace. >> > >> > Signed-off-by: Matt Roper >> >> Ok, cursor planes. I've poked around in this a lot and unfortunately I >> don't think we can achieve nirvana :( > [...] >> - For the pixel format I think it's ok to always assume RGBA. > > How so? We do have hardware on Tegra that doesn't support RGBA cursors > and if at all possible I'd very much like to support that as well. New > generations can do RGBA but still support the old pixel format. Having > the option of supporting all formats would be nice. > > There was some discussion about implementing a bunch of plane properties > so I think having one to enumerate a set of pixel formats wouldn't be > such a big deal. No need for properties to expose formats, we already have that in getplane ioctl. But would be nice to support more than just RGBA. BR, -R > Thierry > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
[PATCH] drm/omap: fix plane rotation
On Mon, Apr 7, 2014 at 6:32 AM, Grazvydas Ignotas wrote: > On Sun, Apr 6, 2014 at 12:45 AM, Rob Clark wrote: >> On Sat, Apr 5, 2014 at 2:33 PM, Grazvydas Ignotas >> wrote: >>> Plane rotation with omapdrm is currently broken. >>> It seems omap_plane_mode_set() expects width and height in screen >>> coordinates, so pass it like that. >>> >>> Cc: Rob Clark >>> Signed-off-by: Grazvydas Ignotas >>> --- >>> drivers/gpu/drm/omapdrm/omap_plane.c |8 >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c >>> b/drivers/gpu/drm/omapdrm/omap_plane.c >>> index 370580c..5611f15 100644 >>> --- a/drivers/gpu/drm/omapdrm/omap_plane.c >>> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c >>> @@ -253,6 +253,14 @@ static int omap_plane_update(struct drm_plane *plane, >>> >>> drm_framebuffer_reference(fb); >>> >>> + /* omap_plane_mode_set() takes adjusted src */ >>> + switch (omap_plane->win.rotation & 0xf) { >>> + case BIT(DRM_ROTATE_90): >>> + case BIT(DRM_ROTATE_270): >>> + swap(src_w, src_h); >>> + break; >>> + } >>> + >> >> hmm, I think that the better thing would be to do this in >> omap_framebuffer_update_scanout(). In fact we do already swap >> src_w/src_h there. Only we don't swap the width/height parameters we >> pass down to omapdss. Not quite sure if something changed in omapdss >> with regards to rotation_type, but keeping it with the rest of the >> rotation related maths in _update_scanout() seems cleaner. > > But then it has to know somehow if apply was triggered from > omap_crtc_mode_set() or omap_plane_update(). The problem is > omap_crtc_mode_set() passes rotated width/height (and crtc rotation > works correctly), but omap_plane_update() uses unrotated width/height > (so plane rotation is currently broken). Something still seems a bit suspicious.. drm core is not swapping width/height for crtc (other than for validating the configuration... which might also be needed for planes). I guess it is getting already-rotated width/height from userspace. So probably we want to do the same for planes, so it might be a good idea to move crtc->invert_dimensions into plane. BR, -R. > > Gra?vydas
[PATCH 01/13] drm/mgag200: Remove unnecessary NULL check in bo_unref
On 04/07/2014 12:56 PM, Daniel Vetter wrote: > On Mon, Apr 7, 2014 at 5:19 PM, Ian Romanick wrote: >> On 04/05/2014 02:44 AM, Daniel Vetter wrote: >>> ttm_bo_unref unconditionally calls kref_put on it's argument, so the >>> thing can't be NULL without already causing Oopses. >> >> Doesn't this mean the NULL check is in the wrong place (rather than the >> NULL check should be removed)? > > Afaics chasing callchains it's a bug to call this with NULL pointer > and no one really should be doing it. Like David Herrmann said it's > sometimes useful if unref/free functions automatically cope with NULL, > but ttm buffers don't seem to be of this kind. So consistency with > other ttm drivers seems better, same with all the gem_free_object > callbacks. That's fair. I'm convinced. Patches 1, 3, and 5 are also Reviewed-by: Ian Romanick > -Daniel
[PATCH] DRM: Armada: Use devm_ioremap_resource()
On Thu, Apr 03, 2014 at 09:31:04AM +0900, Jingoo Han wrote: > Use devm_ioremap_resource() because devm_request_and_ioremap() is > obsoleted by devm_ioremap_resource(). > > Signed-off-by: Jingoo Han > --- > drivers/gpu/drm/armada/armada_crtc.c |6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/armada/armada_crtc.c > b/drivers/gpu/drm/armada/armada_crtc.c > index d8e3982..23b0123 100644 > --- a/drivers/gpu/drm/armada/armada_crtc.c > +++ b/drivers/gpu/drm/armada/armada_crtc.c > @@ -1037,10 +1037,10 @@ int armada_drm_crtc_create(struct drm_device *dev, > unsigned num, > if (ret) > return ret; > > - base = devm_request_and_ioremap(dev->dev, res); > - if (!base) { > + base = devm_ioremap_resource(dev->dev, res); > + if (IS_ERR(base)) { > DRM_ERROR("failed to ioremap register\n"); > - return -ENOMEM; > + return PTR_ERR(base); While at it, perhaps you should drop the error message too because devm_ioremap_resource() already prints one in all failure cases. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/ae3b2dc3/attachment.sig>
[PATCH] drm/i915: support address only i2c-over-aux transactions
To support bare address requests used by the drm dp helpers. Signed-off-by: Jani Nikula --- Hi Alex, similar to Thierry's patch for i915. BR, Jani. --- drivers/gpu/drm/i915/intel_dp.c |7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index e48d47ced57b..b435d07fbc08 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -575,7 +575,8 @@ out: return ret; } -#define HEADER_SIZE4 +#define BARE_ADDRESS_SIZE 3 +#define HEADER_SIZE(BARE_ADDRESS_SIZE + 1) static ssize_t intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) { @@ -592,7 +593,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) switch (msg->request & ~DP_AUX_I2C_MOT) { case DP_AUX_NATIVE_WRITE: case DP_AUX_I2C_WRITE: - txsize = HEADER_SIZE + msg->size; + txsize = msg->size ? HEADER_SIZE + msg->size : BARE_ADDRESS_SIZE; rxsize = 1; if (WARN_ON(txsize > 20)) @@ -611,7 +612,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) case DP_AUX_NATIVE_READ: case DP_AUX_I2C_READ: - txsize = HEADER_SIZE; + txsize = msg->size ? HEADER_SIZE : BARE_ADDRESS_SIZE; rxsize = msg->size + 1; if (WARN_ON(rxsize > 20)) -- 1.7.9.5
[PATCH v5 00/11] imx-drm dt bindings
On Tue, Mar 11, 2014 at 11:46:11AM +0800, Shawn Guo wrote: > I just came across a couple problems when testing the series on > my imx6dl-sabresd board in dual display case - HDMI + LVDS. I tested it > using Russell's branch below, which I believe has all the pieces put > together. > > git://ftp.arm.linux.org.uk/~rmk/linux-arm.git imx-drm-staging > > - When I enable HDMI and LVDS support in both kernel build and device > tree, HDMI seems working fine but LVDS color is corrupted quite badly. Philipp, Did you get any chance to reproduce this dual display issue? Now it shows on mainline kernel. And I see another HDMI regression with my testing on mainline kernel. I can have my HDMI work at 1920x1080 with v3.14 kernel, but it can only probes 1024x768 with the mainline today. The Xorg.0.log are attached below. The hardware and user space are same, so I guess this is another issue introduced by the recently kernel driver changes? Shawn mainline kernel === [20.606] (II) LoadModule: "modesetting" [20.607] (II) Loading /usr/lib/xorg/modules/drivers/modesetting_drv.so [20.609] (II) Module modesetting: vendor="X.Org Foundation" [20.609]compiled for 1.12.1.902, module version = 0.3.0 [20.610]Module class: X.Org Video Driver [20.610]ABI class: X.Org Video Driver, version 12.0 [20.610] (II) modesetting: Driver for Modesetting Kernel Drivers: kms [20.610] (++) using VT number 7 [20.624] (WW) Falling back to old probe method for modesetting [20.624] (II) modesetting(0): using default device [20.627] (II) modesetting(0): Creating default Display subsection in Screen section "Default Screen Section" for depth/fbbpp 24/32 [20.627] (==) modesetting(0): Depth 24, (==) framebuffer bpp 32 [20.628] (==) modesetting(0): RGB weight 888 [20.628] (==) modesetting(0): Default visual is TrueColor [20.628] (II) modesetting(0): ShadowFB: preferred NO, enabled NO [20.628] (II) modesetting(0): Output HDMI-0 has no monitor section [20.629] (II) modesetting(0): EDID for output HDMI-0 [20.629] (II) modesetting(0): Printing probed modes for output HDMI-0 [20.629] (II) modesetting(0): Modeline "1024x768"x60.0 65.00 1024 1048 1184 1344 768 771 777 806 -hsync -vsync (48.4 kHz e) [20.629] (II) modesetting(0): Modeline "800x600"x60.3 40.00 800 840 968 1056 600 601 605 628 +hsync +vsync (37.9 kHz e) [20.629] (II) modesetting(0): Modeline "800x600"x56.2 36.00 800 824 896 1024 600 601 603 625 +hsync +vsync (35.2 kHz e) [20.630] (II) modesetting(0): Modeline "848x480"x60.0 33.75 848 864 976 1088 480 486 494 517 +hsync +vsync (31.0 kHz e) [20.630] (II) modesetting(0): Modeline "640x480"x59.9 25.18 640 656 752 800 480 489 492 525 -hsync -vsync (31.5 kHz e) [20.630] (II) modesetting(0): Output HDMI-0 connected [20.630] (II) modesetting(0): Using exact sizes for initial modes [20.630] (II) modesetting(0): Output HDMI-0 using initial mode 1024x768 [20.630] (II) modesetting(0): Using default gamma of (1.0, 1.0, 1.0) unless otherwise stated. [20.630] (==) modesetting(0): DPI set to (96, 96) v3.14 kernel [20.214] (II) LoadModule: "modesetting" [20.215] (II) Loading /usr/lib/xorg/modules/drivers/modesetting_drv.so [20.217] (II) Module modesetting: vendor="X.Org Foundation" [20.217]compiled for 1.12.1.902, module version = 0.3.0 [20.217]Module class: X.Org Video Driver [20.217]ABI class: X.Org Video Driver, version 12.0 [20.217] (II) modesetting: Driver for Modesetting Kernel Drivers: kms [20.217] (++) using VT number 7 [20.240] (WW) Falling back to old probe method for modesetting [20.241] (II) modesetting(0): using default device [20.241] (WW) VGA arbiter: cannot open kernel arbiter, no multi-card support [20.244] (II) modesetting(0): Creating default Display subsection in Screen section "Default Screen Section" for depth/fbbpp 24/32 [20.244] (==) modesetting(0): Depth 24, (==) framebuffer bpp 32 [20.244] (==) modesetting(0): RGB weight 888 [20.244] (==) modesetting(0): Default visual is TrueColor [20.244] (II) modesetting(0): ShadowFB: preferred NO, enabled NO [20.282] (II) modesetting(0): Output HDMI-0 has no monitor section [20.329] (II) modesetting(0): EDID for output HDMI-0 [20.330] (II) modesetting(0): Manufacturer: RAW Model: 0 Serial#: 1 [20.330] (II) modesetting(0): Year: 2012 Week: 6 [20.330] (II) modesetting(0): EDID Version: 1.3 [20.331] (II) modesetting(0): Digital Display Input [20.331] (II) modesetting(0): Indeterminate output size [20.331] (II) modesetting(0): Gamma: 2.20 [20.331] (II) modesetting(0): No DPMS capabilities specified [20.331] (II) modesetting(0): Supported color encodings: RGB 4:4:4 YCrCb 4:4:4 [20.332] (II) modesetting(0): First detailed timing is preferred mode [20.332] (II) modesetting(0): r
[RFCv3 12/14] drm: Specify cursor plane at CRTC initialization
On Fri, Mar 28, 2014 at 10:04:09PM +0100, Daniel Vetter wrote: > On Tue, Mar 18, 2014 at 05:22:57PM -0700, Matt Roper wrote: > > Add cursor plane as a parameter to drm_crtc_init() and update all > > existing DRM drivers to use a helper-provided primary plane. Passing > > NULL for this parameter indicates that there is no hardware cursor > > supported by the driver and no cursor plane should be provided to > > userspace. > > > > Signed-off-by: Matt Roper > > Ok, cursor planes. I've poked around in this a lot and unfortunately I > don't think we can achieve nirvana :( [...] > - For the pixel format I think it's ok to always assume RGBA. How so? We do have hardware on Tegra that doesn't support RGBA cursors and if at all possible I'd very much like to support that as well. New generations can do RGBA but still support the old pixel format. Having the option of supporting all formats would be nice. There was some discussion about implementing a bunch of plane properties so I think having one to enumerate a set of pixel formats wouldn't be such a big deal. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/a9808a95/attachment.sig>
[PATCH v5 00/11] imx-drm dt bindings
Hi Shawn, Am Montag, den 07.04.2014, 12:23 +0800 schrieb Shawn Guo: > On Tue, Mar 11, 2014 at 11:46:11AM +0800, Shawn Guo wrote: > > I just came across a couple problems when testing the series on > > my imx6dl-sabresd board in dual display case - HDMI + LVDS. I tested it > > using Russell's branch below, which I believe has all the pieces put > > together. > > > > git://ftp.arm.linux.org.uk/~rmk/linux-arm.git imx-drm-staging > > > > - When I enable HDMI and LVDS support in both kernel build and device > > tree, HDMI seems working fine but LVDS color is corrupted quite badly. > > Philipp, > > Did you get any chance to reproduce this dual display issue? Now it > shows on mainline kernel. I have not yet reproduced, but I've sent a patch today ("imx-drm: imx-drm-core: Fix imx_drm_encoder_get_mux_id") that might fix this. > And I see another HDMI regression with my testing on mainline kernel. I > can have my HDMI work at 1920x1080 with v3.14 kernel, but it can only > probes 1024x768 with the mainline today. The Xorg.0.log are attached > below. The hardware and user space are same, so I guess this is another > issue introduced by the recently kernel driver changes? Hmm, not sure. Have you updated to the i2c-ddc-bus phandle property as described in Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt? regards Philipp
[PATCH] DRM: armada: fix corruption while loading cursors
Loading cursors to the LCD controller's SRAM can be corrupted when the configured pixel clock is relatively slow. This seems to be caused when we write back-to-back to the SRAM registers. There doesn't appear to be any status register we can read to check when an access has completed. Inserting a dummy read between the writes appears to fix the problem. Cc: # 3.13 Signed-off-by: Russell King --- David, Could you take this patch - it's also a good idea that it's back ported to stable kernels as it affects all releases which this driver has been in. I don't see the need for a pull request as this is just a single patch. Thanks. drivers/gpu/drm/armada/armada_crtc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 19ffd5c22944..357e5eb59e89 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -678,6 +678,7 @@ static void armada_load_cursor_argb(void __iomem *base, uint32_t *pix, base + LCD_SPU_SRAM_WRDAT); writel_relaxed(addr | SRAM_WRITE, base + LCD_SPU_SRAM_CTRL); + readl_relaxed(base + LCD_SPU_HWC_OVSA_HPXL_VLN); addr += 1; if ((addr & 0x00ff) == 0) addr += 0xf00; -- 1.8.3.1
[PATCH V2] gpu: host1x: handle the correct # of syncpt regs
On 07.04.2014 11:41, Thierry Reding wrote: > On Mon, Apr 07, 2014 at 11:34:22AM +0300, Terje Bergstr?m wrote: >> On 07.04.2014 11:18, Thierry Reding wrote: >>> If I understand correctly there's no immediate need for this to go to >>> stable kernels, nor for it to be queued for 3.15, right? That is the >>> potential extra write isn't causing any harm on actual hardware, is it? >>> >>> In that case I'll queue this up for 3.16. >> >> The reads and writes would get ignored on 32-bit kernel. The change does >> fix sync point behavior in 64-bit kernel, so it is fixing a real issue. > > Okay, but given that we don't support any 64 bit Tegra hardware upstream > yet, 3.16 would still be enough, wouldn't it? Sure, sounds good. Terje
[PATCH V2] gpu: host1x: handle the correct # of syncpt regs
On 07.04.2014 11:18, Thierry Reding wrote: > If I understand correctly there's no immediate need for this to go to > stable kernels, nor for it to be queued for 3.15, right? That is the > potential extra write isn't causing any harm on actual hardware, is it? > > In that case I'll queue this up for 3.16. The reads and writes would get ignored on 32-bit kernel. The change does fix sync point behavior in 64-bit kernel, so it is fixing a real issue. Terje
[PATCH V2] gpu: host1x: handle the correct # of syncpt regs
On 05.04.2014 01:31, Stephen Warren wrote: > From: Stephen Warren > > diff --git a/drivers/gpu/host1x/hw/intr_hw.c b/drivers/gpu/host1x/hw/intr_hw.c > index db9017adfe2b..498b37e39058 100644 > --- a/drivers/gpu/host1x/hw/intr_hw.c > +++ b/drivers/gpu/host1x/hw/intr_hw.c > @@ -47,7 +47,7 @@ static irqreturn_t syncpt_thresh_isr(int irq, void *dev_id) > unsigned long reg; > int i, id; > > - for (i = 0; i <= BIT_WORD(host->info->nb_pts); i++) { > + for (i = 0; i < DIV_ROUND_UP(host->info->nb_pts, 32); i++) { > reg = host1x_sync_readl(host, > HOST1X_SYNC_SYNCPT_THRESH_CPU0_INT_STATUS(i)); > for_each_set_bit(id, ®, BITS_PER_LONG) { > @@ -64,7 +64,7 @@ static void _host1x_intr_disable_all_syncpt_intrs(struct > host1x *host) > { > u32 i; > > - for (i = 0; i <= BIT_WORD(host->info->nb_pts); ++i) { > + for (i = 0; i < DIV_ROUND_UP(host->info->nb_pts, 32); ++i) { > host1x_sync_writel(host, 0xu, > HOST1X_SYNC_SYNCPT_THRESH_INT_DISABLE(i)); > host1x_sync_writel(host, 0xu, > Acked-By: Terje Bergstrom Terje
Required backwards support level?
On Son, 2014-04-06 at 15:26 +0300, Lauri Kasanen wrote: > Hi, > > Obviously old userspace + new kernel combo needs to be supported. But > I'm not so sure about a mixed case, does old userspace + new userspace > need to be supported to run at the same time? > > For example, a new 64-bit mesa+libdrm and a 32-bit old set, both > running apps at the same time. > > Or is it acceptable to assume that once a new userspace has been run, > an old one won't be run for this boot? I'm afraid not. Trivial counter-example: Build Git snapshot from after your changes, hit regression (not necessarily related to your changes), run git bisect involving snapshots from before your changes. -- Earthling Michel D?nzer| http://www.amd.com Libre software enthusiast |Mesa and X developer
[PATCH 1/4] drm/radeon/dp: handle zero sized i2c over aux transactions
On Fri, 04 Apr 2014, Alex Deucher wrote: > Needed for proper i2c over aux handling for certain > monitors and configurations (e.g., dp bridges or > adapters). > > Signed-off-by: Alex Deucher > --- > drivers/gpu/drm/radeon/atombios_dp.c | 15 +++ > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/atombios_dp.c > b/drivers/gpu/drm/radeon/atombios_dp.c > index 8b0ab17..e914008 100644 > --- a/drivers/gpu/drm/radeon/atombios_dp.c > +++ b/drivers/gpu/drm/radeon/atombios_dp.c > @@ -143,6 +143,7 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan > *chan, > } > > #define HEADER_SIZE 4 > +#define BARE_ADDRESS_SIZE 3 > > static ssize_t > radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > @@ -160,13 +161,16 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct > drm_dp_aux_msg *msg) > tx_buf[0] = msg->address & 0xff; > tx_buf[1] = msg->address >> 8; > tx_buf[2] = msg->request << 4; > - tx_buf[3] = msg->size - 1; > + tx_buf[3] = msg->size ? (msg->size - 1) : 0; > > switch (msg->request & ~DP_AUX_I2C_MOT) { > case DP_AUX_NATIVE_WRITE: > case DP_AUX_I2C_WRITE: > tx_size = HEADER_SIZE + msg->size; > - tx_buf[3] |= tx_size << 4; > + if (msg->size == 0) > + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; > + else > + tx_buf[3] |= tx_size << 4; > memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size); > ret = radeon_process_aux_ch(chan, > tx_buf, tx_size, NULL, 0, delay, > &ack); I think your tz_size and tx_buf[3] are confused. Shouldn't you only send 3 bytes of tx_buf when msg->size == 0? Disclaimer, I don't know anything about your hw and all that... :) BR, Jani. > @@ -177,7 +181,10 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct > drm_dp_aux_msg *msg) > case DP_AUX_NATIVE_READ: > case DP_AUX_I2C_READ: > tx_size = HEADER_SIZE; > - tx_buf[3] |= tx_size << 4; > + if (msg->size == 0) > + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; > + else > + tx_buf[3] |= tx_size << 4; > ret = radeon_process_aux_ch(chan, > tx_buf, tx_size, msg->buffer, > msg->size, delay, &ack); > break; > @@ -186,7 +193,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct > drm_dp_aux_msg *msg) > break; > } > > - if (ret > 0) > + if (ret >= 0) > msg->reply = ack >> 4; > > return ret; > -- > 1.8.3.1 > -- Jani Nikula, Intel Open Source Technology Center
[PATCH 2/4] drm/dp/i2c: send bare addresses to properly reset i2c connections (v3)
On Fri, Apr 04, 2014 at 03:58:37PM -0400, Alex Deucher wrote: [...] > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c [...] > + /* send a bare address packet to close out the connection */ Missed this one earlier. Perhaps s/connection/transaction/? Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/eb0a812c/attachment.sig>
[PATCH V2] gpu: host1x: handle the correct # of syncpt regs
On Mon, Apr 07, 2014 at 11:34:22AM +0300, Terje Bergstr?m wrote: > On 07.04.2014 11:18, Thierry Reding wrote: > > If I understand correctly there's no immediate need for this to go to > > stable kernels, nor for it to be queued for 3.15, right? That is the > > potential extra write isn't causing any harm on actual hardware, is it? > > > > In that case I'll queue this up for 3.16. > > The reads and writes would get ignored on 32-bit kernel. The change does > fix sync point behavior in 64-bit kernel, so it is fixing a real issue. Okay, but given that we don't support any 64 bit Tegra hardware upstream yet, 3.16 would still be enough, wouldn't it? Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/77c9b71b/attachment.sig>
[PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions
From: Thierry Reding Certain types of I2C-over-AUX transactions require that only the address is transferred. Detect this by looking at the AUX message's size and set the address-only bit appropriately. Signed-off-by: Thierry Reding --- Hi Alex, This patch is required to make eDP work properly on Tegra with your I2C- over-AUX patch series applied. Would you consider carrying this in your series so that bisectability can be maintained? It would need to be applied prior to the core change (patch 2/4 in the latest series) to do so. Thanks, Thierry drivers/gpu/drm/tegra/dpaux.c | 44 ++- drivers/gpu/drm/tegra/dpaux.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c index d536ed381fbd..005c19bd92df 100644 --- a/drivers/gpu/drm/tegra/dpaux.c +++ b/drivers/gpu/drm/tegra/dpaux.c @@ -99,55 +99,73 @@ static void tegra_dpaux_read_fifo(struct tegra_dpaux *dpaux, u8 *buffer, static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) { - unsigned long value = DPAUX_DP_AUXCTL_TRANSACTREQ; unsigned long timeout = msecs_to_jiffies(250); struct tegra_dpaux *dpaux = to_dpaux(aux); unsigned long status; ssize_t ret = 0; + u32 value; - if (msg->size < 1 || msg->size > 16) + /* Tegra has 4x4 byte DP AUX transmit and receive FIFOs. */ + if (msg->size > 16) return -EINVAL; - tegra_dpaux_writel(dpaux, msg->address, DPAUX_DP_AUXADDR); + /* +* Allow zero-sized messages only for I2C, in which case they specify +* address-only transactions. +*/ + if (msg->size < 1) { + switch (msg->request & ~DP_AUX_I2C_MOT) { + case DP_AUX_I2C_WRITE: + case DP_AUX_I2C_READ: + value = DPAUX_DP_AUXCTL_CMD_ADDRESS_ONLY; + break; + + default: + return -EINVAL; + } + } else { + /* For non-zero-sized messages, set the CMDLEN field. */ + value = DPAUX_DP_AUXCTL_CMDLEN(msg->size - 1); + } switch (msg->request & ~DP_AUX_I2C_MOT) { case DP_AUX_I2C_WRITE: if (msg->request & DP_AUX_I2C_MOT) - value = DPAUX_DP_AUXCTL_CMD_MOT_WR; + value |= DPAUX_DP_AUXCTL_CMD_MOT_WR; else - value = DPAUX_DP_AUXCTL_CMD_I2C_WR; + value |= DPAUX_DP_AUXCTL_CMD_I2C_WR; break; case DP_AUX_I2C_READ: if (msg->request & DP_AUX_I2C_MOT) - value = DPAUX_DP_AUXCTL_CMD_MOT_RD; + value |= DPAUX_DP_AUXCTL_CMD_MOT_RD; else - value = DPAUX_DP_AUXCTL_CMD_I2C_RD; + value |= DPAUX_DP_AUXCTL_CMD_I2C_RD; break; case DP_AUX_I2C_STATUS: if (msg->request & DP_AUX_I2C_MOT) - value = DPAUX_DP_AUXCTL_CMD_MOT_RQ; + value |= DPAUX_DP_AUXCTL_CMD_MOT_RQ; else - value = DPAUX_DP_AUXCTL_CMD_I2C_RQ; + value |= DPAUX_DP_AUXCTL_CMD_I2C_RQ; break; case DP_AUX_NATIVE_WRITE: - value = DPAUX_DP_AUXCTL_CMD_AUX_WR; + value |= DPAUX_DP_AUXCTL_CMD_AUX_WR; break; case DP_AUX_NATIVE_READ: - value = DPAUX_DP_AUXCTL_CMD_AUX_RD; + value |= DPAUX_DP_AUXCTL_CMD_AUX_RD; break; default: return -EINVAL; } - value |= DPAUX_DP_AUXCTL_CMDLEN(msg->size - 1); + tegra_dpaux_writel(dpaux, msg->address, DPAUX_DP_AUXADDR); tegra_dpaux_writel(dpaux, value, DPAUX_DP_AUXCTL); if ((msg->request & DP_AUX_I2C_READ) == 0) { @@ -198,7 +216,7 @@ static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux, break; } - if (msg->reply == DP_AUX_NATIVE_REPLY_ACK) { + if ((msg->size > 0) && (msg->reply == DP_AUX_NATIVE_REPLY_ACK)) { if (msg->request & DP_AUX_I2C_READ) { size_t count = value & DPAUX_DP_AUXSTAT_REPLY_MASK; diff --git a/drivers/gpu/drm/tegra/dpaux.h b/drivers/gpu/drm/tegra/dpaux.h index 4f5bf10fdff9..806e245ca787 100644 --- a/drivers/gpu/drm/tegra/dpaux.h +++ b/drivers/gpu/drm/tegra/dpaux.h @@ -32,6 +32,7 @@ #define DPAUX_DP_AUXCTL_CMD_I2C_RQ (2 << 12) #define DPAUX_DP_AUXCTL_CMD_I2C_RD (1 << 12) #define DPAUX_DP_AUXCTL_CMD_I2C_WR (0 << 12) +#define DPAUX_DP_AUXCTL_CMD_ADDRESS_ONLY (1 << 8) #define DPAUX_DP_AUXCTL_CMDLEN(x) ((x) & 0xff) #define DPAUX_DP_AUXSTAT 0x31 -- 1.9.1
[PATCH] drm/i915: support address only i2c-over-aux transactions
On Mon, Apr 7, 2014 at 5:37 AM, Jani Nikula wrote: > To support bare address requests used by the drm dp helpers. > > Signed-off-by: Jani Nikula > > --- > > Hi Alex, similar to Thierry's patch for i915. > Looks good to me. Reviewed-by: Alex Deucher Do you want me to add this to the patch set? Alex > BR, > Jani. > --- > drivers/gpu/drm/i915/intel_dp.c |7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index e48d47ced57b..b435d07fbc08 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -575,7 +575,8 @@ out: > return ret; > } > > -#define HEADER_SIZE4 > +#define BARE_ADDRESS_SIZE 3 > +#define HEADER_SIZE(BARE_ADDRESS_SIZE + 1) > static ssize_t > intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > { > @@ -592,7 +593,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct > drm_dp_aux_msg *msg) > switch (msg->request & ~DP_AUX_I2C_MOT) { > case DP_AUX_NATIVE_WRITE: > case DP_AUX_I2C_WRITE: > - txsize = HEADER_SIZE + msg->size; > + txsize = msg->size ? HEADER_SIZE + msg->size : > BARE_ADDRESS_SIZE; > rxsize = 1; > > if (WARN_ON(txsize > 20)) > @@ -611,7 +612,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct > drm_dp_aux_msg *msg) > > case DP_AUX_NATIVE_READ: > case DP_AUX_I2C_READ: > - txsize = HEADER_SIZE; > + txsize = msg->size ? HEADER_SIZE : BARE_ADDRESS_SIZE; > rxsize = msg->size + 1; > > if (WARN_ON(rxsize > 20)) > -- > 1.7.9.5 >
[PATCH 4/4] drm/radeon/dp: switch to the common i2c over aux code
Provides a nice cleanup in radeon. Signed-off-by: Alex Deucher --- drivers/gpu/drm/radeon/atombios_dp.c | 117 + drivers/gpu/drm/radeon/radeon_connectors.c | 44 ++- drivers/gpu/drm/radeon/radeon_display.c| 11 ++- drivers/gpu/drm/radeon/radeon_i2c.c| 60 --- drivers/gpu/drm/radeon/radeon_mode.h | 12 +-- 5 files changed, 44 insertions(+), 200 deletions(-) diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c index e448304..1593652 100644 --- a/drivers/gpu/drm/radeon/atombios_dp.c +++ b/drivers/gpu/drm/radeon/atombios_dp.c @@ -207,98 +207,15 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) void radeon_dp_aux_init(struct radeon_connector *radeon_connector) { - struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv; - - dig_connector->dp_i2c_bus->aux.dev = radeon_connector->base.kdev; - dig_connector->dp_i2c_bus->aux.transfer = radeon_dp_aux_transfer; -} - -int radeon_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode, -u8 write_byte, u8 *read_byte) -{ - struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data; - struct radeon_i2c_chan *auxch = i2c_get_adapdata(adapter); - u16 address = algo_data->address; - u8 msg[5]; - u8 reply[2]; - unsigned retry; - int msg_bytes; - int reply_bytes = 1; int ret; - u8 ack; - - /* Set up the address */ - msg[0] = address; - msg[1] = address >> 8; - - /* Set up the command byte */ - if (mode & MODE_I2C_READ) { - msg[2] = DP_AUX_I2C_READ << 4; - msg_bytes = 4; - msg[3] = msg_bytes << 4; - } else { - msg[2] = DP_AUX_I2C_WRITE << 4; - msg_bytes = 5; - msg[3] = msg_bytes << 4; - msg[4] = write_byte; - } - /* special handling for start/stop */ - if (mode & (MODE_I2C_START | MODE_I2C_STOP)) - msg[3] = 3 << 4; - - /* Set MOT bit for all but stop */ - if ((mode & MODE_I2C_STOP) == 0) - msg[2] |= DP_AUX_I2C_MOT << 4; - - for (retry = 0; retry < 7; retry++) { - ret = radeon_process_aux_ch(auxch, - msg, msg_bytes, reply, reply_bytes, 0, &ack); - if (ret == -EBUSY) - continue; - else if (ret < 0) { - DRM_DEBUG_KMS("aux_ch failed %d\n", ret); - return ret; - } + radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev; + radeon_connector->ddc_bus->aux.transfer = radeon_dp_aux_transfer; + ret = drm_dp_aux_register_i2c_bus(&radeon_connector->ddc_bus->aux); + if (!ret) + radeon_connector->ddc_bus->has_aux = true; - switch ((ack >> 4) & DP_AUX_NATIVE_REPLY_MASK) { - case DP_AUX_NATIVE_REPLY_ACK: - /* I2C-over-AUX Reply field is only valid -* when paired with AUX ACK. -*/ - break; - case DP_AUX_NATIVE_REPLY_NACK: - DRM_DEBUG_KMS("aux_ch native nack\n"); - return -EREMOTEIO; - case DP_AUX_NATIVE_REPLY_DEFER: - DRM_DEBUG_KMS("aux_ch native defer\n"); - usleep_range(500, 600); - continue; - default: - DRM_ERROR("aux_ch invalid native reply 0x%02x\n", ack); - return -EREMOTEIO; - } - - switch ((ack >> 4) & DP_AUX_I2C_REPLY_MASK) { - case DP_AUX_I2C_REPLY_ACK: - if (mode == MODE_I2C_READ) - *read_byte = reply[0]; - return ret; - case DP_AUX_I2C_REPLY_NACK: - DRM_DEBUG_KMS("aux_i2c nack\n"); - return -EREMOTEIO; - case DP_AUX_I2C_REPLY_DEFER: - DRM_DEBUG_KMS("aux_i2c defer\n"); - usleep_range(400, 500); - break; - default: - DRM_ERROR("aux_i2c invalid reply 0x%02x\n", ack); - return -EREMOTEIO; - } - } - - DRM_DEBUG_KMS("aux i2c too many retries, giving up\n"); - return -EREMOTEIO; + WARN(ret, "drm_dp_aux_register_i2c_bus() failed with error %d\n", ret); } /* general DP utility functions */ @@ -433,12 +350,11 @@ static u8 radeon_dp_encoder_service(struct radeon_device *rdev, u8 radeon_dp_getsinktype(struct radeon_connector *radeon_connector) { - struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv; struct drm_devi
[PATCH 3/4] drm/dp/i2c: Update comments about common i2c over dp assumptions (v2)
If you are using the common dp over i2c functionality, it is asumed that the aux transfer function does not modify the any of the msg structure other than the reply field. Doing so breaks the logic in the common code. v2: update struct drm_dp_aux comments about assumptions Signed-off-by: Alex Deucher Cc: Ville Syrj?l? Cc: Jani Nikula Cc: Thierry Reding Reviewed-by: Ville Syrj?l? --- drivers/gpu/drm/drm_dp_helper.c | 4 +++- include/drm/drm_dp_helper.h | 4 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 6122a4f..e1c1347 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -577,7 +577,9 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter) /* * Transfer a single I2C-over-AUX message and handle various error conditions, - * retrying the transaction as appropriate. + * retrying the transaction as appropriate. It is assumed that the + * aux->transfer function does not modify anything in the msg other than the + * reply field. */ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) { diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index b7488c9..10d67018 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -443,6 +443,10 @@ struct drm_dp_aux_msg { * transactions. The drm_dp_aux_register_i2c_bus() function registers an * I2C adapter that can be passed to drm_probe_ddc(). Upon removal, drivers * should call drm_dp_aux_unregister_i2c_bus() to remove the I2C adapter. + * + * Note that the aux helper code assumes that the .transfer() function + * only modifies the reply field of the drm_dp_aux_msg structure. The + * retry logic and i2c helpers assume this is the case. */ struct drm_dp_aux { struct i2c_adapter ddc; -- 1.8.3.1
[PATCH 2/4] drm/dp/i2c: send bare addresses to properly reset i2c connections (v4)
We need bare address packets at the start and end of each i2c over aux transaction to properly reset the connection between transactions. This mirrors what the existing dp i2c over aux algo currently does. This fixes EDID fetches on certain monitors especially with dp bridges. v2: update as per Ville's comments - Set buffer to NULL for zero sized packets - abort the entre transaction if one of the messages fails v3: drop leftover debugging code v4: integrate Thierry's comments - add comments about address only transactions - switch back to i and j Signed-off-by: Alex Deucher Cc: Ville Syrj?l? Cc: Jani Nikula Cc: Thierry Reding Reviewed-by: Ville Syrj?l? --- drivers/gpu/drm/drm_dp_helper.c | 51 ++--- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 74724aa..6122a4f 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -665,11 +665,26 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, { struct drm_dp_aux *aux = adapter->algo_data; unsigned int i, j; + struct drm_dp_aux_msg msg; + int err = 0; - for (i = 0; i < num; i++) { - struct drm_dp_aux_msg msg; - int err; + memset(&msg, 0, sizeof(msg)); + for (i = 0; i < num; i++) { + msg.address = msgs[i].addr; + msg.request = (msgs[i].flags & I2C_M_RD) ? + DP_AUX_I2C_READ : + DP_AUX_I2C_WRITE; + msg.request |= DP_AUX_I2C_MOT; + /* Send a bare address packet to start the transaction. +* Zero sized messages specify an address only (bare +* address) transaction. +*/ + msg.buffer = NULL; + msg.size = 0; + err = drm_dp_i2c_do_msg(aux, &msg); + if (err < 0) + break; /* * Many hardware implementations support FIFOs larger than a * single byte, but it has been empirically determined that @@ -678,30 +693,28 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, * transferred byte-by-byte. */ for (j = 0; j < msgs[i].len; j++) { - memset(&msg, 0, sizeof(msg)); - msg.address = msgs[i].addr; - - msg.request = (msgs[i].flags & I2C_M_RD) ? - DP_AUX_I2C_READ : - DP_AUX_I2C_WRITE; - - /* -* All messages except the last one are middle-of- -* transfer messages. -*/ - if ((i < num - 1) || (j < msgs[i].len - 1)) - msg.request |= DP_AUX_I2C_MOT; - msg.buffer = msgs[i].buf + j; msg.size = 1; err = drm_dp_i2c_do_msg(aux, &msg); if (err < 0) - return err; + break; } + if (err < 0) + break; } + if (err >= 0) + err = num; + /* Send a bare address packet to close out the transaction. +* Zero sized messages specify an address only (bare +* address) transaction. +*/ + msg.request &= ~DP_AUX_I2C_MOT; + msg.buffer = NULL; + msg.size = 0; + (void)drm_dp_i2c_do_msg(aux, &msg); - return num; + return err; } static const struct i2c_algorithm drm_dp_i2c_algo = { -- 1.8.3.1
[PATCH 1/4] drm/radeon/dp: handle zero sized i2c over aux transactions (v2)
Needed for proper i2c over aux handling for certain monitors and configurations (e.g., dp bridges or adapters). v2: add comments clarifying tx_size setting. Signed-off-by: Alex Deucher --- drivers/gpu/drm/radeon/atombios_dp.c | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c index 8b0ab17..e448304 100644 --- a/drivers/gpu/drm/radeon/atombios_dp.c +++ b/drivers/gpu/drm/radeon/atombios_dp.c @@ -142,7 +142,8 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan, return recv_bytes; } -#define HEADER_SIZE 4 +#define BARE_ADDRESS_SIZE 3 +#define HEADER_SIZE (BARE_ADDRESS_SIZE + 1) static ssize_t radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) @@ -160,13 +161,19 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) tx_buf[0] = msg->address & 0xff; tx_buf[1] = msg->address >> 8; tx_buf[2] = msg->request << 4; - tx_buf[3] = msg->size - 1; + tx_buf[3] = msg->size ? (msg->size - 1) : 0; switch (msg->request & ~DP_AUX_I2C_MOT) { case DP_AUX_NATIVE_WRITE: case DP_AUX_I2C_WRITE: + /* tx_size needs to be 4 even for bare address packets since the atom +* table needs the info in tx_buf[3]. +*/ tx_size = HEADER_SIZE + msg->size; - tx_buf[3] |= tx_size << 4; + if (msg->size == 0) + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; + else + tx_buf[3] |= tx_size << 4; memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size); ret = radeon_process_aux_ch(chan, tx_buf, tx_size, NULL, 0, delay, &ack); @@ -176,8 +183,14 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) break; case DP_AUX_NATIVE_READ: case DP_AUX_I2C_READ: + /* tx_size needs to be 4 even for bare address packets since the atom +* table needs the info in tx_buf[3]. +*/ tx_size = HEADER_SIZE; - tx_buf[3] |= tx_size << 4; + if (msg->size == 0) + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; + else + tx_buf[3] |= tx_size << 4; ret = radeon_process_aux_ch(chan, tx_buf, tx_size, msg->buffer, msg->size, delay, &ack); break; @@ -186,7 +199,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) break; } - if (ret > 0) + if (ret >= 0) msg->reply = ack >> 4; return ret; -- 1.8.3.1
[PATCH 0/4] Reset i2c connection between xfers (v4)
Updated with Thierry's comments. Alex Deucher (4): drm/radeon/dp: handle zero sized i2c over aux transactions (v2) drm/dp/i2c: send bare addresses to properly reset i2c connections (v4) drm/dp/i2c: Update comments about common i2c over dp assumptions (v2) drm/radeon/dp: switch to the common i2c over aux code drivers/gpu/drm/drm_dp_helper.c| 55 +++- drivers/gpu/drm/radeon/atombios_dp.c | 140 - drivers/gpu/drm/radeon/radeon_connectors.c | 44 ++--- drivers/gpu/drm/radeon/radeon_display.c| 11 ++- drivers/gpu/drm/radeon/radeon_i2c.c| 60 +++-- drivers/gpu/drm/radeon/radeon_mode.h | 12 +-- include/drm/drm_dp_helper.h| 4 + 7 files changed, 101 insertions(+), 225 deletions(-) -- 1.8.3.1
Required backwards support level?
On Sun, 6 Apr 2014 19:58:48 +0200 Daniel Vetter wrote: > On Sun, Apr 6, 2014 at 7:28 PM, Lauri Kasanen wrote: > > This is related to my memory management work. As the VRAM queue is > > global, it cannot be determined on a per-app basis. If at least one > > client is running old userspace, the new scoring cannot be used. > > > > Switching live between the two would bring additional complexity. I'd > > hope to be able to determine by the first 3d app if the userspace is > > new enough. But that depends on if it's expected to be able to run > > mixed loads. > > At least thus far we've worked under the example that any > explicitly-enabled new behaviour is only enabled per-fd. That's also > useful when you install all your developer versions into a prefix, so > still have 2 versions of X driver, libdrm, mesa, everything else lying > around and want to switch at runtime even. > > Can't you fake a default score somehow for all legacy userspace and > always run with the new code? Reimplementing the old interfaces in > terms of the new ones also allows you to kick out duplicated code > (usually) compared to having both old and new code around. Certainly, that can be done. - Lauri
[PATCH] imx-drm: imx-drm-core: Fix imx_drm_encoder_get_mux_id
The decoder mux id is equal to the port id of the encoder's input port that is connected to the given crtc, not to the endpoint id (which is arbitrary and usually zero). Signed-off-by: Philipp Zabel --- drivers/staging/imx-drm/imx-drm-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c index 4144a75..bc7f8bd 100644 --- a/drivers/staging/imx-drm/imx-drm-core.c +++ b/drivers/staging/imx-drm/imx-drm-core.c @@ -517,7 +517,7 @@ int imx_drm_encoder_get_mux_id(struct device_node *node, of_node_put(port); if (port == imx_crtc->port) { ret = of_graph_parse_endpoint(ep, &endpoint); - return ret ? ret : endpoint.id; + return ret ? ret : endpoint.port; } } while (ep); -- 1.9.1
[PATCH V2] gpu: host1x: handle the correct # of syncpt regs
On Fri, Apr 04, 2014 at 04:31:05PM -0600, Stephen Warren wrote: > From: Stephen Warren > > BIT_WORD() truncates rather than rounds, so the loops in > syncpt_thresh_isr() and _host1x_intr_disable_all_syncpt_intrs() use <= > rather than < in an attempt to process the correct number of registers > when rounding of the conversion of count of bits to count of words is > necessary. However, when rounding isn't necessary because the value is > already a multiple of the divisor (as is the case for all values of > nb_pts the code actually sees), this causes one too many registers to > be processed. > > Solve this by using and explicit DIV_ROUND_UP() call, rather than > BIT_WORD(), and comparing with < rather than <=. > > Signed-off-by: Stephen Warren > --- > v2: Use DIV_ROUND_UP rather than BITS_TO_LONGS to avoid problems on 64-bit. > --- > drivers/gpu/host1x/hw/intr_hw.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) If I understand correctly there's no immediate need for this to go to stable kernels, nor for it to be queued for 3.15, right? That is the potential extra write isn't causing any harm on actual hardware, is it? In that case I'll queue this up for 3.16. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/c6c11806/attachment-0001.sig>
[PATCH v5 00/11] imx-drm dt bindings
On Mon, Apr 07, 2014 at 12:23:37PM +0800, Shawn Guo wrote: > And I see another HDMI regression with my testing on mainline kernel. I > can have my HDMI work at 1920x1080 with v3.14 kernel, but it can only > probes 1024x768 with the mainline today. Works for me here. > [20.606] (II) LoadModule: "modesetting" > [20.607] (II) Loading /usr/lib/xorg/modules/drivers/modesetting_drv.so > [20.609] (II) Module modesetting: vendor="X.Org Foundation" > [20.609] compiled for 1.12.1.902, module version = 0.3.0 > [20.610] Module class: X.Org Video Driver > [20.610] ABI class: X.Org Video Driver, version 12.0 > [20.610] (II) modesetting: Driver for Modesetting Kernel Drivers: kms > [20.610] (++) using VT number 7 > > [20.624] (WW) Falling back to old probe method for modesetting > [20.624] (II) modesetting(0): using default device > [20.627] (II) modesetting(0): Creating default Display subsection in > Screen section > "Default Screen Section" for depth/fbbpp 24/32 > [20.627] (==) modesetting(0): Depth 24, (==) framebuffer bpp 32 > [20.628] (==) modesetting(0): RGB weight 888 > [20.628] (==) modesetting(0): Default visual is TrueColor > [20.628] (II) modesetting(0): ShadowFB: preferred NO, enabled NO > [20.628] (II) modesetting(0): Output HDMI-0 has no monitor section > [20.629] (II) modesetting(0): EDID for output HDMI-0 > [20.629] (II) modesetting(0): Printing probed modes for output HDMI-0 So no EDID. Did you update the "ddc" property to "ddc-i2c-bus" ? -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it.
[PATCH 2/4] drm/dp/i2c: send bare addresses to properly reset i2c connections (v3)
On Fri, Apr 04, 2014 at 03:58:37PM -0400, Alex Deucher wrote: > We need bare address packets at the start and end of > each i2c over aux transaction to properly reset the connection > between transactions. This mirrors what the existing dp i2c > over aux algo currently does. > > This fixes EDID fetches on certain monitors especially with > dp bridges. > > v2: update as per Ville's comments > - Set buffer to NULL for zero sized packets > - abort the entre transaction if one of the messages fails > v3: drop leftover debugging code > > Signed-off-by: Alex Deucher > Cc: Ville Syrj?l? > Cc: Jani Nikula > Cc: Thierry Reding > Reviewed-by: Ville Syrj?l? > --- > drivers/gpu/drm/drm_dp_helper.c | 52 > +++-- > 1 file changed, 29 insertions(+), 23 deletions(-) Can we please document that zero-sized messages specify address-only transactions? Perhaps it would also be useful to mention that these can only happen for I2C-over-AUX messages. > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 74724aa..dfe4cf4 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -664,12 +664,23 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, > struct i2c_msg *msgs, > int num) > { > struct drm_dp_aux *aux = adapter->algo_data; > - unsigned int i, j; > + unsigned int m, b; I don't see why these would need to be changed. i and j are perfectly fine loop variable names. > - for (i = 0; i < num; i++) { > - struct drm_dp_aux_msg msg; > - int err; > + memset(&msg, 0, sizeof(msg)); > > + for (m = 0; m < num; m++) { > + msg.address = msgs[m].addr; > + msg.request = (msgs[m].flags & I2C_M_RD) ? > + DP_AUX_I2C_READ : > + DP_AUX_I2C_WRITE; > + msg.request |= DP_AUX_I2C_MOT; > + msg.buffer = NULL; > + msg.size = 0; > + err = drm_dp_i2c_do_msg(aux, &msg); > + if (err < 0) > + break; This seems somewhat brittle to me. Even though I notice that patch 3/4 updates a comment that documents these assumptions, I don't see a reason for these assumptions in the first place. I'd prefer if we simply provided the complete message rather than rely on drivers not to touch anything but the reply field. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140407/92f2bb18/attachment.sig>
[PATCH 2/4] drm/dp/i2c: send bare addresses to properly reset i2c connections (v3)
On Mon, Apr 7, 2014 at 3:49 AM, Thierry Reding wrote: > On Fri, Apr 04, 2014 at 03:58:37PM -0400, Alex Deucher wrote: >> We need bare address packets at the start and end of >> each i2c over aux transaction to properly reset the connection >> between transactions. This mirrors what the existing dp i2c >> over aux algo currently does. >> >> This fixes EDID fetches on certain monitors especially with >> dp bridges. >> >> v2: update as per Ville's comments >> - Set buffer to NULL for zero sized packets >> - abort the entre transaction if one of the messages fails >> v3: drop leftover debugging code >> >> Signed-off-by: Alex Deucher >> Cc: Ville Syrj?l? >> Cc: Jani Nikula >> Cc: Thierry Reding >> Reviewed-by: Ville Syrj?l? >> --- >> drivers/gpu/drm/drm_dp_helper.c | 52 >> +++-- >> 1 file changed, 29 insertions(+), 23 deletions(-) > > Can we please document that zero-sized messages specify address-only > transactions? Perhaps it would also be useful to mention that these can > only happen for I2C-over-AUX messages. Can do. I don't know of any uses for bare address packets with regular aux off hand, but there may be cases I'm not familiar with. Does anyone know of any use for a bare address packet with regular aux? > >> diff --git a/drivers/gpu/drm/drm_dp_helper.c >> b/drivers/gpu/drm/drm_dp_helper.c >> index 74724aa..dfe4cf4 100644 >> --- a/drivers/gpu/drm/drm_dp_helper.c >> +++ b/drivers/gpu/drm/drm_dp_helper.c >> @@ -664,12 +664,23 @@ static int drm_dp_i2c_xfer(struct i2c_adapter >> *adapter, struct i2c_msg *msgs, >> int num) >> { >> struct drm_dp_aux *aux = adapter->algo_data; >> - unsigned int i, j; >> + unsigned int m, b; > > I don't see why these would need to be changed. i and j are perfectly > fine loop variable names. It was easier for me to follow the code since the variables matched the objects they were iterating, but I can change them back if you'd prefer. > >> - for (i = 0; i < num; i++) { >> - struct drm_dp_aux_msg msg; >> - int err; >> + memset(&msg, 0, sizeof(msg)); >> >> + for (m = 0; m < num; m++) { >> + msg.address = msgs[m].addr; >> + msg.request = (msgs[m].flags & I2C_M_RD) ? >> + DP_AUX_I2C_READ : >> + DP_AUX_I2C_WRITE; >> + msg.request |= DP_AUX_I2C_MOT; >> + msg.buffer = NULL; >> + msg.size = 0; >> + err = drm_dp_i2c_do_msg(aux, &msg); >> + if (err < 0) >> + break; > > This seems somewhat brittle to me. Even though I notice that patch 3/4 > updates a comment that documents these assumptions, I don't see a reason > for these assumptions in the first place. We already assume that in drm_dp_i2c_do_msg() for the retry loop. > > I'd prefer if we simply provided the complete message rather than rely > on drivers not to touch anything but the reply field. Are you suggesting we re-write drm_dp_i2c_do_msg() or move the retry logic into drm_dp_i2c_xfer()? Do you mind if we do that in a follow up patch so we can keep it separate from the addition of the bare address packets? Alex
[PATCH V2] gpu: host1x: handle the correct # of syncpt regs
On 04/07/2014 02:18 AM, Thierry Reding wrote: > On Fri, Apr 04, 2014 at 04:31:05PM -0600, Stephen Warren wrote: >> From: Stephen Warren >> >> BIT_WORD() truncates rather than rounds, so the loops in >> syncpt_thresh_isr() and _host1x_intr_disable_all_syncpt_intrs() use <= >> rather than < in an attempt to process the correct number of registers >> when rounding of the conversion of count of bits to count of words is >> necessary. However, when rounding isn't necessary because the value is >> already a multiple of the divisor (as is the case for all values of >> nb_pts the code actually sees), this causes one too many registers to >> be processed. >> >> Solve this by using and explicit DIV_ROUND_UP() call, rather than >> BIT_WORD(), and comparing with < rather than <=. >> >> Signed-off-by: Stephen Warren >> --- >> v2: Use DIV_ROUND_UP rather than BITS_TO_LONGS to avoid problems on 64-bit. >> --- >> drivers/gpu/host1x/hw/intr_hw.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) > > If I understand correctly there's no immediate need for this to go to > stable kernels, nor for it to be queued for 3.15, right? That is the > potential extra write isn't causing any harm on actual hardware, is it? > > In that case I'll queue this up for 3.16. We should definitely apply this, and as far back as the code exists, since the SW is touching non-existent registers, and that is presumably undefined behaviour, which could potentially cause hard-to-diagnose bugs. Besides, I want the mainline kernel to run on our simulator without having to maintain patches for fixed issues.
[PATCH 1/4] drm/radeon/dp: handle zero sized i2c over aux transactions
On Mon, Apr 7, 2014 at 9:24 AM, Alex Deucher wrote: > On Mon, Apr 7, 2014 at 3:57 AM, Jani Nikula > wrote: >> On Fri, 04 Apr 2014, Alex Deucher wrote: >>> Needed for proper i2c over aux handling for certain >>> monitors and configurations (e.g., dp bridges or >>> adapters). >>> >>> Signed-off-by: Alex Deucher >>> --- >>> drivers/gpu/drm/radeon/atombios_dp.c | 15 +++ >>> 1 file changed, 11 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c >>> b/drivers/gpu/drm/radeon/atombios_dp.c >>> index 8b0ab17..e914008 100644 >>> --- a/drivers/gpu/drm/radeon/atombios_dp.c >>> +++ b/drivers/gpu/drm/radeon/atombios_dp.c >>> @@ -143,6 +143,7 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan >>> *chan, >>> } >>> >>> #define HEADER_SIZE 4 >>> +#define BARE_ADDRESS_SIZE 3 >>> >>> static ssize_t >>> radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >>> @@ -160,13 +161,16 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct >>> drm_dp_aux_msg *msg) >>> tx_buf[0] = msg->address & 0xff; >>> tx_buf[1] = msg->address >> 8; >>> tx_buf[2] = msg->request << 4; >>> - tx_buf[3] = msg->size - 1; >>> + tx_buf[3] = msg->size ? (msg->size - 1) : 0; >>> >>> switch (msg->request & ~DP_AUX_I2C_MOT) { >>> case DP_AUX_NATIVE_WRITE: >>> case DP_AUX_I2C_WRITE: >>> tx_size = HEADER_SIZE + msg->size; >>> - tx_buf[3] |= tx_size << 4; >>> + if (msg->size == 0) >>> + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; >>> + else >>> + tx_buf[3] |= tx_size << 4; >>> memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size); >>> ret = radeon_process_aux_ch(chan, >>> tx_buf, tx_size, NULL, 0, delay, >>> &ack); >> >> I think your tz_size and tx_buf[3] are confused. Shouldn't you only send >> 3 bytes of tx_buf when msg->size == 0? >> >> Disclaimer, I don't know anything about your hw and all that... :) > > It doesn't really matter. The hw only cares about the size specified > in tx_buf[3]. tx_size is only used to determine how much data to the > buffer used by the atom table. We end up copying an extra byte that > never gets used. I suppose I should fix it up for clarify. Actually, I was wrong. We need all 4 bytes of the tx_buf, but the hw only sends 3 based on the size specified in tx_buf[3]. so the code is correct as is. Alex > > Alex > >> >> BR, >> Jani. >> >> >>> @@ -177,7 +181,10 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct >>> drm_dp_aux_msg *msg) >>> case DP_AUX_NATIVE_READ: >>> case DP_AUX_I2C_READ: >>> tx_size = HEADER_SIZE; >>> - tx_buf[3] |= tx_size << 4; >>> + if (msg->size == 0) >>> + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; >>> + else >>> + tx_buf[3] |= tx_size << 4; >>> ret = radeon_process_aux_ch(chan, >>> tx_buf, tx_size, msg->buffer, >>> msg->size, delay, &ack); >>> break; >>> @@ -186,7 +193,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct >>> drm_dp_aux_msg *msg) >>> break; >>> } >>> >>> - if (ret > 0) >>> + if (ret >= 0) >>> msg->reply = ack >> 4; >>> >>> return ret; >>> -- >>> 1.8.3.1 >>> >> >> -- >> Jani Nikula, Intel Open Source Technology Center
[PATCH 1/4] drm/radeon/dp: handle zero sized i2c over aux transactions
On Mon, Apr 7, 2014 at 3:57 AM, Jani Nikula wrote: > On Fri, 04 Apr 2014, Alex Deucher wrote: >> Needed for proper i2c over aux handling for certain >> monitors and configurations (e.g., dp bridges or >> adapters). >> >> Signed-off-by: Alex Deucher >> --- >> drivers/gpu/drm/radeon/atombios_dp.c | 15 +++ >> 1 file changed, 11 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c >> b/drivers/gpu/drm/radeon/atombios_dp.c >> index 8b0ab17..e914008 100644 >> --- a/drivers/gpu/drm/radeon/atombios_dp.c >> +++ b/drivers/gpu/drm/radeon/atombios_dp.c >> @@ -143,6 +143,7 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan >> *chan, >> } >> >> #define HEADER_SIZE 4 >> +#define BARE_ADDRESS_SIZE 3 >> >> static ssize_t >> radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >> @@ -160,13 +161,16 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct >> drm_dp_aux_msg *msg) >> tx_buf[0] = msg->address & 0xff; >> tx_buf[1] = msg->address >> 8; >> tx_buf[2] = msg->request << 4; >> - tx_buf[3] = msg->size - 1; >> + tx_buf[3] = msg->size ? (msg->size - 1) : 0; >> >> switch (msg->request & ~DP_AUX_I2C_MOT) { >> case DP_AUX_NATIVE_WRITE: >> case DP_AUX_I2C_WRITE: >> tx_size = HEADER_SIZE + msg->size; >> - tx_buf[3] |= tx_size << 4; >> + if (msg->size == 0) >> + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; >> + else >> + tx_buf[3] |= tx_size << 4; >> memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size); >> ret = radeon_process_aux_ch(chan, >> tx_buf, tx_size, NULL, 0, delay, >> &ack); > > I think your tz_size and tx_buf[3] are confused. Shouldn't you only send > 3 bytes of tx_buf when msg->size == 0? > > Disclaimer, I don't know anything about your hw and all that... :) It doesn't really matter. The hw only cares about the size specified in tx_buf[3]. tx_size is only used to determine how much data to the buffer used by the atom table. We end up copying an extra byte that never gets used. I suppose I should fix it up for clarify. Alex > > BR, > Jani. > > >> @@ -177,7 +181,10 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct >> drm_dp_aux_msg *msg) >> case DP_AUX_NATIVE_READ: >> case DP_AUX_I2C_READ: >> tx_size = HEADER_SIZE; >> - tx_buf[3] |= tx_size << 4; >> + if (msg->size == 0) >> + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; >> + else >> + tx_buf[3] |= tx_size << 4; >> ret = radeon_process_aux_ch(chan, >> tx_buf, tx_size, msg->buffer, >> msg->size, delay, &ack); >> break; >> @@ -186,7 +193,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct >> drm_dp_aux_msg *msg) >> break; >> } >> >> - if (ret > 0) >> + if (ret >= 0) >> msg->reply = ack >> 4; >> >> return ret; >> -- >> 1.8.3.1 >> > > -- > Jani Nikula, Intel Open Source Technology Center
[PATCH 12/13] drm/i2c/tda998x: Fix signed overflow issue
On 04/05/2014 02:45 AM, Daniel Vetter wrote: > This is C standard hair-splitting, but afaict > - sum will be promoted to signed int in computation since > uint8_t fits > - signed overflow is undefined. > > No we need to add up an awful lot of bytes to actually make it ^^ Now > overflow. But I guess the real risk is gcc spotting this and going > bananas. Fix this by simply using unsigned in to force all computations > to use the well-defined unsigned behaviour. Seems reasonable... it also seems impossible (ha!) to break anything. Reviewed-by: Ian Romanick > Spotted by coverity. > > Cc: Russell King > Cc: Rob Clark > Cc: Jean-Francois Moine > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c > b/drivers/gpu/drm/i2c/tda998x_drv.c > index 48af5cac1902..ae2754760d77 100644 > --- a/drivers/gpu/drm/i2c/tda998x_drv.c > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > @@ -568,7 +568,7 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) > > static uint8_t tda998x_cksum(uint8_t *buf, size_t bytes) > { > - uint8_t sum = 0; > + unsigned sum = 0; > > while (bytes--) > sum += *buf++; >
[PATCH 11/13] drm/bochs: Remove unecessary NULL check in gem_free
On 04/05/2014 02:45 AM, Daniel Vetter wrote: > The ->gem_free_object never gets called with a NULL pointer, the check > is redundant. Also checking after the upcast allows compilers to elide > it anyway. > > Noticed while chasing coverity reports, somehow this one here was not > flagged. > > Signed-off-by: Daniel Vetter Same as MGA change. Reviewed-by: Ian Romanick > --- > drivers/gpu/drm/bochs/bochs_mm.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/bochs/bochs_mm.c > b/drivers/gpu/drm/bochs/bochs_mm.c > index 4a239e931aff..b9a695d92792 100644 > --- a/drivers/gpu/drm/bochs/bochs_mm.c > +++ b/drivers/gpu/drm/bochs/bochs_mm.c > @@ -441,8 +441,6 @@ void bochs_gem_free_object(struct drm_gem_object *obj) > { > struct bochs_bo *bochs_bo = gem_to_bochs_bo(obj); > > - if (!bochs_bo) > - return; > bochs_bo_unref(&bochs_bo); > } > >
[PATCH 08/13] drm/ast: Remove dead code from cbr_scan2
On 04/05/2014 02:44 AM, Daniel Vetter wrote: > The outer if already checks for data != 0, so it can't really be > 0. Hence remove it. > > Now I don't have specs or anything for this beast, so I have no > idea whether this was actually intended or whether the logic > should be different. At least the code still seems to be doing > something useful. > > Spotted by coverity. > > Cc: Dave Airlie > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/ast/ast_post.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c > index 977cfb35837a..6263116054b6 100644 > --- a/drivers/gpu/drm/ast/ast_post.c > +++ b/drivers/gpu/drm/ast/ast_post.c > @@ -572,8 +572,6 @@ static u32 cbr_scan2(struct ast_private *ast) > for (loop = 0; loop < CBR_PASSNUM2; loop++) { > if ((data = cbr_test2(ast)) != 0) { > data2 &= data; > - if (!data) > - return 0; That feels like a typo... was that supposed to be 'if (!data2)'? > break; > } > } >