[PATCH libdrm 5/5] modetest: Reduce the length of the connector type string

2013-04-17 Thread ville.syrj...@linux.intel.com
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

2013-04-17 Thread ville.syrj...@linux.intel.com
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

2013-04-17 Thread ville.syrj...@linux.intel.com
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

2013-04-17 Thread ville.syrj...@linux.intel.com
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

2013-04-17 Thread ville.syrj...@linux.intel.com
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]

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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]

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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

2013-04-17 Thread Daniel Vetter
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]

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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]

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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

2013-04-17 Thread ville.syrj...@linux.intel.com
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

2013-04-17 Thread ville.syrj...@linux.intel.com
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()

2013-04-17 Thread ville.syrj...@linux.intel.com
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()

2013-04-17 Thread ville.syrj...@linux.intel.com
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

2013-04-17 Thread ville.syrj...@linux.intel.com
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

2013-04-17 Thread ville.syrj...@linux.intel.com
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]

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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

2013-04-17 Thread Kero
> > 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

2013-04-17 Thread Daniel Vetter
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)

2013-04-17 Thread Imre Deak
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)

2013-04-17 Thread Dave Airlie
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]

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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)

2013-04-17 Thread Randy Dunlap
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]

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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-04-17 Thread Paulo Zanoni
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)

2013-04-17 Thread Huacai Chen
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]

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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

2013-04-17 Thread Byron Stanoszek
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]

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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()

2013-04-17 Thread Paulo Zanoni
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

2013-04-17 Thread Alex Deucher
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

2013-04-17 Thread Pawel Moll
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

2013-04-17 Thread Pawel Moll
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

2013-04-17 Thread Pawel Moll
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

2013-04-17 Thread Pawel Moll
... 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

2013-04-17 Thread Pawel Moll
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

2013-04-17 Thread Pawel Moll
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

2013-04-17 Thread Pawel Moll
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

2013-04-17 Thread Pawel Moll
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

2013-04-17 Thread Pawel Moll
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

2013-04-17 Thread Pawel Moll
From: Laurent Pinchart 

Signed-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

2013-04-17 Thread Pawel Moll
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

2013-04-17 Thread Alex Deucher
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)

2013-04-17 Thread Dave Airlie
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

2013-04-17 Thread Inki Dae
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

2013-04-17 Thread Bjorn Helgaas
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

2013-04-17 Thread Bjorn Helgaas
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)

2013-04-17 Thread Dave Airlie
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.

2013-04-17 Thread Arnd Bergmann
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.

2013-04-17 Thread Borislav Petkov
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 Lankhorst 
Date: 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.

2013-04-17 Thread Jon Medhurst (Tixy)
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

2013-04-17 Thread Sylwester Nawrocki
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

2013-04-17 Thread Sylwester Nawrocki
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()

2013-04-17 Thread Sylwester Nawrocki
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

2013-04-17 Thread Sylwester Nawrocki
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

2013-04-17 Thread Sylwester Nawrocki
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)

2013-04-17 Thread Daniel Vetter
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

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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

2013-04-17 Thread Lucas Kannebley Tavares
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

2013-04-17 Thread Lucas Kannebley Tavares
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

2013-04-17 Thread Lucas Kannebley Tavares
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

2013-04-17 Thread Sachin Kamat
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]

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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.

2013-04-17 Thread Rob Clark
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

2013-04-17 Thread Laurent Pinchart
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

2013-04-17 Thread bugzilla-dae...@bugzilla.kernel.org
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

2013-04-17 Thread bugzilla-dae...@freedesktop.org
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

2013-04-17 Thread David Rientjes
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.

2013-04-17 Thread Nishanth Menon
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

2013-04-17 Thread Sachin Kamat
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)

2013-04-17 Thread Dave Airlie
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

2013-04-17 Thread Inki Dae
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

2013-04-17 Thread David Rientjes
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)

2013-04-17 Thread Daniel Vetter
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]

2013-04-17 Thread bugzilla-daemon
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

2013-04-17 Thread Sylwester Nawrocki
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)

2013-04-17 Thread Huacai Chen
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

2013-04-17 Thread Sylwester Nawrocki
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()

2013-04-17 Thread Sylwester Nawrocki
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

2013-04-17 Thread Sylwester Nawrocki
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

2013-04-17 Thread Sylwester Nawrocki
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

2013-04-17 Thread bugzilla-daemon
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.

2013-04-17 Thread Arnd Bergmann
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.

2013-04-17 Thread Rob Clark
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

2013-04-17 Thread Lucas Kannebley Tavares

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

2013-04-17 Thread Lucas Kannebley Tavares

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

2013-04-17 Thread Lucas Kannebley Tavares

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.

2013-04-17 Thread Jon Medhurst (Tixy)
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)

2013-04-17 Thread Imre Deak
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]

2013-04-17 Thread bugzilla-daemon
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

2013-04-17 Thread ville . syrjala
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]

2013-04-17 Thread bugzilla-daemon
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

2013-04-17 Thread ville . syrjala
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()

2013-04-17 Thread ville . syrjala
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()

2013-04-17 Thread ville . syrjala
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

2013-04-17 Thread ville . syrjala
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

2013-04-17 Thread ville . syrjala
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

2013-04-17 Thread Daniel Vetter
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]

2013-04-17 Thread bugzilla-daemon
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]

2013-04-17 Thread bugzilla-daemon
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


  1   2   >