[PATCH libdrm 5/5] modetest: Reduce the length of the connector type string
From: Ville Syrj?l?Spelling out eDP or DP make for a ridicilously long string which plays havoc with formatting. Just say eDP or DP. Signed-off-by: Ville Syrj?l? --- tests/modetest/modetest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 27cd2ce..8afd2b1 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -106,11 +106,11 @@ struct type_name connector_type_names[] = { { DRM_MODE_CONNECTOR_LVDS, "LVDS" }, { DRM_MODE_CONNECTOR_Component, "component" }, { DRM_MODE_CONNECTOR_9PinDIN, "9-pin DIN" }, - { DRM_MODE_CONNECTOR_DisplayPort, "displayport" }, + { DRM_MODE_CONNECTOR_DisplayPort, "DP" }, { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A" }, { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B" }, { DRM_MODE_CONNECTOR_TV, "TV" }, - { DRM_MODE_CONNECTOR_eDP, "embedded displayport" }, + { DRM_MODE_CONNECTOR_eDP, "eDP" }, }; type_name_fn(connector_type) -- 1.8.1.5
[PATCH libdrm 4/5] modetest: Print possible_crtcs for planes
From: Ville Syrj?l?Signed-off-by: Ville Syrj?l? --- tests/modetest/modetest.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index c91bb9d..27cd2ce 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -429,7 +429,7 @@ static void dump_planes(void) } printf("Planes:\n"); - printf("id\tcrtc\tfb\tCRTC x,y\tx,y\tgamma size\n"); + printf("id\tcrtc\tfb\tCRTC x,y\tx,y\tgamma size\tpossible crtcs\n"); for (i = 0; i < plane_resources->count_planes; i++) { ovr = drmModeGetPlane(fd, plane_resources->planes[i]); if (!ovr) { @@ -438,10 +438,10 @@ static void dump_planes(void) continue; } - printf("%d\t%d\t%d\t%d,%d\t\t%d,%d\t%d\n", + printf("%d\t%d\t%d\t%d,%d\t\t%d,%d\t%-8d\t0x%08x\n", ovr->plane_id, ovr->crtc_id, ovr->fb_id, ovr->crtc_x, ovr->crtc_y, ovr->x, ovr->y, - ovr->gamma_size); + ovr->gamma_size, ovr->possible_crtcs); if (!ovr->count_formats) continue; -- 1.8.1.5
[PATCH libdrm 3/5] modetest: Add support for all 16/32 bpp RGB formats
From: Ville Syrj?l?Signed-off-by: Ville Syrj?l? --- tests/modetest/buffers.c | 120 +-- 1 file changed, 115 insertions(+), 5 deletions(-) diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c index 7f534a1..3b1685c 100644 --- a/tests/modetest/buffers.c +++ b/tests/modetest/buffers.c @@ -102,17 +102,44 @@ static const struct format_info format_info[] = { /* YUV planar */ { DRM_FORMAT_YVU420, "YV12", MAKE_YUV_INFO(YUV_YCrCb, 2, 2, 1) }, /* RGB16 */ + { DRM_FORMAT_ARGB, "AR12", MAKE_RGB_INFO(4, 8, 4, 4, 4, 0, 4, 12) }, + { DRM_FORMAT_XRGB, "XR12", MAKE_RGB_INFO(4, 8, 4, 4, 4, 0, 0, 0) }, + { DRM_FORMAT_ABGR, "AB12", MAKE_RGB_INFO(4, 0, 4, 4, 4, 8, 4, 12) }, + { DRM_FORMAT_XBGR, "XB12", MAKE_RGB_INFO(4, 0, 4, 4, 4, 8, 0, 0) }, + { DRM_FORMAT_RGBA, "RA12", MAKE_RGB_INFO(4, 12, 4, 8, 4, 4, 4, 0) }, + { DRM_FORMAT_RGBX, "RX12", MAKE_RGB_INFO(4, 12, 4, 8, 4, 4, 0, 0) }, + { DRM_FORMAT_BGRA, "BA12", MAKE_RGB_INFO(4, 4, 4, 8, 4, 12, 4, 0) }, + { DRM_FORMAT_BGRX, "BX12", MAKE_RGB_INFO(4, 4, 4, 8, 4, 12, 0, 0) }, { DRM_FORMAT_ARGB1555, "AR15", MAKE_RGB_INFO(5, 10, 5, 5, 5, 0, 1, 15) }, { DRM_FORMAT_XRGB1555, "XR15", MAKE_RGB_INFO(5, 10, 5, 5, 5, 0, 0, 0) }, + { DRM_FORMAT_ABGR1555, "AB15", MAKE_RGB_INFO(5, 0, 5, 5, 5, 10, 1, 15) }, + { DRM_FORMAT_XBGR1555, "XB15", MAKE_RGB_INFO(5, 0, 5, 5, 5, 10, 0, 0) }, + { DRM_FORMAT_RGBA5551, "RA15", MAKE_RGB_INFO(5, 11, 5, 6, 5, 1, 1, 0) }, + { DRM_FORMAT_RGBX5551, "RX15", MAKE_RGB_INFO(5, 11, 5, 6, 5, 1, 0, 0) }, + { DRM_FORMAT_BGRA5551, "BA15", MAKE_RGB_INFO(5, 1, 5, 6, 5, 11, 1, 0) }, + { DRM_FORMAT_BGRX5551, "BX15", MAKE_RGB_INFO(5, 1, 5, 6, 5, 11, 0, 0) }, { DRM_FORMAT_RGB565, "RG16", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) }, + { DRM_FORMAT_BGR565, "BG16", MAKE_RGB_INFO(5, 0, 6, 5, 5, 11, 0, 0) }, /* RGB24 */ { DRM_FORMAT_BGR888, "BG24", MAKE_RGB_INFO(8, 0, 8, 8, 8, 16, 0, 0) }, { DRM_FORMAT_RGB888, "RG24", MAKE_RGB_INFO(8, 16, 8, 8, 8, 0, 0, 0) }, /* RGB32 */ { DRM_FORMAT_ARGB, "AR24", MAKE_RGB_INFO(8, 16, 8, 8, 8, 0, 8, 24) }, - { DRM_FORMAT_BGRA, "BA24", MAKE_RGB_INFO(8, 8, 8, 16, 8, 24, 8, 0) }, { DRM_FORMAT_XRGB, "XR24", MAKE_RGB_INFO(8, 16, 8, 8, 8, 0, 0, 0) }, + { DRM_FORMAT_ABGR, "AB24", MAKE_RGB_INFO(8, 0, 8, 8, 8, 16, 8, 24) }, + { DRM_FORMAT_XBGR, "XB24", MAKE_RGB_INFO(8, 0, 8, 8, 8, 16, 0, 0) }, + { DRM_FORMAT_RGBA, "RA24", MAKE_RGB_INFO(8, 24, 8, 16, 8, 8, 8, 0) }, + { DRM_FORMAT_RGBX, "RX24", MAKE_RGB_INFO(8, 24, 8, 16, 8, 8, 0, 0) }, + { DRM_FORMAT_BGRA, "BA24", MAKE_RGB_INFO(8, 8, 8, 16, 8, 24, 8, 0) }, { DRM_FORMAT_BGRX, "BX24", MAKE_RGB_INFO(8, 8, 8, 16, 8, 24, 0, 0) }, + { DRM_FORMAT_ARGB2101010, "AR30", MAKE_RGB_INFO(10, 20, 10, 10, 10, 0, 2, 30) }, + { DRM_FORMAT_XRGB2101010, "XR30", MAKE_RGB_INFO(10, 20, 10, 10, 10, 0, 0, 0) }, + { DRM_FORMAT_ABGR2101010, "AB30", MAKE_RGB_INFO(10, 0, 10, 10, 10, 20, 2, 30) }, + { DRM_FORMAT_XBGR2101010, "XB30", MAKE_RGB_INFO(10, 0, 10, 10, 10, 20, 0, 0) }, + { DRM_FORMAT_RGBA1010102, "RA30", MAKE_RGB_INFO(10, 22, 10, 12, 10, 2, 2, 0) }, + { DRM_FORMAT_RGBX1010102, "RX30", MAKE_RGB_INFO(10, 22, 10, 12, 10, 2, 0, 0) }, + { DRM_FORMAT_BGRA1010102, "BA30", MAKE_RGB_INFO(10, 2, 10, 12, 10, 22, 2, 0) }, + { DRM_FORMAT_BGRX1010102, "BX30", MAKE_RGB_INFO(10, 2, 10, 12, 10, 22, 0, 0) }, }; unsigned int format_fourcc(const char *name) @@ -577,19 +604,47 @@ fill_smpte(const struct format_info *info, void *planes[3], unsigned int width, return fill_smpte_yuv_planar(>yuv, planes[0], planes[1], planes[2], width, height, stride); + case DRM_FORMAT_ARGB: + case DRM_FORMAT_XRGB: + case DRM_FORMAT_ABGR: + case DRM_FORMAT_XBGR: + case DRM_FORMAT_RGBA: + case DRM_FORMAT_RGBX: + case DRM_FORMAT_BGRA: + case DRM_FORMAT_BGRX: case DRM_FORMAT_RGB565: + case DRM_FORMAT_BGR565: case DRM_FORMAT_ARGB1555: case DRM_FORMAT_XRGB1555: + case DRM_FORMAT_ABGR1555: + case DRM_FORMAT_XBGR1555: + case DRM_FORMAT_RGBA5551: + case DRM_FORMAT_RGBX5551: + case DRM_FORMAT_BGRA5551: + case DRM_FORMAT_BGRX5551: return fill_smpte_rgb16(>rgb, planes[0], width, height, stride); + case DRM_FORMAT_BGR888: case DRM_FORMAT_RGB888: return fill_smpte_rgb24(>rgb, planes[0], width, height, stride); case DRM_FORMAT_ARGB: - case DRM_FORMAT_BGRA: case
[PATCH libdrm 2/5] modetest: Fix pitches, somewhat
From: Ville Syrj?l?libkms only has the xrgb format, so we're overallocating the bo by quite a lot in some cases. But we still need to get the pitch from the libkms since it's the driver that decides how to align it. Signed-off-by: Ville Syrj?l? --- tests/modetest/buffers.c | 34 ++ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c index 6b117b4..7f534a1 100644 --- a/tests/modetest/buffers.c +++ b/tests/modetest/buffers.c @@ -948,9 +948,9 @@ create_test_buffer(struct kms_driver *kms, unsigned int format, case DRM_FORMAT_VYUY: case DRM_FORMAT_YUYV: case DRM_FORMAT_YVYU: - pitches[0] = width * 2; offsets[0] = 0; kms_bo_get_prop(bo, KMS_HANDLE, [0]); + kms_bo_get_prop(bo, KMS_PITCH, [0]); planes[0] = virtual; break; @@ -959,11 +959,11 @@ create_test_buffer(struct kms_driver *kms, unsigned int format, case DRM_FORMAT_NV21: case DRM_FORMAT_NV16: case DRM_FORMAT_NV61: - pitches[0] = width; offsets[0] = 0; kms_bo_get_prop(bo, KMS_HANDLE, [0]); - pitches[1] = width; - offsets[1] = width * height; + kms_bo_get_prop(bo, KMS_PITCH, [0]); + pitches[1] = pitches[0]; + offsets[1] = pitches[0] * height; kms_bo_get_prop(bo, KMS_HANDLE, [1]); planes[0] = virtual; @@ -971,14 +971,14 @@ create_test_buffer(struct kms_driver *kms, unsigned int format, break; case DRM_FORMAT_YVU420: - pitches[0] = width; offsets[0] = 0; kms_bo_get_prop(bo, KMS_HANDLE, [0]); - pitches[1] = width / 2; - offsets[1] = width * height; + kms_bo_get_prop(bo, KMS_PITCH, [0]); + pitches[1] = pitches[0] / 2; + offsets[1] = pitches[0] * height; kms_bo_get_prop(bo, KMS_HANDLE, [1]); - pitches[2] = width / 2; - offsets[2] = offsets[1] + (width * height) / 4; + pitches[2] = pitches[1]; + offsets[2] = offsets[1] + pitches[1] * height / 2; kms_bo_get_prop(bo, KMS_HANDLE, [2]); planes[0] = virtual; @@ -989,29 +989,15 @@ create_test_buffer(struct kms_driver *kms, unsigned int format, case DRM_FORMAT_RGB565: case DRM_FORMAT_ARGB1555: case DRM_FORMAT_XRGB1555: - pitches[0] = width * 2; - offsets[0] = 0; - kms_bo_get_prop(bo, KMS_HANDLE, [0]); - - planes[0] = virtual; - break; - case DRM_FORMAT_BGR888: case DRM_FORMAT_RGB888: - pitches[0] = width * 3; - offsets[0] = 0; - kms_bo_get_prop(bo, KMS_HANDLE, [0]); - - planes[0] = virtual; - break; - case DRM_FORMAT_ARGB: case DRM_FORMAT_BGRA: case DRM_FORMAT_XRGB: case DRM_FORMAT_BGRX: - pitches[0] = width * 4; offsets[0] = 0; kms_bo_get_prop(bo, KMS_HANDLE, [0]); + kms_bo_get_prop(bo, KMS_PITCH, [0]); planes[0] = virtual; break; -- 1.8.1.5
[PATCH libdrm 1/5] modetest: Make RGB565 pwetty too
From: Ville Syrj?l?Signed-off-by: Ville Syrj?l? --- tests/modetest/buffers.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c index 5086381..6b117b4 100644 --- a/tests/modetest/buffers.c +++ b/tests/modetest/buffers.c @@ -601,7 +601,7 @@ fill_smpte(const struct format_info *info, void *planes[3], unsigned int width, #define BLUE 0 static void -make_pwetty(void *data, int width, int height, int stride) +make_pwetty(void *data, int width, int height, int stride, int rgb16) { #ifdef HAVE_CAIRO cairo_surface_t *surface; @@ -609,7 +609,7 @@ make_pwetty(void *data, int width, int height, int stride) int x, y; surface = cairo_image_surface_create_for_data(data, - CAIRO_FORMAT_ARGB32, + rgb16 ? CAIRO_FORMAT_RGB16_565 : CAIRO_FORMAT_ARGB32, width, height, stride); cr = cairo_create(surface); @@ -716,6 +716,7 @@ static void fill_tiles_rgb16(const struct rgb_info *rgb, unsigned char *mem, unsigned int width, unsigned int height, unsigned int stride) { + unsigned char *mem_base = mem; unsigned int x, y; for (y = 0; y < height; ++y) { @@ -732,6 +733,8 @@ fill_tiles_rgb16(const struct rgb_info *rgb, unsigned char *mem, } mem += stride; } + + make_pwetty(mem_base, width, height, stride, 1); } static void @@ -777,7 +780,7 @@ fill_tiles_rgb32(const struct rgb_info *rgb, unsigned char *mem, mem += stride; } - make_pwetty(mem_base, width, height, stride); + make_pwetty(mem_base, width, height, stride, 0); } static void -- 1.8.1.5
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #18 from Ian Romanick --- (In reply to comment #17) > (In reply to comment #16) > > Making the support general (instead of just for preprocessor directives) > > simplified the code greatly. Since I'm responsible for maintaining this > > code base as my job, that's a strong incentive for me. > > I'm sorry if this is a bit blunt, but wow. That's one of the least appealing > arguments I've heard in a long time. Not only is someone paying you for your > time, you think that's a *justification* for not supporting the standard? > That's pretty much the oppositte of how this usually works. > > So, as a paying customer of Intel, where would I file a bug-report that > Intel will deal with in a responsible way? What are you talking about? This game works on the i965 driver. I'm sorry that you don't think the maintainability of a multimillion line code base is an appealing argument. -- 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/20130417/4499b828/attachment-0001.html>
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #17 from Erik Faye-Lund --- (In reply to comment #16) > (In reply to comment #15) > > Thanks for the clarification, but I'm still not entirely convinced. > > > > I've just tested the latter. So apparently, Mesa is the only major > > OpenGL implementation that currently implements this. > > They all support it in preprocessor directives. We verified this in 2010 > when we added that level of support back. As I said before, we've > encountered shaders in games that use line continuation for multi-line > macros. > > #define foo(a, b) \ > do { \ > b = bar(a); \ > } while(0) That might very well be the case. But this ticket is not about line continuation in pre-processor directives. My test were in comments, as is the issue with this ticket. And none of them implementations listed above supports them in comments before a "#version 420" statement (if supported at all). > Making the support general (instead of just for preprocessor directives) > simplified the code greatly. Since I'm responsible for maintaining this > code base as my job, that's a strong incentive for me. I'm sorry if this is a bit blunt, but wow. That's one of the least appealing arguments I've heard in a long time. Not only is someone paying you for your time, you think that's a *justification* for not supporting the standard? That's pretty much the oppositte of how this usually works. So, as a paying customer of Intel, where would I file a bug-report that Intel will deal with in a responsible way? > > By the way, the WebGL conformance tests also checks that line continuation > > does not work. So there are at least two known, publically available shaders > > that depends on no line-continuation to work. Of course, the latter is > > synthetic, but at least it's based on wording in a specification. > > I'm not going to add complexity or overhead to the preprocessor for this > case. If WebGL tests non-continuation behavior, we can add the browsers to > the workaround list. Fair enough. At least when considered in isolation. -- 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/20130417/cbc87b40/attachment.html>
[PATCH v2 2/3] mutex: add support for reservation style locks, v2
On Wed, Apr 10, 2013 at 12:28 AM, Steven Rostedt wrote: > On Thu, Apr 04, 2013 at 06:41:02PM +0200, Peter Zijlstra wrote: >> On Thu, 2013-04-04 at 15:31 +0200, Daniel Vetter wrote: >> > The thing is now that you're not expected to hold these locks for a >> > long >> > time - if you need to synchronously stall while holding a lock >> > performance >> > will go down the gutters anyway. And since most current >> > gpus/co-processors >> > still can't really preempt fairness isn't that high a priority, >> > either. >> > So we didn't think too much about that. >> >> Yeah but you're proposing a new synchronization primitive for the core >> kernel.. all such 'fun' details need to be considered, not only those >> few that bear on the one usecase. > > Which bares the question, what other use cases are there? Just stumbled over one I think: If we have a big graph of connected things (happens really often for video pipelines). And we want multiple users to use them in parallel. But sometimes a configuration change could take way too long and so would unduly stall a 2nd thread with just a global mutex, then per-object ww_mutexes would be a fit: You'd start with grabbing all the locks for the objects you want to change anything with, then grab anything in the graph that you also need to check. Thanks to loop detection and self-recursion this would all nicely work out, even for cyclic graphs of objects. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 63532] Heroes of Newerth Water renders Black instead of transparent with reflections on Ultra [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63532 --- Comment #14 from Ian Romanick --- (In reply to comment #13) > I managed to start the game and it seems there are different problems with > water rendering, and I don't see framebuffer-related errors in the game. > Possibly I was wrong, I'll look into it to see if it's something related to > mesa or the driver. > > Btw, Ian, here is a backtrace for BindTexture error (target is 0). > Breakpoint on _mesa_problem didn't catch it, so I used _mesa_error: Right, I always get those backwards. Sorry. :( > Breakpoint 1, _mesa_error (ctx=0x6f31ec0, error=1280, fmtString= > 0x7f4466e38864 "glBindTexture(target)") at > ../../src/mesa/main/errors.c:935 > 935 debug_get_id(_msg_id); > (gdb) bt full > #0 _mesa_error (ctx=0x6f31ec0, error=1280, fmtString= > 0x7f4466e38864 "glBindTexture(target)") at > ../../src/mesa/main/errors.c:935 > do_output = 0 '\000' > do_log = 0 '\000' > error_msg_id = 0 > __PRETTY_FUNCTION__ = "_mesa_error" > #1 0x7f4466aba48f in _mesa_BindTexture (target=0, texName=2079) > at ../../src/mesa/main/texobj.c:1238 > ctx = 0x6f31ec0 > texUnit = 0x6f34f38 > newTexObj = 0x0 > targetIndex = -1 > __PRETTY_FUNCTION__ = "_mesa_BindTexture" > __func__ = "_mesa_BindTexture" > #2 0x7f4485b7a63e in glBindTexture (target=0, texture=2079) That's definitely no good. 0 isn't a valid target, and 2079 (0x081F) isn't either. Weird. > at ../../../src/mapi/glapi/glapi_mapi_tmp.h:3574 > _tbl = 0x6f42cd0 > _func = 0x7f4466aba3ca <_mesa_BindTexture> > #3 0x7f4467bb1f85 in ?? () from /home/vg/HoN/vid_gl2-x86_64.so -- 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/20130417/78194a18/attachment.html>
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #16 from Ian Romanick --- (In reply to comment #15) > Thanks for the clarification, but I'm still not entirely convinced. > > I agree that this per-spec for OpenGL ES 3.0 (although I'm a bit > disappointed that the ES3-group missed that we in the ES2-group had made it > easy for you by requiring #version to be the first bytes, if present), and > that there are spec-justification to rejecting the shader in question to > compile (due to the character being outside of the character set). And I > think we both agree that doing the latter would be a bad idea. > > But I don't agree that this is per-spec for OpenGL nor OpenGL ES 2.0. It's > the ratified spec that is the standard, not whatever discussions were held > during the meeting. And even though you have a large collection of shaders > that does not use it, I don't think breaking existing (unknown) applications > is a good idea. How many shaders besides syntetic glsparsertest-shaders > requires line-continuation to work correctly? My guess would be zero; > shaders like these would not compile on AMD, NVIDIA, nor Intel's Windows > drivers. I've just tested the latter. So apparently, Mesa is the only major > OpenGL implementation that currently implements this. They all support it in preprocessor directives. We verified this in 2010 when we added that level of support back. As I said before, we've encountered shaders in games that use line continuation for multi-line macros. #define foo(a, b) \ do { \ b = bar(a); \ } while(0) Making the support general (instead of just for preprocessor directives) simplified the code greatly. Since I'm responsible for maintaining this code base as my job, that's a strong incentive for me. > By the way, the WebGL conformance tests also checks that line continuation > does not work. So there are at least two known, publically available shaders > that depends on no line-continuation to work. Of course, the latter is > synthetic, but at least it's based on wording in a specification. I'm not going to add complexity or overhead to the preprocessor for this case. If WebGL tests non-continuation behavior, we can add the browsers to the workaround list. > I'm not trying to be a pain here, I just think you're pushing for a > direction that just leads to even more fragmentation and pain. -- 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/20130417/eb3ee167/attachment.html>
[PATCH 4/4] drm/edid: Check both 60Hz and 59.94Hz when looking for a CEA mode
From: Ville Syrj?l?drm_match_cea_mode() should be able to match both the 60Hz version, and the 59.94Hz version of modes. We only store one pixel clock value per mode in edid_cea_modes, so the other value must be calculated. Depending on the mode, edid_cea_modes contains the pixel clock for either the 60Hz version or the 59.94Hz version, so a bit of care is needed so that the calculation produces the correct result. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46800 Signed-off-by: Ville Syrj?l? --- drivers/gpu/drm/drm_edid.c | 27 --- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e233ff5..71d49f2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2330,13 +2330,34 @@ EXPORT_SYMBOL(drm_find_cea_extension); */ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) { - struct drm_display_mode *cea_mode; u8 mode; + if (!to_match->clock) + return 0; + for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) { - cea_mode = (struct drm_display_mode *)_cea_modes[mode]; + const struct drm_display_mode *cea_mode = _cea_modes[mode]; + unsigned int clock1, clock2; - if (drm_mode_equal(to_match, cea_mode)) + clock1 = clock2 = cea_mode->clock; + + /* Check both 60Hz and 59.94Hz */ + if (cea_mode->vrefresh % 6 == 0) { + /* +* edid_cea_modes contains the 59.94Hz +* variant for 240 and 480 line modes, +* and the 60Hz variant otherwise. +*/ + if (cea_mode->vdisplay == 240 || + cea_mode->vdisplay == 480) + clock1 = clock1 * 1001 / 1000; + else + clock2 = DIV_ROUND_UP(clock2 * 1000, 1001); + } + + if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || +KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && + drm_mode_equal_no_clocks(to_match, cea_mode)) return mode + 1; } return 0; -- 1.8.1.5
[PATCH 3/4] drm/edid: Populate vrefresh for CEA modes
From: Ville Syrj?l?Well have use for the vrefresh information of CEA modes later. Just populate the information into the table to avoid having to calculate it. I'm too lazy to check if someone relies on newly allocated CEA modes having 0 vrefresh, so just clear vrefresh back to 0 when adding the mode to the connector's modelist. Signed-off-by: Ville Syrj?l? --- drivers/gpu/drm/drm_edid.c | 193 ++--- 1 file changed, 129 insertions(+), 64 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e2acfdb..e233ff5 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -587,284 +587,348 @@ static const struct drm_display_mode edid_cea_modes[] = { /* 1 - 640x480 at 60Hz */ { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 25175, 640, 656, 752, 800, 0, 480, 490, 492, 525, 0, - DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) }, + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), + .vrefresh = 60, }, /* 2 - 720x480 at 60Hz */ { DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 27000, 720, 736, 798, 858, 0, 480, 489, 495, 525, 0, - DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) }, + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), + .vrefresh = 60, }, /* 3 - 720x480 at 60Hz */ { DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 27000, 720, 736, 798, 858, 0, 480, 489, 495, 525, 0, - DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) }, + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), + .vrefresh = 60, }, /* 4 - 1280x720 at 60Hz */ { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1390, 1430, 1650, 0, 720, 725, 730, 750, 0, - DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC) }, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 60, }, /* 5 - 1920x1080i at 60Hz */ { DRM_MODE("1920x1080i", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2008, 2052, 2200, 0, 1080, 1084, 1094, 1125, 0, DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC | - DRM_MODE_FLAG_INTERLACE) }, + DRM_MODE_FLAG_INTERLACE), + .vrefresh = 60, }, /* 6 - 1440x480i at 60Hz */ { DRM_MODE("1440x480i", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, 1602, 1716, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK) }, + DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), + .vrefresh = 60, }, /* 7 - 1440x480i at 60Hz */ { DRM_MODE("1440x480i", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, 1602, 1716, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK) }, + DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), + .vrefresh = 60, }, /* 8 - 1440x240 at 60Hz */ { DRM_MODE("1440x240", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, 1602, 1716, 0, 240, 244, 247, 262, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_DBLCLK) }, + DRM_MODE_FLAG_DBLCLK), + .vrefresh = 60, }, /* 9 - 1440x240 at 60Hz */ { DRM_MODE("1440x240", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, 1602, 1716, 0, 240, 244, 247, 262, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_DBLCLK) }, + DRM_MODE_FLAG_DBLCLK), + .vrefresh = 60, }, /* 10 - 2880x480i at 60Hz */ { DRM_MODE("2880x480i", DRM_MODE_TYPE_DRIVER, 54000, 2880, 2956, 3204, 3432, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_INTERLACE) }, + DRM_MODE_FLAG_INTERLACE), + .vrefresh = 60, }, /* 11 - 2880x480i at 60Hz */ { DRM_MODE("2880x480i", DRM_MODE_TYPE_DRIVER, 54000, 2880, 2956, 3204, 3432, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_INTERLACE) }, + DRM_MODE_FLAG_INTERLACE), + .vrefresh = 60, }, /* 12 - 2880x240 at 60Hz */ { DRM_MODE("2880x240", DRM_MODE_TYPE_DRIVER, 54000, 2880, 2956, 3204, 3432, 0, 240, 244, 247, 262, 0, - DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) }, + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), + .vrefresh
[PATCH 2/4] drm: Add drm_mode_equal_no_clocks()
From: Ville Syrj?l?drm_mode_equal_no_clocks() is like drm_mode_equal() except it doesn't compare the clock or vrefresh values. drm_mode_equal() is now implemented by first doing the clock checks, and then calling drm_mode_equal_no_clocks(). Signed-off-by: Ville Syrj?l? --- drivers/gpu/drm/drm_modes.c | 20 +++- include/drm/drm_crtc.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 04fa6f1..db85d0b9 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -848,6 +848,24 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_displ } else if (mode1->clock != mode2->clock) return false; + return drm_mode_equal_no_clocks(mode1, mode2); +} +/** + * drm_mode_equal_no_clocks - test modes for equality + * @mode1: first mode + * @mode2: second mode + * + * LOCKING: + * None. + * + * Check to see if @mode1 and @mode2 are equivalent, but + * don't check the pixel clocks. + * + * RETURNS: + * True if the modes are equal, false otherwise. + */ +bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2) +{ if (mode1->hdisplay == mode2->hdisplay && mode1->hsync_start == mode2->hsync_start && mode1->hsync_end == mode2->hsync_end && @@ -863,7 +881,7 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_displ return false; } -EXPORT_SYMBOL(drm_mode_equal); +EXPORT_SYMBOL(drm_mode_equal_no_clocks); /** * drm_mode_validate_size - make sure modes adhere to size constraints diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b85575b..836438d 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -922,6 +922,7 @@ extern void drm_mode_config_reset(struct drm_device *dev); extern void drm_mode_config_cleanup(struct drm_device *dev); extern void drm_mode_set_name(struct drm_display_mode *mode); extern bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); +extern bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); extern int drm_mode_width(const struct drm_display_mode *mode); extern int drm_mode_height(const struct drm_display_mode *mode); -- 1.8.1.5
[PATCH 1/4] drm: Remove explicit vrefresh initialization from DRM_MODE()
From: Ville Syrj?l?No need to zero initialize .vrefresh in DRM_MODE() since it's using desgignated initializers. This will also avoid some duplicate initialization warnings later. Signed-off-by: Ville Syrj?l? --- include/drm/drm_crtc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8c7846b..b85575b 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -120,7 +120,7 @@ enum drm_mode_status { .hdisplay = (hd), .hsync_start = (hss), .hsync_end = (hse), \ .htotal = (ht), .hskew = (hsk), .vdisplay = (vd), \ .vsync_start = (vss), .vsync_end = (vse), .vtotal = (vt), \ - .vscan = (vs), .flags = (f), .vrefresh = 0, \ + .vscan = (vs), .flags = (f), \ .base.type = DRM_MODE_OBJECT_MODE #define CRTC_INTERLACE_HALVE_V 0x1 /* halve V values for interlacing */ -- 1.8.1.5
[PATCH 0/4] drm/edid: Recognize 60Hz and 59.94Hz CEA modes
This series attempts to make our CEA mode matching recognize both the 60Hz and 59.94Hz variants of the modes (and similarly for 24/23.97, 30/29.97, etc.). The benefits should include: - Send the correct VIC in the AVI infoframe - Pick the correct RGB quantization range in automatic mode
[RFC][PATCH] drm: Insane but more fine grained locking for planes
From: Ville Syrj?l?Instead of locking all modeset locks during plane updates, use just a single CRTC mutex. To make that work, track the CRTC that "owns" the plane currently. During enable/update that means the new CRTC, and during disable it means the old CRTC. Since the plane state is no longer protected by a single lock, we need to sprinkle some additional memory barriers when relinquishing ownership. Otherwise the next CRTC might observe some stale state even though the crtc_mutex already got updated. drm_framebuffer_remove() doesn't need extra barriers since it already holds all CRTC locks, and thus no-one can be poking around at the same time. On the read side cmpxchg() already should have the necessary memory barriers. This design implies that before a plane can be moved to another CRTC, it must be disabled first, even if the hardware would offer some kind of mechanism to move an active plane over directly. I believe everyone has agreed that this an acceptable compromise. Signed-off-by: Ville Syrj?l? --- drivers/gpu/drm/drm_crtc.c | 43 +++ include/drm/drm_crtc.h | 3 +++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 957fb70..6f7385e 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -576,6 +576,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) __drm_framebuffer_unreference(plane->fb); plane->fb = NULL; plane->crtc = NULL; + plane->crtc_mutex = NULL; } } drm_modeset_unlock_all(dev); @@ -1785,6 +1786,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data, int ret = 0; unsigned int fb_width, fb_height; int i; + struct mutex *old_crtc_mutex; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; @@ -1804,12 +1806,33 @@ int drm_mode_setplane(struct drm_device *dev, void *data, /* No fb means shut it down */ if (!plane_req->fb_id) { - drm_modeset_lock_all(dev); + struct mutex *crtc_mutex; + + retry: + crtc_mutex = ACCESS_ONCE(plane->crtc_mutex); + + /* plane was already disabled? */ + if (!crtc_mutex) + return 0; + + mutex_lock(crtc_mutex); + + /* re-check that plane is still on the same crtc... */ + if (crtc_mutex != plane->crtc_mutex) { + mutex_unlock(crtc_mutex); + goto retry; + } + old_fb = plane->fb; plane->funcs->disable_plane(plane); plane->crtc = NULL; plane->fb = NULL; - drm_modeset_unlock_all(dev); + + smp_wmb(); + plane->crtc_mutex = NULL; + + mutex_unlock(crtc_mutex); + goto out; } @@ -1875,7 +1898,15 @@ int drm_mode_setplane(struct drm_device *dev, void *data, goto out; } - drm_modeset_lock_all(dev); + mutex_lock(>mutex); + + old_crtc_mutex = cmpxchg(>crtc_mutex, NULL, >mutex); + if (old_crtc_mutex != NULL && old_crtc_mutex != >mutex) { + mutex_unlock(>mutex); + ret = -EBUSY; + goto out; + } + ret = plane->funcs->update_plane(plane, crtc, fb, plane_req->crtc_x, plane_req->crtc_y, plane_req->crtc_w, plane_req->crtc_h, @@ -1886,8 +1917,12 @@ int drm_mode_setplane(struct drm_device *dev, void *data, plane->crtc = crtc; plane->fb = fb; fb = NULL; + } else { + smp_wmb(); + plane->crtc_mutex = old_crtc_mutex; } - drm_modeset_unlock_all(dev); + + mutex_unlock(>mutex); out: if (fb) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8c7846b..cc3779f 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -651,6 +651,7 @@ struct drm_plane_funcs { * @dev: DRM device this plane belongs to * @head: for list management * @base: base mode object + * @crtc_mutex: points to the mutex of the current "owner" CRTC * @possible_crtcs: pipes this plane can be bound to * @format_types: array of formats supported by this plane * @format_count: number of formats supported @@ -669,6 +670,8 @@ struct drm_plane { struct drm_mode_object base; + struct mutex *crtc_mutex; + uint32_t possible_crtcs; uint32_t *format_types; uint32_t format_count; -- 1.8.1.5
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #15 from Erik Faye-Lund --- Thanks for the clarification, but I'm still not entirely convinced. I agree that this per-spec for OpenGL ES 3.0 (although I'm a bit disappointed that the ES3-group missed that we in the ES2-group had made it easy for you by requiring #version to be the first bytes, if present), and that there are spec-justification to rejecting the shader in question to compile (due to the character being outside of the character set). And I think we both agree that doing the latter would be a bad idea. But I don't agree that this is per-spec for OpenGL nor OpenGL ES 2.0. It's the ratified spec that is the standard, not whatever discussions were held during the meeting. And even though you have a large collection of shaders that does not use it, I don't think breaking existing (unknown) applications is a good idea. How many shaders besides syntetic glsparsertest-shaders requires line-continuation to work correctly? My guess would be zero; shaders like these would not compile on AMD, NVIDIA, nor Intel's Windows drivers. I've just tested the latter. So apparently, Mesa is the only major OpenGL implementation that currently implements this. By the way, the WebGL conformance tests also checks that line continuation does not work. So there are at least two known, publically available shaders that depends on no line-continuation to work. Of course, the latter is synthetic, but at least it's based on wording in a specification. I'm not trying to be a pain here, I just think you're pushing for a direction that just leads to even more fragmentation and pain. -- 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/20130417/bef34017/attachment.html>
gma500: Other things that I could work on
> > 4) when using modules, initialization from hibernation is not good enough: > >my screen stays black; without using modules, the kernel boots normally, > > and everything is fine. > > 5) initialization from suspend is not good enough: my Asus stays in > >some text mode (80x25?), but shows garbage (possibly data from the > > desired console or > >graphics mode, since sometimes there are reactions correlated to actions) > >Switching tty or using `chvt` do not improve anything. > >NB: hibernating and booting solves this. > > I may have fixed some of your issues, could you please try the latest > gma500-next and report back. > > https://github.com/patjak/drm-gma500.git gma500-next Like I said, I am on vacation. So it took a while before I had some time to spend on this experiment. The good news, both problems seem to be solved on my hardware. Bye, Kero. -- Are you master of your time here? Are you master of your own fate? -- Deep Inside -- Exile -- To-Mera
[RFC][PATCH] drm: Insane but more fine grained locking for planes
On Wed, Apr 17, 2013 at 08:04:52PM +0300, ville.syrjala at linux.intel.com wrote: > From: Ville Syrj?l? > > Instead of locking all modeset locks during plane updates, use just > a single CRTC mutex. To make that work, track the CRTC that "owns" > the plane currently. During enable/update that means the new > CRTC, and during disable it means the old CRTC. > > Since the plane state is no longer protected by a single lock, we > need to sprinkle some additional memory barriers when relinquishing > ownership. Otherwise the next CRTC might observe some stale state > even though the crtc_mutex already got updated. > drm_framebuffer_remove() doesn't need extra barriers since it > already holds all CRTC locks, and thus no-one can be poking around > at the same time. On the read side cmpxchg() already should have > the necessary memory barriers. > > This design implies that before a plane can be moved to another CRTC, > it must be disabled first, even if the hardware would offer some kind > of mechanism to move an active plane over directly. I believe everyone > has agreed that this an acceptable compromise. > > Signed-off-by: Ville Syrj?l? Since the insanity is around plane->crtc_mutex, why not just add a plane mutex which _only_ protects that? That way we could partially resurect the old semantics by simply first grabbing the old crtc mutex, removing the fb, then grabbing the new crtc mutex and displaying it there. Whoever shows up with hw which can do that in one atomic step gets to fix the resulting mess then ;-) -Daniel > --- > drivers/gpu/drm/drm_crtc.c | 43 +++ > include/drm/drm_crtc.h | 3 +++ > 2 files changed, 42 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 957fb70..6f7385e 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -576,6 +576,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) > __drm_framebuffer_unreference(plane->fb); > plane->fb = NULL; > plane->crtc = NULL; > + plane->crtc_mutex = NULL; > } > } > drm_modeset_unlock_all(dev); > @@ -1785,6 +1786,7 @@ int drm_mode_setplane(struct drm_device *dev, void > *data, > int ret = 0; > unsigned int fb_width, fb_height; > int i; > + struct mutex *old_crtc_mutex; > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) > return -EINVAL; > @@ -1804,12 +1806,33 @@ int drm_mode_setplane(struct drm_device *dev, void > *data, > > /* No fb means shut it down */ > if (!plane_req->fb_id) { > - drm_modeset_lock_all(dev); > + struct mutex *crtc_mutex; > + > + retry: > + crtc_mutex = ACCESS_ONCE(plane->crtc_mutex); > + > + /* plane was already disabled? */ > + if (!crtc_mutex) > + return 0; > + > + mutex_lock(crtc_mutex); > + > + /* re-check that plane is still on the same crtc... */ > + if (crtc_mutex != plane->crtc_mutex) { > + mutex_unlock(crtc_mutex); > + goto retry; > + } > + > old_fb = plane->fb; > plane->funcs->disable_plane(plane); > plane->crtc = NULL; > plane->fb = NULL; > - drm_modeset_unlock_all(dev); > + > + smp_wmb(); > + plane->crtc_mutex = NULL; > + > + mutex_unlock(crtc_mutex); > + > goto out; > } > > @@ -1875,7 +1898,15 @@ int drm_mode_setplane(struct drm_device *dev, void > *data, > goto out; > } > > - drm_modeset_lock_all(dev); > + mutex_lock(>mutex); > + > + old_crtc_mutex = cmpxchg(>crtc_mutex, NULL, >mutex); > + if (old_crtc_mutex != NULL && old_crtc_mutex != >mutex) { > + mutex_unlock(>mutex); > + ret = -EBUSY; > + goto out; > + } > + > ret = plane->funcs->update_plane(plane, crtc, fb, >plane_req->crtc_x, plane_req->crtc_y, >plane_req->crtc_w, plane_req->crtc_h, > @@ -1886,8 +1917,12 @@ int drm_mode_setplane(struct drm_device *dev, void > *data, > plane->crtc = crtc; > plane->fb = fb; > fb = NULL; > + } else { > + smp_wmb(); > + plane->crtc_mutex = old_crtc_mutex; > } > - drm_modeset_unlock_all(dev); > + > + mutex_unlock(>mutex); > > out: > if (fb) > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 8c7846b..cc3779f 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -651,6 +651,7 @@ struct drm_plane_funcs { > * @dev: DRM device this plane belongs to > * @head: for list
[PATCH] drm/prime: keep a reference from the handle to exported dma-buf (v4)
On Wed, 2013-04-17 at 10:21 +0200, Daniel Vetter wrote: > On Wed, Apr 17, 2013 at 02:38:02PM +1000, Dave Airlie wrote: > > Currently we have a problem with this: > > 1. i915: create gem object > > 2. i915: export gem object to prime > > 3. radeon: import gem object > > 4. close prime fd > > 5. radeon: unref object > > 6. i915: unref object > > > > i915 has an imported object reference in its file priv, that isn't > > cleaned up properly until fd close. The reference gets added at step 2, > > but at step 6 we don't have enough info to clean it up. > > > > The solution is to take a reference on the dma-buf when we export it, > > and drop the reference when the gem handle goes away. > > > > So when we export a dma_buf from a gem object, we keep track of it > > with the handle, we take a reference to the dma_buf. When we close > > the handle (i.e. userspace is finished with the buffer), we drop > > the reference to the dma_buf, and it gets collected. > > > > This patch isn't meant to fix any other problem or bikesheds, and it doesn't > > fix any races with other scenarios. > > > > v1.1: move export symbol line back up. > > > > v2: okay I had to do a bit more, as the first patch showed a leak > > on one of my tests, that I found using the dma-buf debugfs support, > > the problem case is exporting a buffer twice with the same handle, > > we'd add another export handle for it unnecessarily, however > > we now fail if we try to export the same object with a different gem handle, > > however I'm not sure if that is a case I want to support, and I've > > gotten the code to WARN_ON if we hit something like that. > > > > v2.1: rebase this patch, write better commit msg. > > v3: cleanup error handling, track import vs export in linked list, > > these two patches were separate previously, but seem to work better > > like this. > > v4: danvet is correct, this code is no longer useful, since the buffer > > better exist, so remove it. > > > > Signed-off-by: Dave Airlie > > Reviewed-by: Daniel Vetter > > On bikeshed review level I wonder whether we shouldn't just grab a > reference unconditonally when inserting it into the handle_to_fd lookup > lists. But I need to think through a few other cases and apparently the > self-import test is still a bit broken. So this is material for follow-up > patches. Yes, this is needed, otherwise closing the prime fd will leave stale pointers to the dma buf in the importer's lookup table. I've sent an update to igt/prime_self_test to intel-gfx for showing this. --Imre > -Daniel > > > --- > > drivers/gpu/drm/drm_gem.c | 4 +- > > drivers/gpu/drm/drm_prime.c | 95 > > + > > include/drm/drmP.h | 4 +- > > 3 files changed, 65 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > index af779ae..cf919e3 100644 > > --- a/drivers/gpu/drm/drm_gem.c > > +++ b/drivers/gpu/drm/drm_gem.c > > @@ -205,11 +205,11 @@ static void > > drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file > > *filp) > > { > > if (obj->import_attach) { > > - drm_prime_remove_imported_buf_handle(>prime, > > + drm_prime_remove_buf_handle(>prime, > > obj->import_attach->dmabuf); > > } > > if (obj->export_dma_buf) { > > - drm_prime_remove_imported_buf_handle(>prime, > > + drm_prime_remove_buf_handle(>prime, > > obj->export_dma_buf); > > } > > } > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > > index 366910d..b4ed808 100644 > > --- a/drivers/gpu/drm/drm_prime.c > > +++ b/drivers/gpu/drm/drm_prime.c > > @@ -57,10 +57,14 @@ > > * use the drm_gem_prime_{import,export} helpers. > > */ > > > > +#define PRIME_IMPORT 1 > > +#define PRIME_EXPORT 2 > > + > > struct drm_prime_member { > > struct list_head entry; > > struct dma_buf *dma_buf; > > uint32_t handle; > > + int type; > > }; > > > > static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment > > *attach, > > @@ -157,6 +161,8 @@ static const struct dma_buf_ops > > drm_gem_prime_dmabuf_ops = { > > .vunmap = drm_gem_dmabuf_vunmap, > > }; > > > > +static int drm_prime_add_exported_buf_handle(struct drm_prime_file_private > > *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle); > > + > > /** > > * DOC: PRIME Helpers > > * > > @@ -200,7 +206,9 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, > > { > > struct drm_gem_object *obj; > > void *buf; > > - int ret; > > + int ret = 0; > > + struct dma_buf *dmabuf; > > + uint32_t exp_handle; > > > > obj = drm_gem_object_lookup(dev, file_priv, handle); > > if (!obj) > > @@ -209,43 +217,44 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, > > mutex_lock(_priv->prime.lock); > > /* re-export the original imported object */ > > if (obj->import_attach)
mmotm 2013-04-17-16-02 uploaded (gpu/drm/qxl)
On Wed, Apr 17, 2013 at 6:44 PM, Randy Dunlap wrote: > On 04/17/13 16:03, akpm at linux-foundation.org wrote: >> The mm-of-the-moment snapshot 2013-04-17-16-02 has been uploaded to >> >>http://www.ozlabs.org/~akpm/mmotm/ >> > > > I saw this in linux-next a few days ago and forgot to post it. > > When CONFIG_DEBUG_FS is not enabled: just pushed a fix to my drm-next branch. Thanks. Dave.
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #14 from Ian Romanick --- (In reply to comment #12) > Well, Khronos meetings don't define the spec, the specification does. And > the specification is pretty clear here. The discussion was at the time of the vote, and I stand by that. I can't make the meeting minutes public, and I'm not sure that's the most important thing here. However, I can explain the rationale behind our position for Mesa's compiler. Since you won't be satisfied by the short version of the story, prepare for a long, twisting tale... The problem is that, following the C model of compilation, line continuation is a separate compilation phase, and it occurs before preprocessing. As a result, it's a shocking amount of work to *correctly* dynamically switch behaviors depending on the contents of the shader. The implementation is also likely to have compilation overhead for every shader. Consider a pathological shader like: // \ /* #version 120 // */ \ #version 420 Shaders similar to this one were discussed by Khronos during OpenGL ES 3.0 and OpenGL 4.2 development. Suffice to say, more time than seems reasonable was spent trying to decide 1. what that shader should do, and 2. how to craft spec language to describe that. This is also why issue 12.19 in the GLSL ES 3.0 spec is resolved, "Line-continuation to be made optional in GLSL ES 1.00." Mesa's current GLSL compiler originally had line continuation, but, after having a problem with this game, we removed it. I can't find a specific commit, so this may have been removed before the preprocessor was merged. We encountered a small number of shaders that wanted continuation in preprocessor lines (#define and #line), and these shaders all compile on both NVIDIA and AMD. We added continuation support for those cases in 2010 (commit bc1097d). Both 3.00 ES and 4.20 require continuation, so we had to add it back for all cases. In doing so, of course, we noticed that the Savage2 shader stopped compiling. We thought about just supporting continuation in ES contexts, but the problem still exists since we expose GL_ARB_ES3_compatibility. We searched our internal database of a few thousand shaders from shipping applications (open- and closed-source). This was the only shader didn't want continuation behavior. There was a very small number of shaders that used continuation. If it weren't for the Savage2 shader, frankly, we wouldn't have implemented the non-continuation behavior at all... and nobody would have ever noticed. There were zero shaders that both wanted continuation and non-continuation behavior, and there were zero applications that contained both a continuation shader and a non-continuation shader. We just couldn't justify either the effort or the added code complexity to support dynamic selection. We also couldn't justify breaking a shipping application that previously worked. The right compromise was to have a workaround for the one case that required the non-continuation behavior, and this game works fine on the i965 driver. The Gallium drivers just need to use the workaround, and attachment #78052 should do that. We probably could have done a better job telling the other driver authors that they needed to support this option when it was added. We were overwhelmingly busy trying to get ES3 support finished, so some things like that fell through the cracks. We put the patches on the mailing list (and the commit message in 4b00ece specifically says it fixes Savage2), so it's not like we were being sneaky. We can only do better in the future. The part you didn't mention from section 3.1 of the 1.20 spec is that \ is not listed as part of the character set. If we really want to argue the letter of the spec, the shader should fail to compile because it contains an invalid character. :) -- 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/20130417/6fee35f4/attachment.html>
mmotm 2013-04-17-16-02 uploaded (gpu/drm/qxl)
On 04/17/13 16:03, akpm at linux-foundation.org wrote: > The mm-of-the-moment snapshot 2013-04-17-16-02 has been uploaded to > >http://www.ozlabs.org/~akpm/mmotm/ > I saw this in linux-next a few days ago and forgot to post it. When CONFIG_DEBUG_FS is not enabled: drivers/gpu/drm/qxl/qxl_ttm.c:568:33: error: 'qxl_mm_dump_table' undeclared (first use in this function) drivers/gpu/drm/qxl/qxl_debugfs.c:76:2: error: implicit declaration of function 'drm_debugfs_create_files' [-Werror=implicit-function-declaration] drivers/gpu/drm/qxl/qxl_debugfs.c:84:2: error: implicit declaration of function 'drm_debugfs_remove_files' [-Werror=implicit-function-declaration] -- ~Randy
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #13 from Ian Romanick --- Comment on attachment 78052 --> https://bugs.freedesktop.org/attachment.cgi?id=78052 [PATCH] gallium: handle drirc disable_glsl_line_continuations option You'll probably want review from someone that actually works on Gallium code, but this patch is Reviewed-by: Ian Romanick You should also add NOTE: This is a candidate for the 9.1 branch. to the commit message. -- 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/20130417/d26336f4/attachment.html>
[PATCH 0/4] drm/edid: Recognize 60Hz and 59.94Hz CEA modes
2013/4/17 : > This series attempts to make our CEA mode matching recognize both the > 60Hz and 59.94Hz variants of the modes (and similarly for 24/23.97, > 30/29.97, etc.). > > The benefits should include: > - Send the correct VIC in the AVI infoframe > - Pick the correct RGB quantization range in automatic mode Everything looks correct, but I really didn't test anything. If you apply my comments from patch 2, then you have "Reviewed-by: Paulo Zanoni " for all the 4 patches. Optional bikeshedding: you could add a follow-up patch fixing the comments inside edid_cea_modes to reflect the correct Hz used, replacing, for example, "AxB at 60Hz" with "AxB at 59.94Hz". But I can certainly live without this :) > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Paulo Zanoni
[Bug 56139] [bisected] kernel 3.7.0-rc1 breaks 6950 (CAYMAN)
We have a similar problem: A HD6570(TURKS) card with RS780E chipset. When booting, DVI output is good, but VGA output is black screen; after X started, DVI is still good and VGA blinks forever. We have tried 3.4~3.9-rc7 kernel, all kernels after the commit "drm/radeon: properly handle mc_stop/mc_resume on evergreen+" has the same issue. On Mon, Nov 12, 2012 at 2:55 AM, wrote: > Florian Mickler changed bug > 56139<https://bugs.freedesktop.org/show_bug.cgi?id=56139> > What Removed Added CC florian at mickler.org > > *Comment # 25 <https://bugs.freedesktop.org/show_bug.cgi?id=56139#c25>on bug > 56139 <https://bugs.freedesktop.org/show_bug.cgi?id=56139> from Florian > Mickler * > > A patch referencing this bug report has been merged in Linux v3.7-rc5: > > commit 695ddeb457584a602f2ba117d08ce37cf6ec1589 > Author: Alex Deucher > Date: Mon Nov 5 16:34:58 2012 + > > drm/radeon: fix typo in evergreen_mc_resume() > > -- > You are receiving this mail because: > >- You are the assignee for the bug. > > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130417/a75d4971/attachment.html>
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #12 from Erik Faye-Lund --- Well, Khronos meetings don't define the spec, the specification does. And the specification is pretty clear here. -- 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/20130417/9f8cac0d/attachment.html>
Standalone DRM application
David, I'm developing a small application that uses libdrm (DRM ioctls) to change the resolution of a single graphics display and show a framebuffer. I've run into two problems with this implementation that I'm hoping you can address. 1. Each application is its own process, which is designed to control 1 graphics display. This is unlike X, for instance, which could be configured to grab all of the displays in the system at once. Depending on our stackup, there can be as many as 4 displays connected to a single graphics card. One process could open /dev/dri/card0 and call drmModeSetCrtc() to initialize one of its displays to the requested resolution. However, whenever a second process calls drmModeSetCrtc() to control a second display on the same card, it gets -EPERM back from the ioctl. I've traced this down to the following line in linux/drivers/gpu/drm/drm_drv.c: DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), If I remove the DRM_MASTER flag, then my application behaves correctly, and 4 separate processes can then control each individual display on the card without issue. My question is, is there any real benefit to restricting drm_mode_setcrtc() with DRM_MASTER, or can we lose this flag in order to support one-process-per- display programs like the above? 2. My application has the design requirement that "screen 1" always refers to the card that was initialized by the PC BIOS for bootup. This is the same card that the Linux Console framebuffer will come up on by default, and therefore extra processing is required to handle VT switches (e.g. pause the display, restore original CRTC mode, etc.) Depending on the "Boot Display First [Onboard] or [PCI Slot]" option in the BIOS, this might mean either /dev/dri/card0 or /dev/dri/card1 becomes the default VGA card, as set by the vga_set_default_device() call in arch/x86/pci/fixup.c. Is there a way in userspace to identify which card# is the default card? Or alternatively, is there some way to get the underlying PCI bus/slot ID from a /dev/dri/card# device. Thanks, -Byron
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #11 from Ian Romanick --- (In reply to comment #9) > I don't know where you have the retroactive-story from, but the > specification does not mention it being retroactive, and even goes as far as > to say: I have the story from being in the Khronos meetings. -- 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/20130417/3e318931/attachment.html>
[PATCH 2/4] drm: Add drm_mode_equal_no_clocks()
Hi 2013/4/17 : > From: Ville Syrj?l? > > drm_mode_equal_no_clocks() is like drm_mode_equal() except it doesn't > compare the clock or vrefresh values. drm_mode_equal() is now > implemented by first doing the clock checks, and then calling > drm_mode_equal_no_clocks(). > > Signed-off-by: Ville Syrj?l? > --- > drivers/gpu/drm/drm_modes.c | 20 +++- > include/drm/drm_crtc.h | 1 + > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index 04fa6f1..db85d0b9 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -848,6 +848,24 @@ bool drm_mode_equal(const struct drm_display_mode > *mode1, const struct drm_displ > } else if (mode1->clock != mode2->clock) > return false; > > + return drm_mode_equal_no_clocks(mode1, mode2); > +} EXPORT_SYMBOL(drm_mode_equal) is gone. I'd also add a newline here. > +/** > + * drm_mode_equal_no_clocks - test modes for equality > + * @mode1: first mode > + * @mode2: second mode > + * > + * LOCKING: > + * None. > + * > + * Check to see if @mode1 and @mode2 are equivalent, but > + * don't check the pixel clocks. > + * > + * RETURNS: > + * True if the modes are equal, false otherwise. > + */ > +bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const > struct drm_display_mode *mode2) > +{ > if (mode1->hdisplay == mode2->hdisplay && > mode1->hsync_start == mode2->hsync_start && > mode1->hsync_end == mode2->hsync_end && > @@ -863,7 +881,7 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, > const struct drm_displ > > return false; > } > -EXPORT_SYMBOL(drm_mode_equal); > +EXPORT_SYMBOL(drm_mode_equal_no_clocks); > > /** > * drm_mode_validate_size - make sure modes adhere to size constraints > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index b85575b..836438d 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -922,6 +922,7 @@ extern void drm_mode_config_reset(struct drm_device *dev); > extern void drm_mode_config_cleanup(struct drm_device *dev); > extern void drm_mode_set_name(struct drm_display_mode *mode); > extern bool drm_mode_equal(const struct drm_display_mode *mode1, const > struct drm_display_mode *mode2); > +extern bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, > const struct drm_display_mode *mode2); > extern int drm_mode_width(const struct drm_display_mode *mode); > extern int drm_mode_height(const struct drm_display_mode *mode); > > -- > 1.8.1.5 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Paulo Zanoni
[PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
On Wed, Apr 17, 2013 at 4:11 PM, Bjorn Helgaas wrote: > On Wed, Apr 17, 2013 at 2:04 PM, Alex Deucher > wrote: >> On Wed, Apr 17, 2013 at 8:38 AM, Lucas Kannebley Tavares >> wrote: >>> On 04/12/2013 01:38 PM, Bjorn Helgaas wrote: On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares wrote: > > radeon currently uses a drm function to get the speed capabilities for > the bus. However, this is a non-standard method of performing this > detection and this patch changes it to use the max_bus_speed attribute. > --- > drivers/gpu/drm/radeon/evergreen.c |9 ++--- > drivers/gpu/drm/radeon/r600.c |8 +--- > drivers/gpu/drm/radeon/rv770.c |8 +--- > 3 files changed, 4 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/evergreen.c > b/drivers/gpu/drm/radeon/evergreen.c > index 305a657..3291f62 100644 > --- a/drivers/gpu/drm/radeon/evergreen.c > +++ b/drivers/gpu/drm/radeon/evergreen.c > @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev) > > void evergreen_pcie_gen2_enable(struct radeon_device *rdev) > { > - u32 link_width_cntl, speed_cntl, mask; > - int ret; > + u32 link_width_cntl, speed_cntl; > > if (radeon_pcie_gen2 == 0) > return; > @@ -3871,11 +3870,7 @@ void evergreen_pcie_gen2_enable(struct > radeon_device *rdev) > if (ASIC_IS_X2(rdev)) > return; > > - ret = drm_pcie_get_speed_cap_mask(rdev->ddev,); > - if (ret != 0) > - return; > - > - if (!(mask& DRM_PCIE_SPEED_50)) > > + if (rdev->pdev->bus->max_bus_speed< PCIE_SPEED_5_0GT) For devices on a root bus, we previously dereferenced a NULL pointer in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a root bus. (I think this is the original problem you tripped over, Lucas.) These patches fix that problem. On pseries, where the device *is* on a root bus, your patches set max_bus_speed so this will work as expected. On most other systems, max_bus_speed for root buses will be PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because most arches don't have code like the pseries code you're adding). PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on the root bus, we'll attempt to enable Gen2 on the device even though we have no idea what the bus will support. That's why I originally suggested skipping the Gen2 stuff if "max_bus_speed == PCI_SPEED_UNKNOWN". I was just being conservative, thinking that it's better to have a functional but slow GPU rather than the unknown (to me) effects of enabling Gen2 on a link that might not support it. But I'm fine with this being either way. >>> >>> >>> You're right here, of course. I'll test for it being different from 5_0GT >>> and 8_0GT instead. Though at some point I suppose someone will want to >>> tackle Gen3 speeds. >> >> drm_pcie_get_speed_cap_mask() actually checked the pci bridge to see >> what speeds it supported. If the new code doesn't actually check the >> bridge caps then the new code does not seem like a valid replacement >> unless I'm missing something. > > The max_bus_speed in struct pci_bus is set in pci_set_bus_speed() > based on the PCIe, PCI-X, or AGP capabilities. This happens when we > enumerate the bridge device. Or, in this case, Lucas added > powerpc-specific code to set max_bus_speed for the root bus based on > platform-specific host bridge knowledge. > > So it still does check the PCI bridge capabilities, just not as > directly as it did before. Ah, ok. Thanks. The previous comments about PCI_SPEED_UNKNOWN being set in pci_alloc_bus() and never updated confused me. Alex
[RFC 10/10] ARM: vexpress: Add CLCD Device Tree properties
Signed-off-by: Pawel Moll --- arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 17 + arch/arm/boot/dts/vexpress-v2m.dtsi | 17 + arch/arm/boot/dts/vexpress-v2p-ca9.dts |5 + 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi index ac870fb..aac9459 100644 --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi @@ -41,7 +41,7 @@ bank-width = <4>; }; - vram at 2, { + v2m_vram: vram at 2, { compatible = "arm,vexpress-vram"; reg = <2 0x 0x0080>; }; @@ -233,6 +233,12 @@ interrupts = <14>; clocks = <_oscclk1>, <>; clock-names = "clcdclk", "apb_pclk"; + label = "IOFPGA CLCD"; + video-ram = <_vram>; + display = <_muxfpga 0>; + max-hactive = <640>; + max-vactive = <480>; + max-bpp = <16>; }; }; @@ -282,7 +288,7 @@ /* CLCD clock */ compatible = "arm,vexpress-osc"; arm,vexpress-sysreg,func = <1 1>; - freq-range = <2375 6350>; + freq-range = <2375 6500>; #clock-cells = <0>; clock-output-names = "v2m:oscclk1"; }; @@ -317,9 +323,11 @@ arm,vexpress-sysreg,func = <5 0>; }; - muxfpga at 0 { + v2m_muxfpga: muxfpga at 0 { compatible = "arm,vexpress-muxfpga"; arm,vexpress-sysreg,func = <7 0>; + #display-cells = <1>; + display = <_dvimode>; }; shutdown at 0 { @@ -332,9 +340,10 @@ arm,vexpress-sysreg,func = <9 0>; }; - dvimode at 0 { + v2m_dvimode: dvimode at 0 { compatible = "arm,vexpress-dvimode"; arm,vexpress-sysreg,func = <11 0>; + #display-cells = <0>; }; }; }; diff --git a/arch/arm/boot/dts/vexpress-v2m.dtsi b/arch/arm/boot/dts/vexpress-v2m.dtsi index f142036..4d080d0 100644 --- a/arch/arm/boot/dts/vexpress-v2m.dtsi +++ b/arch/arm/boot/dts/vexpress-v2m.dtsi @@ -40,7 +40,7 @@ bank-width = <4>; }; - vram at 3, { + v2m_vram: vram at 3, { compatible = "arm,vexpress-vram"; reg = <3 0x 0x0080>; }; @@ -232,6 +232,12 @@ interrupts = <14>; clocks = <_oscclk1>, <>; clock-names = "clcdclk", "apb_pclk"; + label = "IOFPGA CLCD"; + video-ram = <_vram>; + display = <_muxfpga 0>; + max-hactive = <640>; + max-vactive = <480>; + max-bpp = <16>; }; }; @@ -281,7 +287,7 @@ /* CLCD clock */ compatible = "arm,vexpress-osc"; arm,vexpress-sysreg,func = <1 1>; - freq-range = <2375 6350>; + freq-range = <2375 6500>; #clock-cells = <0>; clock-output-names = "v2m:oscclk1"; }; @@ -316,9 +322,11 @@ arm,vexpress-sysreg,func = <5 0>; }; - muxfpga at 0 { + v2m_muxfpga: muxfpga at 0 { compatible = "arm,vexpress-muxfpga"; arm,vexpress-sysreg,func = <7 0>; + #display-cells = <1>; + display = <_dvimode>; }; shutdown at 0 { @@ -331,9 +339,10 @@ arm,vexpress-sysreg,func = <9 0>; }; -
[RFC 09/10] video: Versatile Express DVI mode driver
Versatile Express DVI output is driven by a Sii9022 chip. It can be controller to a limited extend by the Motherboard Config Controller, and that's what the driver is doing now. It is a temporary measure till there's a full I2C driver for the chip. Signed-off-by: Pawel Moll --- drivers/video/Makefile |1 + drivers/video/vexpress-dvimode.c | 158 ++ 2 files changed, 159 insertions(+) create mode 100644 drivers/video/vexpress-dvimode.c diff --git a/drivers/video/Makefile b/drivers/video/Makefile index 84c6083..9347e00 100644 --- a/drivers/video/Makefile +++ b/drivers/video/Makefile @@ -179,3 +179,4 @@ obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o # platform specific output drivers obj-$(CONFIG_VEXPRESS_CONFIG)+= vexpress-muxfpga.o +obj-$(CONFIG_VEXPRESS_CONFIG)+= vexpress-dvimode.o diff --git a/drivers/video/vexpress-dvimode.c b/drivers/video/vexpress-dvimode.c new file mode 100644 index 000..85d5608 --- /dev/null +++ b/drivers/video/vexpress-dvimode.c @@ -0,0 +1,158 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * 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. + * + * Copyright (C) 2013 ARM Limited + */ + +#define pr_fmt(fmt) "vexpress-dvimode: " fmt + +#include +#include +#include +#include +#include +#include + + +static struct vexpress_config_func *vexpress_dvimode_func; + + +static int vexpress_dvimode_display_update(struct display_entity *display, + const struct videomode *mode) +{ + static const struct { + u32 hactive, vactive, dvimode; + } dvimodes[] = { + { 640, 480, 0 }, /* VGA */ + { 800, 600, 1 }, /* SVGA */ + { 1024, 768, 2 }, /* XGA */ + { 1280, 1024, 3 }, /* SXGA */ + { 1600, 1200, 4 }, /* UXGA */ + { 1920, 1080, 5 }, /* HD1080 */ + }; + int err = -ENOENT; + int i; + + for (i = 0; i < ARRAY_SIZE(dvimodes); i++) { + if (dvimodes[i].hactive == mode->hactive && + dvimodes[i].vactive == mode->vactive) { + pr_debug("mode: %ux%u = %d\n", mode->hactive, + mode->vactive, dvimodes[i].dvimode); + err = vexpress_config_write(vexpress_dvimode_func, 0, + dvimodes[i].dvimode); + break; + } + } + + if (err) + pr_warn("Failed to set %ux%u mode! (%d)\n", mode->hactive, + mode->vactive, err); + + return err; +} + +static int vexpress_dvimode_display_get_modes(struct display_entity *display, + const struct videomode **modes) +{ + static const struct videomode m[] = { + { + /* VGA */ + .pixelclock = 25175000, + .hactive= 640, + .hback_porch= 40, + .hfront_porch = 24, + .vfront_porch = 11, + .hsync_len = 96, + .vactive= 480, + .vback_porch= 32, + .vsync_len = 2, + }, { + /* XGA */ + .pixelclock = 63500127, + .hactive= 1024, + .hback_porch= 152, + .hfront_porch = 48, + .hsync_len = 104, + .vactive= 768, + .vback_porch= 23, + .vfront_porch = 3, + .vsync_len = 4, + }, { + /* SXGA */ + .pixelclock = 10800, + .hactive= 1280, + .hback_porch= 248, + .hfront_porch = 48, + .hsync_len = 112, + .vactive= 1024, + .vback_porch= 38, + .vfront_porch = 1, + .vsync_len = 3, + }, + }; + + *modes = m; + + return ARRAY_SIZE(m); +} + +static int vexpress_dvimode_display_get_params(struct display_entity *display, + struct display_entity_interface_params *params) +{ + params->type = DISPLAY_ENTITY_INTERFACE_TFT_PARALLEL; + params->p.tft_parallel.r_bits = 8; + params->p.tft_parallel.g_bits = 8; +
[RFC 08/10] video: Versatile Express MUXFPGA driver
Versatile Express' DVI video output can be connected to one the three sources - motherboard's CLCD controller or a video signal generated by one of the daughterboards. This driver provides a Common Display Framework driver for the muxer FPGA, which acts as a switch selecting one of the video data sources. The default source is selected basing on the priority list (which itself can be modified via module paramter), but the user can make his own decision about it using the device's sysfs "source" attribute. Signed-off-by: Pawel Moll --- .../testing/sysfs-driver-video-vexpress-muxfpga|5 + drivers/video/Makefile |3 + drivers/video/vexpress-muxfpga.c | 228 3 files changed, 236 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-video-vexpress-muxfpga create mode 100644 drivers/video/vexpress-muxfpga.c diff --git a/Documentation/ABI/testing/sysfs-driver-video-vexpress-muxfpga b/Documentation/ABI/testing/sysfs-driver-video-vexpress-muxfpga new file mode 100644 index 000..bfd568d --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-video-vexpress-muxfpga @@ -0,0 +1,5 @@ +What: /sys/bus/platform/drivers/vexpress-muxfpga//source +Date: April 2013 +Contant: Pawel Moll +Description: This file stores the id of the video signal source + supposed to be routed to the board's DVI output. diff --git a/drivers/video/Makefile b/drivers/video/Makefile index b989e8e..84c6083 100644 --- a/drivers/video/Makefile +++ b/drivers/video/Makefile @@ -176,3 +176,6 @@ obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o obj-$(CONFIG_OF_DISPLAY_TIMING) += of_display_timing.o obj-$(CONFIG_VIDEOMODE) += videomode.o obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o + +# platform specific output drivers +obj-$(CONFIG_VEXPRESS_CONFIG)+= vexpress-muxfpga.o diff --git a/drivers/video/vexpress-muxfpga.c b/drivers/video/vexpress-muxfpga.c new file mode 100644 index 000..1731ad0 --- /dev/null +++ b/drivers/video/vexpress-muxfpga.c @@ -0,0 +1,228 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * 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. + * + * Copyright (C) 2013 ARM Limited + */ + +#define pr_fmt(fmt) "vexpress-muxfpga: " fmt + +#include +#include +#include +#include +#include +#include + + +static struct vexpress_config_func *vexpress_muxfpga_func; +static struct display_entity *vexpress_muxfpga_output; + + +static struct vexpress_muxfpga_source { + struct display_entity display; + struct videomode mode; + bool updated; +} vexpress_muxfpga_sources[__VEXPRESS_SITE_LAST]; +static u32 vexpress_muxfpga_source_site = VEXPRESS_SITE_MB; +static bool vexpress_muxfpga_source_stored; + + +static int vexpress_muxfpga_set_site(u32 site) +{ + int err; + + if (site >= ARRAY_SIZE(vexpress_muxfpga_sources)) + return -EINVAL; + + err = vexpress_config_write(vexpress_muxfpga_func, 0, site); + if (!err) { + pr_debug("Selected site %d as source\n", site); + vexpress_muxfpga_source_site = site; + } else { + pr_warn("Failed to select site %d as source! (%d)\n", + site, err); + } + + return err; +} + +static unsigned int vexpress_muxfpga_preferred_sites[] = { + VEXPRESS_SITE_MASTER, + VEXPRESS_SITE_DB1, + VEXPRESS_SITE_DB2, + VEXPRESS_SITE_MB, +}; +static unsigned int vexpress_muxfpga_preferred_sites_num = + ARRAY_SIZE(vexpress_muxfpga_preferred_sites); +module_param_array_named(preferred_sites, vexpress_muxfpga_preferred_sites, + uint, _muxfpga_preferred_sites_num, S_IRUGO); +MODULE_PARM_DESC(preferred_sites, "Preferred order of MUXFPGA (DVI output) " + "sources; values can be a daughterboard site ID (1-2), the " + "motherboard ID (0) or a value describing the master site " + "(0xf)."); + +static int vexpress_muxfpga_get_priority(u32 site) +{ + int i; + + for (i = 0; i < vexpress_muxfpga_preferred_sites_num; i++) { + u32 preference = vexpress_muxfpga_preferred_sites[i]; + + if (site == vexpress_get_site(preference)) + return i; + } + + return INT_MAX; +} + +static void vexpress_muxfpga_set_preffered_site(u32 site) +{ + int current_priority = vexpress_muxfpga_get_priority( + vexpress_muxfpga_source_site); + int new_priority = vexpress_muxfpga_get_priority(site); + + if (new_priority <=
[RFC 07/10] mfd: vexpress: Allow external drivers to parse site ids
... by providing a function translating the MASTER value into the currently valid site number and a _LAST constant providing all possible site id values. Signed-off-by: Pawel Moll --- drivers/mfd/vexpress-sysreg.c |5 + include/linux/vexpress.h |2 ++ 2 files changed, 7 insertions(+) diff --git a/drivers/mfd/vexpress-sysreg.c b/drivers/mfd/vexpress-sysreg.c index bf75e96..4158e26 100644 --- a/drivers/mfd/vexpress-sysreg.c +++ b/drivers/mfd/vexpress-sysreg.c @@ -81,6 +81,11 @@ void vexpress_flags_set(u32 data) writel(data, vexpress_sysreg_base + SYS_FLAGSSET); } +u32 vexpress_get_site(int site) +{ + return site == VEXPRESS_SITE_MASTER ? vexpress_master_site : site; +} + u32 vexpress_get_procid(int site) { if (site == VEXPRESS_SITE_MASTER) diff --git a/include/linux/vexpress.h b/include/linux/vexpress.h index 7581874..1ebbcf5 100644 --- a/include/linux/vexpress.h +++ b/include/linux/vexpress.h @@ -19,6 +19,7 @@ #define VEXPRESS_SITE_MB 0 #define VEXPRESS_SITE_DB1 1 #define VEXPRESS_SITE_DB2 2 +#define __VEXPRESS_SITE_LAST 3 #define VEXPRESS_SITE_MASTER 0xf #define VEXPRESS_CONFIG_STATUS_DONE0 @@ -103,6 +104,7 @@ int vexpress_config_write(struct vexpress_config_func *func, int offset, /* Platform control */ +u32 vexpress_get_site(int site); u32 vexpress_get_procid(int site); u32 vexpress_get_hbi(int site); void *vexpress_get_24mhz_clock_base(void); -- 1.7.10.4
[RFC 06/10] video: ARM CLCD: Add DT & CDF support
This patch adds basic DT bindings for the PL11x CLCD cells and make their fbdev driver use them, together with the Common Display Framework. The DT provides information about the hardware configuration and limitations (eg. the largest supported resolution) but the video modes come exclusively from the Common Display Framework drivers, referenced to by the standard CDF binding. Signed-off-by: Pawel Moll --- .../devicetree/bindings/video/arm,pl11x.txt| 35 +++ drivers/video/Kconfig |1 + drivers/video/amba-clcd.c | 244 include/linux/amba/clcd.h |2 + 4 files changed, 282 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/arm,pl11x.txt diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt new file mode 100644 index 000..ee9534a --- /dev/null +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt @@ -0,0 +1,35 @@ +* ARM PrimeCell Color LCD Controller (CLCD) PL110/PL111 + +Required properties: + +- compatible : must be one of: + "arm,pl110", "arm,primecell" + "arm,pl111", "arm,primecell" +- reg : base address and size of the control registers block +- interrupts : the combined interrupt +- clocks : phandles to the CLCD (pixel) clock and the APB clocks +- clock-names : "clcdclk", "apb_pclk" +- display : phandle to the display entity connected to the controller + +Optional properties: + +- label : string describing the controller location and/or usage +- video-ram : phandle to DT node of the specialized video RAM to be used +- max-hactive : maximum frame buffer width in pixels +- max-vactive : maximum frame buffer height in pixels +- max-bpp : maximum number of bits per pixel +- big-endian-pixels : defining this property makes the pixel bytes being + accessed in Big Endian organization + +Example: + + clcd at 1f { + compatible = "arm,pl111", "arm,primecell"; + reg = <0x1f 0x1000>; + interrupts = <14>; + clocks = <_oscclk1>, <>; + clock-names = "clcdclk", "apb_pclk"; + label = "IOFPGA CLCD"; + video-ram = <_vram>; + display = <_muxfpga>; + }; diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 281e548..bad8166 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -340,6 +340,7 @@ config FB_ARMCLCD select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT + select FB_MODE_HELPERS if OF help This framebuffer device driver is for the ARM PrimeCell PL110 Colour LCD controller. ARM PrimeCells provide the building diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c index 0a2cce7..778dc03 100644 --- a/drivers/video/amba-clcd.c +++ b/drivers/video/amba-clcd.c @@ -25,6 +25,11 @@ #include #include #include +#include +#include +#include +#include +#include #include @@ -62,6 +67,10 @@ static void clcdfb_disable(struct clcd_fb *fb) { u32 val; + if (fb->panel->display) + display_entity_set_state(fb->panel->display, + DISPLAY_ENTITY_STATE_OFF); + if (fb->board->disable) fb->board->disable(fb); @@ -115,6 +124,11 @@ static void clcdfb_enable(struct clcd_fb *fb, u32 cntl) */ if (fb->board->enable) fb->board->enable(fb); + + if (fb->panel->display) + display_entity_set_state(fb->panel->display, + DISPLAY_ENTITY_STATE_ON); + } static int @@ -304,6 +318,13 @@ static int clcdfb_set_par(struct fb_info *info) clcdfb_enable(fb, regs.cntl); + if (fb->panel->display) { + struct videomode mode; + + videomode_from_fb_var_screeninfo(>fb.var, ); + display_entity_update(fb->panel->display, ); + } + #ifdef DEBUG printk(KERN_INFO "CLCD: Registers set to\n" @@ -542,6 +563,226 @@ static int clcdfb_register(struct clcd_fb *fb) return ret; } +#if defined(CONFIG_OF) +static int clcdfb_of_get_tft_parallel_panel(struct clcd_panel *panel, + struct display_entity_interface_params *params) +{ + int r = params->p.tft_parallel.r_bits; + int g = params->p.tft_parallel.g_bits; + int b = params->p.tft_parallel.b_bits; + + /* Bypass pixel clock divider, data output on the falling edge */ + panel->tim2 = TIM2_BCD | TIM2_IPC; + + /* TFT display, vert. comp. interrupt at the start of the back porch */ + panel->cntl |= CNTL_LCDTFT | CNTL_LCDVCOMP(1);
[RFC 05/10] fbmon: Add extra video helper
This function converts the fb_var_screeninfo to the videomode structure, to be used in fbdev drivers working with the Common Display Framework. Signed-off-by: Pawel Moll --- drivers/video/fbmon.c | 29 + include/linux/fb.h|3 +++ 2 files changed, 32 insertions(+) diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c index 7f67099..f0ff2bf 100644 --- a/drivers/video/fbmon.c +++ b/drivers/video/fbmon.c @@ -1424,6 +1424,35 @@ int fb_videomode_from_videomode(const struct videomode *vm, return 0; } EXPORT_SYMBOL_GPL(fb_videomode_from_videomode); + +void videomode_from_fb_var_screeninfo(const struct fb_var_screeninfo *var, + struct videomode *vm) +{ + vm->pixelclock = PICOS2KHZ(var->pixclock) * 1000; + + vm->hactive = var->xres; + vm->hfront_porch = var->right_margin; + vm->hback_porch = var->left_margin; + vm->hsync_len = var->hsync_len; + + vm->vactive = var->yres; + vm->vfront_porch = var->lower_margin; + vm->vback_porch = var->upper_margin; + vm->vsync_len = var->vsync_len; + + vm->dmt_flags = 0; + if (var->sync & FB_SYNC_HOR_HIGH_ACT) + vm->dmt_flags |= VESA_DMT_HSYNC_HIGH; + if (var->sync & FB_SYNC_VERT_HIGH_ACT) + vm->dmt_flags |= VESA_DMT_VSYNC_HIGH; + + vm->data_flags = 0; + if (var->vmode & FB_VMODE_INTERLACED) + vm->data_flags |= DISPLAY_FLAGS_INTERLACED; + if (var->vmode & FB_VMODE_DOUBLE) + vm->data_flags |= DISPLAY_FLAGS_DOUBLESCAN; +} +EXPORT_SYMBOL_GPL(videomode_from_fb_var_screeninfo); #endif #if IS_ENABLED(CONFIG_OF_VIDEOMODE) diff --git a/include/linux/fb.h b/include/linux/fb.h index 58b9860..aae2ed3 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -721,6 +721,9 @@ extern int of_get_fb_videomode(struct device_node *np, int index); extern int fb_videomode_from_videomode(const struct videomode *vm, struct fb_videomode *fbmode); +extern void videomode_from_fb_var_screeninfo( + const struct fb_var_screeninfo *var, + struct videomode *vm); /* drivers/video/modedb.c */ #define VESA_MODEDB_SIZE 34 -- 1.7.10.4
[RFC 04/10] video: display: Add generic TFT display type
TFT panels may be interfaced via a simple parallel interface carrying RGB data, pixel clock and synchronisation signals. >From the video generator point of view the width of the data channels (number of bits per R/G/B components) may be an important factor in setting up the display model. Above information is based on the presentations by Dave Anders available here: http://elinux.org/Elc-lcd This patch adds the parallel TFT display type and basic parameters structure. Maybe it should be split into a separate header, eg. include/video/tft.h? Or maybe it's just the INTERFACE_DPI I'm talking about? Signed-off-by: Pawel Moll --- include/video/display.h |9 + 1 file changed, 9 insertions(+) diff --git a/include/video/display.h b/include/video/display.h index 7fe8b2f..875e230 100644 --- a/include/video/display.h +++ b/include/video/display.h @@ -69,10 +69,19 @@ enum display_entity_stream_state { enum display_entity_interface_type { DISPLAY_ENTITY_INTERFACE_DPI, + DISPLAY_ENTITY_INTERFACE_TFT_PARALLEL, +}; + +struct tft_parallel_interface_params { + int r_bits, g_bits, b_bits; + int r_b_swapped; }; struct display_entity_interface_params { enum display_entity_interface_type type; + union { + struct tft_parallel_interface_params tft_parallel; + } p; }; struct display_entity_control_ops { -- 1.7.10.4
[RFC 03/10] video: display: Add Device Tree bindings
Modelled after the common clock solution, the bindings are based on the idea of display entity "providers" and "consumers". Signed-off-by: Pawel Moll --- .../devicetree/bindings/video/display-bindings.txt | 75 + drivers/video/display/display-core.c | 84 include/video/display.h| 11 +++ 3 files changed, 170 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/display-bindings.txt diff --git a/Documentation/devicetree/bindings/video/display-bindings.txt b/Documentation/devicetree/bindings/video/display-bindings.txt new file mode 100644 index 000..6d8b888 --- /dev/null +++ b/Documentation/devicetree/bindings/video/display-bindings.txt @@ -0,0 +1,75 @@ +[this is an RFC] + +Common Display Framework define a display entity (eg. LCD panel), +being a sink for video data generated by a video signal generator +(eg. LCD controller/driver). + +This set of bindings allow to represent connections between them +in the Device Tree. + +Devices nodes representing display sinks are called "display +providers" and nodes representing display sources are called +"display consumers". + +Notice that in both cases a device represented by a node can +provide or consume more than one display entity. For example +a LCD controller can be able to driver more than one LCD +panel at the same time, while a panel (or a special signal +multiplexer) may have more than one input (sink) and switch +between them. + +== Display provider == + +Required properties: + +* #clock-cells:Number of cells in the display specifier. Typically + 0 for nodes providing single display entity and 1 + for nodes providing multiple displays. + +Example: + dvi-output: dvi-output at 0 { + #display-cells = <0>; + }; + +== Display consumer == + +Required properties: + +* display: List of phandle and clock specifier pairs, one pair + for each display (sink). Note: if the display provider + specifies '0' for #display-cells, then only the phandle + portion of the pair will appear. + +Example: + + display-driver { + display = <>; + }; + +== Larger example == + + clcd at 1002 { + compatible = "arm,pl111", "arm,primecell"; + reg = <0x1002 0x1000>; + interrupts = <0 44 4>; + clocks = <>, <>; + clock-names = "clcdclk", "apb_pclk"; + label = "V2P-CA9 CLCD"; + display = <_muxfpga 0xf>; + max-hactive = <1024>; + max-vactive = <768>; + max-bpp = <16>; + }; + + v2m_muxfpga: muxfpga at 0 { + compatible = "arm,vexpress-muxfpga"; + arm,vexpress-sysreg,func = <7 0>; + #display-cells = <1>; + display = <_dvimode>; + }; + + v2m_dvimode: dvimode at 0 { + compatible = "arm,vexpress-dvimode"; + arm,vexpress-sysreg,func = <11 0>; + #display-cells = <0>; + }; diff --git a/drivers/video/display/display-core.c b/drivers/video/display/display-core.c index 4b8e45a..9827a5d 100644 --- a/drivers/video/display/display-core.c +++ b/drivers/video/display/display-core.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -230,6 +231,89 @@ void display_entity_put(struct display_entity *entity) } EXPORT_SYMBOL_GPL(display_entity_put); +#if defined(CONFIG_OF) +struct of_display_entity_provider { + struct list_head list; + struct device_node *node; + struct display_entity *(*get)(struct of_phandle_args *spec, void *data); + void *data; +}; + +static LIST_HEAD(of_display_entity_providers); +static DEFINE_MUTEX(of_display_entity_providers_lock); + +int of_display_entity_add_provider(struct device_node *node, + struct display_entity *(*get)(struct of_phandle_args *spec, + void *data), void *data) +{ + struct of_display_entity_provider *provider = + kzalloc(sizeof(*provider), GFP_KERNEL); + + if (!provider) + return -ENOMEM; + + provider->node = of_node_get(node); + provider->get = get; + provider->data = data; + + mutex_lock(_display_entity_providers_lock); + list_add(>list, _display_entity_providers); + mutex_unlock(_display_entity_providers_lock); + +
[RFC 02/10] video: display: Update the display with the video mode data
The display entity (sink) may need to know about the mode being changed, eg. to update timings. Alternatively there could be a separate set_mode() operation... Signed-off-by: Pawel Moll --- drivers/video/display/display-core.c |5 +++-- include/video/display.h |6 -- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/video/display/display-core.c b/drivers/video/display/display-core.c index d2daa15..4b8e45a 100644 --- a/drivers/video/display/display-core.c +++ b/drivers/video/display/display-core.c @@ -69,12 +69,13 @@ EXPORT_SYMBOL_GPL(display_entity_set_state); * * Return 0 on success or a negative error code otherwise. */ -int display_entity_update(struct display_entity *entity) +int display_entity_update(struct display_entity *entity, +const struct videomode *mode) { if (!entity->ops.ctrl || !entity->ops.ctrl->update) return 0; - return entity->ops.ctrl->update(entity); + return entity->ops.ctrl->update(entity, mode); } EXPORT_SYMBOL_GPL(display_entity_update); diff --git a/include/video/display.h b/include/video/display.h index 90d18ca..64f84d5 100644 --- a/include/video/display.h +++ b/include/video/display.h @@ -77,7 +77,8 @@ struct display_entity_interface_params { struct display_entity_control_ops { int (*set_state)(struct display_entity *ent, enum display_entity_state state); - int (*update)(struct display_entity *ent); + int (*update)(struct display_entity *ent, +const struct videomode *mode); int (*get_modes)(struct display_entity *ent, const struct videomode **modes); int (*get_params)(struct display_entity *ent, @@ -111,7 +112,8 @@ struct display_entity { int display_entity_set_state(struct display_entity *entity, enum display_entity_state state); -int display_entity_update(struct display_entity *entity); +int display_entity_update(struct display_entity *entity, +const struct videomode *mode); int display_entity_get_modes(struct display_entity *entity, const struct videomode **modes); int display_entity_get_params(struct display_entity *entity, -- 1.7.10.4
[RFC 01/10] video: Add generic display entity core
From: Laurent PinchartSigned-off-by: Laurent Pinchart --- drivers/video/Kconfig|1 + drivers/video/Makefile |1 + drivers/video/display/Kconfig|4 + drivers/video/display/Makefile |1 + drivers/video/display/display-core.c | 362 ++ include/video/display.h | 150 ++ 6 files changed, 519 insertions(+) create mode 100644 drivers/video/display/Kconfig create mode 100644 drivers/video/display/Makefile create mode 100644 drivers/video/display/display-core.c create mode 100644 include/video/display.h diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 4c1546f..281e548 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -2456,6 +2456,7 @@ source "drivers/video/omap2/Kconfig" source "drivers/video/exynos/Kconfig" source "drivers/video/mmp/Kconfig" source "drivers/video/backlight/Kconfig" +source "drivers/video/display/Kconfig" if VT source "drivers/video/console/Kconfig" diff --git a/drivers/video/Makefile b/drivers/video/Makefile index 9df3873..b989e8e 100644 --- a/drivers/video/Makefile +++ b/drivers/video/Makefile @@ -15,6 +15,7 @@ fb-objs := $(fb-y) obj-$(CONFIG_VT) += console/ obj-$(CONFIG_LOGO) += logo/ obj-y+= backlight/ +obj-y+= display/ obj-$(CONFIG_EXYNOS_VIDEO) += exynos/ diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig new file mode 100644 index 000..1d533e7 --- /dev/null +++ b/drivers/video/display/Kconfig @@ -0,0 +1,4 @@ +menuconfig DISPLAY_CORE + tristate "Display Core" + ---help--- + Support common display framework for graphics devices. diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile new file mode 100644 index 000..bd93496 --- /dev/null +++ b/drivers/video/display/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_DISPLAY_CORE) += display-core.o diff --git a/drivers/video/display/display-core.c b/drivers/video/display/display-core.c new file mode 100644 index 000..d2daa15 --- /dev/null +++ b/drivers/video/display/display-core.c @@ -0,0 +1,362 @@ +/* + * Display Core + * + * Copyright (C) 2012 Renesas Solutions Corp. + * + * Contacts: Laurent Pinchart + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include + +#include +#include + +static LIST_HEAD(display_entity_list); +static LIST_HEAD(display_entity_notifiers); +static DEFINE_MUTEX(display_entity_mutex); + +/* - + * Control operations + */ + +/** + * display_entity_set_state - Set the display entity operation state + * @entity: The display entity + * @state: Display entity operation state + * + * See display_entity_state for information regarding the entity states. + * + * Return 0 on success or a negative error code otherwise. + */ +int display_entity_set_state(struct display_entity *entity, +enum display_entity_state state) +{ + int ret; + + if (entity->state == state) + return 0; + + if (!entity->ops.ctrl || !entity->ops.ctrl->set_state) + return 0; + + ret = entity->ops.ctrl->set_state(entity, state); + if (ret < 0) + return ret; + + entity->state = state; + return 0; +} +EXPORT_SYMBOL_GPL(display_entity_set_state); + +/** + * display_entity_update - Update the display + * @entity: The display entity + * + * Make the display entity ready to receive pixel data and start frame transfer. + * This operation can only be called if the display entity is in STANDBY or ON + * state. + * + * The display entity will call the upstream entity in the video chain to start + * the video stream. + * + * Return 0 on success or a negative error code otherwise. + */ +int display_entity_update(struct display_entity *entity) +{ + if (!entity->ops.ctrl || !entity->ops.ctrl->update) + return 0; + + return entity->ops.ctrl->update(entity); +} +EXPORT_SYMBOL_GPL(display_entity_update); + +/** + * display_entity_get_modes - Get video modes supported by the display entity + * @entity The display entity + * @modes: Pointer to an array of modes + * + * Fill the modes argument with a pointer to an array of video modes. The array + * is owned by the display entity. + * + * Return the number of supported modes on success (including 0 if no mode is + * supported) or a negative error code otherwise. + */ +int display_entity_get_modes(struct display_entity *entity, +const struct videomode **modes) +{ + if (!entity->ops.ctrl ||
[RFC 00/10] Versatile Express CLCD DVI output support
Hello All, This series implements support for the Versatile Express video output pipeline, which is not the simplest one available... It is meant as a RFC only and I'm hoping to attract all possible feedback (*including* naming ;-). The VE's "MultiMedia Bus" [1] comprises three video signal sources (motherboard's CLCD cell and a implementation-specific driver on each of the daugtherboards) and a FPGA multiplexer routing data from one of the sources to DVI/HDMI formatter chip (Sii9022). +-+ | DB1 |>--+ DVI connector +-+ | +--+ +--+ +-->| | |oo| +-+ | mux |+-+|oo| | DB2 |>->| |>-->| sii9022 |>-->|oo| +-+ | FPGA |+-+|oo| +-->| | |oo| +-+ | +--+ +--+ | MB |>--+ +-+ The series is based on Laurent Pinchart's Common Display Framework patch (not in mainline yet, v2 discussed here: [2]) and extends it by adding DT bindings and basic support for TFT panels. The CLCD driver has been adapted to work with the framework and the Device Tree information. Also a set of drivers for the VE-specific components is included (note that the sii9022 is now driven via the moterboard firmware; this is intended to be replaced by a proper I2C driver for the chip). It is worth mentioning that the CDF caters for both fbdev and DRM so the solution should be suitable for all potential DRM-driven display controllers. [1] http://infocenter.arm.com/help/topic/com.arm.doc.dui0447h/CHDEHEAA.html#CACGIGGC [2] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/57298 Laurent Pinchart (1): video: Add generic display entity core Pawel Moll (9): video: display: Update the display with the video mode data video: display: Add Device Tree bindings video: display: Add generic TFT display type fbmon: Add extra video helper video: ARM CLCD: Add DT & CDF support mfd: vexpress: Allow external drivers to parse site ids video: Versatile Express MUXFPGA driver video: Versatile Express DVI mode driver ARM: vexpress: Add CLCD Device Tree properties .../testing/sysfs-driver-video-vexpress-muxfpga|5 + .../devicetree/bindings/video/arm,pl11x.txt| 35 ++ .../devicetree/bindings/video/display-bindings.txt | 75 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi| 17 +- arch/arm/boot/dts/vexpress-v2m.dtsi| 17 +- arch/arm/boot/dts/vexpress-v2p-ca9.dts |5 + drivers/mfd/vexpress-sysreg.c |5 + drivers/video/Kconfig |2 + drivers/video/Makefile |5 + drivers/video/amba-clcd.c | 244 +++ drivers/video/display/Kconfig |4 + drivers/video/display/Makefile |1 + drivers/video/display/display-core.c | 447 drivers/video/fbmon.c | 29 ++ drivers/video/vexpress-dvimode.c | 158 +++ drivers/video/vexpress-muxfpga.c | 228 ++ include/linux/amba/clcd.h |2 + include/linux/fb.h |3 + include/linux/vexpress.h |2 + include/video/display.h| 172 20 files changed, 1448 insertions(+), 8 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-video-vexpress-muxfpga create mode 100644 Documentation/devicetree/bindings/video/arm,pl11x.txt create mode 100644 Documentation/devicetree/bindings/video/display-bindings.txt create mode 100644 drivers/video/display/Kconfig create mode 100644 drivers/video/display/Makefile create mode 100644 drivers/video/display/display-core.c create mode 100644 drivers/video/vexpress-dvimode.c create mode 100644 drivers/video/vexpress-muxfpga.c create mode 100644 include/video/display.h -- 1.7.10.4
[PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
On Wed, Apr 17, 2013 at 8:38 AM, Lucas Kannebley Tavares wrote: > On 04/12/2013 01:38 PM, Bjorn Helgaas wrote: >> >> On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares >> wrote: >>> >>> radeon currently uses a drm function to get the speed capabilities for >>> the bus. However, this is a non-standard method of performing this >>> detection and this patch changes it to use the max_bus_speed attribute. >>> --- >>> drivers/gpu/drm/radeon/evergreen.c |9 ++--- >>> drivers/gpu/drm/radeon/r600.c |8 +--- >>> drivers/gpu/drm/radeon/rv770.c |8 +--- >>> 3 files changed, 4 insertions(+), 21 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/radeon/evergreen.c >>> b/drivers/gpu/drm/radeon/evergreen.c >>> index 305a657..3291f62 100644 >>> --- a/drivers/gpu/drm/radeon/evergreen.c >>> +++ b/drivers/gpu/drm/radeon/evergreen.c >>> @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev) >>> >>> void evergreen_pcie_gen2_enable(struct radeon_device *rdev) >>> { >>> - u32 link_width_cntl, speed_cntl, mask; >>> - int ret; >>> + u32 link_width_cntl, speed_cntl; >>> >>> if (radeon_pcie_gen2 == 0) >>> return; >>> @@ -3871,11 +3870,7 @@ void evergreen_pcie_gen2_enable(struct >>> radeon_device *rdev) >>> if (ASIC_IS_X2(rdev)) >>> return; >>> >>> - ret = drm_pcie_get_speed_cap_mask(rdev->ddev,); >>> - if (ret != 0) >>> - return; >>> - >>> - if (!(mask& DRM_PCIE_SPEED_50)) >>> >>> + if (rdev->pdev->bus->max_bus_speed< PCIE_SPEED_5_0GT) >> >> >> For devices on a root bus, we previously dereferenced a NULL pointer >> in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a >> root bus. (I think this is the original problem you tripped over, >> Lucas.) >> >> These patches fix that problem. On pseries, where the device *is* on >> a root bus, your patches set max_bus_speed so this will work as >> expected. On most other systems, max_bus_speed for root buses will be >> PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because >> most arches don't have code like the pseries code you're adding). >> >> PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on >> the root bus, we'll attempt to enable Gen2 on the device even though >> we have no idea what the bus will support. >> >> That's why I originally suggested skipping the Gen2 stuff if >> "max_bus_speed == PCI_SPEED_UNKNOWN". I was just being conservative, >> thinking that it's better to have a functional but slow GPU rather >> than the unknown (to me) effects of enabling Gen2 on a link that might >> not support it. But I'm fine with this being either way. > > > You're right here, of course. I'll test for it being different from 5_0GT > and 8_0GT instead. Though at some point I suppose someone will want to > tackle Gen3 speeds. drm_pcie_get_speed_cap_mask() actually checked the pci bridge to see what speeds it supported. If the new code doesn't actually check the bridge caps then the new code does not seem like a valid replacement unless I'm missing something. Alex > > >> >> It would be nice if we could get rid of drm_pcie_get_speed_cap_mask() >> altogether. It is exported, but I have no idea of anybody else uses >> it. Maybe it could at least be marked __deprecated now? >> > > I'll mark it. > >> I don't know who should take these patches. They don't touch >> drivers/pci, but I'd be happy to push them, given the appropriate ACKs >> from DRM and powerpc folks. >> >> Bjorn >> >>> return; >>> >>> speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL); >>> diff --git a/drivers/gpu/drm/radeon/r600.c >>> b/drivers/gpu/drm/radeon/r600.c >>> index 0740db3..64b90c0 100644 >>> --- a/drivers/gpu/drm/radeon/r600.c >>> +++ b/drivers/gpu/drm/radeon/r600.c >>> @@ -4351,8 +4351,6 @@ static void r600_pcie_gen2_enable(struct >>> radeon_device *rdev) >>> { >>> u32 link_width_cntl, lanes, speed_cntl, training_cntl, tmp; >>> u16 link_cntl2; >>> - u32 mask; >>> - int ret; >>> >>> if (radeon_pcie_gen2 == 0) >>> return; >>> @@ -4371,11 +4369,7 @@ static void r600_pcie_gen2_enable(struct >>> radeon_device *rdev) >>> if (rdev->family<= CHIP_R600) >>> return; >>> >>> - ret = drm_pcie_get_speed_cap_mask(rdev->ddev,); >>> - if (ret != 0) >>> - return; >>> - >>> - if (!(mask& DRM_PCIE_SPEED_50)) >>> >>> + if (rdev->pdev->bus->max_bus_speed< PCIE_SPEED_5_0GT) >>> return; >>> >>> speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL); >>> diff --git a/drivers/gpu/drm/radeon/rv770.c >>> b/drivers/gpu/drm/radeon/rv770.c >>> index d63fe1d..c683c36 100644 >>> --- a/drivers/gpu/drm/radeon/rv770.c >>> +++ b/drivers/gpu/drm/radeon/rv770.c >>> @@ -1238,8 +1238,6 @@ static void rv770_pcie_gen2_enable(struct >>> radeon_device *rdev) >>> { >>>
[PATCH] drm/prime: keep a reference from the handle to exported dma-buf (v4)
Currently we have a problem with this: 1. i915: create gem object 2. i915: export gem object to prime 3. radeon: import gem object 4. close prime fd 5. radeon: unref object 6. i915: unref object i915 has an imported object reference in its file priv, that isn't cleaned up properly until fd close. The reference gets added at step 2, but at step 6 we don't have enough info to clean it up. The solution is to take a reference on the dma-buf when we export it, and drop the reference when the gem handle goes away. So when we export a dma_buf from a gem object, we keep track of it with the handle, we take a reference to the dma_buf. When we close the handle (i.e. userspace is finished with the buffer), we drop the reference to the dma_buf, and it gets collected. This patch isn't meant to fix any other problem or bikesheds, and it doesn't fix any races with other scenarios. v1.1: move export symbol line back up. v2: okay I had to do a bit more, as the first patch showed a leak on one of my tests, that I found using the dma-buf debugfs support, the problem case is exporting a buffer twice with the same handle, we'd add another export handle for it unnecessarily, however we now fail if we try to export the same object with a different gem handle, however I'm not sure if that is a case I want to support, and I've gotten the code to WARN_ON if we hit something like that. v2.1: rebase this patch, write better commit msg. v3: cleanup error handling, track import vs export in linked list, these two patches were separate previously, but seem to work better like this. v4: danvet is correct, this code is no longer useful, since the buffer better exist, so remove it. Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_gem.c | 4 +- drivers/gpu/drm/drm_prime.c | 95 + include/drm/drmP.h | 4 +- 3 files changed, 65 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index af779ae..cf919e3 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -205,11 +205,11 @@ static void drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) { if (obj->import_attach) { - drm_prime_remove_imported_buf_handle(>prime, + drm_prime_remove_buf_handle(>prime, obj->import_attach->dmabuf); } if (obj->export_dma_buf) { - drm_prime_remove_imported_buf_handle(>prime, + drm_prime_remove_buf_handle(>prime, obj->export_dma_buf); } } diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 366910d..b4ed808 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -57,10 +57,14 @@ * use the drm_gem_prime_{import,export} helpers. */ +#define PRIME_IMPORT 1 +#define PRIME_EXPORT 2 + struct drm_prime_member { struct list_head entry; struct dma_buf *dma_buf; uint32_t handle; + int type; }; static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, @@ -157,6 +161,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { .vunmap = drm_gem_dmabuf_vunmap, }; +static int drm_prime_add_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle); + /** * DOC: PRIME Helpers * @@ -200,7 +206,9 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, { struct drm_gem_object *obj; void *buf; - int ret; + int ret = 0; + struct dma_buf *dmabuf; + uint32_t exp_handle; obj = drm_gem_object_lookup(dev, file_priv, handle); if (!obj) @@ -209,43 +217,44 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, mutex_lock(_priv->prime.lock); /* re-export the original imported object */ if (obj->import_attach) { - get_dma_buf(obj->import_attach->dmabuf); - *prime_fd = dma_buf_fd(obj->import_attach->dmabuf, flags); - drm_gem_object_unreference_unlocked(obj); - mutex_unlock(_priv->prime.lock); - return 0; + dmabuf = obj->import_attach->dmabuf; + goto out_have_obj; } if (obj->export_dma_buf) { - get_dma_buf(obj->export_dma_buf); - *prime_fd = dma_buf_fd(obj->export_dma_buf, flags); - drm_gem_object_unreference_unlocked(obj); - } else { - buf = dev->driver->gem_prime_export(dev, obj, flags); - if (IS_ERR(buf)) { - /* normally the created dma-buf takes ownership of the ref, -* but if that fails then drop the ref -*/ - drm_gem_object_unreference_unlocked(obj); - mutex_unlock(_priv->prime.lock); - return
[GIT PULL] exynos-drm-next
Hi Dave, This is initial pull request for Exynos. It includes a big change that it makes drm_display_mode for timings parameters to be used for exynos4 and exynos5 commonly and cleans up unnecessary codes. And also it adds device tree support for fimd to get timing values and interrupt source from dts file. In addition, one more patch, device tree support feature for Exynos FIMC, is being reviewed. This patch was posted a little ago like below, http://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg17568.html So we are going to request git pull one more time after reviewed. Please kindly let me know if there is any problem. Thanks, Inki Dae The following changes since commit 62c8ba7c58e4163f975c5f8b5a3dd5f306a2deda: drm/qxl: fix smatch warnings (2013-04-16 13:36:00 +1000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next for you to fetch changes up to dd4d34fde068a9362d09292d881ce4743bb11c7f: Revert "of/exynos_g2d: Add Bindings for exynos G2D driver" (2013-04-17 00:07:29 +0900) Rahul Sharma (2): drm/exynos: hdmi: using drm_display_mode timings for exynos4 drm/exynos: hdmi: move mode_fixup to drm common hdmi Sachin Kamat (5): drm/exynos: hdmi: Fix incorrect usage of IS_ERR_OR_NULL drm/exynos: mixer: Fix incorrect usage of IS_ERR_OR_NULL drm/exynos: drm_rotator: Fix incorrect usage of IS_ERR_OR_NULL drm/exynos: drm_connector: Fix error check condition Revert "of/exynos_g2d: Add Bindings for exynos G2D driver" Vikas Sajjan (3): drm/exynos: Add display-timing node parsing using video helper function drm/exynos: enable OF_VIDEOMODE and FB_MODE_HELPERS for exynos drm fimd drm/exynos: change the method for getting the interrupt .../devicetree/bindings/drm/exynos/g2d.txt | 22 - drivers/gpu/drm/exynos/Kconfig | 4 +- drivers/gpu/drm/exynos/exynos_drm_connector.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 26 +- drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 40 +- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 3 - drivers/gpu/drm/exynos/exynos_drm_rotator.c| 2 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 704 +++-- drivers/gpu/drm/exynos/exynos_mixer.c | 14 +- 9 files changed, 312 insertions(+), 505 deletions(-) delete mode 100644 Documentation/devicetree/bindings/drm/exynos/g2d.txt
[PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
On Wed, Apr 17, 2013 at 2:17 PM, Alex Deucher wrote: > On Wed, Apr 17, 2013 at 4:11 PM, Bjorn Helgaas wrote: >> On Wed, Apr 17, 2013 at 2:04 PM, Alex Deucher >> wrote: >>> On Wed, Apr 17, 2013 at 8:38 AM, Lucas Kannebley Tavares >>> wrote: On 04/12/2013 01:38 PM, Bjorn Helgaas wrote: > > On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares > wrote: >> >> radeon currently uses a drm function to get the speed capabilities for >> the bus. However, this is a non-standard method of performing this >> detection and this patch changes it to use the max_bus_speed attribute. >> --- >> drivers/gpu/drm/radeon/evergreen.c |9 ++--- >> drivers/gpu/drm/radeon/r600.c |8 +--- >> drivers/gpu/drm/radeon/rv770.c |8 +--- >> 3 files changed, 4 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/evergreen.c >> b/drivers/gpu/drm/radeon/evergreen.c >> index 305a657..3291f62 100644 >> --- a/drivers/gpu/drm/radeon/evergreen.c >> +++ b/drivers/gpu/drm/radeon/evergreen.c >> @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev) >> >> void evergreen_pcie_gen2_enable(struct radeon_device *rdev) >> { >> - u32 link_width_cntl, speed_cntl, mask; >> - int ret; >> + u32 link_width_cntl, speed_cntl; >> >> if (radeon_pcie_gen2 == 0) >> return; >> @@ -3871,11 +3870,7 @@ void evergreen_pcie_gen2_enable(struct >> radeon_device *rdev) >> if (ASIC_IS_X2(rdev)) >> return; >> >> - ret = drm_pcie_get_speed_cap_mask(rdev->ddev,); >> - if (ret != 0) >> - return; >> - >> - if (!(mask& DRM_PCIE_SPEED_50)) >> >> + if (rdev->pdev->bus->max_bus_speed< PCIE_SPEED_5_0GT) > > > For devices on a root bus, we previously dereferenced a NULL pointer > in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a > root bus. (I think this is the original problem you tripped over, > Lucas.) > > These patches fix that problem. On pseries, where the device *is* on > a root bus, your patches set max_bus_speed so this will work as > expected. On most other systems, max_bus_speed for root buses will be > PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because > most arches don't have code like the pseries code you're adding). > > PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on > the root bus, we'll attempt to enable Gen2 on the device even though > we have no idea what the bus will support. > > That's why I originally suggested skipping the Gen2 stuff if > "max_bus_speed == PCI_SPEED_UNKNOWN". I was just being conservative, > thinking that it's better to have a functional but slow GPU rather > than the unknown (to me) effects of enabling Gen2 on a link that might > not support it. But I'm fine with this being either way. You're right here, of course. I'll test for it being different from 5_0GT and 8_0GT instead. Though at some point I suppose someone will want to tackle Gen3 speeds. >>> >>> drm_pcie_get_speed_cap_mask() actually checked the pci bridge to see >>> what speeds it supported. If the new code doesn't actually check the >>> bridge caps then the new code does not seem like a valid replacement >>> unless I'm missing something. >> >> The max_bus_speed in struct pci_bus is set in pci_set_bus_speed() >> based on the PCIe, PCI-X, or AGP capabilities. This happens when we >> enumerate the bridge device. Or, in this case, Lucas added >> powerpc-specific code to set max_bus_speed for the root bus based on >> platform-specific host bridge knowledge. >> >> So it still does check the PCI bridge capabilities, just not as >> directly as it did before. > > Ah, ok. Thanks. The previous comments about PCI_SPEED_UNKNOWN being > set in pci_alloc_bus() and never updated confused me. Yeah, that's just for root buses where we don't have the host bridge knowledge to figure it out. The root bus case was broken in drm_pcie_get_speed_cap_mask() anyway, because there is no upstream P2P bridge to look at. (That's why Lucas tripped over the null pointer dereference in the first place.)
[PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
On Wed, Apr 17, 2013 at 2:04 PM, Alex Deucher wrote: > On Wed, Apr 17, 2013 at 8:38 AM, Lucas Kannebley Tavares > wrote: >> On 04/12/2013 01:38 PM, Bjorn Helgaas wrote: >>> >>> On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares >>> wrote: radeon currently uses a drm function to get the speed capabilities for the bus. However, this is a non-standard method of performing this detection and this patch changes it to use the max_bus_speed attribute. --- drivers/gpu/drm/radeon/evergreen.c |9 ++--- drivers/gpu/drm/radeon/r600.c |8 +--- drivers/gpu/drm/radeon/rv770.c |8 +--- 3 files changed, 4 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 305a657..3291f62 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev) void evergreen_pcie_gen2_enable(struct radeon_device *rdev) { - u32 link_width_cntl, speed_cntl, mask; - int ret; + u32 link_width_cntl, speed_cntl; if (radeon_pcie_gen2 == 0) return; @@ -3871,11 +3870,7 @@ void evergreen_pcie_gen2_enable(struct radeon_device *rdev) if (ASIC_IS_X2(rdev)) return; - ret = drm_pcie_get_speed_cap_mask(rdev->ddev,); - if (ret != 0) - return; - - if (!(mask& DRM_PCIE_SPEED_50)) + if (rdev->pdev->bus->max_bus_speed< PCIE_SPEED_5_0GT) >>> >>> >>> For devices on a root bus, we previously dereferenced a NULL pointer >>> in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a >>> root bus. (I think this is the original problem you tripped over, >>> Lucas.) >>> >>> These patches fix that problem. On pseries, where the device *is* on >>> a root bus, your patches set max_bus_speed so this will work as >>> expected. On most other systems, max_bus_speed for root buses will be >>> PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because >>> most arches don't have code like the pseries code you're adding). >>> >>> PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on >>> the root bus, we'll attempt to enable Gen2 on the device even though >>> we have no idea what the bus will support. >>> >>> That's why I originally suggested skipping the Gen2 stuff if >>> "max_bus_speed == PCI_SPEED_UNKNOWN". I was just being conservative, >>> thinking that it's better to have a functional but slow GPU rather >>> than the unknown (to me) effects of enabling Gen2 on a link that might >>> not support it. But I'm fine with this being either way. >> >> >> You're right here, of course. I'll test for it being different from 5_0GT >> and 8_0GT instead. Though at some point I suppose someone will want to >> tackle Gen3 speeds. > > drm_pcie_get_speed_cap_mask() actually checked the pci bridge to see > what speeds it supported. If the new code doesn't actually check the > bridge caps then the new code does not seem like a valid replacement > unless I'm missing something. The max_bus_speed in struct pci_bus is set in pci_set_bus_speed() based on the PCIe, PCI-X, or AGP capabilities. This happens when we enumerate the bridge device. Or, in this case, Lucas added powerpc-specific code to set max_bus_speed for the root bus based on platform-specific host bridge knowledge. So it still does check the PCI bridge capabilities, just not as directly as it did before. Bjorn
[PATCH] drm/prime: keep a reference from the handle to exported dma-buf (v3)
Currently we have a problem with this: 1. i915: create gem object 2. i915: export gem object to prime 3. radeon: import gem object 4. close prime fd 5. radeon: unref object 6. i915: unref object i915 has an imported object reference in its file priv, that isn't cleaned up properly until fd close. The reference gets added at step 2, but at step 6 we don't have enough info to clean it up. The solution is to take a reference on the dma-buf when we export it, and drop the reference when the gem handle goes away. So when we export a dma_buf from a gem object, we keep track of it with the handle, we take a reference to the dma_buf. When we close the handle (i.e. userspace is finished with the buffer), we drop the reference to the dma_buf, and it gets collected. This patch isn't meant to fix any other problem or bikesheds, and it doesn't fix any races with other scenarios. v1.1: move export symbol line back up. v2: okay I had to do a bit more, as the first patch showed a leak on one of my tests, that I found using the dma-buf debugfs support, the problem case is exporting a buffer twice with the same handle, we'd add another export handle for it unnecessarily, however we now fail if we try to export the same object with a different gem handle, however I'm not sure if that is a case I want to support, and I've gotten the code to WARN_ON if we hit something like that. v2.1: rebase this patch, write better commit msg. v3: cleanup error handling, track import vs export in linked list, these two patches were separate previously, but seem to work better like this. Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_gem.c | 4 +- drivers/gpu/drm/drm_prime.c | 100 +--- include/drm/drmP.h | 4 +- 3 files changed, 71 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index af779ae..cf919e3 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -205,11 +205,11 @@ static void drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) { if (obj->import_attach) { - drm_prime_remove_imported_buf_handle(>prime, + drm_prime_remove_buf_handle(>prime, obj->import_attach->dmabuf); } if (obj->export_dma_buf) { - drm_prime_remove_imported_buf_handle(>prime, + drm_prime_remove_buf_handle(>prime, obj->export_dma_buf); } } diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 366910d..37f393e 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -57,10 +57,14 @@ * use the drm_gem_prime_{import,export} helpers. */ +#define PRIME_IMPORT 1 +#define PRIME_EXPORT 2 + struct drm_prime_member { struct list_head entry; struct dma_buf *dma_buf; uint32_t handle; + int type; }; static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, @@ -157,6 +161,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { .vunmap = drm_gem_dmabuf_vunmap, }; +static int drm_prime_add_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle); + /** * DOC: PRIME Helpers * @@ -201,6 +207,8 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, struct drm_gem_object *obj; void *buf; int ret; + struct dma_buf *dmabuf; + uint32_t exp_handle; obj = drm_gem_object_lookup(dev, file_priv, handle); if (!obj) @@ -209,43 +217,51 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, mutex_lock(_priv->prime.lock); /* re-export the original imported object */ if (obj->import_attach) { - get_dma_buf(obj->import_attach->dmabuf); - *prime_fd = dma_buf_fd(obj->import_attach->dmabuf, flags); - drm_gem_object_unreference_unlocked(obj); - mutex_unlock(_priv->prime.lock); - return 0; + dmabuf = obj->import_attach->dmabuf; + goto out_have_obj; } if (obj->export_dma_buf) { - get_dma_buf(obj->export_dma_buf); - *prime_fd = dma_buf_fd(obj->export_dma_buf, flags); - drm_gem_object_unreference_unlocked(obj); - } else { - buf = dev->driver->gem_prime_export(dev, obj, flags); - if (IS_ERR(buf)) { - /* normally the created dma-buf takes ownership of the ref, -* but if that fails then drop the ref -*/ - drm_gem_object_unreference_unlocked(obj); - mutex_unlock(_priv->prime.lock); - return PTR_ERR(buf); - } - obj->export_dma_buf = buf; - *prime_fd = dma_buf_fd(buf, flags); +
[RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.
On Tuesday 16 April 2013, Nishanth Menon wrote: > On 12:50-20130416, Arnd Bergmann wrote: > > On Tuesday 16 April 2013 12:48:28 Paul Sokolovsky wrote: > > > > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > > > > index 8d1e2bb..73a99e4 100644 > > > > --- a/include/uapi/drm/drm.h > > > > +++ b/include/uapi/drm/drm.h > > > > @@ -36,7 +36,7 @@ > > > > #ifndef _DRM_H_ > > > > #define _DRM_H_ > > > > > > > > -#if defined(__linux__) > > > > +#if defined(__KERNEL__) || defined(__linux__) > > > > > > > > #include > > > > #include > > > > This is still completely bogus, the __KERNEL__ symbol has no significance > > here. > > Either make the compiler define __linux__, or remove this #ifdef completely. > > > Searching the v.39-rc7 tag, and greping for _linux_ a few interesting > list pops up. (pruned): > arch/arc/Makefile:cflags-y+= -mA7 -fno-common -pipe -fno-builtin > -D__linux__ > arch/h8300/Makefile:KBUILD_CFLAGS += -D__linux__ > arch/hexagon/Makefile:KBUILD_CFLAGS += -ffixed-$(TIR_NAME) > -DTHREADINFO_REG=$(TIR_NAME) -D__linux__ > arch/score/Makefile: -D__linux__ -ffunction-sections -ffreestanding > arch/xtensa/Makefile:KBUILD_CFLAGS += -ffreestanding -D__linux__ > ^^ these architectures seem to bypass the pain entirely by defining > __linux__ Right, that seems like a reasonable approach when the compilers are actually known to be compatible. > arch/mips/include/uapi/asm/sgidefs.h:#ifndef __linux__ On MIPS, they are not. If you are building a Linux kernel with a gcc that was targetted at another ABI, the system call interface may change, so they forbid that here. > drivers/gpu/drm/radeon/radeon_cp.c:#ifdef __linux__ > {snip} > drivers/scsi/aic7xxx/aic7770.c:#ifdef __linux__ > drivers/scsi/aic7xxx/aic79xx.h:#ifndef __linux__ > {snip} > drivers/scsi/aic7xxx/aic79xx_core.c:#ifdef __linux__ > {snip} > drivers/scsi/aic7xxx/aic79xx_pci.c:#ifdef __linux__ > drivers/scsi/aic7xxx/aic7xxx.h:#ifndef __linux__ > drivers/scsi/aic7xxx/aic7xxx.h:#ifndef __linux__ > drivers/scsi/aic7xxx/aic7xxx_93cx6.c:#ifdef __linux__ > drivers/scsi/aic7xxx/aic7xxx_core.c:#ifdef __linux__ > {snip} > drivers/scsi/aic7xxx/aic7xxx_pci.c:#ifdef __linux__ > drivers/scsi/aic7xxx/aicasm/aicasm.h:#ifdef __linux__ > drivers/scsi/aic7xxx/aicasm/aicasm_macro_scan.l:#ifdef __linux__ > drivers/scsi/aic7xxx/aicasm/aicasm_scan.l:#ifdef __linux__ > drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c:#ifdef __linux__ > drivers/scsi/aic7xxx/aicasm/aicasm_symbol.h:#ifdef __linux__ > drivers/scsi/dpt/osd_defs.h:#if (defined(__linux__)) > drivers/staging/ced1401/machine.h:#if (defined(__linux__) || defined(_linux) > || defined(__linux)) && !defined(LINUX) These are all drivers that are shared with another OS, or used to be. In most of them, we can probably just remove the #else path, since I don't think they are getting synchronized any more. > include/acpi/platform/acenv.h:#if defined(_LINUX) || defined(__linux__) The acpi header files are maintained outside of Linux and are kept OS-independent. > include/linux/coda.h:#if defined(__linux__) > include/uapi/drm/drm.h:#if defined(__linux__) > include/uapi/linux/coda.h:#if defined(__linux__) > include/uapi/linux/fuse.h:#ifdef __linux__ In case of coda, we should not need to care any more, that header just got broken by the uapi-split for other operating systems. The drm.h and fuse.h header files are in theory still kept OS-agnostic and are actively maintained. > And then we have the following as well.. > fs/ext4/ext4.h:#if defined(__KERNEL__) || defined(__linux__) This seems to have been copied from the ext2 utils. The ext2/ext3 versions of this file don't have support for other operating systems. > Trying out a few different prebuilt compilers I had around, I see: > http://pastebin.com/bTVDLTb1 > > So, is our approach just to use __linux__ for builds? I am trying to > understand rationale behind why #include #include > > would want __linux__ and why __KERNEL__ check is un-wanted. > Ofcourse, I cant comment about the "One of the BSDs" in else options.. > and why we'd like to keep it around in kernel :) We might be in a similar situation on ARM that we are in on MIPS. For instance, there are some compilers that are targetting (old) Android that have a slightly different ABI, and building a kernel with those results in a system call ABI that is incompatible with user space built with a standard compiler. The safest approach is probably to bail out early if __linux__ is not set, and force anyone that wants to use a strange compiler to set the macro manually. Arnd
[PATCH v2] drm/nouveau: always select ACPI_VIDEO if ACPI is enabled.
On Wed, Feb 20, 2013 at 07:56:33PM +0100, Borislav Petkov wrote: > On Tue, Feb 05, 2013 at 05:22:06PM +0100, Borislav Petkov wrote: > > On Tue, Feb 05, 2013 at 04:38:35PM +0100, Maarten Lankhorst wrote: > > > Argh, next attempt, based on i915's Kconfig. > > > > > > It seems that not only I have to select ACPI_VIDEO, I also have to select > > > all the dependencies. > > > Is this a Kconfig bug or working as intended? i915 seems to have a > > > workaround, so I copied it from > > > there. Except it's currently missing select THERMAL, so I guess it didn't > > > get updated when that got > > > added. > > > > > > >8 > > > Having nouveau builtin would still allow ACPI_VIDEO to be used as > > > external module > > > if some of the deps for acpi_video have not been met, which would result > > > in a linking > > > failure. Solve this by selecting all dependencies as well. > > > > > > Signed-off-by: Maarten Lankhorst > > > > Yep, this takes care of all deps, > > > > Tested-by: Borislav Petkov > > Just to check whether anyone has picked this already - it is triggering > on 3.8 too so it most probably needs a stable tag too. Ok, this is not good. So we have -rc7+, this patch has been out there for 2 months and still not in Linus' tree. Linus, the patch below fixes a build failure here with my custom config. I think this late in the game, we need some kind of a fix before 3.9 is released. I'm adding the patch here in case you want to pick it up yourself. Thanks. -- From: Maarten LankhorstDate: Wed, 17 Apr 2013 12:04:09 +0200 Subject: [PATCH] drm/nouveau: always select ACPI_VIDEO if ACPI is enabled. Having nouveau builtin would still allow ACPI_VIDEO to be used as external module if some of the deps for acpi_video have not been met, which would result in a linking failure. Solve this by selecting all dependencies as well. Signed-off-by: Maarten Lankhorst Tested-by: Borislav Petkov --- drivers/gpu/drm/nouveau/Kconfig | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index a7ff6d5a34b9..845b7caa9ee1 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -10,11 +10,17 @@ config DRM_NOUVEAU select FB select FRAMEBUFFER_CONSOLE if !EXPERT select FB_BACKLIGHT if DRM_NOUVEAU_BACKLIGHT - select ACPI_VIDEO if ACPI && X86 && BACKLIGHT_CLASS_DEVICE && VIDEO_OUTPUT_CONTROL && INPUT select X86_PLATFORM_DEVICES if ACPI && X86 select ACPI_WMI if ACPI && X86 select MXM_WMI if ACPI && X86 select POWER_SUPPLY + # Similar to i915, we need to select ACPI_VIDEO and it's dependencies + select BACKLIGHT_LCD_SUPPORT if ACPI && X86 + select BACKLIGHT_CLASS_DEVICE if ACPI && X86 + select VIDEO_OUTPUT_CONTROL if ACPI && X86 + select INPUT if ACPI && X86 + select THERMAL if ACPI && X86 + select ACPI_VIDEO if ACPI && X86 help Choose this option for open-source nVidia support. -- 1.8.2.135.g7b592fa -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. - -
[RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.
On Tue, 2013-04-16 at 12:50 +0200, Arnd Bergmann wrote: > On Tuesday 16 April 2013 12:48:28 Paul Sokolovsky wrote: > > > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > > > index 8d1e2bb..73a99e4 100644 > > > --- a/include/uapi/drm/drm.h > > > +++ b/include/uapi/drm/drm.h > > > @@ -36,7 +36,7 @@ > > > #ifndef _DRM_H_ > > > #define _DRM_H_ > > > > > > -#if defined(__linux__) > > > +#if defined(__KERNEL__) || defined(__linux__) > > > > > > #include > > > #include > > This is still completely bogus, the __KERNEL__ symbol has no significance > here. > Either make the compiler define __linux__, or remove this #ifdef completely. I don't have the full background here but thought I would point out that a similar issue seems to have just cropped up with fuse.h ... http://lkml.org/lkml/2013/4/15/487 -- Tixy
[PATCH v2 3/3] drm/exynos: Add device tree support for fimc ipp driver
This patch adds OF initialization support for the FIMC driver. The binding documentation can be found at Documentation/devicetree/ bindings/media/samsung-fimc.txt. The syscon regmap interface is used to serialize access to the shared CAMBLK registers from within the V4L2 FIMC-IS and the DRM FIMC drivers. The DRM driver uses this interface for setting up the FIFO data link between FIMD and FIMC IP blocks, while the V4L2 one for setting up a data link between the camera ISP and FIMC for camera capture. The CAMBLK registers are not accessed any more through a statically mapped IO. Synchronized access to these registers is required for simultaneous operation of the camera ISP and the DRM IPP on Exynos4x12. Exynos4 is going to be a dt-only platform since 3.10, thus the driver data and driver_ids static data structures are removed. Camera input signal polarities are not currently parsed from the device tree. Signed-off-by: Sylwester Nawrocki Signed-off-by: Kyungmin Park --- drivers/gpu/drm/exynos/Kconfig |2 +- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 101 +++--- drivers/gpu/drm/exynos/regs-fimc.h |7 +-- 3 files changed, 53 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index 406f32a..5c4be2a 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -56,7 +56,7 @@ config DRM_EXYNOS_IPP config DRM_EXYNOS_FIMC bool "Exynos DRM FIMC" - depends on DRM_EXYNOS_IPP + depends on DRM_EXYNOS_IPP && MFD_SYSCON && OF help Choose this option if you want to use Exynos FIMC for DRM. diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index bc8411a..2e182ba 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -12,11 +12,12 @@ * */ #include +#include #include #include +#include #include #include -#include #include #include @@ -140,15 +141,6 @@ struct fimc_capability { }; /* - * A structure of fimc driver data. - * - * @parent_clk: name of parent clock. - */ -struct fimc_driverdata { - char*parent_clk; -}; - -/* * A structure of fimc context. * * @ippdrv: prepare initialization using ippdrv. @@ -158,6 +150,7 @@ struct fimc_driverdata { * @lock: locking of operations. * @clocks: fimc clocks. * @clk_frequency: LCLK clock frequency. + * @sysreg: handle to SYSREG block regmap. * @sc: scaler infomations. * @pol: porarity of writeback. * @id: fimc id. @@ -172,8 +165,8 @@ struct fimc_context { struct mutexlock; struct clk *clocks[FIMC_CLKS_MAX]; u32 clk_frequency; + struct regmap *sysreg; struct fimc_scaler sc; - struct fimc_driverdata *ddata; struct exynos_drm_ipp_pol pol; int id; int irq; @@ -217,17 +210,13 @@ static void fimc_sw_reset(struct fimc_context *ctx) fimc_write(0x0, EXYNOS_CIFCNTSEQ); } -static void fimc_set_camblk_fimd0_wb(struct fimc_context *ctx) +static int fimc_set_camblk_fimd0_wb(struct fimc_context *ctx) { - u32 camblk_cfg; - DRM_DEBUG_KMS("%s\n", __func__); - camblk_cfg = readl(SYSREG_CAMERA_BLK); - camblk_cfg &= ~(SYSREG_FIMD0WB_DEST_MASK); - camblk_cfg |= ctx->id << (SYSREG_FIMD0WB_DEST_SHIFT); - - writel(camblk_cfg, SYSREG_CAMERA_BLK); + return regmap_update_bits(ctx->sysreg, SYSREG_CAMERA_BLK, + SYSREG_FIMD0WB_DEST_MASK, + ctx->id << SYSREG_FIMD0WB_DEST_SHIFT); } static void fimc_set_type_ctrl(struct fimc_context *ctx, enum fimc_wb wb) @@ -1628,7 +1617,9 @@ static int fimc_ippdrv_start(struct device *dev, enum drm_exynos_ipp_cmd cmd) fimc_handle_lastend(ctx, true); /* setup FIMD */ - fimc_set_camblk_fimd0_wb(ctx); + ret = fimc_set_camblk_fimd0_wb(ctx); + if (ret < 0) + return ret; set_wb.enable = 1; set_wb.refresh = property->refresh_rate; @@ -1784,26 +1775,50 @@ e_clk_free: return ret; } +static int fimc_parse_dt(struct fimc_context *ctx) +{ + struct device_node *node = ctx->dev->of_node; + + if (!of_property_read_bool(node, "samsung,lcd-wb")) + return -ENODEV; + + if (of_property_read_u32(node, "clock-frequency", + >clk_frequency)) + ctx->clk_frequency = FIMC_DEFAULT_LCLK_FREQUENCY; + + ctx->id = of_alias_get_id(node, "fimc"); + if (ctx->id < 0) + return -EINVAL; + + return 0; +} + static int fimc_probe(struct platform_device *pdev) { struct device *dev = >dev; struct fimc_context *ctx; struct resource *res; struct exynos_drm_ippdrv *ippdrv; - struct
[PATCH v2 2/3] drm/exynos: Rework fimc clocks handling
The clocks handling is refactored and a "mux" clock handling is added to account for changes in the clocks driver. After switching to the common clock framework the sclk_fimc clock is now split into two clocks: a gate and a mux clock. In order to retain the exisiting functionality two additional consumer clocks are passed to the driver from device tree: "mux" and "parent". Then the driver sets "parent" clock as a parent clock of the "mux" clock. These two additional clocks are optional, and should go away when there is a standard way of setting up parent clocks on DT platforms. Signed-off-by: Sylwester Nawrocki Signed-off-by: Kyungmin Park --- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 167 +- 1 file changed, 97 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index d812c57..bc8411a 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -76,6 +76,27 @@ enum fimc_wb { FIMC_WB_B, }; +enum { + FIMC_CLK_LCLK, + FIMC_CLK_GATE, + FIMC_CLK_WB_A, + FIMC_CLK_WB_B, + FIMC_CLK_MUX, + FIMC_CLK_PARENT, + FIMC_CLKS_MAX +}; + +static const char * fimc_clock_names[] = { + [FIMC_CLK_LCLK] = "sclk_fimc", + [FIMC_CLK_GATE] = "fimc", + [FIMC_CLK_WB_A] = "pxl_async0", + [FIMC_CLK_WB_B] = "pxl_async1", + [FIMC_CLK_MUX]= "mux", + [FIMC_CLK_PARENT] = "parent", +}; + +#define FIMC_DEFAULT_LCLK_FREQUENCY 13300UL + /* * A structure of scaler. * @@ -132,15 +153,12 @@ struct fimc_driverdata { * * @ippdrv: prepare initialization using ippdrv. * @regs_res: register resources. + * @dev: pointer to the fimc device structure. * @regs: memory mapped io registers. * @lock: locking of operations. - * @sclk_fimc_clk: fimc source clock. - * @fimc_clk: fimc clock. - * @wb_clk: writeback a clock. - * @wb_b_clk: writeback b clock. + * @clocks: fimc clocks. + * @clk_frequency: LCLK clock frequency. * @sc: scaler infomations. - * @odr: ordering of YUV. - * @ver: fimc version. * @pol: porarity of writeback. * @id: fimc id. * @irq: irq number. @@ -148,13 +166,12 @@ struct fimc_driverdata { */ struct fimc_context { struct exynos_drm_ippdrvippdrv; + struct device *dev; struct resource *regs_res; void __iomem*regs; struct mutexlock; - struct clk *sclk_fimc_clk; - struct clk *fimc_clk; - struct clk *wb_clk; - struct clk *wb_b_clk; + struct clk *clocks[FIMC_CLKS_MAX]; + u32 clk_frequency; struct fimc_scaler sc; struct fimc_driverdata *ddata; struct exynos_drm_ipp_pol pol; @@ -1301,14 +1318,12 @@ static int fimc_clk_ctrl(struct fimc_context *ctx, bool enable) DRM_DEBUG_KMS("%s:enable[%d]\n", __func__, enable); if (enable) { - clk_enable(ctx->sclk_fimc_clk); - clk_enable(ctx->fimc_clk); - clk_enable(ctx->wb_clk); + clk_prepare_enable(ctx->clocks[FIMC_CLK_GATE]); + clk_prepare_enable(ctx->clocks[FIMC_CLK_WB_A]); ctx->suspended = false; } else { - clk_disable(ctx->sclk_fimc_clk); - clk_disable(ctx->fimc_clk); - clk_disable(ctx->wb_clk); + clk_disable_unprepare(ctx->clocks[FIMC_CLK_GATE]); + clk_disable_unprepare(ctx->clocks[FIMC_CLK_WB_A]); ctx->suspended = true; } @@ -1713,11 +1728,66 @@ static void fimc_ippdrv_stop(struct device *dev, enum drm_exynos_ipp_cmd cmd) fimc_write(cfg, EXYNOS_CIGCTRL); } +static void fimc_put_clocks(struct fimc_context *ctx) +{ + int i; + + for (i = 0; i < FIMC_CLKS_MAX; i++) { + if (IS_ERR(ctx->clocks[i])) + continue; + clk_put(ctx->clocks[i]); + ctx->clocks[i] = ERR_PTR(-EINVAL); + } +} + +static int fimc_setup_clocks(struct fimc_context *ctx) +{ + struct device *dev; + int ret, i; + + for (i = 0; i < FIMC_CLKS_MAX; i++) + ctx->clocks[i] = ERR_PTR(-EINVAL); + + for (i = 0; i < FIMC_CLKS_MAX; i++) { + if (i == FIMC_CLK_WB_A || i == FIMC_CLK_WB_B) + dev = ctx->dev->parent; + else + dev = ctx->dev; + + ctx->clocks[i] = clk_get(dev, fimc_clock_names[i]); + if (IS_ERR(ctx->clocks[i])) { + if (i >= FIMC_CLK_MUX) + break; + ret = PTR_ERR(ctx->clocks[i]); + goto e_clk_free; + } + } + + if (!IS_ERR(ctx->clocks[FIMC_CLK_PARENT])) { + ret = clk_set_parent(ctx->clocks[FIMC_CLK_MUX], +
[PATCH v2 1/3] drm/exynos: Remove redundant devm_kfree()
There is no need for explicit calls of devm_kfree(), as the allocated memory will be freed during driver's detach. Remove the redundant devm_kfree() calls from probe() and remove() callbacks. Signed-off-by: Sylwester Nawrocki Signed-off-by: Kyungmin Park --- drivers/gpu/drm/exynos/exynos_drm_fimc.c |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index 411f69b7..d812c57 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1843,7 +1843,6 @@ static int fimc_probe(struct platform_device *pdev) return 0; err_ippdrv_register: - devm_kfree(dev, ippdrv->prop_list); pm_runtime_disable(dev); err_get_irq: free_irq(ctx->irq, ctx); @@ -1857,7 +1856,6 @@ static int fimc_remove(struct platform_device *pdev) struct fimc_context *ctx = get_fimc_context(dev); struct exynos_drm_ippdrv *ippdrv = >ippdrv; - devm_kfree(dev, ippdrv->prop_list); exynos_drm_ippdrv_unregister(ippdrv); mutex_destroy(>lock); -- 1.7.9.5
[PATCH v2 0/3] drm/exynos: Add device tree support for IPP driver
Hi Inki, This small patch series adds device tree support for the DRM FIMC driver. The binding documentation can be found in -next at Documentation/devicetree/ bindings/media/samsung-fimc.txt. It will make the driver dependent on OF. This patch series is needed in 3.10 to ensure simultaneous operation of the DRM FIMC and the camera ISP on Exynos4x12. Changes since v1: - removed devm_kfree() that got erroneously re-added in patch 2/3 during rebase. Thanks, Sylwester Sylwester Nawrocki (3): drm/exynos: Remove redundant devm_kfree() drm/exynos: Rework fimc clocks handling drm/exynos: Add device tree support for fimc ipp driver drivers/gpu/drm/exynos/Kconfig |2 +- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 264 -- drivers/gpu/drm/exynos/regs-fimc.h |7 +- 3 files changed, 147 insertions(+), 126 deletions(-) -- 1.7.9.5
[PATCH 2/3] drm/exynos: Rework fimc clocks handling
On 04/17/2013 06:02 AM, Sachin Kamat wrote: > Hi Sylwester, > > On 16 April 2013 23:01, Sylwester Nawrocki wrote: >> @@ -1835,16 +1859,19 @@ static int fimc_probe(struct platform_device *pdev) >> ret = exynos_drm_ippdrv_register(ippdrv); >> if (ret < 0) { >> dev_err(dev, "failed to register drm fimc device.\n"); >> - goto err_ippdrv_register; >> + goto err_pm_dis; >> } >> >> dev_info(>dev, "drm fimc registered successfully.\n"); >> >> return 0; >> >> -err_ippdrv_register: >> +err_pm_dis: >> + devm_kfree(dev, ippdrv->prop_list); > > devm_kfree was removed in patch1 of this series. Do we need it back? Certainly not, that's a rebase error. I'll resend it fixed. Thanks for your review. Regards, Sylwester
[PATCH] drm/prime: keep a reference from the handle to exported dma-buf (v4)
On Wed, Apr 17, 2013 at 02:38:02PM +1000, Dave Airlie wrote: > Currently we have a problem with this: > 1. i915: create gem object > 2. i915: export gem object to prime > 3. radeon: import gem object > 4. close prime fd > 5. radeon: unref object > 6. i915: unref object > > i915 has an imported object reference in its file priv, that isn't > cleaned up properly until fd close. The reference gets added at step 2, > but at step 6 we don't have enough info to clean it up. > > The solution is to take a reference on the dma-buf when we export it, > and drop the reference when the gem handle goes away. > > So when we export a dma_buf from a gem object, we keep track of it > with the handle, we take a reference to the dma_buf. When we close > the handle (i.e. userspace is finished with the buffer), we drop > the reference to the dma_buf, and it gets collected. > > This patch isn't meant to fix any other problem or bikesheds, and it doesn't > fix any races with other scenarios. > > v1.1: move export symbol line back up. > > v2: okay I had to do a bit more, as the first patch showed a leak > on one of my tests, that I found using the dma-buf debugfs support, > the problem case is exporting a buffer twice with the same handle, > we'd add another export handle for it unnecessarily, however > we now fail if we try to export the same object with a different gem handle, > however I'm not sure if that is a case I want to support, and I've > gotten the code to WARN_ON if we hit something like that. > > v2.1: rebase this patch, write better commit msg. > v3: cleanup error handling, track import vs export in linked list, > these two patches were separate previously, but seem to work better > like this. > v4: danvet is correct, this code is no longer useful, since the buffer > better exist, so remove it. > > Signed-off-by: Dave Airlie Reviewed-by: Daniel Vetter On bikeshed review level I wonder whether we shouldn't just grab a reference unconditonally when inserting it into the handle_to_fd lookup lists. But I need to think through a few other cases and apparently the self-import test is still a bit broken. So this is material for follow-up patches. -Daniel > --- > drivers/gpu/drm/drm_gem.c | 4 +- > drivers/gpu/drm/drm_prime.c | 95 > + > include/drm/drmP.h | 4 +- > 3 files changed, 65 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index af779ae..cf919e3 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -205,11 +205,11 @@ static void > drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file > *filp) > { > if (obj->import_attach) { > - drm_prime_remove_imported_buf_handle(>prime, > + drm_prime_remove_buf_handle(>prime, > obj->import_attach->dmabuf); > } > if (obj->export_dma_buf) { > - drm_prime_remove_imported_buf_handle(>prime, > + drm_prime_remove_buf_handle(>prime, > obj->export_dma_buf); > } > } > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 366910d..b4ed808 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -57,10 +57,14 @@ > * use the drm_gem_prime_{import,export} helpers. > */ > > +#define PRIME_IMPORT 1 > +#define PRIME_EXPORT 2 > + > struct drm_prime_member { > struct list_head entry; > struct dma_buf *dma_buf; > uint32_t handle; > + int type; > }; > > static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment > *attach, > @@ -157,6 +161,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops > = { > .vunmap = drm_gem_dmabuf_vunmap, > }; > > +static int drm_prime_add_exported_buf_handle(struct drm_prime_file_private > *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle); > + > /** > * DOC: PRIME Helpers > * > @@ -200,7 +206,9 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, > { > struct drm_gem_object *obj; > void *buf; > - int ret; > + int ret = 0; > + struct dma_buf *dmabuf; > + uint32_t exp_handle; > > obj = drm_gem_object_lookup(dev, file_priv, handle); > if (!obj) > @@ -209,43 +217,44 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, > mutex_lock(_priv->prime.lock); > /* re-export the original imported object */ > if (obj->import_attach) { > - get_dma_buf(obj->import_attach->dmabuf); > - *prime_fd = dma_buf_fd(obj->import_attach->dmabuf, flags); > - drm_gem_object_unreference_unlocked(obj); > - mutex_unlock(_priv->prime.lock); > - return 0; > + dmabuf = obj->import_attach->dmabuf; > + goto out_have_obj; > } > > if (obj->export_dma_buf) { > - get_dma_buf(obj->export_dma_buf); > -
[Bug 63632] New: RS880 + mesa/llvm heads - segfault
https://bugs.freedesktop.org/show_bug.cgi?id=63632 Priority: medium Bug ID: 63632 Assignee: dri-devel at lists.freedesktop.org Summary: RS880 + mesa/llvm heads - segfault Severity: normal Classification: Unclassified OS: Linux (All) Reporter: adf.lists at gmail.com Hardware: x86-64 (AMD64) Status: NEW Version: git Component: Drivers/Gallium/r600 Product: Mesa With current mesa and llvm heads I get a segfault running anything, R600_LLVM=0 is OK. Haven't had time to bisect what with the extra hassle of two trees with inter dependencies. I did find a recent working mesa on commit 1d6eb23f2dc1bb53636802cb698e6788ca0a26ac Author: Roland Scheidegger Date: Mon Apr 15 03:57:23 2013 +0200 gallivm: fix small but severe bug in handling multiple lod level strides llvm on the commit before commit ef1762b6a1d3353790bdb415788e7d8963e70372 Author: Nico Rieck Date: Sun Apr 14 21:18:36 2013 + Use object file specific section type for initial text section with llvm on that commit and mesa on above I get /mnt/sdb1/Src64/llvm/include/llvm/MC/MCStreamer.h:224: void llvm::MCStreamer::SwitchSection(const llvm::MCSection*): Assertion `Section && "Cannot switch to a null section!"' failed With both on head I segfault Program received signal SIGSEGV, Segmentation fault. 0x71e9b68c in r600_bytecode_from_byte_stream (num_bytes=0, bytes=0x0, ctx=0x7fffc670) at r600_shader.c:593 593 ctx->bc->nstack = bytes[bytes_read++]; (gdb) bt #0 0x71e9b68c in r600_bytecode_from_byte_stream (num_bytes=0, bytes=0x0, ctx=0x7fffc670) at r600_shader.c:593 #1 r600_shader_from_tgsi (rscreen=0x634630, pipeshader=0x61da70, key=...) at r600_shader.c:1558 #2 0x71e9dd73 in r600_pipe_shader_create (ctx=0x6478f0, shader=0x61da70, key=...) at r600_shader.c:132 #3 0x71eb06db in r600_shader_select (ctx=0x6478f0, sel=0x61d910, dirty=0x0) at r600_state_common.c:747 #4 0x71eb0876 in r600_create_shader_state (ctx=0x6478f0, pipe_shader_type=0, state=) at r600_state_common.c:794 #5 0x71dc2f16 in ureg_create_shader (ureg=0x618fa0, pipe=0x6478f0, so=0x0) at tgsi/tgsi_ureg.c:1701 #6 0x71de8172 in ureg_create_shader_with_so_and_destroy (so=0x0, pipe=0x6478f0, p=0x618fa0) at ./tgsi/tgsi_ureg.h:131 #7 util_make_vertex_passthrough_shader_with_so (pipe=0x6478f0, num_attribs=2, semantic_names=0x7fffdd70, semantic_indexes=0x7fffdd80, so=0x0) at util/u_simple_shaders.c:98 #8 0x71dcf4a6 in util_blitter_create (pipe=0x6478f0) at util/u_blitter.c:301 #9 0x71e8e59c in r600_create_context (screen=0x634630, priv=0x0) at r600_pipe.c:466 #10 0x71cf7a56 in st_api_create_context (stapi=, smapi=0x633e60, attribs=0x7fffdee0, error=0x7fffdf0c, shared_stctxi=0x0) at ../../src/mesa/state_tracker/st_manager.c:633 #11 0x71ebdbf3 in dri_create_context (api=, visual=0x6388e0, cPriv=, major_version=, minor_version=, flags=, error=0x7fffdfdc, sharedContextPrivate=0x0) at dri_context.c:122 #12 0x71bc36aa in dri2CreateContextAttribs (screen=0x633ca0, api=, config=0x6388e0, shared=, num_attribs=, attribs=, error=0x7fffdfdc, data=0x647680) at ../../../../src/mesa/drivers/dri/common/dri_util.c:288 #13 0x71bc37dd in dri2CreateNewContextForAPI (screen=, api=, config=, shared=, data=) at ../../../../src/mesa/drivers/dri/common/dri_util.c:306 #14 0x7716dd88 in dri2_create_context (base=0x617290, config_base=0x642ac0, shareList=, renderType=) at dri2_glx.c:230 #15 0x77142b8e in CreateContext (dpy=0x604050, generic_id=185, config=0x642ac0, shareList_user=0x0, allowDirect=, code=24, renderType=32788, screen=0) at glxcmds.c:274 #16 0x77142dba in glXCreateNewContext (dpy=, fbconfig=, renderType=, shareList=, allowDirect=) at glxcmds.c:1591 #17 0x773cdf61 in fghCreateNewContext (window=) at freeglut_window.c:458 #18 0x773ce81b in fgOpenWindow (window=0x6139e0, title=0x402f20 "Gears", positionUse=0 '\000', x=-1, y=-1, sizeUse=1 '\001', w=300, h=300, gameMode=0 '\000', isSubWindow=0 '\000') at freeglut_window.c:1228 #19 0x773cd182 in fgCreateWindow (parent=0x0, title=0x402f20 "Gears", positionUse=0 '\000', x=-1, y=-1, sizeUse=1 '\001', w=300, h=300, gameMode=0 '\000', isMenu=0 '\000') at freeglut_structure.c:108 #20 0x773cea12 in glutCreateWindow (title=0x402f20 "Gears") at freeglut_window.c:1583 #21 0x00401553 in main (argc=1, argv=0x7fffe568) at gears.c:391 -- 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/20130417/068de0ad/attachment-0001.html>
[PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
On 04/15/2013 08:42 AM, Michael Ellerman wrote: > On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote: >> On pseries machines the detection for max_bus_speed should be done >> through an OpenFirmware property. This patch adds a function to perform this >> detection and a hook to perform dynamic adding of the function only for >> pseries. > > This fails to build for me on ppc64_defconfig, with: > > arch/powerpc/include/asm/machdep.h:111:5: error: 'struct pci_host_bridge' > declared inside parameter list [-Werror] > > > Presumably you tested it using some other defconfig? > > cheers > Yes, I tested with another config, I did get warnings, though, so I should've fixed that earlier. Adding a forward declaration to prevent this from even throwing out warnings. -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center
[PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
On 04/15/2013 02:00 AM, Michael Ellerman wrote: > On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote: >> On pseries machines the detection for max_bus_speed should be done >> through an OpenFirmware property. This patch adds a function to perform this >> detection and a hook to perform dynamic adding of the function only for >> pseries. > > The crucial detail you didn't mention is that pcibios_root_bridge_prepare() > already exists as a weak function in the PCI code and is called from > pci_create_root_bus(). Adding this comment. > >> diff --git a/arch/powerpc/platforms/pseries/setup.c >> b/arch/powerpc/platforms/pseries/setup.c >> index 8bcc9ca..15796b5 100644 >> --- a/arch/powerpc/platforms/pseries/setup.c >> +++ b/arch/powerpc/platforms/pseries/setup.c >> @@ -430,6 +430,8 @@ static void pSeries_machine_kexec(struct kimage *image) >> } >> #endif >> >> +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge); >> + > > Don't do that, put it in a header where it belongs. > And done as well. > cheers > -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center
[PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
On 04/12/2013 01:38 PM, Bjorn Helgaas wrote: > On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares > wrote: >> radeon currently uses a drm function to get the speed capabilities for >> the bus. However, this is a non-standard method of performing this >> detection and this patch changes it to use the max_bus_speed attribute. >> --- >> drivers/gpu/drm/radeon/evergreen.c |9 ++--- >> drivers/gpu/drm/radeon/r600.c |8 +--- >> drivers/gpu/drm/radeon/rv770.c |8 +--- >> 3 files changed, 4 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/evergreen.c >> b/drivers/gpu/drm/radeon/evergreen.c >> index 305a657..3291f62 100644 >> --- a/drivers/gpu/drm/radeon/evergreen.c >> +++ b/drivers/gpu/drm/radeon/evergreen.c >> @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev) >> >> void evergreen_pcie_gen2_enable(struct radeon_device *rdev) >> { >> - u32 link_width_cntl, speed_cntl, mask; >> - int ret; >> + u32 link_width_cntl, speed_cntl; >> >> if (radeon_pcie_gen2 == 0) >> return; >> @@ -3871,11 +3870,7 @@ void evergreen_pcie_gen2_enable(struct radeon_device >> *rdev) >> if (ASIC_IS_X2(rdev)) >> return; >> >> - ret = drm_pcie_get_speed_cap_mask(rdev->ddev,); >> - if (ret != 0) >> - return; >> - >> - if (!(mask& DRM_PCIE_SPEED_50)) >> + if (rdev->pdev->bus->max_bus_speed< PCIE_SPEED_5_0GT) > > For devices on a root bus, we previously dereferenced a NULL pointer > in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a > root bus. (I think this is the original problem you tripped over, > Lucas.) > > These patches fix that problem. On pseries, where the device *is* on > a root bus, your patches set max_bus_speed so this will work as > expected. On most other systems, max_bus_speed for root buses will be > PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because > most arches don't have code like the pseries code you're adding). > > PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on > the root bus, we'll attempt to enable Gen2 on the device even though > we have no idea what the bus will support. > > That's why I originally suggested skipping the Gen2 stuff if > "max_bus_speed == PCI_SPEED_UNKNOWN". I was just being conservative, > thinking that it's better to have a functional but slow GPU rather > than the unknown (to me) effects of enabling Gen2 on a link that might > not support it. But I'm fine with this being either way. You're right here, of course. I'll test for it being different from 5_0GT and 8_0GT instead. Though at some point I suppose someone will want to tackle Gen3 speeds. > > It would be nice if we could get rid of drm_pcie_get_speed_cap_mask() > altogether. It is exported, but I have no idea of anybody else uses > it. Maybe it could at least be marked __deprecated now? > I'll mark it. > I don't know who should take these patches. They don't touch > drivers/pci, but I'd be happy to push them, given the appropriate ACKs > from DRM and powerpc folks. > > Bjorn > >> return; >> >> speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL); >> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c >> index 0740db3..64b90c0 100644 >> --- a/drivers/gpu/drm/radeon/r600.c >> +++ b/drivers/gpu/drm/radeon/r600.c >> @@ -4351,8 +4351,6 @@ static void r600_pcie_gen2_enable(struct radeon_device >> *rdev) >> { >> u32 link_width_cntl, lanes, speed_cntl, training_cntl, tmp; >> u16 link_cntl2; >> - u32 mask; >> - int ret; >> >> if (radeon_pcie_gen2 == 0) >> return; >> @@ -4371,11 +4369,7 @@ static void r600_pcie_gen2_enable(struct >> radeon_device *rdev) >> if (rdev->family<= CHIP_R600) >> return; >> >> - ret = drm_pcie_get_speed_cap_mask(rdev->ddev,); >> - if (ret != 0) >> - return; >> - >> - if (!(mask& DRM_PCIE_SPEED_50)) >> + if (rdev->pdev->bus->max_bus_speed< PCIE_SPEED_5_0GT) >> return; >> >> speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL); >> diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c >> index d63fe1d..c683c36 100644 >> --- a/drivers/gpu/drm/radeon/rv770.c >> +++ b/drivers/gpu/drm/radeon/rv770.c >> @@ -1238,8 +1238,6 @@ static void rv770_pcie_gen2_enable(struct >> radeon_device *rdev) >> { >> u32 link_width_cntl, lanes, speed_cntl, tmp; >> u16 link_cntl2; >> - u32 mask; >> - int ret; >> >> if (radeon_pcie_gen2 == 0) >> return; >> @@ -1254,11 +1252,7 @@ static void rv770_pcie_gen2_enable(struct >> radeon_device *rdev) >> if (ASIC_IS_X2(rdev)) >> return; >> >> - ret = drm_pcie_get_speed_cap_mask(rdev->ddev,); >> - if (ret != 0) >> - return; >>
[PATCH 2/3] drm/exynos: Rework fimc clocks handling
Hi Sylwester, On 16 April 2013 23:01, Sylwester Nawrocki wrote: > @@ -1835,16 +1859,19 @@ static int fimc_probe(struct platform_device *pdev) > ret = exynos_drm_ippdrv_register(ippdrv); > if (ret < 0) { > dev_err(dev, "failed to register drm fimc device.\n"); > - goto err_ippdrv_register; > + goto err_pm_dis; > } > > dev_info(>dev, "drm fimc registered successfully.\n"); > > return 0; > > -err_ippdrv_register: > +err_pm_dis: > + devm_kfree(dev, ippdrv->prop_list); devm_kfree was removed in patch1 of this series. Do we need it back? -- With warm regards, Sachin
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #10 from Erik Faye-Lund --- Another data-point: My AMD driver also does not support line continuation characters by default, although they do *after* a "#version 420" line. So in conclusion, I think blindly supporting line-continuation without a "#version 420"-line is both in violation of the spec as well as diverging from other implementations in a way that can only create problems. -- 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/20130417/2804cbac/attachment.html>
[RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.
On Wed, Apr 17, 2013 at 6:43 AM, Arnd Bergmann wrote: >> drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c:#ifdef __linux__ >> drivers/scsi/aic7xxx/aicasm/aicasm_symbol.h:#ifdef __linux__ >> drivers/scsi/dpt/osd_defs.h:#if (defined(__linux__)) >> drivers/staging/ced1401/machine.h:#if (defined(__linux__) || defined(_linux) >> || defined(__linux)) && !defined(LINUX) > > These are all drivers that are shared with another OS, or used > to be. In most of them, we can probably just remove the > #else path, since I don't think they are getting synchronized > any more. note that DRM gets periodically ported to the *BSD's, so possibly the same applies here.. BR, -R
[RESEND PATCH 3/3] drm: Destroy property blobs at mode config cleanup time
Hi Daniel, On Tuesday 16 April 2013 21:06:43 Daniel Vetter wrote: > On Tue, Apr 16, 2013 at 01:12:21PM +1000, Dave Airlie wrote: > > On Mon, Apr 15, 2013 at 11:37 PM, Laurent Pinchart wrote: > > > Property blob objects need to be destroyed when cleaning up to avoid > > > memory leaks. Go through the list of all blobs in the > > > drm_mode_config_cleanup() function and destroy them. > > > > > > The drm_mode_config_cleanup() function needs to be moved after the > > > drm_property_destroy_blob() declaration. Move drm_mode_config_init() as > > > well to keep the functions together. > > > > > > Signed-off-by: Laurent Pinchart > > >> > > Reviewed-by: Daniel Vetter > > > > I've applied this one, the other two I don't mind, but I'm not sure > > they aren't a bit restrictive in the generic code, > > Patch 2 is just for the crtc helpers, and matches what we've just done in > the i915 code. I think it fits in with the general design of the crtc > helpers. > > Patch 1 matches a check in the intel code, too. So since applications > can't really know whether a flip to a different pixel format works I don't > think that capability is useful at all. And at least for i915 it's just > broken ;-) On patch 1 itself though: Just checking fb->pixel_format should > be good enough. I've fixed that and I'll resubmit after we reach an agreement on 1/3 and 2/3. -- Regards, Laurent Pinchart
[Bug 43751] [TTM] Out of kernel memory
https://bugzilla.kernel.org/show_bug.cgi?id=43751 John Paul Funk changed: What|Removed |Added CC||funk at funktronix.com --- Comment #14 from John Paul Funk 2013-04-17 00:54:36 --- I'm running kernel 3.8.7 and still having this issue. dmesg is full of these lines: [126466.384749] [TTM] Out of kernel memory [126466.384752] [drm:radeon_gem_object_create] *ERROR* Failed to allocate GEM object (4096, 6, 4096, -12) This problem started after updating Kubuntu 12.10 a few days ago. Latest kernel does not fix the issue. -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug.
[Bug 57567] 3.7 radeonfb broken on efivga screens
https://bugs.freedesktop.org/show_bug.cgi?id=57567 --- Comment #63 from Alexandre Demers --- (In reply to comment #62) > Created attachment 78090 [details] [review] > another test > > Please also try this patch on top of only attachment 77705 [details] > [review]. No other patches. Both patches don't fix anything over here. They do pretty much as if nothing was applied. -- 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/20130417/b2104e25/attachment.html>
[PATCH 4/6] char: use vma_pages() to replace (vm_end - vm_start) >> PAGE_SHIFT
On Mon, 15 Apr 2013, Libin wrote: > diff --git a/drivers/char/mspec.c b/drivers/char/mspec.c > index e1f60f9..ed0703f 100644 > --- a/drivers/char/mspec.c > +++ b/drivers/char/mspec.c > @@ -168,7 +168,7 @@ mspec_close(struct vm_area_struct *vma) > if (!atomic_dec_and_test(>refcnt)) > return; > > - last_index = (vdata->vm_end - vdata->vm_start) >> PAGE_SHIFT; > + last_index = vma_pages(vdata); > for (index = 0; index < last_index; index++) { > if (vdata->maddr[index] == 0) > continue; vdata is of type struct vma_data * and vma_pages() takes a formal of type struct vm_area_struct *, so these are incompatible. Hopefully you tested the other changes and simply lack an ia64 cross compiler for this one, because it will emit a warning.
Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.
On 12:50-20130416, Arnd Bergmann wrote: On Tuesday 16 April 2013 12:48:28 Paul Sokolovsky wrote: diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 8d1e2bb..73a99e4 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -36,7 +36,7 @@ #ifndef _DRM_H_ #define _DRM_H_ -#if defined(__linux__) +#if defined(__KERNEL__) || defined(__linux__) #include linux/types.h #include asm/ioctl.h This is still completely bogus, the __KERNEL__ symbol has no significance here. Either make the compiler define __linux__, or remove this #ifdef completely. Searching the v.39-rc7 tag, and greping for _linux_ a few interesting list pops up. (pruned): arch/arc/Makefile:cflags-y += -mA7 -fno-common -pipe -fno-builtin -D__linux__ arch/h8300/Makefile:KBUILD_CFLAGS += -D__linux__ arch/hexagon/Makefile:KBUILD_CFLAGS += -ffixed-$(TIR_NAME) -DTHREADINFO_REG=$(TIR_NAME) -D__linux__ arch/score/Makefile:-D__linux__ -ffunction-sections -ffreestanding arch/xtensa/Makefile:KBUILD_CFLAGS += -ffreestanding -D__linux__ ^^ these architectures seem to bypass the pain entirely by defining __linux__ arch/mips/include/uapi/asm/sgidefs.h:#ifndef __linux__ drivers/gpu/drm/radeon/radeon_cp.c:#ifdef __linux__ {snip} drivers/scsi/aic7xxx/aic7770.c:#ifdef __linux__ drivers/scsi/aic7xxx/aic79xx.h:#ifndef __linux__ {snip} drivers/scsi/aic7xxx/aic79xx_core.c:#ifdef __linux__ {snip} drivers/scsi/aic7xxx/aic79xx_pci.c:#ifdef __linux__ drivers/scsi/aic7xxx/aic7xxx.h:#ifndef __linux__ drivers/scsi/aic7xxx/aic7xxx.h:#ifndef __linux__ drivers/scsi/aic7xxx/aic7xxx_93cx6.c:#ifdef __linux__ drivers/scsi/aic7xxx/aic7xxx_core.c:#ifdef __linux__ {snip} drivers/scsi/aic7xxx/aic7xxx_pci.c:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm.h:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm_gram.y:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm_macro_scan.l:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm_scan.l:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm_symbol.h:#ifdef __linux__ drivers/scsi/dpt/osd_defs.h:#if (defined(__linux__)) drivers/staging/ced1401/machine.h:#if (defined(__linux__) || defined(_linux) || defined(__linux)) !defined(LINUX) include/acpi/platform/acenv.h:#if defined(_LINUX) || defined(__linux__) include/linux/coda.h:#if defined(__linux__) include/uapi/drm/drm.h:#if defined(__linux__) include/uapi/linux/coda.h:#if defined(__linux__) include/uapi/linux/fuse.h:#ifdef __linux__ And then we have the following as well.. fs/ext4/ext4.h:#if defined(__KERNEL__) || defined(__linux__) Trying out a few different prebuilt compilers I had around, I see: http://pastebin.com/bTVDLTb1 So, is our approach just to use __linux__ for builds? I am trying to understand rationale behind why #include linux/types.h #include asm/ioctl.h would want __linux__ and why __KERNEL__ check is un-wanted. Ofcourse, I cant comment about the One of the BSDs in else options.. and why we'd like to keep it around in kernel :) -- Regards, Nishanth Menon ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/exynos: Rework fimc clocks handling
Hi Sylwester, On 16 April 2013 23:01, Sylwester Nawrocki s.nawro...@samsung.com wrote: @@ -1835,16 +1859,19 @@ static int fimc_probe(struct platform_device *pdev) ret = exynos_drm_ippdrv_register(ippdrv); if (ret 0) { dev_err(dev, failed to register drm fimc device.\n); - goto err_ippdrv_register; + goto err_pm_dis; } dev_info(pdev-dev, drm fimc registered successfully.\n); return 0; -err_ippdrv_register: +err_pm_dis: + devm_kfree(dev, ippdrv-prop_list); devm_kfree was removed in patch1 of this series. Do we need it back? -- With warm regards, Sachin ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/prime: keep a reference from the handle to exported dma-buf (v4)
Currently we have a problem with this: 1. i915: create gem object 2. i915: export gem object to prime 3. radeon: import gem object 4. close prime fd 5. radeon: unref object 6. i915: unref object i915 has an imported object reference in its file priv, that isn't cleaned up properly until fd close. The reference gets added at step 2, but at step 6 we don't have enough info to clean it up. The solution is to take a reference on the dma-buf when we export it, and drop the reference when the gem handle goes away. So when we export a dma_buf from a gem object, we keep track of it with the handle, we take a reference to the dma_buf. When we close the handle (i.e. userspace is finished with the buffer), we drop the reference to the dma_buf, and it gets collected. This patch isn't meant to fix any other problem or bikesheds, and it doesn't fix any races with other scenarios. v1.1: move export symbol line back up. v2: okay I had to do a bit more, as the first patch showed a leak on one of my tests, that I found using the dma-buf debugfs support, the problem case is exporting a buffer twice with the same handle, we'd add another export handle for it unnecessarily, however we now fail if we try to export the same object with a different gem handle, however I'm not sure if that is a case I want to support, and I've gotten the code to WARN_ON if we hit something like that. v2.1: rebase this patch, write better commit msg. v3: cleanup error handling, track import vs export in linked list, these two patches were separate previously, but seem to work better like this. v4: danvet is correct, this code is no longer useful, since the buffer better exist, so remove it. Signed-off-by: Dave Airlie airl...@redhat.com --- drivers/gpu/drm/drm_gem.c | 4 +- drivers/gpu/drm/drm_prime.c | 95 + include/drm/drmP.h | 4 +- 3 files changed, 65 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index af779ae..cf919e3 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -205,11 +205,11 @@ static void drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) { if (obj-import_attach) { - drm_prime_remove_imported_buf_handle(filp-prime, + drm_prime_remove_buf_handle(filp-prime, obj-import_attach-dmabuf); } if (obj-export_dma_buf) { - drm_prime_remove_imported_buf_handle(filp-prime, + drm_prime_remove_buf_handle(filp-prime, obj-export_dma_buf); } } diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 366910d..b4ed808 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -57,10 +57,14 @@ * use the drm_gem_prime_{import,export} helpers. */ +#define PRIME_IMPORT 1 +#define PRIME_EXPORT 2 + struct drm_prime_member { struct list_head entry; struct dma_buf *dma_buf; uint32_t handle; + int type; }; static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, @@ -157,6 +161,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { .vunmap = drm_gem_dmabuf_vunmap, }; +static int drm_prime_add_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle); + /** * DOC: PRIME Helpers * @@ -200,7 +206,9 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, { struct drm_gem_object *obj; void *buf; - int ret; + int ret = 0; + struct dma_buf *dmabuf; + uint32_t exp_handle; obj = drm_gem_object_lookup(dev, file_priv, handle); if (!obj) @@ -209,43 +217,44 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, mutex_lock(file_priv-prime.lock); /* re-export the original imported object */ if (obj-import_attach) { - get_dma_buf(obj-import_attach-dmabuf); - *prime_fd = dma_buf_fd(obj-import_attach-dmabuf, flags); - drm_gem_object_unreference_unlocked(obj); - mutex_unlock(file_priv-prime.lock); - return 0; + dmabuf = obj-import_attach-dmabuf; + goto out_have_obj; } if (obj-export_dma_buf) { - get_dma_buf(obj-export_dma_buf); - *prime_fd = dma_buf_fd(obj-export_dma_buf, flags); - drm_gem_object_unreference_unlocked(obj); - } else { - buf = dev-driver-gem_prime_export(dev, obj, flags); - if (IS_ERR(buf)) { - /* normally the created dma-buf takes ownership of the ref, -* but if that fails then drop the ref -*/ - drm_gem_object_unreference_unlocked(obj); - mutex_unlock(file_priv-prime.lock); -
[GIT PULL] exynos-drm-next
Hi Dave, This is initial pull request for Exynos. It includes a big change that it makes drm_display_mode for timings parameters to be used for exynos4 and exynos5 commonly and cleans up unnecessary codes. And also it adds device tree support for fimd to get timing values and interrupt source from dts file. In addition, one more patch, device tree support feature for Exynos FIMC, is being reviewed. This patch was posted a little ago like below, http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg17568.html So we are going to request git pull one more time after reviewed. Please kindly let me know if there is any problem. Thanks, Inki Dae The following changes since commit 62c8ba7c58e4163f975c5f8b5a3dd5f306a2deda: drm/qxl: fix smatch warnings (2013-04-16 13:36:00 +1000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next for you to fetch changes up to dd4d34fde068a9362d09292d881ce4743bb11c7f: Revert of/exynos_g2d: Add Bindings for exynos G2D driver (2013-04-17 00:07:29 +0900) Rahul Sharma (2): drm/exynos: hdmi: using drm_display_mode timings for exynos4 drm/exynos: hdmi: move mode_fixup to drm common hdmi Sachin Kamat (5): drm/exynos: hdmi: Fix incorrect usage of IS_ERR_OR_NULL drm/exynos: mixer: Fix incorrect usage of IS_ERR_OR_NULL drm/exynos: drm_rotator: Fix incorrect usage of IS_ERR_OR_NULL drm/exynos: drm_connector: Fix error check condition Revert of/exynos_g2d: Add Bindings for exynos G2D driver Vikas Sajjan (3): drm/exynos: Add display-timing node parsing using video helper function drm/exynos: enable OF_VIDEOMODE and FB_MODE_HELPERS for exynos drm fimd drm/exynos: change the method for getting the interrupt .../devicetree/bindings/drm/exynos/g2d.txt | 22 - drivers/gpu/drm/exynos/Kconfig | 4 +- drivers/gpu/drm/exynos/exynos_drm_connector.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 26 +- drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 40 +- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 3 - drivers/gpu/drm/exynos/exynos_drm_rotator.c| 2 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 704 +++-- drivers/gpu/drm/exynos/exynos_mixer.c | 14 +- 9 files changed, 312 insertions(+), 505 deletions(-) delete mode 100644 Documentation/devicetree/bindings/drm/exynos/g2d.txt ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/6] char: use vma_pages() to replace (vm_end - vm_start) PAGE_SHIFT
On Mon, 15 Apr 2013, Libin wrote: diff --git a/drivers/char/mspec.c b/drivers/char/mspec.c index e1f60f9..ed0703f 100644 --- a/drivers/char/mspec.c +++ b/drivers/char/mspec.c @@ -168,7 +168,7 @@ mspec_close(struct vm_area_struct *vma) if (!atomic_dec_and_test(vdata-refcnt)) return; - last_index = (vdata-vm_end - vdata-vm_start) PAGE_SHIFT; + last_index = vma_pages(vdata); for (index = 0; index last_index; index++) { if (vdata-maddr[index] == 0) continue; vdata is of type struct vma_data * and vma_pages() takes a formal of type struct vm_area_struct *, so these are incompatible. Hopefully you tested the other changes and simply lack an ia64 cross compiler for this one, because it will emit a warning. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/prime: keep a reference from the handle to exported dma-buf (v4)
On Wed, Apr 17, 2013 at 02:38:02PM +1000, Dave Airlie wrote: Currently we have a problem with this: 1. i915: create gem object 2. i915: export gem object to prime 3. radeon: import gem object 4. close prime fd 5. radeon: unref object 6. i915: unref object i915 has an imported object reference in its file priv, that isn't cleaned up properly until fd close. The reference gets added at step 2, but at step 6 we don't have enough info to clean it up. The solution is to take a reference on the dma-buf when we export it, and drop the reference when the gem handle goes away. So when we export a dma_buf from a gem object, we keep track of it with the handle, we take a reference to the dma_buf. When we close the handle (i.e. userspace is finished with the buffer), we drop the reference to the dma_buf, and it gets collected. This patch isn't meant to fix any other problem or bikesheds, and it doesn't fix any races with other scenarios. v1.1: move export symbol line back up. v2: okay I had to do a bit more, as the first patch showed a leak on one of my tests, that I found using the dma-buf debugfs support, the problem case is exporting a buffer twice with the same handle, we'd add another export handle for it unnecessarily, however we now fail if we try to export the same object with a different gem handle, however I'm not sure if that is a case I want to support, and I've gotten the code to WARN_ON if we hit something like that. v2.1: rebase this patch, write better commit msg. v3: cleanup error handling, track import vs export in linked list, these two patches were separate previously, but seem to work better like this. v4: danvet is correct, this code is no longer useful, since the buffer better exist, so remove it. Signed-off-by: Dave Airlie airl...@redhat.com Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch On bikeshed review level I wonder whether we shouldn't just grab a reference unconditonally when inserting it into the handle_to_fd lookup lists. But I need to think through a few other cases and apparently the self-import test is still a bit broken. So this is material for follow-up patches. -Daniel --- drivers/gpu/drm/drm_gem.c | 4 +- drivers/gpu/drm/drm_prime.c | 95 + include/drm/drmP.h | 4 +- 3 files changed, 65 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index af779ae..cf919e3 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -205,11 +205,11 @@ static void drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) { if (obj-import_attach) { - drm_prime_remove_imported_buf_handle(filp-prime, + drm_prime_remove_buf_handle(filp-prime, obj-import_attach-dmabuf); } if (obj-export_dma_buf) { - drm_prime_remove_imported_buf_handle(filp-prime, + drm_prime_remove_buf_handle(filp-prime, obj-export_dma_buf); } } diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 366910d..b4ed808 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -57,10 +57,14 @@ * use the drm_gem_prime_{import,export} helpers. */ +#define PRIME_IMPORT 1 +#define PRIME_EXPORT 2 + struct drm_prime_member { struct list_head entry; struct dma_buf *dma_buf; uint32_t handle; + int type; }; static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, @@ -157,6 +161,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { .vunmap = drm_gem_dmabuf_vunmap, }; +static int drm_prime_add_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle); + /** * DOC: PRIME Helpers * @@ -200,7 +206,9 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, { struct drm_gem_object *obj; void *buf; - int ret; + int ret = 0; + struct dma_buf *dmabuf; + uint32_t exp_handle; obj = drm_gem_object_lookup(dev, file_priv, handle); if (!obj) @@ -209,43 +217,44 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, mutex_lock(file_priv-prime.lock); /* re-export the original imported object */ if (obj-import_attach) { - get_dma_buf(obj-import_attach-dmabuf); - *prime_fd = dma_buf_fd(obj-import_attach-dmabuf, flags); - drm_gem_object_unreference_unlocked(obj); - mutex_unlock(file_priv-prime.lock); - return 0; + dmabuf = obj-import_attach-dmabuf; + goto out_have_obj; } if (obj-export_dma_buf) { - get_dma_buf(obj-export_dma_buf); - *prime_fd = dma_buf_fd(obj-export_dma_buf, flags); -
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #10 from Erik Faye-Lund kusmab...@gmail.com --- Another data-point: My AMD driver also does not support line continuation characters by default, although they do *after* a #version 420 line. So in conclusion, I think blindly supporting line-continuation without a #version 420-line is both in violation of the spec as well as diverging from other implementations in a way that can only create problems. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/exynos: Rework fimc clocks handling
On 04/17/2013 06:02 AM, Sachin Kamat wrote: Hi Sylwester, On 16 April 2013 23:01, Sylwester Nawrocki s.nawro...@samsung.com wrote: @@ -1835,16 +1859,19 @@ static int fimc_probe(struct platform_device *pdev) ret = exynos_drm_ippdrv_register(ippdrv); if (ret 0) { dev_err(dev, failed to register drm fimc device.\n); - goto err_ippdrv_register; + goto err_pm_dis; } dev_info(pdev-dev, drm fimc registered successfully.\n); return 0; -err_ippdrv_register: +err_pm_dis: + devm_kfree(dev, ippdrv-prop_list); devm_kfree was removed in patch1 of this series. Do we need it back? Certainly not, that's a rebase error. I'll resend it fixed. Thanks for your review. Regards, Sylwester ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Bug 56139] [bisected] kernel 3.7.0-rc1 breaks 6950 (CAYMAN)
We have a similar problem: A HD6570(TURKS) card with RS780E chipset. When booting, DVI output is good, but VGA output is black screen; after X started, DVI is still good and VGA blinks forever. We have tried 3.4~3.9-rc7 kernel, all kernels after the commit drm/radeon: properly handle mc_stop/mc_resume on evergreen+ has the same issue. On Mon, Nov 12, 2012 at 2:55 AM, bugzilla-dae...@freedesktop.org wrote: Florian Mickler flor...@mickler.org changed bug 56139https://bugs.freedesktop.org/show_bug.cgi?id=56139 What Removed Added CC flor...@mickler.org *Comment # 25 https://bugs.freedesktop.org/show_bug.cgi?id=56139#c25on bug 56139 https://bugs.freedesktop.org/show_bug.cgi?id=56139 from Florian Mickler flor...@mickler.org * A patch referencing this bug report has been merged in Linux v3.7-rc5: commit 695ddeb457584a602f2ba117d08ce37cf6ec1589 Author: Alex Deucher alexander.deuc...@amd.com Date: Mon Nov 5 16:34:58 2012 + drm/radeon: fix typo in evergreen_mc_resume() -- You are receiving this mail because: - You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] drm/exynos: Add device tree support for IPP driver
Hi Inki, This small patch series adds device tree support for the DRM FIMC driver. The binding documentation can be found in -next at Documentation/devicetree/ bindings/media/samsung-fimc.txt. It will make the driver dependent on OF. This patch series is needed in 3.10 to ensure simultaneous operation of the DRM FIMC and the camera ISP on Exynos4x12. Changes since v1: - removed devm_kfree() that got erroneously re-added in patch 2/3 during rebase. Thanks, Sylwester Sylwester Nawrocki (3): drm/exynos: Remove redundant devm_kfree() drm/exynos: Rework fimc clocks handling drm/exynos: Add device tree support for fimc ipp driver drivers/gpu/drm/exynos/Kconfig |2 +- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 264 -- drivers/gpu/drm/exynos/regs-fimc.h |7 +- 3 files changed, 147 insertions(+), 126 deletions(-) -- 1.7.9.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/3] drm/exynos: Remove redundant devm_kfree()
There is no need for explicit calls of devm_kfree(), as the allocated memory will be freed during driver's detach. Remove the redundant devm_kfree() calls from probe() and remove() callbacks. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_fimc.c |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index 411f69b7..d812c57 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -1843,7 +1843,6 @@ static int fimc_probe(struct platform_device *pdev) return 0; err_ippdrv_register: - devm_kfree(dev, ippdrv-prop_list); pm_runtime_disable(dev); err_get_irq: free_irq(ctx-irq, ctx); @@ -1857,7 +1856,6 @@ static int fimc_remove(struct platform_device *pdev) struct fimc_context *ctx = get_fimc_context(dev); struct exynos_drm_ippdrv *ippdrv = ctx-ippdrv; - devm_kfree(dev, ippdrv-prop_list); exynos_drm_ippdrv_unregister(ippdrv); mutex_destroy(ctx-lock); -- 1.7.9.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] drm/exynos: Rework fimc clocks handling
The clocks handling is refactored and a mux clock handling is added to account for changes in the clocks driver. After switching to the common clock framework the sclk_fimc clock is now split into two clocks: a gate and a mux clock. In order to retain the exisiting functionality two additional consumer clocks are passed to the driver from device tree: mux and parent. Then the driver sets parent clock as a parent clock of the mux clock. These two additional clocks are optional, and should go away when there is a standard way of setting up parent clocks on DT platforms. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 167 +- 1 file changed, 97 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index d812c57..bc8411a 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -76,6 +76,27 @@ enum fimc_wb { FIMC_WB_B, }; +enum { + FIMC_CLK_LCLK, + FIMC_CLK_GATE, + FIMC_CLK_WB_A, + FIMC_CLK_WB_B, + FIMC_CLK_MUX, + FIMC_CLK_PARENT, + FIMC_CLKS_MAX +}; + +static const char * fimc_clock_names[] = { + [FIMC_CLK_LCLK] = sclk_fimc, + [FIMC_CLK_GATE] = fimc, + [FIMC_CLK_WB_A] = pxl_async0, + [FIMC_CLK_WB_B] = pxl_async1, + [FIMC_CLK_MUX]= mux, + [FIMC_CLK_PARENT] = parent, +}; + +#define FIMC_DEFAULT_LCLK_FREQUENCY 13300UL + /* * A structure of scaler. * @@ -132,15 +153,12 @@ struct fimc_driverdata { * * @ippdrv: prepare initialization using ippdrv. * @regs_res: register resources. + * @dev: pointer to the fimc device structure. * @regs: memory mapped io registers. * @lock: locking of operations. - * @sclk_fimc_clk: fimc source clock. - * @fimc_clk: fimc clock. - * @wb_clk: writeback a clock. - * @wb_b_clk: writeback b clock. + * @clocks: fimc clocks. + * @clk_frequency: LCLK clock frequency. * @sc: scaler infomations. - * @odr: ordering of YUV. - * @ver: fimc version. * @pol: porarity of writeback. * @id: fimc id. * @irq: irq number. @@ -148,13 +166,12 @@ struct fimc_driverdata { */ struct fimc_context { struct exynos_drm_ippdrvippdrv; + struct device *dev; struct resource *regs_res; void __iomem*regs; struct mutexlock; - struct clk *sclk_fimc_clk; - struct clk *fimc_clk; - struct clk *wb_clk; - struct clk *wb_b_clk; + struct clk *clocks[FIMC_CLKS_MAX]; + u32 clk_frequency; struct fimc_scaler sc; struct fimc_driverdata *ddata; struct exynos_drm_ipp_pol pol; @@ -1301,14 +1318,12 @@ static int fimc_clk_ctrl(struct fimc_context *ctx, bool enable) DRM_DEBUG_KMS(%s:enable[%d]\n, __func__, enable); if (enable) { - clk_enable(ctx-sclk_fimc_clk); - clk_enable(ctx-fimc_clk); - clk_enable(ctx-wb_clk); + clk_prepare_enable(ctx-clocks[FIMC_CLK_GATE]); + clk_prepare_enable(ctx-clocks[FIMC_CLK_WB_A]); ctx-suspended = false; } else { - clk_disable(ctx-sclk_fimc_clk); - clk_disable(ctx-fimc_clk); - clk_disable(ctx-wb_clk); + clk_disable_unprepare(ctx-clocks[FIMC_CLK_GATE]); + clk_disable_unprepare(ctx-clocks[FIMC_CLK_WB_A]); ctx-suspended = true; } @@ -1713,11 +1728,66 @@ static void fimc_ippdrv_stop(struct device *dev, enum drm_exynos_ipp_cmd cmd) fimc_write(cfg, EXYNOS_CIGCTRL); } +static void fimc_put_clocks(struct fimc_context *ctx) +{ + int i; + + for (i = 0; i FIMC_CLKS_MAX; i++) { + if (IS_ERR(ctx-clocks[i])) + continue; + clk_put(ctx-clocks[i]); + ctx-clocks[i] = ERR_PTR(-EINVAL); + } +} + +static int fimc_setup_clocks(struct fimc_context *ctx) +{ + struct device *dev; + int ret, i; + + for (i = 0; i FIMC_CLKS_MAX; i++) + ctx-clocks[i] = ERR_PTR(-EINVAL); + + for (i = 0; i FIMC_CLKS_MAX; i++) { + if (i == FIMC_CLK_WB_A || i == FIMC_CLK_WB_B) + dev = ctx-dev-parent; + else + dev = ctx-dev; + + ctx-clocks[i] = clk_get(dev, fimc_clock_names[i]); + if (IS_ERR(ctx-clocks[i])) { + if (i = FIMC_CLK_MUX) + break; + ret = PTR_ERR(ctx-clocks[i]); + goto e_clk_free; + } + } + + if (!IS_ERR(ctx-clocks[FIMC_CLK_PARENT])) { + ret = clk_set_parent(ctx-clocks[FIMC_CLK_MUX], +
[PATCH v2 3/3] drm/exynos: Add device tree support for fimc ipp driver
This patch adds OF initialization support for the FIMC driver. The binding documentation can be found at Documentation/devicetree/ bindings/media/samsung-fimc.txt. The syscon regmap interface is used to serialize access to the shared CAMBLK registers from within the V4L2 FIMC-IS and the DRM FIMC drivers. The DRM driver uses this interface for setting up the FIFO data link between FIMD and FIMC IP blocks, while the V4L2 one for setting up a data link between the camera ISP and FIMC for camera capture. The CAMBLK registers are not accessed any more through a statically mapped IO. Synchronized access to these registers is required for simultaneous operation of the camera ISP and the DRM IPP on Exynos4x12. Exynos4 is going to be a dt-only platform since 3.10, thus the driver data and driver_ids static data structures are removed. Camera input signal polarities are not currently parsed from the device tree. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/gpu/drm/exynos/Kconfig |2 +- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 101 +++--- drivers/gpu/drm/exynos/regs-fimc.h |7 +-- 3 files changed, 53 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index 406f32a..5c4be2a 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -56,7 +56,7 @@ config DRM_EXYNOS_IPP config DRM_EXYNOS_FIMC bool Exynos DRM FIMC - depends on DRM_EXYNOS_IPP + depends on DRM_EXYNOS_IPP MFD_SYSCON OF help Choose this option if you want to use Exynos FIMC for DRM. diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c index bc8411a..2e182ba 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c @@ -12,11 +12,12 @@ * */ #include linux/kernel.h +#include linux/mfd/syscon.h #include linux/module.h #include linux/platform_device.h +#include linux/regmap.h #include linux/clk.h #include linux/pm_runtime.h -#include plat/map-base.h #include drm/drmP.h #include drm/exynos_drm.h @@ -140,15 +141,6 @@ struct fimc_capability { }; /* - * A structure of fimc driver data. - * - * @parent_clk: name of parent clock. - */ -struct fimc_driverdata { - char*parent_clk; -}; - -/* * A structure of fimc context. * * @ippdrv: prepare initialization using ippdrv. @@ -158,6 +150,7 @@ struct fimc_driverdata { * @lock: locking of operations. * @clocks: fimc clocks. * @clk_frequency: LCLK clock frequency. + * @sysreg: handle to SYSREG block regmap. * @sc: scaler infomations. * @pol: porarity of writeback. * @id: fimc id. @@ -172,8 +165,8 @@ struct fimc_context { struct mutexlock; struct clk *clocks[FIMC_CLKS_MAX]; u32 clk_frequency; + struct regmap *sysreg; struct fimc_scaler sc; - struct fimc_driverdata *ddata; struct exynos_drm_ipp_pol pol; int id; int irq; @@ -217,17 +210,13 @@ static void fimc_sw_reset(struct fimc_context *ctx) fimc_write(0x0, EXYNOS_CIFCNTSEQ); } -static void fimc_set_camblk_fimd0_wb(struct fimc_context *ctx) +static int fimc_set_camblk_fimd0_wb(struct fimc_context *ctx) { - u32 camblk_cfg; - DRM_DEBUG_KMS(%s\n, __func__); - camblk_cfg = readl(SYSREG_CAMERA_BLK); - camblk_cfg = ~(SYSREG_FIMD0WB_DEST_MASK); - camblk_cfg |= ctx-id (SYSREG_FIMD0WB_DEST_SHIFT); - - writel(camblk_cfg, SYSREG_CAMERA_BLK); + return regmap_update_bits(ctx-sysreg, SYSREG_CAMERA_BLK, + SYSREG_FIMD0WB_DEST_MASK, + ctx-id SYSREG_FIMD0WB_DEST_SHIFT); } static void fimc_set_type_ctrl(struct fimc_context *ctx, enum fimc_wb wb) @@ -1628,7 +1617,9 @@ static int fimc_ippdrv_start(struct device *dev, enum drm_exynos_ipp_cmd cmd) fimc_handle_lastend(ctx, true); /* setup FIMD */ - fimc_set_camblk_fimd0_wb(ctx); + ret = fimc_set_camblk_fimd0_wb(ctx); + if (ret 0) + return ret; set_wb.enable = 1; set_wb.refresh = property-refresh_rate; @@ -1784,26 +1775,50 @@ e_clk_free: return ret; } +static int fimc_parse_dt(struct fimc_context *ctx) +{ + struct device_node *node = ctx-dev-of_node; + + if (!of_property_read_bool(node, samsung,lcd-wb)) + return -ENODEV; + + if (of_property_read_u32(node, clock-frequency, + ctx-clk_frequency)) + ctx-clk_frequency = FIMC_DEFAULT_LCLK_FREQUENCY; + + ctx-id = of_alias_get_id(node, fimc); + if (ctx-id 0) + return -EINVAL; + + return 0; +} + static int fimc_probe(struct
[Bug 63632] New: RS880 + mesa/llvm heads - segfault
https://bugs.freedesktop.org/show_bug.cgi?id=63632 Priority: medium Bug ID: 63632 Assignee: dri-devel@lists.freedesktop.org Summary: RS880 + mesa/llvm heads - segfault Severity: normal Classification: Unclassified OS: Linux (All) Reporter: adf.li...@gmail.com Hardware: x86-64 (AMD64) Status: NEW Version: git Component: Drivers/Gallium/r600 Product: Mesa With current mesa and llvm heads I get a segfault running anything, R600_LLVM=0 is OK. Haven't had time to bisect what with the extra hassle of two trees with inter dependencies. I did find a recent working mesa on commit 1d6eb23f2dc1bb53636802cb698e6788ca0a26ac Author: Roland Scheidegger srol...@vmware.com Date: Mon Apr 15 03:57:23 2013 +0200 gallivm: fix small but severe bug in handling multiple lod level strides llvm on the commit before commit ef1762b6a1d3353790bdb415788e7d8963e70372 Author: Nico Rieck nico.ri...@gmail.com Date: Sun Apr 14 21:18:36 2013 + Use object file specific section type for initial text section with llvm on that commit and mesa on above I get /mnt/sdb1/Src64/llvm/include/llvm/MC/MCStreamer.h:224: void llvm::MCStreamer::SwitchSection(const llvm::MCSection*): Assertion `Section Cannot switch to a null section!' failed With both on head I segfault Program received signal SIGSEGV, Segmentation fault. 0x71e9b68c in r600_bytecode_from_byte_stream (num_bytes=0, bytes=0x0, ctx=0x7fffc670) at r600_shader.c:593 593 ctx-bc-nstack = bytes[bytes_read++]; (gdb) bt #0 0x71e9b68c in r600_bytecode_from_byte_stream (num_bytes=0, bytes=0x0, ctx=0x7fffc670) at r600_shader.c:593 #1 r600_shader_from_tgsi (rscreen=0x634630, pipeshader=0x61da70, key=...) at r600_shader.c:1558 #2 0x71e9dd73 in r600_pipe_shader_create (ctx=0x6478f0, shader=0x61da70, key=...) at r600_shader.c:132 #3 0x71eb06db in r600_shader_select (ctx=0x6478f0, sel=0x61d910, dirty=0x0) at r600_state_common.c:747 #4 0x71eb0876 in r600_create_shader_state (ctx=0x6478f0, pipe_shader_type=0, state=optimized out) at r600_state_common.c:794 #5 0x71dc2f16 in ureg_create_shader (ureg=0x618fa0, pipe=0x6478f0, so=0x0) at tgsi/tgsi_ureg.c:1701 #6 0x71de8172 in ureg_create_shader_with_so_and_destroy (so=0x0, pipe=0x6478f0, p=0x618fa0) at ./tgsi/tgsi_ureg.h:131 #7 util_make_vertex_passthrough_shader_with_so (pipe=0x6478f0, num_attribs=2, semantic_names=0x7fffdd70, semantic_indexes=0x7fffdd80, so=0x0) at util/u_simple_shaders.c:98 #8 0x71dcf4a6 in util_blitter_create (pipe=0x6478f0) at util/u_blitter.c:301 #9 0x71e8e59c in r600_create_context (screen=0x634630, priv=0x0) at r600_pipe.c:466 #10 0x71cf7a56 in st_api_create_context (stapi=optimized out, smapi=0x633e60, attribs=0x7fffdee0, error=0x7fffdf0c, shared_stctxi=0x0) at ../../src/mesa/state_tracker/st_manager.c:633 #11 0x71ebdbf3 in dri_create_context (api=optimized out, visual=0x6388e0, cPriv=optimized out, major_version=optimized out, minor_version=optimized out, flags=optimized out, error=0x7fffdfdc, sharedContextPrivate=0x0) at dri_context.c:122 #12 0x71bc36aa in dri2CreateContextAttribs (screen=0x633ca0, api=optimized out, config=0x6388e0, shared=optimized out, num_attribs=optimized out, attribs=optimized out, error=0x7fffdfdc, data=0x647680) at ../../../../src/mesa/drivers/dri/common/dri_util.c:288 #13 0x71bc37dd in dri2CreateNewContextForAPI (screen=optimized out, api=optimized out, config=optimized out, shared=optimized out, data=optimized out) at ../../../../src/mesa/drivers/dri/common/dri_util.c:306 #14 0x7716dd88 in dri2_create_context (base=0x617290, config_base=0x642ac0, shareList=optimized out, renderType=optimized out) at dri2_glx.c:230 #15 0x77142b8e in CreateContext (dpy=0x604050, generic_id=185, config=0x642ac0, shareList_user=0x0, allowDirect=optimized out, code=24, renderType=32788, screen=0) at glxcmds.c:274 #16 0x77142dba in glXCreateNewContext (dpy=optimized out, fbconfig=optimized out, renderType=optimized out, shareList=optimized out, allowDirect=optimized out) at glxcmds.c:1591 #17 0x773cdf61 in fghCreateNewContext (window=optimized out) at freeglut_window.c:458 #18 0x773ce81b in fgOpenWindow (window=0x6139e0, title=0x402f20 Gears, positionUse=0 '\000', x=-1, y=-1, sizeUse=1 '\001', w=300, h=300, gameMode=0 '\000', isSubWindow=0 '\000') at freeglut_window.c:1228 #19 0x773cd182 in fgCreateWindow (parent=0x0, title=0x402f20 Gears, positionUse=0 '\000', x=-1, y=-1, sizeUse=1 '\001', w=300, h=300, gameMode=0 '\000', isMenu=0 '\000') at freeglut_structure.c:108 #20 0x773cea12 in glutCreateWindow (title=0x402f20 Gears) at freeglut_window.c:1583 #21 0x00401553 in main (argc=1, argv=0x7fffe568) at gears.c:391 -- You are receiving
Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.
On Tuesday 16 April 2013, Nishanth Menon wrote: On 12:50-20130416, Arnd Bergmann wrote: On Tuesday 16 April 2013 12:48:28 Paul Sokolovsky wrote: diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 8d1e2bb..73a99e4 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -36,7 +36,7 @@ #ifndef _DRM_H_ #define _DRM_H_ -#if defined(__linux__) +#if defined(__KERNEL__) || defined(__linux__) #include linux/types.h #include asm/ioctl.h This is still completely bogus, the __KERNEL__ symbol has no significance here. Either make the compiler define __linux__, or remove this #ifdef completely. Searching the v.39-rc7 tag, and greping for _linux_ a few interesting list pops up. (pruned): arch/arc/Makefile:cflags-y+= -mA7 -fno-common -pipe -fno-builtin -D__linux__ arch/h8300/Makefile:KBUILD_CFLAGS += -D__linux__ arch/hexagon/Makefile:KBUILD_CFLAGS += -ffixed-$(TIR_NAME) -DTHREADINFO_REG=$(TIR_NAME) -D__linux__ arch/score/Makefile: -D__linux__ -ffunction-sections -ffreestanding arch/xtensa/Makefile:KBUILD_CFLAGS += -ffreestanding -D__linux__ ^^ these architectures seem to bypass the pain entirely by defining __linux__ Right, that seems like a reasonable approach when the compilers are actually known to be compatible. arch/mips/include/uapi/asm/sgidefs.h:#ifndef __linux__ On MIPS, they are not. If you are building a Linux kernel with a gcc that was targetted at another ABI, the system call interface may change, so they forbid that here. drivers/gpu/drm/radeon/radeon_cp.c:#ifdef __linux__ {snip} drivers/scsi/aic7xxx/aic7770.c:#ifdef __linux__ drivers/scsi/aic7xxx/aic79xx.h:#ifndef __linux__ {snip} drivers/scsi/aic7xxx/aic79xx_core.c:#ifdef __linux__ {snip} drivers/scsi/aic7xxx/aic79xx_pci.c:#ifdef __linux__ drivers/scsi/aic7xxx/aic7xxx.h:#ifndef __linux__ drivers/scsi/aic7xxx/aic7xxx.h:#ifndef __linux__ drivers/scsi/aic7xxx/aic7xxx_93cx6.c:#ifdef __linux__ drivers/scsi/aic7xxx/aic7xxx_core.c:#ifdef __linux__ {snip} drivers/scsi/aic7xxx/aic7xxx_pci.c:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm.h:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm_macro_scan.l:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm_scan.l:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm_symbol.h:#ifdef __linux__ drivers/scsi/dpt/osd_defs.h:#if (defined(__linux__)) drivers/staging/ced1401/machine.h:#if (defined(__linux__) || defined(_linux) || defined(__linux)) !defined(LINUX) These are all drivers that are shared with another OS, or used to be. In most of them, we can probably just remove the #else path, since I don't think they are getting synchronized any more. include/acpi/platform/acenv.h:#if defined(_LINUX) || defined(__linux__) The acpi header files are maintained outside of Linux and are kept OS-independent. include/linux/coda.h:#if defined(__linux__) include/uapi/drm/drm.h:#if defined(__linux__) include/uapi/linux/coda.h:#if defined(__linux__) include/uapi/linux/fuse.h:#ifdef __linux__ In case of coda, we should not need to care any more, that header just got broken by the uapi-split for other operating systems. The drm.h and fuse.h header files are in theory still kept OS-agnostic and are actively maintained. And then we have the following as well.. fs/ext4/ext4.h:#if defined(__KERNEL__) || defined(__linux__) This seems to have been copied from the ext2 utils. The ext2/ext3 versions of this file don't have support for other operating systems. Trying out a few different prebuilt compilers I had around, I see: http://pastebin.com/bTVDLTb1 So, is our approach just to use __linux__ for builds? I am trying to understand rationale behind why #include linux/types.h #include asm/ioctl.h would want __linux__ and why __KERNEL__ check is un-wanted. Ofcourse, I cant comment about the One of the BSDs in else options.. and why we'd like to keep it around in kernel :) We might be in a similar situation on ARM that we are in on MIPS. For instance, there are some compilers that are targetting (old) Android that have a slightly different ABI, and building a kernel with those results in a system call ABI that is incompatible with user space built with a standard compiler. The safest approach is probably to bail out early if __linux__ is not set, and force anyone that wants to use a strange compiler to set the macro manually. Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.
On Wed, Apr 17, 2013 at 6:43 AM, Arnd Bergmann a...@arndb.de wrote: drivers/scsi/aic7xxx/aicasm/aicasm_symbol.c:#ifdef __linux__ drivers/scsi/aic7xxx/aicasm/aicasm_symbol.h:#ifdef __linux__ drivers/scsi/dpt/osd_defs.h:#if (defined(__linux__)) drivers/staging/ced1401/machine.h:#if (defined(__linux__) || defined(_linux) || defined(__linux)) !defined(LINUX) These are all drivers that are shared with another OS, or used to be. In most of them, we can probably just remove the #else path, since I don't think they are getting synchronized any more. note that DRM gets periodically ported to the *BSD's, so possibly the same applies here.. BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
On 04/12/2013 01:38 PM, Bjorn Helgaas wrote: On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares luca...@linux.vnet.ibm.com wrote: radeon currently uses a drm function to get the speed capabilities for the bus. However, this is a non-standard method of performing this detection and this patch changes it to use the max_bus_speed attribute. --- drivers/gpu/drm/radeon/evergreen.c |9 ++--- drivers/gpu/drm/radeon/r600.c |8 +--- drivers/gpu/drm/radeon/rv770.c |8 +--- 3 files changed, 4 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 305a657..3291f62 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev) void evergreen_pcie_gen2_enable(struct radeon_device *rdev) { - u32 link_width_cntl, speed_cntl, mask; - int ret; + u32 link_width_cntl, speed_cntl; if (radeon_pcie_gen2 == 0) return; @@ -3871,11 +3870,7 @@ void evergreen_pcie_gen2_enable(struct radeon_device *rdev) if (ASIC_IS_X2(rdev)) return; - ret = drm_pcie_get_speed_cap_mask(rdev-ddev,mask); - if (ret != 0) - return; - - if (!(mask DRM_PCIE_SPEED_50)) + if (rdev-pdev-bus-max_bus_speed PCIE_SPEED_5_0GT) For devices on a root bus, we previously dereferenced a NULL pointer in drm_pcie_get_speed_cap_mask() because pdev-bus-self is NULL on a root bus. (I think this is the original problem you tripped over, Lucas.) These patches fix that problem. On pseries, where the device *is* on a root bus, your patches set max_bus_speed so this will work as expected. On most other systems, max_bus_speed for root buses will be PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because most arches don't have code like the pseries code you're adding). PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on the root bus, we'll attempt to enable Gen2 on the device even though we have no idea what the bus will support. That's why I originally suggested skipping the Gen2 stuff if max_bus_speed == PCI_SPEED_UNKNOWN. I was just being conservative, thinking that it's better to have a functional but slow GPU rather than the unknown (to me) effects of enabling Gen2 on a link that might not support it. But I'm fine with this being either way. You're right here, of course. I'll test for it being different from 5_0GT and 8_0GT instead. Though at some point I suppose someone will want to tackle Gen3 speeds. It would be nice if we could get rid of drm_pcie_get_speed_cap_mask() altogether. It is exported, but I have no idea of anybody else uses it. Maybe it could at least be marked __deprecated now? I'll mark it. I don't know who should take these patches. They don't touch drivers/pci, but I'd be happy to push them, given the appropriate ACKs from DRM and powerpc folks. Bjorn return; speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL); diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index 0740db3..64b90c0 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -4351,8 +4351,6 @@ static void r600_pcie_gen2_enable(struct radeon_device *rdev) { u32 link_width_cntl, lanes, speed_cntl, training_cntl, tmp; u16 link_cntl2; - u32 mask; - int ret; if (radeon_pcie_gen2 == 0) return; @@ -4371,11 +4369,7 @@ static void r600_pcie_gen2_enable(struct radeon_device *rdev) if (rdev-family= CHIP_R600) return; - ret = drm_pcie_get_speed_cap_mask(rdev-ddev,mask); - if (ret != 0) - return; - - if (!(mask DRM_PCIE_SPEED_50)) + if (rdev-pdev-bus-max_bus_speed PCIE_SPEED_5_0GT) return; speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL); diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index d63fe1d..c683c36 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -1238,8 +1238,6 @@ static void rv770_pcie_gen2_enable(struct radeon_device *rdev) { u32 link_width_cntl, lanes, speed_cntl, tmp; u16 link_cntl2; - u32 mask; - int ret; if (radeon_pcie_gen2 == 0) return; @@ -1254,11 +1252,7 @@ static void rv770_pcie_gen2_enable(struct radeon_device *rdev) if (ASIC_IS_X2(rdev)) return; - ret = drm_pcie_get_speed_cap_mask(rdev-ddev,mask); - if (ret != 0) - return; - - if (!(mask DRM_PCIE_SPEED_50)) + if (rdev-pdev-bus-max_bus_speed PCIE_SPEED_5_0GT) return; DRM_INFO(enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n); -- 1.7.4.4 -- Lucas Kannebley Tavares Software Engineer IBM Linux
Re: [PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
On 04/15/2013 02:00 AM, Michael Ellerman wrote: On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote: On pseries machines the detection for max_bus_speed should be done through an OpenFirmware property. This patch adds a function to perform this detection and a hook to perform dynamic adding of the function only for pseries. The crucial detail you didn't mention is that pcibios_root_bridge_prepare() already exists as a weak function in the PCI code and is called from pci_create_root_bus(). Adding this comment. diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 8bcc9ca..15796b5 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -430,6 +430,8 @@ static void pSeries_machine_kexec(struct kimage *image) } #endif +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge); + Don't do that, put it in a header where it belongs. And done as well. cheers -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
On 04/15/2013 08:42 AM, Michael Ellerman wrote: On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote: On pseries machines the detection for max_bus_speed should be done through an OpenFirmware property. This patch adds a function to perform this detection and a hook to perform dynamic adding of the function only for pseries. This fails to build for me on ppc64_defconfig, with: arch/powerpc/include/asm/machdep.h:111:5: error: 'struct pci_host_bridge' declared inside parameter list [-Werror] Presumably you tested it using some other defconfig? cheers Yes, I tested with another config, I did get warnings, though, so I should've fixed that earlier. Adding a forward declaration to prevent this from even throwing out warnings. -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm.h: Fix DRM compilation with bare-metal toolchain.
On Tue, 2013-04-16 at 12:50 +0200, Arnd Bergmann wrote: On Tuesday 16 April 2013 12:48:28 Paul Sokolovsky wrote: diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 8d1e2bb..73a99e4 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -36,7 +36,7 @@ #ifndef _DRM_H_ #define _DRM_H_ -#if defined(__linux__) +#if defined(__KERNEL__) || defined(__linux__) #include linux/types.h #include asm/ioctl.h This is still completely bogus, the __KERNEL__ symbol has no significance here. Either make the compiler define __linux__, or remove this #ifdef completely. I don't have the full background here but thought I would point out that a similar issue seems to have just cropped up with fuse.h ... http://lkml.org/lkml/2013/4/15/487 -- Tixy ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/prime: keep a reference from the handle to exported dma-buf (v4)
On Wed, 2013-04-17 at 10:21 +0200, Daniel Vetter wrote: On Wed, Apr 17, 2013 at 02:38:02PM +1000, Dave Airlie wrote: Currently we have a problem with this: 1. i915: create gem object 2. i915: export gem object to prime 3. radeon: import gem object 4. close prime fd 5. radeon: unref object 6. i915: unref object i915 has an imported object reference in its file priv, that isn't cleaned up properly until fd close. The reference gets added at step 2, but at step 6 we don't have enough info to clean it up. The solution is to take a reference on the dma-buf when we export it, and drop the reference when the gem handle goes away. So when we export a dma_buf from a gem object, we keep track of it with the handle, we take a reference to the dma_buf. When we close the handle (i.e. userspace is finished with the buffer), we drop the reference to the dma_buf, and it gets collected. This patch isn't meant to fix any other problem or bikesheds, and it doesn't fix any races with other scenarios. v1.1: move export symbol line back up. v2: okay I had to do a bit more, as the first patch showed a leak on one of my tests, that I found using the dma-buf debugfs support, the problem case is exporting a buffer twice with the same handle, we'd add another export handle for it unnecessarily, however we now fail if we try to export the same object with a different gem handle, however I'm not sure if that is a case I want to support, and I've gotten the code to WARN_ON if we hit something like that. v2.1: rebase this patch, write better commit msg. v3: cleanup error handling, track import vs export in linked list, these two patches were separate previously, but seem to work better like this. v4: danvet is correct, this code is no longer useful, since the buffer better exist, so remove it. Signed-off-by: Dave Airlie airl...@redhat.com Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch On bikeshed review level I wonder whether we shouldn't just grab a reference unconditonally when inserting it into the handle_to_fd lookup lists. But I need to think through a few other cases and apparently the self-import test is still a bit broken. So this is material for follow-up patches. Yes, this is needed, otherwise closing the prime fd will leave stale pointers to the dma buf in the importer's lookup table. I've sent an update to igt/prime_self_test to intel-gfx for showing this. --Imre -Daniel --- drivers/gpu/drm/drm_gem.c | 4 +- drivers/gpu/drm/drm_prime.c | 95 + include/drm/drmP.h | 4 +- 3 files changed, 65 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index af779ae..cf919e3 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -205,11 +205,11 @@ static void drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) { if (obj-import_attach) { - drm_prime_remove_imported_buf_handle(filp-prime, + drm_prime_remove_buf_handle(filp-prime, obj-import_attach-dmabuf); } if (obj-export_dma_buf) { - drm_prime_remove_imported_buf_handle(filp-prime, + drm_prime_remove_buf_handle(filp-prime, obj-export_dma_buf); } } diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 366910d..b4ed808 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -57,10 +57,14 @@ * use the drm_gem_prime_{import,export} helpers. */ +#define PRIME_IMPORT 1 +#define PRIME_EXPORT 2 + struct drm_prime_member { struct list_head entry; struct dma_buf *dma_buf; uint32_t handle; + int type; }; static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, @@ -157,6 +161,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { .vunmap = drm_gem_dmabuf_vunmap, }; +static int drm_prime_add_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle); + /** * DOC: PRIME Helpers * @@ -200,7 +206,9 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, { struct drm_gem_object *obj; void *buf; - int ret; + int ret = 0; + struct dma_buf *dmabuf; + uint32_t exp_handle; obj = drm_gem_object_lookup(dev, file_priv, handle); if (!obj) @@ -209,43 +217,44 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, mutex_lock(file_priv-prime.lock); /* re-export the original imported object */ if (obj-import_attach) { - get_dma_buf(obj-import_attach-dmabuf); - *prime_fd = dma_buf_fd(obj-import_attach-dmabuf, flags); - drm_gem_object_unreference_unlocked(obj);
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #11 from Ian Romanick i...@freedesktop.org --- (In reply to comment #9) I don't know where you have the retroactive-story from, but the specification does not mention it being retroactive, and even goes as far as to say: I have the story from being in the Khronos meetings. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC][PATCH] drm: Insane but more fine grained locking for planes
From: Ville Syrjälä ville.syrj...@linux.intel.com Instead of locking all modeset locks during plane updates, use just a single CRTC mutex. To make that work, track the CRTC that owns the plane currently. During enable/update that means the new CRTC, and during disable it means the old CRTC. Since the plane state is no longer protected by a single lock, we need to sprinkle some additional memory barriers when relinquishing ownership. Otherwise the next CRTC might observe some stale state even though the crtc_mutex already got updated. drm_framebuffer_remove() doesn't need extra barriers since it already holds all CRTC locks, and thus no-one can be poking around at the same time. On the read side cmpxchg() already should have the necessary memory barriers. This design implies that before a plane can be moved to another CRTC, it must be disabled first, even if the hardware would offer some kind of mechanism to move an active plane over directly. I believe everyone has agreed that this an acceptable compromise. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 43 +++ include/drm/drm_crtc.h | 3 +++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 957fb70..6f7385e 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -576,6 +576,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) __drm_framebuffer_unreference(plane-fb); plane-fb = NULL; plane-crtc = NULL; + plane-crtc_mutex = NULL; } } drm_modeset_unlock_all(dev); @@ -1785,6 +1786,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data, int ret = 0; unsigned int fb_width, fb_height; int i; + struct mutex *old_crtc_mutex; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; @@ -1804,12 +1806,33 @@ int drm_mode_setplane(struct drm_device *dev, void *data, /* No fb means shut it down */ if (!plane_req-fb_id) { - drm_modeset_lock_all(dev); + struct mutex *crtc_mutex; + + retry: + crtc_mutex = ACCESS_ONCE(plane-crtc_mutex); + + /* plane was already disabled? */ + if (!crtc_mutex) + return 0; + + mutex_lock(crtc_mutex); + + /* re-check that plane is still on the same crtc... */ + if (crtc_mutex != plane-crtc_mutex) { + mutex_unlock(crtc_mutex); + goto retry; + } + old_fb = plane-fb; plane-funcs-disable_plane(plane); plane-crtc = NULL; plane-fb = NULL; - drm_modeset_unlock_all(dev); + + smp_wmb(); + plane-crtc_mutex = NULL; + + mutex_unlock(crtc_mutex); + goto out; } @@ -1875,7 +1898,15 @@ int drm_mode_setplane(struct drm_device *dev, void *data, goto out; } - drm_modeset_lock_all(dev); + mutex_lock(crtc-mutex); + + old_crtc_mutex = cmpxchg(plane-crtc_mutex, NULL, crtc-mutex); + if (old_crtc_mutex != NULL old_crtc_mutex != crtc-mutex) { + mutex_unlock(crtc-mutex); + ret = -EBUSY; + goto out; + } + ret = plane-funcs-update_plane(plane, crtc, fb, plane_req-crtc_x, plane_req-crtc_y, plane_req-crtc_w, plane_req-crtc_h, @@ -1886,8 +1917,12 @@ int drm_mode_setplane(struct drm_device *dev, void *data, plane-crtc = crtc; plane-fb = fb; fb = NULL; + } else { + smp_wmb(); + plane-crtc_mutex = old_crtc_mutex; } - drm_modeset_unlock_all(dev); + + mutex_unlock(crtc-mutex); out: if (fb) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8c7846b..cc3779f 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -651,6 +651,7 @@ struct drm_plane_funcs { * @dev: DRM device this plane belongs to * @head: for list management * @base: base mode object + * @crtc_mutex: points to the mutex of the current owner CRTC * @possible_crtcs: pipes this plane can be bound to * @format_types: array of formats supported by this plane * @format_count: number of formats supported @@ -669,6 +670,8 @@ struct drm_plane { struct drm_mode_object base; + struct mutex *crtc_mutex; + uint32_t possible_crtcs; uint32_t *format_types; uint32_t format_count; -- 1.8.1.5 ___ dri-devel
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #12 from Erik Faye-Lund kusmab...@gmail.com --- Well, Khronos meetings don't define the spec, the specification does. And the specification is pretty clear here. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/4] drm/edid: Recognize 60Hz and 59.94Hz CEA modes
This series attempts to make our CEA mode matching recognize both the 60Hz and 59.94Hz variants of the modes (and similarly for 24/23.97, 30/29.97, etc.). The benefits should include: - Send the correct VIC in the AVI infoframe - Pick the correct RGB quantization range in automatic mode ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm: Remove explicit vrefresh initialization from DRM_MODE()
From: Ville Syrjälä ville.syrj...@linux.intel.com No need to zero initialize .vrefresh in DRM_MODE() since it's using desgignated initializers. This will also avoid some duplicate initialization warnings later. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- include/drm/drm_crtc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8c7846b..b85575b 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -120,7 +120,7 @@ enum drm_mode_status { .hdisplay = (hd), .hsync_start = (hss), .hsync_end = (hse), \ .htotal = (ht), .hskew = (hsk), .vdisplay = (vd), \ .vsync_start = (vss), .vsync_end = (vse), .vtotal = (vt), \ - .vscan = (vs), .flags = (f), .vrefresh = 0, \ + .vscan = (vs), .flags = (f), \ .base.type = DRM_MODE_OBJECT_MODE #define CRTC_INTERLACE_HALVE_V 0x1 /* halve V values for interlacing */ -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm: Add drm_mode_equal_no_clocks()
From: Ville Syrjälä ville.syrj...@linux.intel.com drm_mode_equal_no_clocks() is like drm_mode_equal() except it doesn't compare the clock or vrefresh values. drm_mode_equal() is now implemented by first doing the clock checks, and then calling drm_mode_equal_no_clocks(). Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/drm_modes.c | 20 +++- include/drm/drm_crtc.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 04fa6f1..db85d0b9 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -848,6 +848,24 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_displ } else if (mode1-clock != mode2-clock) return false; + return drm_mode_equal_no_clocks(mode1, mode2); +} +/** + * drm_mode_equal_no_clocks - test modes for equality + * @mode1: first mode + * @mode2: second mode + * + * LOCKING: + * None. + * + * Check to see if @mode1 and @mode2 are equivalent, but + * don't check the pixel clocks. + * + * RETURNS: + * True if the modes are equal, false otherwise. + */ +bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2) +{ if (mode1-hdisplay == mode2-hdisplay mode1-hsync_start == mode2-hsync_start mode1-hsync_end == mode2-hsync_end @@ -863,7 +881,7 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_displ return false; } -EXPORT_SYMBOL(drm_mode_equal); +EXPORT_SYMBOL(drm_mode_equal_no_clocks); /** * drm_mode_validate_size - make sure modes adhere to size constraints diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b85575b..836438d 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -922,6 +922,7 @@ extern void drm_mode_config_reset(struct drm_device *dev); extern void drm_mode_config_cleanup(struct drm_device *dev); extern void drm_mode_set_name(struct drm_display_mode *mode); extern bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); +extern bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); extern int drm_mode_width(const struct drm_display_mode *mode); extern int drm_mode_height(const struct drm_display_mode *mode); -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/edid: Populate vrefresh for CEA modes
From: Ville Syrjälä ville.syrj...@linux.intel.com Well have use for the vrefresh information of CEA modes later. Just populate the information into the table to avoid having to calculate it. I'm too lazy to check if someone relies on newly allocated CEA modes having 0 vrefresh, so just clear vrefresh back to 0 when adding the mode to the connector's modelist. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 193 ++--- 1 file changed, 129 insertions(+), 64 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e2acfdb..e233ff5 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -587,284 +587,348 @@ static const struct drm_display_mode edid_cea_modes[] = { /* 1 - 640x480@60Hz */ { DRM_MODE(640x480, DRM_MODE_TYPE_DRIVER, 25175, 640, 656, 752, 800, 0, 480, 490, 492, 525, 0, - DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) }, + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), + .vrefresh = 60, }, /* 2 - 720x480@60Hz */ { DRM_MODE(720x480, DRM_MODE_TYPE_DRIVER, 27000, 720, 736, 798, 858, 0, 480, 489, 495, 525, 0, - DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) }, + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), + .vrefresh = 60, }, /* 3 - 720x480@60Hz */ { DRM_MODE(720x480, DRM_MODE_TYPE_DRIVER, 27000, 720, 736, 798, 858, 0, 480, 489, 495, 525, 0, - DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) }, + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), + .vrefresh = 60, }, /* 4 - 1280x720@60Hz */ { DRM_MODE(1280x720, DRM_MODE_TYPE_DRIVER, 74250, 1280, 1390, 1430, 1650, 0, 720, 725, 730, 750, 0, - DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC) }, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 60, }, /* 5 - 1920x1080i@60Hz */ { DRM_MODE(1920x1080i, DRM_MODE_TYPE_DRIVER, 74250, 1920, 2008, 2052, 2200, 0, 1080, 1084, 1094, 1125, 0, DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC | - DRM_MODE_FLAG_INTERLACE) }, + DRM_MODE_FLAG_INTERLACE), + .vrefresh = 60, }, /* 6 - 1440x480i@60Hz */ { DRM_MODE(1440x480i, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, 1602, 1716, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK) }, + DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), + .vrefresh = 60, }, /* 7 - 1440x480i@60Hz */ { DRM_MODE(1440x480i, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, 1602, 1716, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK) }, + DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), + .vrefresh = 60, }, /* 8 - 1440x240@60Hz */ { DRM_MODE(1440x240, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, 1602, 1716, 0, 240, 244, 247, 262, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_DBLCLK) }, + DRM_MODE_FLAG_DBLCLK), + .vrefresh = 60, }, /* 9 - 1440x240@60Hz */ { DRM_MODE(1440x240, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, 1602, 1716, 0, 240, 244, 247, 262, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_DBLCLK) }, + DRM_MODE_FLAG_DBLCLK), + .vrefresh = 60, }, /* 10 - 2880x480i@60Hz */ { DRM_MODE(2880x480i, DRM_MODE_TYPE_DRIVER, 54000, 2880, 2956, 3204, 3432, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_INTERLACE) }, + DRM_MODE_FLAG_INTERLACE), + .vrefresh = 60, }, /* 11 - 2880x480i@60Hz */ { DRM_MODE(2880x480i, DRM_MODE_TYPE_DRIVER, 54000, 2880, 2956, 3204, 3432, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | - DRM_MODE_FLAG_INTERLACE) }, + DRM_MODE_FLAG_INTERLACE), + .vrefresh = 60, }, /* 12 - 2880x240@60Hz */ { DRM_MODE(2880x240, DRM_MODE_TYPE_DRIVER, 54000, 2880, 2956, 3204, 3432, 0, 240, 244, 247, 262, 0, - DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC) }, + DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), + .vrefresh = 60, }, /* 13 -
[PATCH 4/4] drm/edid: Check both 60Hz and 59.94Hz when looking for a CEA mode
From: Ville Syrjälä ville.syrj...@linux.intel.com drm_match_cea_mode() should be able to match both the 60Hz version, and the 59.94Hz version of modes. We only store one pixel clock value per mode in edid_cea_modes, so the other value must be calculated. Depending on the mode, edid_cea_modes contains the pixel clock for either the 60Hz version or the 59.94Hz version, so a bit of care is needed so that the calculation produces the correct result. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46800 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 27 --- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e233ff5..71d49f2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2330,13 +2330,34 @@ EXPORT_SYMBOL(drm_find_cea_extension); */ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) { - struct drm_display_mode *cea_mode; u8 mode; + if (!to_match-clock) + return 0; + for (mode = 0; mode ARRAY_SIZE(edid_cea_modes); mode++) { - cea_mode = (struct drm_display_mode *)edid_cea_modes[mode]; + const struct drm_display_mode *cea_mode = edid_cea_modes[mode]; + unsigned int clock1, clock2; - if (drm_mode_equal(to_match, cea_mode)) + clock1 = clock2 = cea_mode-clock; + + /* Check both 60Hz and 59.94Hz */ + if (cea_mode-vrefresh % 6 == 0) { + /* +* edid_cea_modes contains the 59.94Hz +* variant for 240 and 480 line modes, +* and the 60Hz variant otherwise. +*/ + if (cea_mode-vdisplay == 240 || + cea_mode-vdisplay == 480) + clock1 = clock1 * 1001 / 1000; + else + clock2 = DIV_ROUND_UP(clock2 * 1000, 1001); + } + + if ((KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock1) || +KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock2)) + drm_mode_equal_no_clocks(to_match, cea_mode)) return mode + 1; } return 0; -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] drm: Insane but more fine grained locking for planes
On Wed, Apr 17, 2013 at 08:04:52PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Instead of locking all modeset locks during plane updates, use just a single CRTC mutex. To make that work, track the CRTC that owns the plane currently. During enable/update that means the new CRTC, and during disable it means the old CRTC. Since the plane state is no longer protected by a single lock, we need to sprinkle some additional memory barriers when relinquishing ownership. Otherwise the next CRTC might observe some stale state even though the crtc_mutex already got updated. drm_framebuffer_remove() doesn't need extra barriers since it already holds all CRTC locks, and thus no-one can be poking around at the same time. On the read side cmpxchg() already should have the necessary memory barriers. This design implies that before a plane can be moved to another CRTC, it must be disabled first, even if the hardware would offer some kind of mechanism to move an active plane over directly. I believe everyone has agreed that this an acceptable compromise. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Since the insanity is around plane-crtc_mutex, why not just add a plane mutex which _only_ protects that? That way we could partially resurect the old semantics by simply first grabbing the old crtc mutex, removing the fb, then grabbing the new crtc mutex and displaying it there. Whoever shows up with hw which can do that in one atomic step gets to fix the resulting mess then ;-) -Daniel --- drivers/gpu/drm/drm_crtc.c | 43 +++ include/drm/drm_crtc.h | 3 +++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 957fb70..6f7385e 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -576,6 +576,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) __drm_framebuffer_unreference(plane-fb); plane-fb = NULL; plane-crtc = NULL; + plane-crtc_mutex = NULL; } } drm_modeset_unlock_all(dev); @@ -1785,6 +1786,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data, int ret = 0; unsigned int fb_width, fb_height; int i; + struct mutex *old_crtc_mutex; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; @@ -1804,12 +1806,33 @@ int drm_mode_setplane(struct drm_device *dev, void *data, /* No fb means shut it down */ if (!plane_req-fb_id) { - drm_modeset_lock_all(dev); + struct mutex *crtc_mutex; + + retry: + crtc_mutex = ACCESS_ONCE(plane-crtc_mutex); + + /* plane was already disabled? */ + if (!crtc_mutex) + return 0; + + mutex_lock(crtc_mutex); + + /* re-check that plane is still on the same crtc... */ + if (crtc_mutex != plane-crtc_mutex) { + mutex_unlock(crtc_mutex); + goto retry; + } + old_fb = plane-fb; plane-funcs-disable_plane(plane); plane-crtc = NULL; plane-fb = NULL; - drm_modeset_unlock_all(dev); + + smp_wmb(); + plane-crtc_mutex = NULL; + + mutex_unlock(crtc_mutex); + goto out; } @@ -1875,7 +1898,15 @@ int drm_mode_setplane(struct drm_device *dev, void *data, goto out; } - drm_modeset_lock_all(dev); + mutex_lock(crtc-mutex); + + old_crtc_mutex = cmpxchg(plane-crtc_mutex, NULL, crtc-mutex); + if (old_crtc_mutex != NULL old_crtc_mutex != crtc-mutex) { + mutex_unlock(crtc-mutex); + ret = -EBUSY; + goto out; + } + ret = plane-funcs-update_plane(plane, crtc, fb, plane_req-crtc_x, plane_req-crtc_y, plane_req-crtc_w, plane_req-crtc_h, @@ -1886,8 +1917,12 @@ int drm_mode_setplane(struct drm_device *dev, void *data, plane-crtc = crtc; plane-fb = fb; fb = NULL; + } else { + smp_wmb(); + plane-crtc_mutex = old_crtc_mutex; } - drm_modeset_unlock_all(dev); + + mutex_unlock(crtc-mutex); out: if (fb) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8c7846b..cc3779f 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -651,6 +651,7 @@ struct drm_plane_funcs { * @dev: DRM device this plane belongs to * @head: for list management * @base: base mode object + * @crtc_mutex: points to the
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #13 from Ian Romanick i...@freedesktop.org --- Comment on attachment 78052 -- https://bugs.freedesktop.org/attachment.cgi?id=78052 [PATCH] gallium: handle drirc disable_glsl_line_continuations option You'll probably want review from someone that actually works on Gallium code, but this patch is Reviewed-by: Ian Romanick ian.d.roman...@intel.com You should also add NOTE: This is a candidate for the 9.1 branch. to the commit message. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 63579] Savage 2 Edges render white [r600g]
https://bugs.freedesktop.org/show_bug.cgi?id=63579 --- Comment #14 from Ian Romanick i...@freedesktop.org --- (In reply to comment #12) Well, Khronos meetings don't define the spec, the specification does. And the specification is pretty clear here. The discussion was at the time of the vote, and I stand by that. I can't make the meeting minutes public, and I'm not sure that's the most important thing here. However, I can explain the rationale behind our position for Mesa's compiler. Since you won't be satisfied by the short version of the story, prepare for a long, twisting tale... The problem is that, following the C model of compilation, line continuation is a separate compilation phase, and it occurs before preprocessing. As a result, it's a shocking amount of work to *correctly* dynamically switch behaviors depending on the contents of the shader. The implementation is also likely to have compilation overhead for every shader. Consider a pathological shader like: // \ /* #version 120 // */ \ #version 420 Shaders similar to this one were discussed by Khronos during OpenGL ES 3.0 and OpenGL 4.2 development. Suffice to say, more time than seems reasonable was spent trying to decide 1. what that shader should do, and 2. how to craft spec language to describe that. This is also why issue 12.19 in the GLSL ES 3.0 spec is resolved, Line-continuation to be made optional in GLSL ES 1.00. Mesa's current GLSL compiler originally had line continuation, but, after having a problem with this game, we removed it. I can't find a specific commit, so this may have been removed before the preprocessor was merged. We encountered a small number of shaders that wanted continuation in preprocessor lines (#define and #line), and these shaders all compile on both NVIDIA and AMD. We added continuation support for those cases in 2010 (commit bc1097d). Both 3.00 ES and 4.20 require continuation, so we had to add it back for all cases. In doing so, of course, we noticed that the Savage2 shader stopped compiling. We thought about just supporting continuation in ES contexts, but the problem still exists since we expose GL_ARB_ES3_compatibility. We searched our internal database of a few thousand shaders from shipping applications (open- and closed-source). This was the only shader didn't want continuation behavior. There was a very small number of shaders that used continuation. If it weren't for the Savage2 shader, frankly, we wouldn't have implemented the non-continuation behavior at all... and nobody would have ever noticed. There were zero shaders that both wanted continuation and non-continuation behavior, and there were zero applications that contained both a continuation shader and a non-continuation shader. We just couldn't justify either the effort or the added code complexity to support dynamic selection. We also couldn't justify breaking a shipping application that previously worked. The right compromise was to have a workaround for the one case that required the non-continuation behavior, and this game works fine on the i965 driver. The Gallium drivers just need to use the workaround, and attachment #78052 should do that. We probably could have done a better job telling the other driver authors that they needed to support this option when it was added. We were overwhelmingly busy trying to get ES3 support finished, so some things like that fell through the cracks. We put the patches on the mailing list (and the commit message in 4b00ece specifically says it fixes Savage2), so it's not like we were being sneaky. We can only do better in the future. The part you didn't mention from section 3.1 of the 1.20 spec is that \ is not listed as part of the character set. If we really want to argue the letter of the spec, the shader should fail to compile because it contains an invalid character. :) -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel